Skip to content

feat(tracemetrics): Add metric to aggregate params #102991

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 1 commit into from
Nov 10, 2025
Merged

feat(tracemetrics): Add metric to aggregate params #102991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 13 additions & 5 deletions static/app/views/explore/metrics/hooks/useMetricAggregatesTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
import type {TraceMetric} from 'sentry/views/explore/metrics/metricQuery';
import {useMetricVisualize} from 'sentry/views/explore/metrics/metricsQueryParams';
import {TraceMetricKnownFieldKey} from 'sentry/views/explore/metrics/types';
import {makeMetricsAggregate} from 'sentry/views/explore/metrics/utils';
import {
useQueryParamsAggregateSortBys,
useQueryParamsGroupBys,
Expand All @@ -33,7 +34,13 @@ interface MetricAggregatesTableResult {
result: ReturnType<typeof useSpansQuery<any[]>>;
}

const COUNT_AGGREGATE = `count(${TraceMetricKnownFieldKey.METRIC_NAME})`;
function makeCountAggregate(traceMetric: TraceMetric): string {
return makeMetricsAggregate({
aggregate: 'count',
traceMetric,
attribute: TraceMetricKnownFieldKey.METRIC_NAME,
});
}

export function useMetricAggregatesTable({
enabled,
Expand All @@ -43,14 +50,15 @@ export function useMetricAggregatesTable({
}: UseMetricAggregatesTableOptions) {
const canTriggerHighAccuracy = useCallback(
(result: ReturnType<typeof useMetricAggregatesTableImp>['result']) => {
const countAggregate = makeCountAggregate(traceMetric);
const canGoToHigherAccuracyTier = result.meta?.dataScanned === 'partial';
const hasData =
defined(result.data) &&
(result.data.length > 1 ||
(result.data.length === 1 && Boolean(result.data[0][COUNT_AGGREGATE])));
(result.data.length === 1 && Boolean(result.data[0][countAggregate])));
return !hasData && canGoToHigherAccuracyTier;
},
[]
[traceMetric]
);
return useProgressiveQuery<typeof useMetricAggregatesTableImp>({
queryHookImplementation: useMetricAggregatesTableImp,
Expand Down Expand Up @@ -103,15 +111,15 @@ function useMetricAggregatesTableImp({
const discoverQuery: NewQuery = {
id: undefined,
name: 'Explore - Metric Aggregates',
fields: [...fields, COUNT_AGGREGATE],
fields: [...fields, makeCountAggregate(traceMetric)],
orderby: sortBys.map(formatSort),
query,
version: 2,
dataset: DiscoverDatasets.TRACEMETRICS,
};

return EventView.fromNewQueryWithPageFilters(discoverQuery, selection);
}, [fields, query, selection, sortBys]);
}, [fields, query, selection, sortBys, traceMetric]);

const result = useSpansQuery({
enabled: enabled && Boolean(traceMetric.name) && fields.length > 0,
Expand Down
8 changes: 2 additions & 6 deletions static/app/views/explore/metrics/metricToolbar/aggregateDropdown.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
useMetricVisualize,
useSetMetricVisualize,
} from 'sentry/views/explore/metrics/metricsQueryParams';
import {updateVisualizeYAxis} from 'sentry/views/explore/metrics/utils';

export function AggregateDropdown({traceMetric}: {traceMetric: TraceMetric}) {
const visualize = useMetricVisualize();
Expand All @@ -19,12 +20,7 @@ export function AggregateDropdown({traceMetric}: {traceMetric: TraceMetric}) {
options={OPTIONS_BY_TYPE[traceMetric.type] ?? []}
value={visualize.parsedFunction?.name ?? ''}
onChange={option => {
setVisualize(
visualize.replace({
yAxis: `${option.value}(value)`,
chartType: undefined, // Reset chart type to let determineDefaultChartType decide
})
);
setVisualize(updateVisualizeYAxis(visualize, option.value, traceMetric));
}}
/>
);
Expand Down
26 changes: 14 additions & 12 deletions static/app/views/explore/metrics/multiMetricsQueryParams.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe('MultiMetricsQueryParamsProvider', () => {
expect.objectContaining({
metric: {name: 'foo', type: 'counter'},
queryParams: expect.objectContaining({
aggregateFields: [new VisualizeFunction('per_second(value)')],
aggregateFields: [new VisualizeFunction('per_second(value,foo,counter,-)')],
}),
}),
]);
Expand All @@ -63,7 +63,7 @@ describe('MultiMetricsQueryParamsProvider', () => {
expect.objectContaining({
metric: {name: 'bar', type: 'gauge'},
queryParams: expect.objectContaining({
aggregateFields: [new VisualizeFunction('per_second(value)')],
aggregateFields: [new VisualizeFunction('per_second(value,bar,gauge,-)')],
}),
}),
]);
Expand All @@ -73,7 +73,9 @@ describe('MultiMetricsQueryParamsProvider', () => {
expect.objectContaining({
metric: {name: 'qux', type: 'distribution'},
queryParams: expect.objectContaining({
aggregateFields: [new VisualizeFunction('per_second(value)')],
aggregateFields: [
new VisualizeFunction('per_second(value,qux,distribution,-)'),
],
}),
}),
]);
Expand All @@ -88,15 +90,15 @@ describe('MultiMetricsQueryParamsProvider', () => {
act(() =>
result.current[0]!.setQueryParams(
result.current[0]!.queryParams.replace({
aggregateFields: [new VisualizeFunction('sum(value)')],
aggregateFields: [new VisualizeFunction('sum(value,foo,counter,-)')],
})
)
);
expect(result.current).toEqual([
expect.objectContaining({
metric: {name: 'foo', type: 'counter'},
queryParams: expect.objectContaining({
aggregateFields: [new VisualizeFunction('sum(value)')],
aggregateFields: [new VisualizeFunction('sum(value,foo,counter,-)')],
}),
}),
]);
Expand All @@ -106,23 +108,23 @@ describe('MultiMetricsQueryParamsProvider', () => {
expect.objectContaining({
metric: {name: 'qux', type: 'gauge'},
queryParams: expect.objectContaining({
aggregateFields: [new VisualizeFunction('avg(value)')],
aggregateFields: [new VisualizeFunction('avg(value,qux,gauge,-)')],
}),
}),
]);

act(() =>
result.current[0]!.setQueryParams(
result.current[0]!.queryParams.replace({
aggregateFields: [new VisualizeFunction('last(value)')],
aggregateFields: [new VisualizeFunction('last(value,qux,gauge,-)')],
})
)
);
expect(result.current).toEqual([
expect.objectContaining({
metric: {name: 'qux', type: 'gauge'},
queryParams: expect.objectContaining({
aggregateFields: [new VisualizeFunction('last(value)')],
aggregateFields: [new VisualizeFunction('last(value,qux,gauge,-)')],
}),
}),
]);
Expand All @@ -132,23 +134,23 @@ describe('MultiMetricsQueryParamsProvider', () => {
expect.objectContaining({
metric: {name: 'bar', type: 'distribution'},
queryParams: expect.objectContaining({
aggregateFields: [new VisualizeFunction('p75(value)')],
aggregateFields: [new VisualizeFunction('p75(value,bar,distribution,-)')],
}),
}),
]);

act(() =>
result.current[0]!.setQueryParams(
result.current[0]!.queryParams.replace({
aggregateFields: [new VisualizeFunction('p99(value)')],
aggregateFields: [new VisualizeFunction('p99(value,bar,distribution,-)')],
})
)
);
expect(result.current).toEqual([
expect.objectContaining({
metric: {name: 'bar', type: 'distribution'},
queryParams: expect.objectContaining({
aggregateFields: [new VisualizeFunction('p99(value)')],
aggregateFields: [new VisualizeFunction('p99(value,bar,distribution,-)')],
}),
}),
]);
Expand All @@ -158,7 +160,7 @@ describe('MultiMetricsQueryParamsProvider', () => {
expect.objectContaining({
metric: {name: 'foo', type: 'counter'},
queryParams: expect.objectContaining({
aggregateFields: [new VisualizeFunction('per_second(value)')],
aggregateFields: [new VisualizeFunction('per_second(value,foo,counter,-)')],
}),
}),
]);
Expand Down
29 changes: 11 additions & 18 deletions static/app/views/explore/metrics/multiMetricsQueryParams.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,10 @@ import {
type MetricQuery,
type TraceMetric,
} from 'sentry/views/explore/metrics/metricQuery';
import {updateVisualizeYAxis} from 'sentry/views/explore/metrics/utils';
import {isGroupBy} from 'sentry/views/explore/queryParams/groupBy';
import type {ReadableQueryParams} from 'sentry/views/explore/queryParams/readableQueryParams';
import {
isVisualizeFunction,
VisualizeFunction,
} from 'sentry/views/explore/queryParams/visualize';
import {isVisualizeFunction} from 'sentry/views/explore/queryParams/visualize';

interface MultiMetricsQueryParamsContextValue {
metricQueries: MetricQuery[];
Expand Down Expand Up @@ -93,25 +91,20 @@ export function MultiMetricsQueryParamsProvider({
const allowedAggregations = OPTIONS_BY_TYPE[newTraceMetric.type];

if (
!allowedAggregations?.find(option => option.value === selectedAggregation)
selectedAggregation &&
allowedAggregations?.find(option => option.value === selectedAggregation)
) {
// the currently selected aggregation isn't supported on the new metric
const defaultAggregation =
DEFAULT_YAXIS_BY_TYPE[newTraceMetric.type] || 'per_second';
// the currently selected aggregation changed types
aggregateFields = [
new VisualizeFunction(`${defaultAggregation}(value)`),
updateVisualizeYAxis(visualize, selectedAggregation, newTraceMetric),
...metricQuery.queryParams.aggregateFields.filter(isGroupBy),
];
} else if (
selectedAggregation === 'per_second' ||
selectedAggregation === 'per_minute'
) {
// TODO: this else if branch can go away once the metric type is lifted
// to the top level

// the currently selected aggregation changed types
} else {
// the currently selected aggregation isn't supported on the new metric
const defaultAggregation =
DEFAULT_YAXIS_BY_TYPE[newTraceMetric.type] || 'per_second';
aggregateFields = [
new VisualizeFunction(`${selectedAggregation}(value)`),
updateVisualizeYAxis(visualize, defaultAggregation, newTraceMetric),
...metricQuery.queryParams.aggregateFields.filter(isGroupBy),
];
}
Expand Down
33 changes: 33 additions & 0 deletions static/app/views/explore/metrics/utils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
type SampleTableColumnKey,
} from 'sentry/views/explore/metrics/types';
import {isGroupBy, type GroupBy} from 'sentry/views/explore/queryParams/groupBy';
import type {VisualizeFunction} from 'sentry/views/explore/queryParams/visualize';
import {Visualize} from 'sentry/views/explore/queryParams/visualize';
import type {PickableDays} from 'sentry/views/explore/utils';

Expand Down Expand Up @@ -199,3 +200,35 @@ export function getMetricTableColumnType(
}
return 'value';
}

export function makeMetricsAggregate({
aggregate,
traceMetric,
attribute,
}: {
aggregate: string;
traceMetric: TraceMetric;
attribute?: string;
}) {
const args = [
attribute ?? 'value', // hard coded to `value` for now, but can be other attributes
traceMetric.name,
traceMetric.type,
'-', // hard coded to `-` for now, but can be other units`
];
return `${aggregate}(${args.join(',')})`;
}

export function updateVisualizeYAxis(
visualize: VisualizeFunction,
aggregate: string,
traceMetric: TraceMetric
): VisualizeFunction {
return visualize.replace({
yAxis: makeMetricsAggregate({
aggregate,
traceMetric,
}),
chartType: undefined,
});
}
Copy link
Contributor

@cursor cursor bot Nov 7, 2025

Choose a reason for hiding this comment

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

Bug: Chart Type Persists Incorrectly

The updateVisualizeYAxis function passes chartType: undefined to visualize.replace(), but the replace method uses nullish coalescing (??) which falls back to this.selectedChartType when chartType is undefined. This prevents resetting the chart type to auto-determine based on the new aggregate function, causing the user's previously selected chart type to persist inappropriately when changing metrics or aggregations.

Fix in Cursor Fix in Web

Loading