Skip to content

ref(insights): Refactor Resource Summary + Landing charts into separate modules #89612

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 22, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,83 +1,23 @@
import styled from '@emotion/styled';

import {space} from 'sentry/styles/space';
import {EMPTY_OPTION_VALUE, MutableSearch} from 'sentry/utils/tokenizeSearch';
import {InsightsLineChartWidget} from 'sentry/views/insights/common/components/insightsLineChartWidget';
import {useSpanMetricsSeries} from 'sentry/views/insights/common/queries/useDiscoverSeries';
import type {ModuleFilters} from 'sentry/views/insights/common/views/spans/types';
import {
getDurationChartTitle,
getThroughputChartTitle,
} from 'sentry/views/insights/common/views/spans/types';
import {SpanMetricsField} from 'sentry/views/insights/types';

const {SPAN_SELF_TIME, NORMALIZED_DESCRIPTION, SPAN_DOMAIN} = SpanMetricsField;

type Props = {
appliedFilters: ModuleFilters;
extraQuery?: string[];
};

export function ResourceLandingPageCharts({appliedFilters, extraQuery}: Props) {
let query: string = buildDiscoverQueryConditions(appliedFilters);

if (extraQuery) {
query += ` ${extraQuery.join(' ')}`;
}

const {data, isPending, error} = useSpanMetricsSeries(
{
search: new MutableSearch(query),
yAxis: ['epm()', `avg(${SPAN_SELF_TIME})`],
transformAliasToInputFormat: true,
},
'api.starfish.span-time-charts'
);
import {ResourceLandingDurationChartWidget} from 'sentry/views/insights/common/components/widgets/resourceLandingDurationChartWidget';
import {ResourceLandingThroughputChartWidget} from 'sentry/views/insights/common/components/widgets/resourceLandingThroughputChartWidget';

export function ResourceLandingPageCharts() {
return (
<ChartsContainer>
<ChartsContainerItem>
<InsightsLineChartWidget
title={getThroughputChartTitle('resource')}
series={[data['epm()']]}
isLoading={isPending}
error={error}
/>
<ResourceLandingThroughputChartWidget />
</ChartsContainerItem>

<ChartsContainerItem>
<InsightsLineChartWidget
title={getDurationChartTitle('resource')}
series={[data[`avg(${SPAN_SELF_TIME})`]]}
isLoading={isPending}
error={error}
/>
<ResourceLandingDurationChartWidget />
</ChartsContainerItem>
</ChartsContainer>
);
}

const SPAN_FILTER_KEYS = ['span_operation', SPAN_DOMAIN, 'action'];

const buildDiscoverQueryConditions = (appliedFilters: ModuleFilters) => {
const result = Object.keys(appliedFilters)
.filter(key => SPAN_FILTER_KEYS.includes(key))
// @ts-expect-error TS(7053): Element implicitly has an 'any' type because expre... Remove this comment to see the full error message
.filter(key => Boolean(appliedFilters[key]))
.map(key => {
// @ts-expect-error TS(7053): Element implicitly has an 'any' type because expre... Remove this comment to see the full error message
const value = appliedFilters[key];
if (key === SPAN_DOMAIN && value === EMPTY_OPTION_VALUE) {
return [`!has:${SPAN_DOMAIN}`];
}
return `${key}:${value}`;
});

result.push(`has:${NORMALIZED_DESCRIPTION}`);

return result.join(' ');
};

const ChartsContainer = styled('div')`
display: flex;
flex-direction: row;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,105 +1,23 @@
import {Fragment} from 'react';

import {t} from 'sentry/locale';
import {MutableSearch} from 'sentry/utils/tokenizeSearch';
import {Referrer} from 'sentry/views/insights/browser/resources/referrer';
import {DATA_TYPE, FIELD_ALIASES} from 'sentry/views/insights/browser/resources/settings';
import {useResourceModuleFilters} from 'sentry/views/insights/browser/resources/utils/useResourceFilters';
import {InsightsLineChartWidget} from 'sentry/views/insights/common/components/insightsLineChartWidget';
import * as ModuleLayout from 'sentry/views/insights/common/components/moduleLayout';
import {useSpanMetricsSeries} from 'sentry/views/insights/common/queries/useDiscoverSeries';
import {
getDurationChartTitle,
getThroughputChartTitle,
} from 'sentry/views/insights/common/views/spans/types';
import {SpanMetricsField} from 'sentry/views/insights/types';

const {
SPAN_SELF_TIME,
HTTP_RESPONSE_CONTENT_LENGTH,
HTTP_DECODED_RESPONSE_CONTENT_LENGTH,
HTTP_RESPONSE_TRANSFER_SIZE,
RESOURCE_RENDER_BLOCKING_STATUS,
} = SpanMetricsField;

function ResourceSummaryCharts(props: {groupId: string}) {
const filters = useResourceModuleFilters();

const mutableSearch = MutableSearch.fromQueryObject({
'span.group': props.groupId,
...(filters[RESOURCE_RENDER_BLOCKING_STATUS]
? {
[RESOURCE_RENDER_BLOCKING_STATUS]: filters[RESOURCE_RENDER_BLOCKING_STATUS],
}
: {}),
...(filters[SpanMetricsField.USER_GEO_SUBREGION]
? {
[SpanMetricsField.USER_GEO_SUBREGION]: `[${filters[SpanMetricsField.USER_GEO_SUBREGION].join(',')}]`,
}
: {}),
});

const {
data: spanMetricsSeriesData,
isPending: areSpanMetricsSeriesLoading,
error: spanMetricsSeriesError,
} = useSpanMetricsSeries(
{
search: mutableSearch,
yAxis: [
`epm()`,
`avg(${SPAN_SELF_TIME})`,
`avg(${HTTP_RESPONSE_CONTENT_LENGTH})`,
`avg(${HTTP_DECODED_RESPONSE_CONTENT_LENGTH})`,
`avg(${HTTP_RESPONSE_TRANSFER_SIZE})`,
],
enabled: Boolean(props.groupId),
transformAliasToInputFormat: true,
},
Referrer.RESOURCE_SUMMARY_CHARTS
);

if (spanMetricsSeriesData) {
spanMetricsSeriesData[`avg(${HTTP_RESPONSE_TRANSFER_SIZE})`].lineStyle = {
type: 'dashed',
};
spanMetricsSeriesData[`avg(${HTTP_DECODED_RESPONSE_CONTENT_LENGTH})`].lineStyle = {
type: 'dashed',
};
}
Comment on lines -62 to -69
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All 3 charts shared the data fetching code, but in the new charts only the AverageSeries chart has this code since it is the only chart that uses these series (IIUC).

import {ResourceSummaryAverageSizeChartWidget} from 'sentry/views/insights/common/components/widgets/resourceSummaryAverageSizeChartWidget';
import {ResourceSummaryDurationChartWidget} from 'sentry/views/insights/common/components/widgets/resourceSummaryDurationChartWidget';
import {ResourceSummaryThroughputChartWidget} from 'sentry/views/insights/common/components/widgets/resourceSummaryThroughputChartWidget';

function ResourceSummaryCharts() {
return (
<Fragment>
<ModuleLayout.Third>
<InsightsLineChartWidget
title={getThroughputChartTitle('resource')}
series={[spanMetricsSeriesData?.[`epm()`]]}
isLoading={areSpanMetricsSeriesLoading}
error={spanMetricsSeriesError}
/>
<ResourceSummaryThroughputChartWidget />
</ModuleLayout.Third>

<ModuleLayout.Third>
<InsightsLineChartWidget
title={getDurationChartTitle('resource')}
series={[spanMetricsSeriesData?.[`avg(${SPAN_SELF_TIME})`]]}
isLoading={areSpanMetricsSeriesLoading}
error={spanMetricsSeriesError}
/>
<ResourceSummaryDurationChartWidget />
</ModuleLayout.Third>

<ModuleLayout.Third>
<InsightsLineChartWidget
title={t('Average %s Size', DATA_TYPE)}
series={[
spanMetricsSeriesData?.[`avg(${HTTP_DECODED_RESPONSE_CONTENT_LENGTH})`],
spanMetricsSeriesData?.[`avg(${HTTP_RESPONSE_TRANSFER_SIZE})`],
spanMetricsSeriesData?.[`avg(${HTTP_RESPONSE_CONTENT_LENGTH})`],
]}
aliases={FIELD_ALIASES}
isLoading={areSpanMetricsSeriesLoading}
error={spanMetricsSeriesError}
/>
<ResourceSummaryAverageSizeChartWidget />
</ModuleLayout.Third>
</Fragment>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {trackAnalytics} from 'sentry/utils/analytics';
import {useLocation} from 'sentry/utils/useLocation';
import {useNavigate} from 'sentry/utils/useNavigate';
import useOrganization from 'sentry/utils/useOrganization';
import {getResourceTypeFilter} from 'sentry/views/insights/browser/common/queries/useResourcesQuery';
import RenderBlockingSelector from 'sentry/views/insights/browser/resources/components/renderBlockingSelector';
import ResourceTable from 'sentry/views/insights/browser/resources/components/tables/resourceTable';
import {
Expand All @@ -24,16 +23,13 @@ import {
import {useResourceSort} from 'sentry/views/insights/browser/resources/utils/useResourceSort';
import {QueryParameterNames} from 'sentry/views/insights/common/views/queryParameters';
import {TransactionSelector} from 'sentry/views/insights/common/views/spans/selectors/transactionSelector';
import type {ModuleFilters} from 'sentry/views/insights/common/views/spans/types';

import {ResourceLandingPageCharts} from './charts/resourceLandingPageCharts';

const {
SPAN_OP: RESOURCE_TYPE,
SPAN_DOMAIN,
TRANSACTION,
RESOURCE_RENDER_BLOCKING_STATUS,
USER_GEO_SUBREGION,
} = BrowserStarfishFields;

type Option = {
Expand All @@ -45,25 +41,10 @@ function ResourceView() {
const filters = useResourceModuleFilters();
const sort = useResourceSort();

const spanTimeChartsFilters: ModuleFilters = {
'span.op': `[${DEFAULT_RESOURCE_TYPES.join(',')}]`,
...(filters[SPAN_DOMAIN] ? {[SPAN_DOMAIN]: filters[SPAN_DOMAIN]} : {}),
};

const extraQuery = [
...getResourceTypeFilter(undefined, DEFAULT_RESOURCE_TYPES),
...(filters[USER_GEO_SUBREGION]
? [`user.geo.subregion:[${filters[USER_GEO_SUBREGION].join(',')}]`]
: []),
];

return (
<Fragment>
<SpanTimeChartsContainer>
<ResourceLandingPageCharts
appliedFilters={spanTimeChartsFilters}
extraQuery={extraQuery}
Comment on lines -64 to -65
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved these into the new widget

/>
<ResourceLandingPageCharts />
</SpanTimeChartsContainer>

<DropdownContainer>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ function ResourceSummary() {
</ModuleLayout.Full>
)}

<ResourceSummaryCharts groupId={groupId!} />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved into widget, groupId comes from URL params - currently drawer is directly tied to the chart/page where it was opened, so i think it's fine to have the widget grab this from the URL/router. if there was some world where we would render these charts separate independently, then we'd need to refactor to allow a way to specify the required properties that we'd need to add to the URL for linking and for rendering.

<ResourceSummaryCharts />

<ModuleLayout.Full>
<ResourceSummaryTable />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import type {PageFilters} from 'sentry/types/core';
import {EMPTY_OPTION_VALUE, MutableSearch} from 'sentry/utils/tokenizeSearch';
import {getResourceTypeFilter} from 'sentry/views/insights/browser/common/queries/useResourcesQuery';
import {DEFAULT_RESOURCE_TYPES} from 'sentry/views/insights/browser/resources/settings';
import {
BrowserStarfishFields,
useResourceModuleFilters,
} from 'sentry/views/insights/browser/resources/utils/useResourceFilters';
import {useSpanMetricsSeries} from 'sentry/views/insights/common/queries/useDiscoverSeries';
import type {ModuleFilters} from 'sentry/views/insights/common/views/spans/types';
import {SpanMetricsField} from 'sentry/views/insights/types';

const {SPAN_SELF_TIME, NORMALIZED_DESCRIPTION, SPAN_DOMAIN} = SpanMetricsField;
const {SPAN_DOMAIN: STARFISH_SPAN_DOMAIN, USER_GEO_SUBREGION} = BrowserStarfishFields;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

had to alias BrowserStarfishFields.SPAN_DOMAIN since we moved these down into the new hook and now there are 2 SPAN_DOMAINs in scope.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BrowserStarfishFields is in the process of being deprecated, could we just grab all of these fields from SpanMetricsFeild ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DominikB2014 Oops, should have looked into the enums... updated!


const SPAN_FILTER_KEYS = ['span_operation', SPAN_DOMAIN, 'action'];

const buildDiscoverQueryConditions = (appliedFilters: ModuleFilters) => {
const result = Object.keys(appliedFilters)
.filter(key => SPAN_FILTER_KEYS.includes(key))
// @ts-expect-error TS(7053): Element implicitly has an 'any' type because expre... Remove this comment to see the full error message
.filter(key => Boolean(appliedFilters[key]))
.map(key => {
// @ts-expect-error TS(7053): Element implicitly has an 'any' type because expre... Remove this comment to see the full error message
const value = appliedFilters[key];
if (key === SPAN_DOMAIN && value === EMPTY_OPTION_VALUE) {
return [`!has:${SPAN_DOMAIN}`];
}
return `${key}:${value}`;
});

result.push(`has:${NORMALIZED_DESCRIPTION}`);

return result.join(' ');
};

interface Props {
pageFilters?: PageFilters;
}

export function useResourceLandingSeries(props?: Props) {
const filters = useResourceModuleFilters();

const spanTimeChartsFilters: ModuleFilters = {
'span.op': `[${DEFAULT_RESOURCE_TYPES.join(',')}]`,
...(filters[STARFISH_SPAN_DOMAIN]
? {[STARFISH_SPAN_DOMAIN]: filters[STARFISH_SPAN_DOMAIN]}
: {}),
};

const extraQuery = [
...getResourceTypeFilter(undefined, DEFAULT_RESOURCE_TYPES),
...(filters[USER_GEO_SUBREGION]
? [`user.geo.subregion:[${filters[USER_GEO_SUBREGION].join(',')}]`]
: []),
];

let query: string = buildDiscoverQueryConditions(spanTimeChartsFilters);

if (extraQuery) {
query += ` ${extraQuery.join(' ')}`;
}

return useSpanMetricsSeries(
{
search: new MutableSearch(query),
yAxis: ['epm()', `avg(${SPAN_SELF_TIME})`],
transformAliasToInputFormat: true,
},
'api.starfish.span-time-charts',
props.pageFilters

Check failure on line 71 in static/app/views/insights/common/components/widgets/hooks/useResourceLandingSeries.tsx

View workflow job for this annotation

GitHub Actions / self-hosted

'props' is possibly 'undefined'.

Check failure on line 71 in static/app/views/insights/common/components/widgets/hooks/useResourceLandingSeries.tsx

View workflow job for this annotation

GitHub Actions / typescript

'props' is possibly 'undefined'.
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import type {PageFilters} from 'sentry/types/core';
import {MutableSearch} from 'sentry/utils/tokenizeSearch';
import {Referrer} from 'sentry/views/insights/browser/resources/referrer';
import {useResourceModuleFilters} from 'sentry/views/insights/browser/resources/utils/useResourceFilters';
import {useSpanMetricsSeries} from 'sentry/views/insights/common/queries/useDiscoverSeries';
import {SpanMetricsField} from 'sentry/views/insights/types';

const {
SPAN_SELF_TIME,
HTTP_RESPONSE_CONTENT_LENGTH,
HTTP_DECODED_RESPONSE_CONTENT_LENGTH,
HTTP_RESPONSE_TRANSFER_SIZE,
RESOURCE_RENDER_BLOCKING_STATUS,
} = SpanMetricsField;

interface Props {
groupId?: string;
pageFilters?: PageFilters;
}

export function useResourceSummarySeries({pageFilters, groupId}: Props) {
const filters = useResourceModuleFilters();

const mutableSearch = MutableSearch.fromQueryObject({
'span.group': groupId,
...(filters[RESOURCE_RENDER_BLOCKING_STATUS]
? {
[RESOURCE_RENDER_BLOCKING_STATUS]: filters[RESOURCE_RENDER_BLOCKING_STATUS],
}
: {}),
...(filters[SpanMetricsField.USER_GEO_SUBREGION]
? {
[SpanMetricsField.USER_GEO_SUBREGION]: `[${filters[SpanMetricsField.USER_GEO_SUBREGION].join(',')}]`,
}
: {}),
});

return useSpanMetricsSeries(
{
search: mutableSearch,
yAxis: [
`epm()`,
`avg(${SPAN_SELF_TIME})`,
`avg(${HTTP_RESPONSE_CONTENT_LENGTH})`,
`avg(${HTTP_DECODED_RESPONSE_CONTENT_LENGTH})`,
`avg(${HTTP_RESPONSE_TRANSFER_SIZE})`,
],
enabled: Boolean(groupId),
transformAliasToInputFormat: true,
},
Referrer.RESOURCE_SUMMARY_CHARTS,
pageFilters
);
}
Loading
Loading