Skip to content

ref(insights): Refactor PerformanceScoreBreakdownChart #89632

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.

Sign up for GitHub

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 4 commits into from
Apr 17, 2025
Merged

ref(insights): Refactor PerformanceScoreBreakdownChart #89632

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
86aab8c
ref(insights): Refactor `PerformanceScoreBreakdownChart`
billyvg Apr 15, 2025
d86320c
remove extra props
billyvg Apr 15, 2025
d34e313
Merge branch 'master' into ref-insights-combine-web-vitals-widgets
gggritso Apr 17, 2025
7faa699
Move some files around
gggritso Apr 17, 2025
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
114 changes: 3 additions & 111 deletions ...app/views/insights/browser/webVitals/components/charts/performanceScoreBreakdownChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,10 @@
import {useTheme} from '@emotion/react';
import styled from '@emotion/styled';

import {t} from 'sentry/locale';
import type {Series} from 'sentry/types/echarts';
import {MutableSearch} from 'sentry/utils/tokenizeSearch';
import {ORDER} from 'sentry/views/insights/browser/webVitals/components/charts/performanceScoreChart';
import type {WebVitalsScoreBreakdown} from 'sentry/views/insights/browser/webVitals/queries/storedScoreQueries/useProjectWebVitalsScoresTimeseriesQuery';
import type {WebVitals} from 'sentry/views/insights/browser/webVitals/types';
import {getWeights} from 'sentry/views/insights/browser/webVitals/utils/getWeights';
import type {BrowserType} from 'sentry/views/insights/browser/webVitals/utils/queryParameterDecoders/browserType';
import {useDefaultWebVitalsQuery} from 'sentry/views/insights/browser/webVitals/utils/useDefaultQuery';
import {InsightsTimeSeriesWidget} from 'sentry/views/insights/common/components/insightsTimeSeriesWidget';
import {
type DiscoverSeries,
useMetricsSeries,
} from 'sentry/views/insights/common/queries/useDiscoverSeries';
import {SpanMetricsField, type SubregionCode} from 'sentry/views/insights/types';

import {WebVitalsWeightList} from './webVitalWeightList';

type Props = {
browserTypes?: BrowserType[];
subregions?: SubregionCode[];
transaction?: string;
};
import {PerformanceScoreBreakdownChartWidget} from 'sentry/views/insights/common/components/widgets/performanceScoreBreakdownChartWidget';

export function formatTimeSeriesResultsToChartData(
data: WebVitalsScoreBreakdown,
Expand All @@ -44,99 +25,10 @@ export function formatTimeSeriesResultsToChartData(
});
}

