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..a5303a9fd124da 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; @@ -118,6 +117,7 @@ export function syncEquationMetricQueries( return field.replace({ yAxis: `${EQUATION_PREFIX}${resolvedExpression.text}`, + internalExpression: visualize.internalExpression, }); }), }), 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/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; }