From 7452d3c82e5433dad2d10f834c04a522d0164c6b Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Wed, 24 Jun 2026 12:44:13 -0400 Subject: [PATCH 1/2] fix(tracemetrics): Equations dropping filters --- .../metricQueryRows.tsx | 4 +- .../metricToolbar.tsx | 6 +- .../forms/metric/metricsEquationVisualize.tsx | 10 +- .../explore/metrics/equationBuilder/index.tsx | 5 +- .../metrics/equationBuilder/utils.spec.tsx | 150 ++++++++++++++++++ .../explore/metrics/equationBuilder/utils.ts | 15 +- .../explore/metrics/metricPanel/index.tsx | 5 +- .../explore/metrics/metricToolbar/index.tsx | 8 +- .../metrics/multiMetricsQueryParams.spec.tsx | 6 +- .../views/explore/queryParams/visualize.ts | 23 +++ 10 files changed, 217 insertions(+), 15 deletions(-) diff --git a/static/app/views/dashboards/widgetBuilder/components/visualize/traceMetrics/metricsEquationVisualize/metricQueryRows.tsx b/static/app/views/dashboards/widgetBuilder/components/visualize/traceMetrics/metricsEquationVisualize/metricQueryRows.tsx index e22bf907e1aea5..24617c0bf395a8 100644 --- a/static/app/views/dashboards/widgetBuilder/components/visualize/traceMetrics/metricsEquationVisualize/metricQueryRows.tsx +++ b/static/app/views/dashboards/widgetBuilder/components/visualize/traceMetrics/metricsEquationVisualize/metricQueryRows.tsx @@ -46,7 +46,9 @@ function computeEquationReferencedLabels( return []; } const labelSet = new Set(Object.keys(referenceMap)); - const unresolvedText = unresolveExpression(visualize.expression.text, referenceMap); + const unresolvedText = + visualize.internalExpression ?? + unresolveExpression(visualize.expression.text, referenceMap); return extractReferenceLabels(new Expression(unresolvedText, labelSet)); } diff --git a/static/app/views/dashboards/widgetBuilder/components/visualize/traceMetrics/metricsEquationVisualize/metricToolbar.tsx b/static/app/views/dashboards/widgetBuilder/components/visualize/traceMetrics/metricsEquationVisualize/metricToolbar.tsx index 1b92d173907077..b4c3a08838a5c4 100644 --- a/static/app/views/dashboards/widgetBuilder/components/visualize/traceMetrics/metricsEquationVisualize/metricToolbar.tsx +++ b/static/app/views/dashboards/widgetBuilder/components/visualize/traceMetrics/metricsEquationVisualize/metricToolbar.tsx @@ -64,7 +64,10 @@ export function MetricToolbar({ ) => { if (isVisualizeEquation(visualize)) { setVisualize( - visualize.replace({yAxis: `${EQUATION_PREFIX}${resolvedExpression.text}`}) + visualize.replace({ + yAxis: `${EQUATION_PREFIX}${resolvedExpression.text}`, + internalExpression: internalText, + }) ); const labelSet = new Set(Object.keys(referenceMap)); const expr = new Expression(internalText, labelSet); @@ -118,6 +121,7 @@ export function MetricToolbar({ referenceMap={referenceMap} handleExpressionChange={handleExpressionChange} disabled={disabled} + storedInternalExpression={visualize.internalExpression} /> ) : null} diff --git a/static/app/views/detectors/components/forms/metric/metricsEquationVisualize.tsx b/static/app/views/detectors/components/forms/metric/metricsEquationVisualize.tsx index 879a620ba0ced3..a0b9734bc04e72 100644 --- a/static/app/views/detectors/components/forms/metric/metricsEquationVisualize.tsx +++ b/static/app/views/detectors/components/forms/metric/metricsEquationVisualize.tsx @@ -60,7 +60,9 @@ function computeEquationReferencedLabels( return []; } const labelSet = new Set(Object.keys(referenceMap)); - const unresolvedText = unresolveExpression(visualize.expression.text, referenceMap); + const unresolvedText = + visualize.internalExpression ?? + unresolveExpression(visualize.expression.text, referenceMap); return extractReferenceLabels(new Expression(unresolvedText, labelSet)); } @@ -366,7 +368,10 @@ function MetricToolbar({ ) => { if (isVisualizeEquation(visualize)) { setVisualize( - visualize.replace({yAxis: `${EQUATION_PREFIX}${resolvedExpression.text}`}) + visualize.replace({ + yAxis: `${EQUATION_PREFIX}${resolvedExpression.text}`, + internalExpression: internalText, + }) ); // Report the user's typed labels (pre-resolve) so identical rows don't // collapse the lock onto whichever label sorts first in the map. @@ -425,6 +430,7 @@ function MetricToolbar({ expression={visualize.expression.text} referenceMap={referenceMap} handleExpressionChange={handleExpressionChange} + storedInternalExpression={visualize.internalExpression} /> void; disabled?: boolean; onReferenceLabelsChange?: (labels: string[]) => void; referenceMap?: Record; + storedInternalExpression?: string; }) { const [_, startTransition] = useTransition(); const references = useMemo( @@ -35,7 +37,8 @@ export function EquationBuilder({ [referenceMap] ); - const internalExpression = unresolveExpression(expression, referenceMap); + const internalExpression = + storedInternalExpression ?? unresolveExpression(expression, referenceMap); // Report which labels this equation references after unresolving. // Cleans up on unmount so deleted equations don't block metric deletion. diff --git a/static/app/views/explore/metrics/equationBuilder/utils.spec.tsx b/static/app/views/explore/metrics/equationBuilder/utils.spec.tsx index 74b4cc89f5ace2..ce07eaca2b7e56 100644 --- a/static/app/views/explore/metrics/equationBuilder/utils.spec.tsx +++ b/static/app/views/explore/metrics/equationBuilder/utils.spec.tsx @@ -82,6 +82,156 @@ describe('syncEquationMetricQueries', () => { ); }); + it('correctly updates equations when two references resolve to the same value', () => { + const metricQueries: BaseMetricQuery[] = [ + { + label: 'A', + metric: {name: 'metricA', type: 'distribution', unit: 'none'}, + queryParams: new ReadableQueryParams({ + extrapolate: true, + mode: Mode.SAMPLES, + query: '', + cursor: '', + fields: ['id', 'timestamp'], + sortBys: [], + aggregateCursor: '', + aggregateFields: [ + new VisualizeFunction('sum(value,metricA,distribution,none)'), + ], + aggregateSortBys: [], + }), + }, + { + label: 'B', + metric: {name: 'metricA', type: 'distribution', unit: 'none'}, + queryParams: new ReadableQueryParams({ + extrapolate: true, + mode: Mode.SAMPLES, + query: '', + cursor: '', + fields: ['id', 'timestamp'], + sortBys: [], + aggregateCursor: '', + aggregateFields: [ + new VisualizeFunction('sum(value,metricA,distribution,none)'), + ], + aggregateSortBys: [], + }), + }, + { + label: 'ƒ1', + metric: {name: '', type: ''}, + queryParams: new ReadableQueryParams({ + extrapolate: true, + mode: Mode.AGGREGATE, + query: '', + cursor: '', + fields: ['id', 'timestamp'], + sortBys: [], + aggregateCursor: '', + aggregateFields: [ + new VisualizeEquation( + 'equation|sum(value,metricA,distribution,none) + sum(value,metricA,distribution,none)', + {internalExpression: 'A + B'} + ), + ], + aggregateSortBys: [], + }), + }, + ]; + + const updatedQueries = syncEquationMetricQueries( + metricQueries, + { + A: 'sum(value,metricA,distribution,none)', + B: 'sum(value,metricA,distribution,none)', + }, + { + A: 'sum(value,metricA,distribution,none)', + B: 'avg(value,metricA,distribution,none)', + } + ); + + expect(updatedQueries[2]!.queryParams.visualizes[0]!.yAxis).toBe( + 'equation|sum(value,metricA,distribution,none) + avg(value,metricA,distribution,none)' + ); + }); + + it('correctly updates equations with non-alphabetical label order in expression', () => { + const metricQueries: BaseMetricQuery[] = [ + { + label: 'A', + metric: {name: 'metricA', type: 'distribution', unit: 'none'}, + queryParams: new ReadableQueryParams({ + extrapolate: true, + mode: Mode.SAMPLES, + query: '', + cursor: '', + fields: ['id', 'timestamp'], + sortBys: [], + aggregateCursor: '', + aggregateFields: [ + new VisualizeFunction('sum(value,metricA,distribution,none)'), + ], + aggregateSortBys: [], + }), + }, + { + label: 'B', + metric: {name: 'metricA', type: 'distribution', unit: 'none'}, + queryParams: new ReadableQueryParams({ + extrapolate: true, + mode: Mode.SAMPLES, + query: '', + cursor: '', + fields: ['id', 'timestamp'], + sortBys: [], + aggregateCursor: '', + aggregateFields: [ + new VisualizeFunction('sum(value,metricA,distribution,none)'), + ], + aggregateSortBys: [], + }), + }, + { + label: 'ƒ1', + metric: {name: '', type: ''}, + queryParams: new ReadableQueryParams({ + extrapolate: true, + mode: Mode.AGGREGATE, + query: '', + cursor: '', + fields: ['id', 'timestamp'], + sortBys: [], + aggregateCursor: '', + aggregateFields: [ + new VisualizeEquation( + 'equation|sum(value,metricA,distribution,none) + sum(value,metricA,distribution,none)', + {internalExpression: 'B + A'} + ), + ], + aggregateSortBys: [], + }), + }, + ]; + + const updatedQueries = syncEquationMetricQueries( + metricQueries, + { + A: 'sum(value,metricA,distribution,none)', + B: 'sum(value,metricA,distribution,none)', + }, + { + A: 'sum(value,metricA,distribution,none)', + B: 'avg(value,metricA,distribution,none)', + } + ); + + expect(updatedQueries[2]!.queryParams.visualizes[0]!.yAxis).toBe( + 'equation|avg(value,metricA,distribution,none) + sum(value,metricA,distribution,none)' + ); + }); + it('updates every equation when a referenced metric changes', () => { const metricQueries: BaseMetricQuery[] = [ { diff --git a/static/app/views/explore/metrics/equationBuilder/utils.ts b/static/app/views/explore/metrics/equationBuilder/utils.ts index 176d6646acf0cb..d299d49e37350e 100644 --- a/static/app/views/explore/metrics/equationBuilder/utils.ts +++ b/static/app/views/explore/metrics/equationBuilder/utils.ts @@ -90,11 +90,10 @@ export function syncEquationMetricQueries( return metricQuery; } - const internalExpression = unresolveExpression( - visualize.expression.text, - previousReferenceMap - ); - const expression = new Expression(internalExpression, previousReferences); + const internalExpressionText = + visualize.internalExpression ?? + unresolveExpression(visualize.expression.text, previousReferenceMap); + const expression = new Expression(internalExpressionText, previousReferences); if (!expression.isValid) { return metricQuery; @@ -102,7 +101,10 @@ export function syncEquationMetricQueries( const resolvedExpression = resolveExpression(expression, nextReferenceMap); - if (resolvedExpression.text === visualize.expression.text) { + if ( + resolvedExpression.text === visualize.expression.text && + visualize.internalExpression + ) { return metricQuery; } @@ -118,6 +120,7 @@ export function syncEquationMetricQueries( return field.replace({ yAxis: `${EQUATION_PREFIX}${resolvedExpression.text}`, + internalExpression: internalExpressionText, }); }), }), diff --git a/static/app/views/explore/metrics/metricPanel/index.tsx b/static/app/views/explore/metrics/metricPanel/index.tsx index 8fd3f49cf457c8..4f58154fd636b5 100644 --- a/static/app/views/explore/metrics/metricPanel/index.tsx +++ b/static/app/views/explore/metrics/metricPanel/index.tsx @@ -128,7 +128,10 @@ export function MetricPanel({ const [title, setTitle] = useState(() => { if (isVisualizeEquation(visualize)) { - return unresolveExpression(visualize.expression.text, referenceMap); + return ( + visualize.internalExpression ?? + unresolveExpression(visualize.expression.text, referenceMap) + ); } return; }); diff --git a/static/app/views/explore/metrics/metricToolbar/index.tsx b/static/app/views/explore/metrics/metricToolbar/index.tsx index 1eb1cc670c1ee2..95661401aff121 100644 --- a/static/app/views/explore/metrics/metricToolbar/index.tsx +++ b/static/app/views/explore/metrics/metricToolbar/index.tsx @@ -90,7 +90,12 @@ export function MetricToolbar({ const handleExpressionChange = useCallback( (newExpression: Expression, internalText: string) => { - setVisualize(visualize.replace({yAxis: `${EQUATION_PREFIX}${newExpression.text}`})); + setVisualize( + visualize.replace({ + yAxis: `${EQUATION_PREFIX}${newExpression.text}`, + internalExpression: internalText, + }) + ); onTitleChange?.(internalText); }, [setVisualize, visualize, onTitleChange] @@ -159,6 +164,7 @@ export function MetricToolbar({ handleExpressionChange={handleExpressionChange} onReferenceLabelsChange={handleReferenceLabelsChange} disabled={hasUnresolvedMetrics} + storedInternalExpression={visualize.internalExpression} /> diff --git a/static/app/views/explore/metrics/multiMetricsQueryParams.spec.tsx b/static/app/views/explore/metrics/multiMetricsQueryParams.spec.tsx index bd04df02ee56d5..07d6964b66b05d 100644 --- a/static/app/views/explore/metrics/multiMetricsQueryParams.spec.tsx +++ b/static/app/views/explore/metrics/multiMetricsQueryParams.spec.tsx @@ -368,12 +368,14 @@ describe('MultiMetricsQueryParamsProvider', () => { expect(result.current[3]!.queryParams.aggregateFields).toEqual([ new VisualizeEquation( - 'equation|avg(value,newSelectedMetric,gauge,none) + sum(value,metricB,distribution,none)' + 'equation|avg(value,newSelectedMetric,gauge,none) + sum(value,metricB,distribution,none)', + {internalExpression: ' A + B '} ), ]); expect(result.current[4]!.queryParams.aggregateFields).toEqual([ new VisualizeEquation( - 'equation|avg(value,newSelectedMetric,gauge,none) - sum(value,metricC,distribution,none)' + 'equation|avg(value,newSelectedMetric,gauge,none) - sum(value,metricC,distribution,none)', + {internalExpression: ' A - C '} ), ]); }); diff --git a/static/app/views/explore/queryParams/visualize.ts b/static/app/views/explore/queryParams/visualize.ts index 8c173d72cc3f23..e585a1d95ef557 100644 --- a/static/app/views/explore/queryParams/visualize.ts +++ b/static/app/views/explore/queryParams/visualize.ts @@ -16,6 +16,10 @@ export const MAX_VISUALIZES = 8; interface VisualizeOptions { chartType?: ChartType; + // The internal expression is used to store the reference format for + // equations so we can properly update the correct references when + // dependencies change + internalExpression?: string; visible?: boolean; } @@ -38,8 +42,10 @@ export abstract class Visualize { chartType, visible, yAxis, + internalExpression, }: { chartType?: ChartType; + internalExpression?: string; visible?: boolean; yAxis?: string; }): Visualize; @@ -66,6 +72,7 @@ export abstract class Visualize { return new VisualizeEquation(yAxis, { chartType: json.chartType, visible: json.visible, + internalExpression: json.internalExpression, }); } return new VisualizeFunction(yAxis, { @@ -98,6 +105,7 @@ export class VisualizeFunction extends Visualize { yAxis, }: { chartType?: ChartType; + internalExpression?: string; visible?: boolean; yAxis?: string; }): VisualizeFunction { @@ -111,15 +119,18 @@ export class VisualizeFunction extends Visualize { export class VisualizeEquation extends Visualize { readonly kind = 'equation'; readonly expression: Expression; + readonly internalExpression?: string; constructor(yAxis: string, options?: VisualizeOptions) { super(yAxis, options); this.expression = new Expression(stripEquationPrefix(yAxis)); + this.internalExpression = options?.internalExpression; } clone(): Visualize { return new VisualizeEquation(this.yAxis, { chartType: this.selectedChartType, + internalExpression: this.internalExpression, }); } @@ -127,16 +138,27 @@ export class VisualizeEquation extends Visualize { chartType, visible, yAxis, + internalExpression, }: { chartType?: ChartType; + internalExpression?: string; visible?: boolean; yAxis?: string; }): Visualize { return new VisualizeEquation(yAxis ?? this.yAxis, { chartType: chartType ?? this.selectedChartType, visible: visible ?? this.visible, + internalExpression: internalExpression ?? this.internalExpression, }); } + + override serialize(): BaseVisualize { + const json = super.serialize(); + if (this.internalExpression) { + json.internalExpression = this.internalExpression; + } + return json; + } } export function getVisualizesFromLocation( @@ -188,6 +210,7 @@ export function isVisualizeEquation( export interface BaseVisualize { yAxes: readonly string[]; chartType?: ChartType; + internalExpression?: string; visible?: boolean; } From 31a06181b31ee96fd7a3c5d63364e9cc461cf582 Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Wed, 24 Jun 2026 14:02:26 -0400 Subject: [PATCH 2/2] Avoid pushing naively resolved equation to URL --- static/app/views/explore/metrics/equationBuilder/utils.ts | 7 ++----- .../views/explore/metrics/multiMetricsQueryParams.spec.tsx | 6 ++---- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/static/app/views/explore/metrics/equationBuilder/utils.ts b/static/app/views/explore/metrics/equationBuilder/utils.ts index d299d49e37350e..a5303a9fd124da 100644 --- a/static/app/views/explore/metrics/equationBuilder/utils.ts +++ b/static/app/views/explore/metrics/equationBuilder/utils.ts @@ -101,10 +101,7 @@ export function syncEquationMetricQueries( const resolvedExpression = resolveExpression(expression, nextReferenceMap); - if ( - resolvedExpression.text === visualize.expression.text && - visualize.internalExpression - ) { + if (resolvedExpression.text === visualize.expression.text) { return metricQuery; } @@ -120,7 +117,7 @@ export function syncEquationMetricQueries( return field.replace({ yAxis: `${EQUATION_PREFIX}${resolvedExpression.text}`, - internalExpression: internalExpressionText, + internalExpression: visualize.internalExpression, }); }), }), diff --git a/static/app/views/explore/metrics/multiMetricsQueryParams.spec.tsx b/static/app/views/explore/metrics/multiMetricsQueryParams.spec.tsx index 07d6964b66b05d..bd04df02ee56d5 100644 --- a/static/app/views/explore/metrics/multiMetricsQueryParams.spec.tsx +++ b/static/app/views/explore/metrics/multiMetricsQueryParams.spec.tsx @@ -368,14 +368,12 @@ describe('MultiMetricsQueryParamsProvider', () => { expect(result.current[3]!.queryParams.aggregateFields).toEqual([ new VisualizeEquation( - 'equation|avg(value,newSelectedMetric,gauge,none) + sum(value,metricB,distribution,none)', - {internalExpression: ' A + B '} + 'equation|avg(value,newSelectedMetric,gauge,none) + sum(value,metricB,distribution,none)' ), ]); expect(result.current[4]!.queryParams.aggregateFields).toEqual([ new VisualizeEquation( - 'equation|avg(value,newSelectedMetric,gauge,none) - sum(value,metricC,distribution,none)', - {internalExpression: ' A - C '} + 'equation|avg(value,newSelectedMetric,gauge,none) - sum(value,metricC,distribution,none)' ), ]); });