From c8d2b08a4a17509a9b211c1a73f47c0ddd530916 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Fri, 7 Nov 2025 16:23:56 -0500 Subject: [PATCH] feat(tracemetrics): Add metric to aggregate params --- .../hooks/useMetricAggregatesTable.tsx | 18 +++++++--- .../metricToolbar/aggregateDropdown.tsx | 8 ++--- .../metrics/multiMetricsQueryParams.spec.tsx | 26 ++++++++------- .../metrics/multiMetricsQueryParams.tsx | 29 +++++++--------- static/app/views/explore/metrics/utils.tsx | 33 +++++++++++++++++++ 5 files changed, 73 insertions(+), 41 deletions(-) diff --git a/static/app/views/explore/metrics/hooks/useMetricAggregatesTable.tsx b/static/app/views/explore/metrics/hooks/useMetricAggregatesTable.tsx index a831312529d1a0..0e4d5c27c00908 100644 --- a/static/app/views/explore/metrics/hooks/useMetricAggregatesTable.tsx +++ b/static/app/views/explore/metrics/hooks/useMetricAggregatesTable.tsx @@ -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, @@ -33,7 +34,13 @@ interface MetricAggregatesTableResult { result: ReturnType>; } -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, @@ -43,14 +50,15 @@ export function useMetricAggregatesTable({ }: UseMetricAggregatesTableOptions) { const canTriggerHighAccuracy = useCallback( (result: ReturnType['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({ queryHookImplementation: useMetricAggregatesTableImp, @@ -103,7 +111,7 @@ 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, @@ -111,7 +119,7 @@ function useMetricAggregatesTableImp({ }; 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, diff --git a/static/app/views/explore/metrics/metricToolbar/aggregateDropdown.tsx b/static/app/views/explore/metrics/metricToolbar/aggregateDropdown.tsx index 1f184628d31544..866d3a22e46a6c 100644 --- a/static/app/views/explore/metrics/metricToolbar/aggregateDropdown.tsx +++ b/static/app/views/explore/metrics/metricToolbar/aggregateDropdown.tsx @@ -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(); @@ -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)); }} /> ); diff --git a/static/app/views/explore/metrics/multiMetricsQueryParams.spec.tsx b/static/app/views/explore/metrics/multiMetricsQueryParams.spec.tsx index e88466d6c84d8c..b581af5b310273 100644 --- a/static/app/views/explore/metrics/multiMetricsQueryParams.spec.tsx +++ b/static/app/views/explore/metrics/multiMetricsQueryParams.spec.tsx @@ -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,-)')], }), }), ]); @@ -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,-)')], }), }), ]); @@ -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,-)'), + ], }), }), ]); @@ -88,7 +90,7 @@ describe('MultiMetricsQueryParamsProvider', () => { act(() => result.current[0]!.setQueryParams( result.current[0]!.queryParams.replace({ - aggregateFields: [new VisualizeFunction('sum(value)')], + aggregateFields: [new VisualizeFunction('sum(value,foo,counter,-)')], }) ) ); @@ -96,7 +98,7 @@ describe('MultiMetricsQueryParamsProvider', () => { expect.objectContaining({ metric: {name: 'foo', type: 'counter'}, queryParams: expect.objectContaining({ - aggregateFields: [new VisualizeFunction('sum(value)')], + aggregateFields: [new VisualizeFunction('sum(value,foo,counter,-)')], }), }), ]); @@ -106,7 +108,7 @@ describe('MultiMetricsQueryParamsProvider', () => { expect.objectContaining({ metric: {name: 'qux', type: 'gauge'}, queryParams: expect.objectContaining({ - aggregateFields: [new VisualizeFunction('avg(value)')], + aggregateFields: [new VisualizeFunction('avg(value,qux,gauge,-)')], }), }), ]); @@ -114,7 +116,7 @@ describe('MultiMetricsQueryParamsProvider', () => { act(() => result.current[0]!.setQueryParams( result.current[0]!.queryParams.replace({ - aggregateFields: [new VisualizeFunction('last(value)')], + aggregateFields: [new VisualizeFunction('last(value,qux,gauge,-)')], }) ) ); @@ -122,7 +124,7 @@ describe('MultiMetricsQueryParamsProvider', () => { expect.objectContaining({ metric: {name: 'qux', type: 'gauge'}, queryParams: expect.objectContaining({ - aggregateFields: [new VisualizeFunction('last(value)')], + aggregateFields: [new VisualizeFunction('last(value,qux,gauge,-)')], }), }), ]); @@ -132,7 +134,7 @@ describe('MultiMetricsQueryParamsProvider', () => { expect.objectContaining({ metric: {name: 'bar', type: 'distribution'}, queryParams: expect.objectContaining({ - aggregateFields: [new VisualizeFunction('p75(value)')], + aggregateFields: [new VisualizeFunction('p75(value,bar,distribution,-)')], }), }), ]); @@ -140,7 +142,7 @@ describe('MultiMetricsQueryParamsProvider', () => { act(() => result.current[0]!.setQueryParams( result.current[0]!.queryParams.replace({ - aggregateFields: [new VisualizeFunction('p99(value)')], + aggregateFields: [new VisualizeFunction('p99(value,bar,distribution,-)')], }) ) ); @@ -148,7 +150,7 @@ describe('MultiMetricsQueryParamsProvider', () => { expect.objectContaining({ metric: {name: 'bar', type: 'distribution'}, queryParams: expect.objectContaining({ - aggregateFields: [new VisualizeFunction('p99(value)')], + aggregateFields: [new VisualizeFunction('p99(value,bar,distribution,-)')], }), }), ]); @@ -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,-)')], }), }), ]); diff --git a/static/app/views/explore/metrics/multiMetricsQueryParams.tsx b/static/app/views/explore/metrics/multiMetricsQueryParams.tsx index f10c69a3a658b0..960ff5840276f2 100644 --- a/static/app/views/explore/metrics/multiMetricsQueryParams.tsx +++ b/static/app/views/explore/metrics/multiMetricsQueryParams.tsx @@ -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[]; @@ -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), ]; } diff --git a/static/app/views/explore/metrics/utils.tsx b/static/app/views/explore/metrics/utils.tsx index 216ad4446bf62a..8e0d492211fe2f 100644 --- a/static/app/views/explore/metrics/utils.tsx +++ b/static/app/views/explore/metrics/utils.tsx @@ -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'; @@ -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, + }); +}