From 33b97fd40b9a0c3499fbd4fc7c847a06672b1af6 Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Wed, 15 Apr 2026 16:32:51 -0400 Subject: [PATCH 1/3] wip --- .../views/explore/metrics/equationBuilder.tsx | 156 ---------------- .../index.spec.tsx} | 0 .../explore/metrics/equationBuilder/index.tsx | 69 +++++++ .../metrics/equationBuilder/utils.spec.tsx | 106 +++++++++++ .../explore/metrics/equationBuilder/utils.ts | 128 +++++++++++++ .../metrics/hooks/useMetricReferences.tsx | 27 +-- .../metrics/multiMetricsQueryParams.spec.tsx | 89 +++++++++ .../metrics/multiMetricsQueryParams.tsx | 171 +++++++++--------- 8 files changed, 496 insertions(+), 250 deletions(-) delete mode 100644 static/app/views/explore/metrics/equationBuilder.tsx rename static/app/views/explore/metrics/{equationBuilder.spec.tsx => equationBuilder/index.spec.tsx} (100%) create mode 100644 static/app/views/explore/metrics/equationBuilder/index.tsx create mode 100644 static/app/views/explore/metrics/equationBuilder/utils.spec.tsx create mode 100644 static/app/views/explore/metrics/equationBuilder/utils.ts diff --git a/static/app/views/explore/metrics/equationBuilder.tsx b/static/app/views/explore/metrics/equationBuilder.tsx deleted file mode 100644 index a2a5793df59c2e..00000000000000 --- a/static/app/views/explore/metrics/equationBuilder.tsx +++ /dev/null @@ -1,156 +0,0 @@ -import {useCallback, useEffect, useMemo, useRef, useTransition} from 'react'; -import isEqual from 'lodash/isEqual'; - -import {ArithmeticBuilder} from 'sentry/components/arithmeticBuilder'; -import {Expression} from 'sentry/components/arithmeticBuilder/expression'; -import { - isTokenFreeText, - isTokenReference, -} from 'sentry/components/arithmeticBuilder/token'; -import {tokenizeExpression} from 'sentry/components/arithmeticBuilder/tokenizer'; - -/** - * Extracts the set of reference labels (e.g. ["A", "B"]) from an Expression's tokens. - */ -function extractReferenceLabels(expression: Expression): string[] { - return expression.tokens.filter(isTokenReference).map(token => token.text); -} - -/** - * Takes an expression and map of references and returns the internal string representation that uses the references. - */ -function unresolveExpression( - expression: string, - referenceMap: Record = {} -): string { - // Reverse the keys and values of the reference map, duplicates keep the first reference found - const reversedReferenceMap = Object.entries(referenceMap).reduce( - (reversedMap: Record, [key, value]) => { - if (!reversedMap[value]) { - reversedMap[value] = key; - } - return reversedMap; - }, - {} - ); - - const tokens = tokenizeExpression(expression); - return tokens - .map(token => { - if (token.text in reversedReferenceMap) { - return reversedReferenceMap[token.text] ?? token.text; - } - return token.text; - }) - .join(' '); -} - -/** - * Resolves the expression using references into a format that is compatible with our querying endpoints. - */ -function resolveExpression( - expression: Expression, - referenceMap: Record = {} -): Expression { - const newTokens = expression.tokens - .map(token => { - if (referenceMap && isTokenReference(token) && token.text in referenceMap) { - return referenceMap[token.text]; - } - if (!isTokenFreeText(token)) { - return token.text; - } - return null; - }) - .filter(Boolean); - - return new Expression(newTokens.join(' ')); -} - -/** - * A component that takes an equation in full resolved form and allows - * the user to edit it using "references" to refer to the different components - * of the equation. - * - * The references are used to resolve the equation into a format that is - * compatible with our querying endpoints. - */ -export function EquationBuilder({ - expression, - referenceMap, - handleExpressionChange, - onReferenceLabelsChange, -}: { - expression: string; - handleExpressionChange: (expression: Expression) => void; - onReferenceLabelsChange?: (labels: string[]) => void; - referenceMap?: Record; -}) { - const [_, startTransition] = useTransition(); - const references = useMemo( - () => new Set(Object.keys(referenceMap ?? {})), - [referenceMap] - ); - - // Tracks the reference map that `expression` was last resolved against. - // When referenceMap changes externally, expression still contains values - // resolved against the previous map until we re-resolve and the parent updates. - const expressionMapRef = useRef(referenceMap); - const mapChanged = !isEqual(expressionMapRef.current, referenceMap); - - const internalExpression = unresolveExpression( - expression, - mapChanged ? expressionMapRef.current : referenceMap - ); - - // When the reference map changes, re-resolve the expression and invoke the callback - useEffect(() => { - if (!isEqual(expressionMapRef.current, referenceMap)) { - const expr = new Expression( - internalExpression, - new Set(Object.keys(expressionMapRef.current ?? {})) - ); - const resolved = resolveExpression(expr, referenceMap); - - // Check the validity of the internal expression, not the resolved one because - // there are issues with validating it with the new _if aggregation format and - // this check just needs to ensure the structure is valid. - if (expr.isValid) { - handleExpressionChange(resolved); - expressionMapRef.current = referenceMap; - } - } - }, [referenceMap, internalExpression, handleExpressionChange]); - - // Report which labels this equation references after unresolving. - // Cleans up on unmount so deleted equations don't block metric deletion. - useEffect(() => { - const expr = new Expression(internalExpression, references); - onReferenceLabelsChange?.(extractReferenceLabels(expr)); - return () => { - onReferenceLabelsChange?.([]); - }; - }, [internalExpression, references, onReferenceLabelsChange]); - - const handleInternalExpressionChange = useCallback( - (newExpression: Expression) => { - startTransition(() => { - if (newExpression.isValid) { - handleExpressionChange(resolveExpression(newExpression, referenceMap)); - } - }); - }, - [handleExpressionChange, referenceMap] - ); - - return ( - null} - references={references} - setExpression={handleInternalExpressionChange} - /> - ); -} diff --git a/static/app/views/explore/metrics/equationBuilder.spec.tsx b/static/app/views/explore/metrics/equationBuilder/index.spec.tsx similarity index 100% rename from static/app/views/explore/metrics/equationBuilder.spec.tsx rename to static/app/views/explore/metrics/equationBuilder/index.spec.tsx diff --git a/static/app/views/explore/metrics/equationBuilder/index.tsx b/static/app/views/explore/metrics/equationBuilder/index.tsx new file mode 100644 index 00000000000000..56b6c5cde142c1 --- /dev/null +++ b/static/app/views/explore/metrics/equationBuilder/index.tsx @@ -0,0 +1,69 @@ +import {useCallback, useEffect, useMemo, useTransition} from 'react'; + +import {ArithmeticBuilder} from 'sentry/components/arithmeticBuilder'; +import {Expression} from 'sentry/components/arithmeticBuilder/expression'; +import { + extractReferenceLabels, + resolveExpression, + unresolveExpression, +} from 'sentry/views/explore/metrics/equationBuilder/utils'; + +/** + * A component that takes an equation in full resolved form and allows + * the user to edit it using "references" to refer to the different components + * of the equation. + * + * The references are used to resolve the equation into a format that is + * compatible with our querying endpoints. + */ +export function EquationBuilder({ + expression, + referenceMap, + handleExpressionChange, + onReferenceLabelsChange, +}: { + expression: string; + handleExpressionChange: (expression: Expression) => void; + onReferenceLabelsChange?: (labels: string[]) => void; + referenceMap?: Record; +}) { + const [_, startTransition] = useTransition(); + const references = useMemo( + () => new Set(Object.keys(referenceMap ?? {})), + [referenceMap] + ); + + const internalExpression = unresolveExpression(expression, referenceMap); + + // Report which labels this equation references after unresolving. + // Cleans up on unmount so deleted equations don't block metric deletion. + useEffect(() => { + const expr = new Expression(internalExpression, references); + onReferenceLabelsChange?.(extractReferenceLabels(expr)); + return () => { + onReferenceLabelsChange?.([]); + }; + }, [internalExpression, references, onReferenceLabelsChange]); + + const handleInternalExpressionChange = useCallback( + (newExpression: Expression) => { + startTransition(() => { + if (newExpression.isValid) { + handleExpressionChange(resolveExpression(newExpression, referenceMap)); + } + }); + }, + [handleExpressionChange, referenceMap] + ); + + return ( + null} + references={references} + setExpression={handleInternalExpressionChange} + /> + ); +} diff --git a/static/app/views/explore/metrics/equationBuilder/utils.spec.tsx b/static/app/views/explore/metrics/equationBuilder/utils.spec.tsx new file mode 100644 index 00000000000000..9281338172af5f --- /dev/null +++ b/static/app/views/explore/metrics/equationBuilder/utils.spec.tsx @@ -0,0 +1,106 @@ +import {Mode} from 'sentry/views/explore/contexts/pageParamsContext/mode'; +import {syncEquationMetricQueries} from 'sentry/views/explore/metrics/equationBuilder/utils'; +import type {BaseMetricQuery} from 'sentry/views/explore/metrics/metricQuery'; +import {ReadableQueryParams} from 'sentry/views/explore/queryParams/readableQueryParams'; +import { + VisualizeEquation, + VisualizeFunction, +} from 'sentry/views/explore/queryParams/visualize'; + +describe('syncEquationMetricQueries', () => { + it('updates every equation when a referenced metric changes', () => { + 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: 'metricB', type: 'distribution', unit: 'none'}, + queryParams: new ReadableQueryParams({ + extrapolate: true, + mode: Mode.SAMPLES, + query: '', + cursor: '', + fields: ['id', 'timestamp'], + sortBys: [], + aggregateCursor: '', + aggregateFields: [ + new VisualizeFunction('sum(value,metricB,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,metricB,distribution,none)' + ), + ], + aggregateSortBys: [], + }), + }, + { + label: 'ƒ2', + 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,metricB,distribution,none)' + ), + ], + aggregateSortBys: [], + }), + }, + ]; + + const updatedQueries = syncEquationMetricQueries( + metricQueries, + { + A: 'sum(value,metricA,distribution,none)', + B: 'sum(value,metricB,distribution,none)', + }, + { + A: 'avg(value,metricA,distribution,none)', + B: 'sum(value,metricB,distribution,none)', + } + ); + + expect(updatedQueries[2]!.queryParams.visualizes[0]!.yAxis).toBe( + 'equation|avg(value,metricA,distribution,none) + sum(value,metricB,distribution,none)' + ); + expect(updatedQueries[3]!.queryParams.visualizes[0]!.yAxis).toBe( + 'equation|avg(value,metricA,distribution,none) - sum(value,metricB,distribution,none)' + ); + }); +}); diff --git a/static/app/views/explore/metrics/equationBuilder/utils.ts b/static/app/views/explore/metrics/equationBuilder/utils.ts new file mode 100644 index 00000000000000..176d6646acf0cb --- /dev/null +++ b/static/app/views/explore/metrics/equationBuilder/utils.ts @@ -0,0 +1,128 @@ +import {Expression} from 'sentry/components/arithmeticBuilder/expression'; +import { + isTokenFreeText, + isTokenReference, +} from 'sentry/components/arithmeticBuilder/token'; +import {tokenizeExpression} from 'sentry/components/arithmeticBuilder/tokenizer'; +import {EQUATION_PREFIX} from 'sentry/utils/discover/fields'; +import type {BaseMetricQuery} from 'sentry/views/explore/metrics/metricQuery'; +import { + isVisualize, + isVisualizeEquation, +} from 'sentry/views/explore/queryParams/visualize'; + +/** + * Extracts the set of reference labels (e.g. ["A", "B"]) from an Expression's tokens. + */ +export function extractReferenceLabels(expression: Expression): string[] { + return expression.tokens.filter(isTokenReference).map(token => token.text); +} + +/** + * Takes an expression and map of references and returns the internal string representation that uses the references. + */ +export function unresolveExpression( + expression: string, + referenceMap: Record = {} +): string { + // Reverse the keys and values of the reference map, duplicates keep the first reference found + const reversedReferenceMap = Object.entries(referenceMap).reduce( + (reversedMap: Record, [key, value]) => { + if (!reversedMap[value]) { + reversedMap[value] = key; + } + return reversedMap; + }, + {} + ); + + const tokens = tokenizeExpression(expression); + return tokens + .map(token => { + if (token.text in reversedReferenceMap) { + return reversedReferenceMap[token.text] ?? token.text; + } + return token.text; + }) + .join(' '); +} + +/** + * Resolves the expression using references into a format that is compatible with our querying endpoints. + */ +export function resolveExpression( + expression: Expression, + referenceMap: Record = {} +): Expression { + const newTokens = expression.tokens + .map(token => { + if (referenceMap && isTokenReference(token) && token.text in referenceMap) { + return referenceMap[token.text]; + } + if (!isTokenFreeText(token)) { + return token.text; + } + return null; + }) + .filter(Boolean); + + return new Expression(newTokens.join(' ')); +} + +/** + * Creates a new set of metric queries with equations updated to use the new + * reference map. + * + * If the equations are not changed, then the original metric queries are returned. + */ +export function syncEquationMetricQueries( + metricQueries: BaseMetricQuery[], + previousReferenceMap: Record, + nextReferenceMap: Record +): BaseMetricQuery[] { + const previousReferences = new Set(Object.keys(previousReferenceMap)); + let changed = false; + + const nextMetricQueries = metricQueries.map(metricQuery => { + const visualize = metricQuery.queryParams.visualizes[0]; + + if (!visualize || !isVisualizeEquation(visualize)) { + return metricQuery; + } + + const internalExpression = unresolveExpression( + visualize.expression.text, + previousReferenceMap + ); + const expression = new Expression(internalExpression, previousReferences); + + if (!expression.isValid) { + return metricQuery; + } + + const resolvedExpression = resolveExpression(expression, nextReferenceMap); + + if (resolvedExpression.text === visualize.expression.text) { + return metricQuery; + } + + changed = true; + + return { + ...metricQuery, + queryParams: metricQuery.queryParams.replace({ + aggregateFields: metricQuery.queryParams.aggregateFields.map(field => { + if (!isVisualize(field) || !isVisualizeEquation(field)) { + return field; + } + + return field.replace({ + yAxis: `${EQUATION_PREFIX}${resolvedExpression.text}`, + }); + }), + }), + }; + }); + + return changed ? nextMetricQueries : metricQueries; +} diff --git a/static/app/views/explore/metrics/hooks/useMetricReferences.tsx b/static/app/views/explore/metrics/hooks/useMetricReferences.tsx index ae30fb6ee68033..f29d9d3b0e4ec2 100644 --- a/static/app/views/explore/metrics/hooks/useMetricReferences.tsx +++ b/static/app/views/explore/metrics/hooks/useMetricReferences.tsx @@ -1,6 +1,7 @@ import {useMemo} from 'react'; import {parseFunction} from 'sentry/utils/discover/fields'; +import type {BaseMetricQuery} from 'sentry/views/explore/metrics/metricQuery'; import {useMultiMetricsQueryParams} from 'sentry/views/explore/metrics/multiMetricsQueryParams'; import type {ReadableQueryParams} from 'sentry/views/explore/queryParams/readableQueryParams'; import {isVisualizeFunction} from 'sentry/views/explore/queryParams/visualize'; @@ -18,19 +19,21 @@ function resolveMetricReference(metricConfig: ReadableQueryParams): string { return metricConfig.visualizes[0]?.yAxis!; } +export function getMetricReferences( + metricQueries: BaseMetricQuery[] +): Record { + return Object.fromEntries( + metricQueries + .filter(metricQuery => metricQuery.queryParams.visualizes.some(isVisualizeFunction)) + .map((metricQuery, index) => [ + metricQuery.label ?? getVisualizeLabel(index, false), + resolveMetricReference(metricQuery.queryParams), + ]) + ); +} + export function useMetricReferences() { const metricQueries = useMultiMetricsQueryParams(); - return useMemo(() => { - return Object.fromEntries( - metricQueries - .filter(metricQuery => - metricQuery.queryParams.visualizes.some(isVisualizeFunction) - ) - .map((metricQuery, index) => [ - metricQuery.label ?? getVisualizeLabel(index, false), - resolveMetricReference(metricQuery.queryParams), - ]) - ); - }, [metricQueries]); + return useMemo(() => getMetricReferences(metricQueries), [metricQueries]); } diff --git a/static/app/views/explore/metrics/multiMetricsQueryParams.spec.tsx b/static/app/views/explore/metrics/multiMetricsQueryParams.spec.tsx index 52ca744d43b430..4ae32ab06934b9 100644 --- a/static/app/views/explore/metrics/multiMetricsQueryParams.spec.tsx +++ b/static/app/views/explore/metrics/multiMetricsQueryParams.spec.tsx @@ -285,6 +285,95 @@ describe('MultiMetricsQueryParamsProvider', () => { ]); }); + it('updates dependent equations in the same metric update', () => { + const {result} = renderHookWithProviders(useMultiMetricsQueryParams, { + additionalWrapper: Wrapper, + initialRouterConfig: { + location: { + pathname: '/organizations/org-slug/explore/metrics/', + query: { + metric: [ + JSON.stringify({ + metric: {name: 'metricA', type: 'distribution', unit: 'none'}, + query: '', + aggregateFields: [ + new VisualizeFunction( + 'sum(value,metricA,distribution,none)' + ).serialize(), + ], + aggregateSortBys: [], + mode: 'samples', + }), + JSON.stringify({ + metric: {name: 'metricB', type: 'distribution', unit: 'none'}, + query: '', + aggregateFields: [ + new VisualizeFunction( + 'sum(value,metricB,distribution,none)' + ).serialize(), + ], + aggregateSortBys: [], + mode: 'samples', + }), + JSON.stringify({ + metric: {name: 'metricC', type: 'distribution', unit: 'none'}, + query: '', + aggregateFields: [ + new VisualizeFunction( + 'sum(value,metricC,distribution,none)' + ).serialize(), + ], + aggregateSortBys: [], + mode: 'samples', + }), + JSON.stringify({ + metric: {name: '', type: ''}, + query: '', + aggregateFields: [ + new VisualizeEquation( + 'equation|sum(value,metricA,distribution,none) + sum(value,metricB,distribution,none)' + ).serialize(), + ], + aggregateSortBys: [], + mode: 'aggregate', + }), + JSON.stringify({ + metric: {name: '', type: ''}, + query: '', + aggregateFields: [ + new VisualizeEquation( + 'equation|sum(value,metricA,distribution,none) - sum(value,metricC,distribution,none)' + ).serialize(), + ], + aggregateSortBys: [], + mode: 'aggregate', + }), + ], + }, + }, + }, + }); + + act(() => + result.current[0]!.setTraceMetric({ + name: 'newSelectedMetric', + type: 'gauge', + unit: 'none', + }) + ); + + expect(result.current[3]!.queryParams.aggregateFields).toEqual([ + new VisualizeEquation( + '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)' + ), + ]); + }); + describe('stable labels', () => { it('preserves label B when A is deleted from [A, B]', () => { const {result} = renderHookWithProviders(useMultiMetricsQueryParams, { diff --git a/static/app/views/explore/metrics/multiMetricsQueryParams.tsx b/static/app/views/explore/metrics/multiMetricsQueryParams.tsx index fc965e7b2cc891..dfd50e2591083f 100644 --- a/static/app/views/explore/metrics/multiMetricsQueryParams.tsx +++ b/static/app/views/explore/metrics/multiMetricsQueryParams.tsx @@ -11,6 +11,8 @@ import { DEFAULT_YAXIS_BY_TYPE, OPTIONS_BY_TYPE, } from 'sentry/views/explore/metrics/constants'; +import {syncEquationMetricQueries} from 'sentry/views/explore/metrics/equationBuilder/utils'; +import {getMetricReferences} from 'sentry/views/explore/metrics/hooks/useMetricReferences'; import { getNextLabel, useStableLabels, @@ -34,6 +36,24 @@ import { export const MAX_METRICS_ALLOWED = 8; +function encodeMetricQueries(metricQueries: BaseMetricQuery[]): string[] { + return metricQueries + .map((metricQuery: BaseMetricQuery) => encodeMetricQueryParams(metricQuery)) + .filter(defined) + .filter(Boolean); +} + +function syncUpdatedMetricQueries( + previousMetricQueries: BaseMetricQuery[], + nextMetricQueries: BaseMetricQuery[] +): BaseMetricQuery[] { + return syncEquationMetricQueries( + nextMetricQueries, + getMetricReferences(previousMetricQueries), + getMetricReferences(nextMetricQueries) + ); +} + interface MultiMetricsQueryParamsContextValue { insertLabelAtIndex: (position: number, label: string) => void; metricQueries: MetricQuery[]; @@ -74,78 +94,80 @@ export function MultiMetricsQueryParamsProvider({ label: labels.getLabel(i), })); + function navigateToMetricQueries(nextMetricQueries: BaseMetricQuery[]) { + const target = {...location, query: {...location.query}}; + target.query.metric = encodeMetricQueries(nextMetricQueries); + navigate(target); + } + function setQueryParamsForIndex(i: number) { return function (newQueryParams: ReadableQueryParams) { - const target = {...location, query: {...location.query}}; - - const newMetricQueries = metricQueries - .map((metricQuery: BaseMetricQuery, j: number) => { - if (i !== j) { - return metricQuery; - } - return { - metric: metricQuery.metric, - queryParams: newQueryParams, - label: metricQuery.label, - }; - }) - .map((metricQuery: BaseMetricQuery) => encodeMetricQueryParams(metricQuery)) - .filter(defined) - .filter(Boolean); - target.query.metric = newMetricQueries; - - navigate(target); + navigateToMetricQueries( + syncUpdatedMetricQueries( + metricQueries, + metricQueries.map((metricQuery: BaseMetricQuery, j: number) => { + if (i !== j) { + return metricQuery; + } + return { + metric: metricQuery.metric, + queryParams: newQueryParams, + label: metricQuery.label, + }; + }) + ) + ); }; } function setTraceMetricForIndex(i: number) { return function (newTraceMetric: TraceMetric) { - const target = {...location, query: {...location.query}}; - target.query.metric = metricQueries - .map((metricQuery: BaseMetricQuery, j: number) => { - if (i !== j) { - return metricQuery; - } - - // when changing trace metrics, we need to look at the currently selected - // aggregation and make necessary adjustments - const visualize = metricQuery.queryParams.visualizes[0]; - let aggregateFields = undefined; - if (visualize && isVisualizeFunction(visualize)) { - const selectedAggregation = visualize.parsedFunction?.name; - const allowedAggregations = OPTIONS_BY_TYPE[newTraceMetric.type]; - - if ( - selectedAggregation && - allowedAggregations?.find(option => option.value === selectedAggregation) - ) { - // the currently selected aggregation changed types - aggregateFields = [ - updateVisualizeYAxis(visualize, selectedAggregation, newTraceMetric), - ...metricQuery.queryParams.aggregateFields.filter(isGroupBy), - ]; - } else { - // the currently selected aggregation isn't supported on the new metric - const defaultAggregation = - DEFAULT_YAXIS_BY_TYPE[newTraceMetric.type] || 'sum'; - aggregateFields = [ - updateVisualizeYAxis(visualize, defaultAggregation, newTraceMetric), - ...metricQuery.queryParams.aggregateFields.filter(isGroupBy), - ]; + navigateToMetricQueries( + syncUpdatedMetricQueries( + metricQueries, + metricQueries.map((metricQuery: BaseMetricQuery, j: number) => { + if (i !== j) { + return metricQuery; + } + + // when changing trace metrics, we need to look at the currently selected + // aggregation and make necessary adjustments + const visualize = metricQuery.queryParams.visualizes[0]; + let aggregateFields = undefined; + if (visualize && isVisualizeFunction(visualize)) { + const selectedAggregation = visualize.parsedFunction?.name; + const allowedAggregations = OPTIONS_BY_TYPE[newTraceMetric.type]; + + if ( + selectedAggregation && + allowedAggregations?.find( + option => option.value === selectedAggregation + ) + ) { + // the currently selected aggregation changed types + aggregateFields = [ + updateVisualizeYAxis(visualize, selectedAggregation, newTraceMetric), + ...metricQuery.queryParams.aggregateFields.filter(isGroupBy), + ]; + } else { + // the currently selected aggregation isn't supported on the new metric + const defaultAggregation = + DEFAULT_YAXIS_BY_TYPE[newTraceMetric.type] || 'sum'; + aggregateFields = [ + updateVisualizeYAxis(visualize, defaultAggregation, newTraceMetric), + ...metricQuery.queryParams.aggregateFields.filter(isGroupBy), + ]; + } } - } - - return { - queryParams: metricQuery.queryParams.replace({aggregateFields}), - metric: newTraceMetric, - label: metricQuery.label, - }; - }) - .map((metric: BaseMetricQuery) => encodeMetricQueryParams(metric)) - .filter(defined) - .filter(Boolean); - - navigate(target); + + return { + queryParams: metricQuery.queryParams.replace({aggregateFields}), + metric: newTraceMetric, + label: metricQuery.label, + }; + }) + ) + ); }; } @@ -159,16 +181,7 @@ export function MultiMetricsQueryParamsProvider({ // Update labels before navigating so they stay stable labels.remove(i); - const target = {...location, query: {...location.query}}; - - const newMetricQueries = metricQueries - .filter((_, j) => i !== j) - .map((metricQuery: BaseMetricQuery) => encodeMetricQueryParams(metricQuery)) - .filter(defined) - .filter(Boolean); - target.query.metric = newMetricQueries; - - navigate(target); + navigateToMetricQueries(metricQueries.filter((_, j) => i !== j)); }; } @@ -247,10 +260,7 @@ export function useAddMetricQuery({ const baseQueries: BaseMetricQuery[] = metricQueries; const newMetricQueries = baseQueries.toSpliced(insertAt, 0, newQuery); - target.query.metric = newMetricQueries - .map((metricQuery: BaseMetricQuery) => encodeMetricQueryParams(metricQuery)) - .filter(defined) - .filter(Boolean); + target.query.metric = encodeMetricQueries(newMetricQueries); navigate(target); }; @@ -268,10 +278,7 @@ export function useReorderMetricQueries() { reorderLabels(oldIndex, newIndex); const target = {...location, query: {...location.query}}; - target.query.metric = reorderedQueries - .map((metricQuery: BaseMetricQuery) => encodeMetricQueryParams(metricQuery)) - .filter(defined) - .filter(Boolean); + target.query.metric = encodeMetricQueries(reorderedQueries); navigate(target); }, From 042c595a24674eb028f01c5330d86ca44c89b6ed Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Wed, 15 Apr 2026 16:59:18 -0400 Subject: [PATCH 2/3] simplify navigations --- .../metrics/multiMetricsQueryParams.tsx | 112 +++++++++--------- 1 file changed, 55 insertions(+), 57 deletions(-) diff --git a/static/app/views/explore/metrics/multiMetricsQueryParams.tsx b/static/app/views/explore/metrics/multiMetricsQueryParams.tsx index dfd50e2591083f..fa277c0d25e6d5 100644 --- a/static/app/views/explore/metrics/multiMetricsQueryParams.tsx +++ b/static/app/views/explore/metrics/multiMetricsQueryParams.tsx @@ -102,71 +102,69 @@ export function MultiMetricsQueryParamsProvider({ function setQueryParamsForIndex(i: number) { return function (newQueryParams: ReadableQueryParams) { + const newMetricQueries = metricQueries.map( + (metricQuery: BaseMetricQuery, j: number) => { + if (i !== j) { + return metricQuery; + } + return { + metric: metricQuery.metric, + queryParams: newQueryParams, + label: metricQuery.label, + }; + } + ); navigateToMetricQueries( - syncUpdatedMetricQueries( - metricQueries, - metricQueries.map((metricQuery: BaseMetricQuery, j: number) => { - if (i !== j) { - return metricQuery; - } - return { - metric: metricQuery.metric, - queryParams: newQueryParams, - label: metricQuery.label, - }; - }) - ) + syncUpdatedMetricQueries(metricQueries, newMetricQueries) ); }; } function setTraceMetricForIndex(i: number) { return function (newTraceMetric: TraceMetric) { - navigateToMetricQueries( - syncUpdatedMetricQueries( - metricQueries, - metricQueries.map((metricQuery: BaseMetricQuery, j: number) => { - if (i !== j) { - return metricQuery; - } - - // when changing trace metrics, we need to look at the currently selected - // aggregation and make necessary adjustments - const visualize = metricQuery.queryParams.visualizes[0]; - let aggregateFields = undefined; - if (visualize && isVisualizeFunction(visualize)) { - const selectedAggregation = visualize.parsedFunction?.name; - const allowedAggregations = OPTIONS_BY_TYPE[newTraceMetric.type]; - - if ( - selectedAggregation && - allowedAggregations?.find( - option => option.value === selectedAggregation - ) - ) { - // the currently selected aggregation changed types - aggregateFields = [ - updateVisualizeYAxis(visualize, selectedAggregation, newTraceMetric), - ...metricQuery.queryParams.aggregateFields.filter(isGroupBy), - ]; - } else { - // the currently selected aggregation isn't supported on the new metric - const defaultAggregation = - DEFAULT_YAXIS_BY_TYPE[newTraceMetric.type] || 'sum'; - aggregateFields = [ - updateVisualizeYAxis(visualize, defaultAggregation, newTraceMetric), - ...metricQuery.queryParams.aggregateFields.filter(isGroupBy), - ]; - } + const newMetricQueries = metricQueries.map( + (metricQuery: BaseMetricQuery, j: number) => { + if (i !== j) { + return metricQuery; + } + + // when changing trace metrics, we need to look at the currently selected + // aggregation and make necessary adjustments + const visualize = metricQuery.queryParams.visualizes[0]; + let aggregateFields = undefined; + if (visualize && isVisualizeFunction(visualize)) { + const selectedAggregation = visualize.parsedFunction?.name; + const allowedAggregations = OPTIONS_BY_TYPE[newTraceMetric.type]; + + if ( + selectedAggregation && + allowedAggregations?.find(option => option.value === selectedAggregation) + ) { + // the currently selected aggregation changed types + aggregateFields = [ + updateVisualizeYAxis(visualize, selectedAggregation, newTraceMetric), + ...metricQuery.queryParams.aggregateFields.filter(isGroupBy), + ]; + } else { + // the currently selected aggregation isn't supported on the new metric + const defaultAggregation = + DEFAULT_YAXIS_BY_TYPE[newTraceMetric.type] || 'sum'; + aggregateFields = [ + updateVisualizeYAxis(visualize, defaultAggregation, newTraceMetric), + ...metricQuery.queryParams.aggregateFields.filter(isGroupBy), + ]; } - - return { - queryParams: metricQuery.queryParams.replace({aggregateFields}), - metric: newTraceMetric, - label: metricQuery.label, - }; - }) - ) + } + + return { + queryParams: metricQuery.queryParams.replace({aggregateFields}), + metric: newTraceMetric, + label: metricQuery.label, + }; + } + ); + navigateToMetricQueries( + syncUpdatedMetricQueries(metricQueries, newMetricQueries) ); }; } From 7357dca0f5193ab3e5719dfb6934e7606f473ac6 Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Wed, 15 Apr 2026 23:22:27 -0400 Subject: [PATCH 3/3] Update grammar to allow for backtick attrs Fixes a small bug revealed when testing the sync, some state changes would cause invalid equations and then fail to sync --- .../arithmeticBuilder/grammar.pegjs | 7 +- .../arithmeticBuilder/tokenizer.spec.tsx | 52 +++++++++++++ .../metrics/equationBuilder/utils.spec.tsx | 74 +++++++++++++++++++ 3 files changed, 132 insertions(+), 1 deletion(-) diff --git a/static/app/components/arithmeticBuilder/grammar.pegjs b/static/app/components/arithmeticBuilder/grammar.pegjs index e039be65e414a2..97dd85bc553f2d 100644 --- a/static/app/components/arithmeticBuilder/grammar.pegjs +++ b/static/app/components/arithmeticBuilder/grammar.pegjs @@ -34,7 +34,12 @@ yes_arg arg = attr -attr = typed_attr / untyped_attr +attr = typed_attr / backtick_attr / untyped_attr + +backtick_attr + = "`" [^`]* "`" { + return tc.tokenAttribute(text(), undefined, location()); + } typed_attr = "tags[" name:name "," spaces type:type_name "]" { diff --git a/static/app/components/arithmeticBuilder/tokenizer.spec.tsx b/static/app/components/arithmeticBuilder/tokenizer.spec.tsx index 5d203a2dbf265c..482ec4da1f20d9 100644 --- a/static/app/components/arithmeticBuilder/tokenizer.spec.tsx +++ b/static/app/components/arithmeticBuilder/tokenizer.spec.tsx @@ -99,6 +99,8 @@ describe('tokenizeExpression', () => { ['avg(tags[foo, number])', f(0, 'avg', [a(0, 'foo', 'number')])], ['avg( tags[foo, number] )', f(0, 'avg', [a(0, 'foo', 'number')])], ['epm()', f(0, 'epm', [])], + ['count_if(`test:foo`)', f(0, 'count_if', [a(0, '`test:foo`')])], + ['count_if(`test:"blah blah"`)', f(0, 'count_if', [a(0, '`test:"blah blah"`')])], ])('tokenizes function `%s`', (expression, expected) => { expect(tokenizeExpression(expression)).toEqual([s(0), expected, s(1)]); }); @@ -116,6 +118,20 @@ describe('tokenizeExpression', () => { 'avg( tags[foo, number], equals, 30 )', f(0, 'avg', [a(0, 'foo', 'number'), a(1, 'equals'), a(2, '30')]), ], + [ + 'count_if(`test:"blah blah"`,test,test)', + f(0, 'count_if', [a(0, '`test:"blah blah"`'), a(1, 'test'), a(2, 'test')]), + ], + [ + 'sum_if(`agent_name:["Agent Run","Assisted Query Agent - Traces"]`,value,agent.invocations.error,counter,none)', + f(0, 'sum_if', [ + a(0, '`agent_name:["Agent Run","Assisted Query Agent - Traces"]`'), + a(1, 'value'), + a(2, 'agent.invocations.error'), + a(3, 'counter'), + a(4, 'none'), + ]), + ], ])('tokenizes multi-param function `%s`', (expression, expected) => { expect(tokenizeExpression(expression)).toEqual([s(0), expected, s(1)]); }); @@ -293,6 +309,42 @@ describe('tokenizeExpression', () => { s(3), ], ], + [ + 'count_if(`test:foo`) + epm()', + [ + s(0), + f(0, 'count_if', [a(0, '`test:foo`')]), + s(1), + o(0, '+'), + s(2), + f(1, 'epm', []), + s(3), + ], + ], + [ + 'count_if(`test:"blah blah"`) + epm()', + [ + s(0), + f(0, 'count_if', [a(0, '`test:"blah blah"`')]), + s(1), + o(0, '+'), + s(2), + f(1, 'epm', []), + s(3), + ], + ], + [ + 'count_if(`test:"blah blah"`,test,test) + sum_if(`test:"blah\'blah\'blah"`)', + [ + s(0), + f(0, 'count_if', [a(0, '`test:"blah blah"`'), a(1, 'test'), a(2, 'test')]), + s(1), + o(0, '+'), + s(2), + f(1, 'sum_if', [a(3, '`test:"blah\'blah\'blah"`')]), + s(3), + ], + ], ])('tokenizes binary expressions `%s`', (expression, expected) => { expect(tokenizeExpression(expression)).toEqual(expected); }); diff --git a/static/app/views/explore/metrics/equationBuilder/utils.spec.tsx b/static/app/views/explore/metrics/equationBuilder/utils.spec.tsx index 9281338172af5f..74b4cc89f5ace2 100644 --- a/static/app/views/explore/metrics/equationBuilder/utils.spec.tsx +++ b/static/app/views/explore/metrics/equationBuilder/utils.spec.tsx @@ -8,6 +8,80 @@ import { } from 'sentry/views/explore/queryParams/visualize'; describe('syncEquationMetricQueries', () => { + it('updates equations when a referenced metric with quoted query filter changes', () => { + const metricQueries: BaseMetricQuery[] = [ + { + label: 'A', + metric: {name: 'agent.invocations.error', type: 'counter', unit: 'none'}, + queryParams: new ReadableQueryParams({ + extrapolate: true, + mode: Mode.SAMPLES, + query: 'agent_name:["Agent Run","Assisted Query Agent - Traces"]', + cursor: '', + fields: ['id', 'timestamp'], + sortBys: [], + aggregateCursor: '', + aggregateFields: [ + new VisualizeFunction('sum(value,agent.invocations.error,counter,none)'), + ], + aggregateSortBys: [], + }), + }, + { + label: 'B', + metric: {name: 'agent.invocations', type: 'counter', unit: 'none'}, + queryParams: new ReadableQueryParams({ + extrapolate: true, + mode: Mode.SAMPLES, + query: '', + cursor: '', + fields: ['id', 'timestamp'], + sortBys: [], + aggregateCursor: '', + aggregateFields: [ + new VisualizeFunction('sum(value,agent.invocations,counter,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_if(`agent_name:["Agent Run","Assisted Query Agent - Traces"]`,value,agent.invocations.error,counter,none) / sum(value,agent.invocations,counter,none) * 100' + ), + ], + aggregateSortBys: [], + }), + }, + ]; + + const updatedQueries = syncEquationMetricQueries( + metricQueries, + { + A: 'sum_if(`agent_name:["Agent Run","Assisted Query Agent - Traces"]`,value,agent.invocations.error,counter,none)', + B: 'sum(value,agent.invocations,counter,none)', + }, + { + A: 'sum_if(`agent_name:"Bug Prediction"`,value,agent.invocations.error,counter,none)', + B: 'sum(value,agent.invocations,counter,none)', + } + ); + + expect(updatedQueries[2]!.queryParams.visualizes[0]!.yAxis).toBe( + 'equation|sum_if(`agent_name:"Bug Prediction"`,value,agent.invocations.error,counter,none) / sum(value,agent.invocations,counter,none) * 100' + ); + }); + it('updates every equation when a referenced metric changes', () => { const metricQueries: BaseMetricQuery[] = [ {