export function PerformanceScoreBreakdownChart({
transaction,
browserTypes,
subregions,
}: Props) {
const theme = useTheme();
const segmentColors = theme.chart.getColorPalette(3).slice(0, 5);
const defaultQuery = useDefaultWebVitalsQuery();

const search = new MutableSearch(`${defaultQuery} has:measurements.score.total`);

if (transaction) {
search.addFilterValue('transaction', transaction);
}

if (subregions) {
search.addDisjunctionFilterValues(SpanMetricsField.USER_GEO_SUBREGION, subregions);
}

if (browserTypes) {
search.addDisjunctionFilterValues(SpanMetricsField.BROWSER_NAME, browserTypes);
}

const {
data: vitalScoresData,
isLoading: areVitalScoresLoading,
error: vitalScoresError,
} = useMetricsSeries(
{
search,
yAxis: [
'performance_score(measurements.score.lcp)',
'performance_score(measurements.score.fcp)',
'performance_score(measurements.score.cls)',
'performance_score(measurements.score.inp)',
'performance_score(measurements.score.ttfb)',
'count()',
],
transformAliasToInputFormat: true,
},
'api.performance.browser.web-vitals.timeseries-scores2'
);

const webVitalsThatHaveData: WebVitals[] = vitalScoresData
? ORDER.filter(webVital => {
const key = `performance_score(measurements.score.${webVital})` as const;
const series = vitalScoresData[key];

return series.data.some(datum => datum.value > 0);
})
: [];

const weights = getWeights(webVitalsThatHaveData);

const allSeries: DiscoverSeries[] = vitalScoresData
? ORDER.map((webVital, index) => {
const key = `performance_score(measurements.score.${webVital})` as const;
const series = vitalScoresData[key];

const scaledSeries: DiscoverSeries = {
...series,
data: series.data.map(datum => {
return {
...datum,
value: datum.value * weights[webVital],
};
}),
color: segmentColors[index],
meta: {
// TODO: The backend doesn't return these score fields with the "score" type yet. Fill this in manually for now.
fields: {
...series.meta?.fields,
[key]: 'score',
},
units: series.meta?.units,
},
};

return scaledSeries;
})
: [];

export function PerformanceScoreBreakdownChart() {
return (
<ChartContainer>
<InsightsTimeSeriesWidget
title={t('Score Breakdown')}
height="100%"
visualizationType="area"
isLoading={areVitalScoresLoading}
error={vitalScoresError}
series={allSeries}
description={<WebVitalsWeightList weights={weights} />}
/>
<PerformanceScoreBreakdownChartWidget />
</ChartContainer>
);
}
Expand Down
6 changes: 1 addition & 5 deletions static/app/views/insights/browser/webVitals/views/pageOverview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,11 +196,7 @@ export function PageOverview() {
<BrowserTypeSelector />
</TopMenuContainer>
<Flex>
<PerformanceScoreBreakdownChart
transaction={transaction}
browserTypes={browserTypes}
subregions={subregions}
/>
<PerformanceScoreBreakdownChart />
</Flex>
<WebVitalMetersContainer>
{(isPending || isProjectScoresLoading) && <WebVitalMetersPlaceholder />}
Expand Down
121 changes: 121 additions & 0 deletions static/app/views/insights/common/components/widgets/performanceScoreBreakdownChartWidget.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import {useTheme} from '@emotion/react';

import {t} from 'sentry/locale';
import {decodeList} from 'sentry/utils/queryString';
import {MutableSearch} from 'sentry/utils/tokenizeSearch';
import useLocationQuery from 'sentry/utils/url/useLocationQuery';
import {ORDER} from 'sentry/views/insights/browser/webVitals/components/charts/performanceScoreChart';
import {WebVitalsWeightList} from 'sentry/views/insights/browser/webVitals/components/charts/webVitalWeightList';
import type {WebVitals} from 'sentry/views/insights/browser/webVitals/types';
import {getWeights} from 'sentry/views/insights/browser/webVitals/utils/getWeights';
import decodeBrowserTypes from 'sentry/views/insights/browser/webVitals/utils/queryParameterDecoders/browserType';
import {useDefaultWebVitalsQuery} from 'sentry/views/insights/browser/webVitals/utils/useDefaultQuery';
import {InsightsTimeSeriesWidget} from 'sentry/views/insights/common/components/insightsTimeSeriesWidget';
import type {LoadableChartWidgetProps} from 'sentry/views/insights/common/components/widgets/types';
import {
type DiscoverSeries,
useMetricsSeries,
} from 'sentry/views/insights/common/queries/useDiscoverSeries';
import {SpanIndexedField, SpanMetricsField} from 'sentry/views/insights/types';

export function PerformanceScoreBreakdownChartWidget(props: LoadableChartWidgetProps) {
const {
transaction,
[SpanIndexedField.BROWSER_NAME]: browserTypes,
[SpanIndexedField.USER_GEO_SUBREGION]: subregions,
} = useLocationQuery({
fields: {
[SpanIndexedField.BROWSER_NAME]: decodeBrowserTypes,
[SpanIndexedField.USER_GEO_SUBREGION]: decodeList,
transaction: decodeList,
Copy link

Choose a reason for hiding this comment

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

This new component could benefit from proper TypeScript interfaces for better type safety and documentation. Consider extracting the type definition for the decoded query parameters into a separate interface. Additionally, the useLocationQuery hook's fields object could be moved to a constant to avoid recreating it on every render.

Suggested change
import {SpanIndexedField, SpanMetricsField} from 'sentry/views/insights/types';
export function PerformanceScoreBreakdownChartWidget(props: LoadableChartWidgetProps) {
const {
transaction,
[SpanIndexedField.BROWSER_NAME]: browserTypes,
[SpanIndexedField.USER_GEO_SUBREGION]: subregions,
} = useLocationQuery({
fields: {
[SpanIndexedField.BROWSER_NAME]: decodeBrowserTypes,
[SpanIndexedField.USER_GEO_SUBREGION]: decodeList,
transaction: decodeList,
interface QueryParams {
[SpanIndexedField.BROWSER_NAME]: ReturnType<typeof decodeBrowserTypes>;
[SpanIndexedField.USER_GEO_SUBREGION]: string[];
transaction: string[];
}
const QUERY_FIELDS = {
[SpanIndexedField.BROWSER_NAME]: decodeBrowserTypes,
[SpanIndexedField.USER_GEO_SUBREGION]: decodeList,
transaction: decodeList,
};
export function PerformanceScoreBreakdownChartWidget(props: LoadableChartWidgetProps) {
const {
transaction,
[SpanIndexedField.BROWSER_NAME]: browserTypes,
[SpanIndexedField.USER_GEO_SUBREGION]: subregions,
} = useLocationQuery<QueryParams>({
fields: QUERY_FIELDS,
});

},
});
Copy link
Member Author

Choose a reason for hiding this comment

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

I updated this to use useLocationQuery

const theme = useTheme();
const segmentColors = theme.chart.getColorPalette(3).slice(0, 5);
const defaultQuery = useDefaultWebVitalsQuery();
const search = new MutableSearch(`${defaultQuery} has:measurements.score.total`);

if (transaction.length && transaction[0]) {
search.addFilterValue('transaction', transaction[0]);
}

if (subregions) {
search.addDisjunctionFilterValues(SpanMetricsField.USER_GEO_SUBREGION, subregions);
}

if (browserTypes) {
search.addDisjunctionFilterValues(SpanMetricsField.BROWSER_NAME, browserTypes);
}

const {
data: vitalScoresData,
isLoading: areVitalScoresLoading,
error: vitalScoresError,
} = useMetricsSeries(
{
search,
yAxis: [
'performance_score(measurements.score.lcp)',
'performance_score(measurements.score.fcp)',
'performance_score(measurements.score.cls)',
'performance_score(measurements.score.inp)',
'performance_score(measurements.score.ttfb)',
'count()',
],
transformAliasToInputFormat: true,
},
'api.performance.browser.web-vitals.timeseries-scores2',
props.pageFilters
);

const webVitalsThatHaveData: WebVitals[] = vitalScoresData
? ORDER.filter(webVital => {
const key = `performance_score(measurements.score.${webVital})` as const;
const series = vitalScoresData[key];

return series.data.some(datum => datum.value > 0);
})
: [];

const weights = getWeights(webVitalsThatHaveData);

const allSeries: DiscoverSeries[] = vitalScoresData
? ORDER.map((webVital, index) => {
const key = `performance_score(measurements.score.${webVital})` as const;
const series = vitalScoresData[key];

const scaledSeries: DiscoverSeries = {
...series,
data: series.data.map(datum => {
return {
...datum,
value: datum.value * weights[webVital],
};
}),
color: segmentColors[index],
meta: {
// TODO: The backend doesn't return these score fields with the "score" type yet. Fill this in manually for now.
fields: {
...series.meta?.fields,
[key]: 'score',
},
units: series.meta?.units,
},
};

Copy link

Choose a reason for hiding this comment

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

The series transformation logic is quite complex and could be extracted into a separate utility function for better maintainability and testability. Also consider adding memoization for the allSeries calculation to prevent unnecessary recalculations on re-renders.

Suggested change
const weights = getWeights(webVitalsThatHaveData);
const allSeries: DiscoverSeries[] = vitalScoresData
? ORDER.map((webVital, index) => {
const key = `performance_score(measurements.score.${webVital})` as const;
const series = vitalScoresData[key];
const scaledSeries: DiscoverSeries = {
...series,
data: series.data.map(datum => {
return {
...datum,
value: datum.value * weights[webVital],
};
}),
color: segmentColors[index],
meta: {
// TODO: The backend doesn't return these score fields with the "score" type yet. Fill this in manually for now.
fields: {
...series.meta?.fields,
[key]: 'score',
},
units: series.meta?.units,
},
};
const transformVitalScores = useCallback((vitalScoresData: any, segmentColors: string[], weights: Record<string, number>): DiscoverSeries[] => {
return ORDER.map((webVital, index) => {
const key = `performance_score(measurements.score.${webVital})` as const;
const series = vitalScoresData[key];
return {
...series,
data: series.data.map(datum => ({
...datum,
value: datum.value * weights[webVital],
})),
color: segmentColors[index],
meta: {
fields: {
...series.meta?.fields,
[key]: 'score',
},
units: series.meta?.units,
},
};
});
}, []);
const allSeries = useMemo(
() => (vitalScoresData ? transformVitalScores(vitalScoresData, segmentColors, weights) : []),
[vitalScoresData, segmentColors, weights, transformVitalScores]
);

return scaledSeries;
})
: [];

return (
<InsightsTimeSeriesWidget
title={t('Score Breakdown')}
height="100%"
visualizationType="area"
isLoading={areVitalScoresLoading}
error={vitalScoresError}
series={allSeries}
description={<WebVitalsWeightList weights={weights} />}
/>
);
}
6 changes: 4 additions & 2 deletions static/app/views/insights/common/queries/useDiscoverSeries.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,15 @@ export const useEAPSeries = <

export const useMetricsSeries = <Fields extends MetricsProperty[]>(
options: UseMetricsSeriesOptions<Fields> = {},
referrer: string
referrer: string,
pageFilters?: PageFilters
) => {
const useEap = useInsightsEap();
return useDiscoverSeries<Fields>(
options,
useEap ? DiscoverDatasets.SPANS_EAP_RPC : DiscoverDatasets.METRICS,
referrer
referrer,
pageFilters
);
};

Expand Down
Loading