From 95ad364b143e5d68d26b7b86eb77a6de7d4ff47b Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Mon, 13 Apr 2026 11:54:37 -0400 Subject: [PATCH 01/11] Use prev reference to rebuild internal expression --- .../views/explore/metrics/equationBuilder.tsx | 105 ++++++++++++++++++ .../metrics/hooks/useMetricReferences.tsx | 21 +++- .../metrics/hooks/useMetricTimeseries.tsx | 4 +- .../hooks/useSaveMetricsMultiQuery.tsx | 7 +- .../explore/metrics/metricPanel/index.tsx | 8 +- .../explore/metrics/metricToolbar/index.tsx | 28 +++-- .../explore/metrics/metricsQueryParams.tsx | 1 + .../app/views/explore/metrics/metricsTab.tsx | 10 +- 8 files changed, 155 insertions(+), 29 deletions(-) create mode 100644 static/app/views/explore/metrics/equationBuilder.tsx diff --git a/static/app/views/explore/metrics/equationBuilder.tsx b/static/app/views/explore/metrics/equationBuilder.tsx new file mode 100644 index 00000000000000..850f80dd0edde1 --- /dev/null +++ b/static/app/views/explore/metrics/equationBuilder.tsx @@ -0,0 +1,105 @@ +import {useCallback, useEffect, useMemo} 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'; +import {usePrevious} from 'sentry/utils/usePrevious'; + +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.fromEntries( + Object.entries(referenceMap).map(([key, value]) => [value, key]) + ); + + const tokens = tokenizeExpression(expression); + return tokens + .map(token => { + if (token.text in reversedReferenceMap) { + return reversedReferenceMap[token.text] ?? token.text; + } + return token.text; + }) + .join(' '); +} + +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, +}: { + expression: string; + handleExpressionChange: (expression: Expression) => void; + referenceMap?: Record; +}) { + const prevReferenceMap = usePrevious(referenceMap); + const references = useMemo( + () => new Set(Object.keys(referenceMap ?? {})), + [referenceMap] + ); + const internalExpression = unresolveExpression(expression, referenceMap); + + const handleInternalExpressionChange = useCallback( + (newExpression: Expression) => { + handleExpressionChange(resolveExpression(newExpression, referenceMap)); + }, + [handleExpressionChange, referenceMap] + ); + + // Trigger the expression change when the reference map changes to ensure the query is showing the correct data + useEffect(() => { + if (!isEqual(prevReferenceMap, referenceMap)) { + const internalRepresentation = unresolveExpression(expression, prevReferenceMap); + const expr = new Expression( + internalRepresentation, + new Set(Object.keys(prevReferenceMap ?? {})) + ); + handleExpressionChange(resolveExpression(expr, referenceMap)); + } + }, [prevReferenceMap, referenceMap, expression, handleExpressionChange]); + + return ( + null} + references={references} + setExpression={handleInternalExpressionChange} + /> + ); +} diff --git a/static/app/views/explore/metrics/hooks/useMetricReferences.tsx b/static/app/views/explore/metrics/hooks/useMetricReferences.tsx index 9cbf86baa36a89..ae30fb6ee68033 100644 --- a/static/app/views/explore/metrics/hooks/useMetricReferences.tsx +++ b/static/app/views/explore/metrics/hooks/useMetricReferences.tsx @@ -1,19 +1,36 @@ import {useMemo} from 'react'; +import {parseFunction} from 'sentry/utils/discover/fields'; 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'; import {getVisualizeLabel} from 'sentry/views/explore/toolbar/toolbarVisualize'; +function resolveMetricReference(metricConfig: ReadableQueryParams): string { + if (metricConfig.query) { + // convert the aggregate to an "if" format + const visualize = metricConfig.visualizes[0]!; + const parsed = parseFunction(visualize.yAxis); + if (parsed) { + return `${parsed.name}_if(\`${metricConfig.query}\`,${parsed.arguments.join(',')})`; + } + } + return metricConfig.visualizes[0]?.yAxis!; +} + export function useMetricReferences() { const metricQueries = useMultiMetricsQueryParams(); return useMemo(() => { - return new Set( + return Object.fromEntries( metricQueries .filter(metricQuery => metricQuery.queryParams.visualizes.some(isVisualizeFunction) ) - .map((metricQuery, index) => metricQuery.label ?? getVisualizeLabel(index, false)) + .map((metricQuery, index) => [ + metricQuery.label ?? getVisualizeLabel(index, false), + resolveMetricReference(metricQuery.queryParams), + ]) ); }, [metricQueries]); } diff --git a/static/app/views/explore/metrics/hooks/useMetricTimeseries.tsx b/static/app/views/explore/metrics/hooks/useMetricTimeseries.tsx index fed39442140bf7..573ce6b6c1a82d 100644 --- a/static/app/views/explore/metrics/hooks/useMetricTimeseries.tsx +++ b/static/app/views/explore/metrics/hooks/useMetricTimeseries.tsx @@ -16,6 +16,7 @@ import { useQueryParamsGroupBys, useQueryParamsSearch, } from 'sentry/views/explore/queryParams/context'; +import {isVisualizeEquation} from 'sentry/views/explore/queryParams/visualize'; import {useSortedTimeSeries} from 'sentry/views/insights/common/queries/useSortedTimeSeries'; interface UseMetricTimeseriesOptions { @@ -68,7 +69,8 @@ function useMetricTimeseriesImpl({ yAxis, interval, fields: [...groupBys, ...yAxis], - enabled: enabled && Boolean(traceMetric.name), + enabled: + enabled && (Boolean(traceMetric.name) || visualizes.some(isVisualizeEquation)), topEvents, orderby: sortBys.map(formatSort), ...queryExtras, diff --git a/static/app/views/explore/metrics/hooks/useSaveMetricsMultiQuery.tsx b/static/app/views/explore/metrics/hooks/useSaveMetricsMultiQuery.tsx index 83eebc9d9e4e99..122669e1b49143 100644 --- a/static/app/views/explore/metrics/hooks/useSaveMetricsMultiQuery.tsx +++ b/static/app/views/explore/metrics/hooks/useSaveMetricsMultiQuery.tsx @@ -13,7 +13,10 @@ import {getTitleFromLocation} from 'sentry/views/explore/contexts/pageParamsCont import {useInvalidateSavedQueries} from 'sentry/views/explore/hooks/useGetSavedQueries'; import {useMultiMetricsQueryParams} from 'sentry/views/explore/metrics/multiMetricsQueryParams'; import {isGroupBy} from 'sentry/views/explore/queryParams/groupBy'; -import {isVisualize} from 'sentry/views/explore/queryParams/visualize'; +import { + isVisualize, + isVisualizeFunction, +} from 'sentry/views/explore/queryParams/visualize'; const METRICS_DATASET = 'metrics'; @@ -70,7 +73,7 @@ export function useSaveMetricsMultiQuery() { ...groupBys, ...(yAxes.length > 0 ? [{yAxes, chartType}] : []), ], - metric: metricQuery.metric, + ...(isVisualizeFunction(visualize) ? {metric: metricQuery.metric} : {}), fields: metricQuery.queryParams.fields, orderby: metricQuery.queryParams.sortBys[0] ? encodeSort(metricQuery.queryParams.sortBys[0]) diff --git a/static/app/views/explore/metrics/metricPanel/index.tsx b/static/app/views/explore/metrics/metricPanel/index.tsx index 1a36e0e43033a8..19b9af089279db 100644 --- a/static/app/views/explore/metrics/metricPanel/index.tsx +++ b/static/app/views/explore/metrics/metricPanel/index.tsx @@ -36,14 +36,14 @@ interface MetricPanelProps { queryIndex: number; queryLabel: string; traceMetric: TraceMetric; - references?: Set; + referenceMap?: Record; } export function MetricPanel({ traceMetric, queryIndex, queryLabel, - references, + referenceMap, }: MetricPanelProps) { const organization = useOrganization(); const { @@ -55,7 +55,7 @@ export function MetricPanel({ const {isMetricOptionsEmpty} = useMetricOptions({enabled: Boolean(traceMetric.name)}); const {result: timeseriesResult} = useMetricTimeseries({ traceMetric, - enabled: Boolean(traceMetric.name) && !isMetricOptionsEmpty, + enabled: !isMetricOptionsEmpty, }); const hasMetricsUIRefresh = canUseMetricsUIRefresh(organization); @@ -104,7 +104,7 @@ export function MetricPanel({ {visualize.visible ? ( diff --git a/static/app/views/explore/metrics/metricToolbar/index.tsx b/static/app/views/explore/metrics/metricToolbar/index.tsx index 618a1728249fb0..420ba0ce860a10 100644 --- a/static/app/views/explore/metrics/metricToolbar/index.tsx +++ b/static/app/views/explore/metrics/metricToolbar/index.tsx @@ -2,10 +2,10 @@ import {Fragment, useCallback} from 'react'; import {Flex, Grid} from '@sentry/scraps/layout'; -import {ArithmeticBuilder} from 'sentry/components/arithmeticBuilder'; import type {Expression} from 'sentry/components/arithmeticBuilder/expression'; import {EQUATION_PREFIX} from 'sentry/utils/discover/fields'; import {useOrganization} from 'sentry/utils/useOrganization'; +import {EquationBuilder} from 'sentry/views/explore/metrics/equationBuilder'; import {type TraceMetric} from 'sentry/views/explore/metrics/metricQuery'; import {canUseMetricsUIRefresh} from 'sentry/views/explore/metrics/metricsFlags'; import { @@ -28,10 +28,14 @@ import { interface MetricToolbarProps { queryLabel: string; traceMetric: TraceMetric; - references?: Set; + referenceMap?: Record; } -export function MetricToolbar({traceMetric, queryLabel, references}: MetricToolbarProps) { +export function MetricToolbar({ + traceMetric, + queryLabel, + referenceMap, +}: MetricToolbarProps) { const organization = useOrganization(); const metricQueries = useMultiMetricsQueryParams(); const visualize = useMetricVisualize(); @@ -97,13 +101,10 @@ export function MetricToolbar({traceMetric, queryLabel, references}: MetricToolb ) : isVisualizeEquation(visualize) ? ( - null} - references={references} - setExpression={handleExpressionChange} + referenceMap={referenceMap} + handleExpressionChange={handleExpressionChange} /> ) : null} {canRemoveMetric && } @@ -146,13 +147,10 @@ export function MetricToolbar({traceMetric, queryLabel, references}: MetricToolb ) : isVisualizeEquation(visualize) ? ( - null} - references={references} - setExpression={handleExpressionChange} + referenceMap={referenceMap} + handleExpressionChange={handleExpressionChange} /> ) : null} {canRemoveMetric && } diff --git a/static/app/views/explore/metrics/metricsQueryParams.tsx b/static/app/views/explore/metrics/metricsQueryParams.tsx index bb55a99c9a328c..a9c867d9a1bbcc 100644 --- a/static/app/views/explore/metrics/metricsQueryParams.tsx +++ b/static/app/views/explore/metrics/metricsQueryParams.tsx @@ -159,6 +159,7 @@ export function useMetricLabel(): string { const {metric} = useTraceMetricContext(); if (isVisualizeEquation(visualize)) { + // TODO: This should show the unresolved expression from the equation builder return visualize.expression.text; } if (isVisualizeFunction(visualize) && visualize.parsedFunction) { diff --git a/static/app/views/explore/metrics/metricsTab.tsx b/static/app/views/explore/metrics/metricsTab.tsx index c0762795e28337..74d31d3bb6c98e 100644 --- a/static/app/views/explore/metrics/metricsTab.tsx +++ b/static/app/views/explore/metrics/metricsTab.tsx @@ -146,7 +146,7 @@ function MetricsQueryBuilderSection() { const addMetricQuery = useAddMetricQuery(); const addEquationQuery = useAddMetricQuery({type: 'equation'}); const hasEquations = canUseMetricsEquations(organization); - const references = useMetricReferences(); + const referenceMap = useMetricReferences(); if (canUseMetricsUIRefresh(organization)) { return null; @@ -173,7 +173,7 @@ function MetricsQueryBuilderSection() { ); @@ -213,7 +213,7 @@ function MetricsTabBodySection() { areToolbarsLoading, isMetricOptionsEmpty, }); - const references = useMetricReferences(); + const referenceMap = useMetricReferences(); // Cannot add metric queries beyond Z const isAddMetricDisabled = @@ -239,7 +239,7 @@ function MetricsTabBodySection() { traceMetric={metricQuery.metric} queryIndex={index} queryLabel={metricQuery.label ?? ''} - references={references} + referenceMap={referenceMap} /> ); @@ -285,7 +285,7 @@ function MetricsTabBodySection() { traceMetric={metricQuery.metric} queryIndex={index} queryLabel={metricQuery.label ?? ''} - references={references} + referenceMap={referenceMap} /> ); From fd0d077b178b7622a4b3d64062cabaed714a806b Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Mon, 13 Apr 2026 12:12:08 -0400 Subject: [PATCH 02/11] useTransition for better input performance --- static/app/views/explore/metrics/equationBuilder.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/static/app/views/explore/metrics/equationBuilder.tsx b/static/app/views/explore/metrics/equationBuilder.tsx index 850f80dd0edde1..c5b7644a62a68e 100644 --- a/static/app/views/explore/metrics/equationBuilder.tsx +++ b/static/app/views/explore/metrics/equationBuilder.tsx @@ -1,4 +1,4 @@ -import {useCallback, useEffect, useMemo} from 'react'; +import {useCallback, useEffect, useMemo, useTransition} from 'react'; import isEqual from 'lodash/isEqual'; import {ArithmeticBuilder} from 'sentry/components/arithmeticBuilder'; @@ -66,6 +66,7 @@ export function EquationBuilder({ handleExpressionChange: (expression: Expression) => void; referenceMap?: Record; }) { + const [_, startTransition] = useTransition(); const prevReferenceMap = usePrevious(referenceMap); const references = useMemo( () => new Set(Object.keys(referenceMap ?? {})), @@ -75,7 +76,9 @@ export function EquationBuilder({ const handleInternalExpressionChange = useCallback( (newExpression: Expression) => { - handleExpressionChange(resolveExpression(newExpression, referenceMap)); + startTransition(() => { + handleExpressionChange(resolveExpression(newExpression, referenceMap)); + }); }, [handleExpressionChange, referenceMap] ); From 53ca6e82d0e6bb14bb877147023485c9e1807694 Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Mon, 13 Apr 2026 12:39:37 -0400 Subject: [PATCH 03/11] do not query if empty equation --- .../app/views/explore/metrics/hooks/useMetricTimeseries.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/static/app/views/explore/metrics/hooks/useMetricTimeseries.tsx b/static/app/views/explore/metrics/hooks/useMetricTimeseries.tsx index 573ce6b6c1a82d..99a140d70d4b25 100644 --- a/static/app/views/explore/metrics/hooks/useMetricTimeseries.tsx +++ b/static/app/views/explore/metrics/hooks/useMetricTimeseries.tsx @@ -70,7 +70,11 @@ function useMetricTimeseriesImpl({ interval, fields: [...groupBys, ...yAxis], enabled: - enabled && (Boolean(traceMetric.name) || visualizes.some(isVisualizeEquation)), + enabled && + (Boolean(traceMetric.name) || + visualizes.some( + visualize => isVisualizeEquation(visualize) && visualize.expression.text + )), topEvents, orderby: sortBys.map(formatSort), ...queryExtras, From 7f48f20015c25bee4b4635b969c02169720e266c Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Mon, 13 Apr 2026 12:41:19 -0400 Subject: [PATCH 04/11] Comments --- static/app/views/explore/metrics/equationBuilder.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/static/app/views/explore/metrics/equationBuilder.tsx b/static/app/views/explore/metrics/equationBuilder.tsx index c5b7644a62a68e..c6db36a1267756 100644 --- a/static/app/views/explore/metrics/equationBuilder.tsx +++ b/static/app/views/explore/metrics/equationBuilder.tsx @@ -10,6 +10,9 @@ import { import {tokenizeExpression} from 'sentry/components/arithmeticBuilder/tokenizer'; import {usePrevious} from 'sentry/utils/usePrevious'; +/** + * Takes an expression and map of references and returns the internal string representation that uses the references. + */ function unresolveExpression( expression: string, referenceMap: Record = {} @@ -30,6 +33,9 @@ function unresolveExpression( .join(' '); } +/** + * Resolves the expression using references into a format that is compatible with our querying endpoints. + */ function resolveExpression( expression: Expression, referenceMap: Record = {} From b1900e46c22dc3dffc064412f0346a3a8f3b7bb5 Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Mon, 13 Apr 2026 12:45:17 -0400 Subject: [PATCH 05/11] fix reverse map calculation to only keep first references --- static/app/views/explore/metrics/equationBuilder.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/static/app/views/explore/metrics/equationBuilder.tsx b/static/app/views/explore/metrics/equationBuilder.tsx index c6db36a1267756..c77684dc0f701d 100644 --- a/static/app/views/explore/metrics/equationBuilder.tsx +++ b/static/app/views/explore/metrics/equationBuilder.tsx @@ -18,8 +18,14 @@ function unresolveExpression( referenceMap: Record = {} ): string { // Reverse the keys and values of the reference map, duplicates keep the first reference found - const reversedReferenceMap = Object.fromEntries( - Object.entries(referenceMap).map(([key, value]) => [value, key]) + const reversedReferenceMap = Object.entries(referenceMap).reduce( + (reversedMap: Record, [key, value]) => { + if (!reversedMap[value]) { + reversedMap[value] = key; + } + return reversedMap; + }, + {} ); const tokens = tokenizeExpression(expression); From 1e1bc54e76897bcb9718c0e979748f6224bbfeb4 Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Mon, 13 Apr 2026 13:02:36 -0400 Subject: [PATCH 06/11] add tests --- .../explore/metrics/equationBuilder.spec.tsx | 61 +++++++++++++++ .../hooks/useMetricReferences.spec.tsx | 78 +++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 static/app/views/explore/metrics/equationBuilder.spec.tsx create mode 100644 static/app/views/explore/metrics/hooks/useMetricReferences.spec.tsx diff --git a/static/app/views/explore/metrics/equationBuilder.spec.tsx b/static/app/views/explore/metrics/equationBuilder.spec.tsx new file mode 100644 index 00000000000000..938c7151cb332a --- /dev/null +++ b/static/app/views/explore/metrics/equationBuilder.spec.tsx @@ -0,0 +1,61 @@ +import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; + +import {EquationBuilder} from 'sentry/views/explore/metrics/equationBuilder'; + +describe('EquationBuilder', () => { + it('takes an equation and represents the equation using the provided reference map', async () => { + const expression = 'count(metricA) + sum(metricB)'; + const referenceMap = { + A: 'count(metricA)', + F: 'sum(metricB)', + }; + + render( + {}} + /> + ); + + const tokens = await screen.findAllByRole('row'); + + // tokens are flanked by empty text nodes for cursor movement, + // and '+' is encoded as 'op:0' + expect(tokens.map(token => token.getAttribute('aria-label'))).toEqual([ + '', + 'A', + '', + 'op:0', + '', + 'F', + '', + ]); + }); + + it('calls the handleExpressionChange callback when the expression changes', async () => { + const expression = ''; + + const handleExpressionChange = jest.fn(); + + render( + + ); + + // Typing the reference de-references it into the metric call + await userEvent.type( + await screen.findByRole('combobox', {name: 'Add a term'}), + 'A * 2' + ); + + expect(handleExpressionChange).toHaveBeenCalledWith( + expect.objectContaining({ + text: 'count(value,metricA,distribution,none) * 2', + }) + ); + }); +}); diff --git a/static/app/views/explore/metrics/hooks/useMetricReferences.spec.tsx b/static/app/views/explore/metrics/hooks/useMetricReferences.spec.tsx new file mode 100644 index 00000000000000..1dae7a626a0f2f --- /dev/null +++ b/static/app/views/explore/metrics/hooks/useMetricReferences.spec.tsx @@ -0,0 +1,78 @@ +import type {ReactNode} from 'react'; + +import {renderHookWithProviders} from 'sentry-test/reactTestingLibrary'; + +import {Mode} from 'sentry/views/explore/contexts/pageParamsContext/mode'; +import {encodeMetricQueryParams} from 'sentry/views/explore/metrics/metricQuery'; +import { + MultiMetricsQueryParamsProvider, + useMultiMetricsQueryParams, +} from 'sentry/views/explore/metrics/multiMetricsQueryParams'; +import {ReadableQueryParams} from 'sentry/views/explore/queryParams/readableQueryParams'; +import {VisualizeFunction} from 'sentry/views/explore/queryParams/visualize'; + +import {useMetricReferences} from './useMetricReferences'; + +function Wrapper({children}: {children: ReactNode}) { + return {children}; +} + +describe('useMetricReferences', () => { + it('returns _if form for metrics with a filter and plain yAxis for metrics without', () => { + const metricWithFilter = encodeMetricQueryParams({ + metric: {name: 'metric_a', type: 'counter', unit: 'none'}, + queryParams: new ReadableQueryParams({ + extrapolate: true, + mode: Mode.SAMPLES, + query: 'status:ok', + cursor: '', + fields: ['id', 'timestamp'], + sortBys: [{field: 'timestamp', kind: 'desc'}], + aggregateCursor: '', + aggregateFields: [new VisualizeFunction('count(value,metric_a,counter,none)')], + aggregateSortBys: [{field: 'count(value,metric_a,counter,none)', kind: 'desc'}], + }), + }); + + const metricWithoutFilter = encodeMetricQueryParams({ + metric: {name: 'metric_b', type: 'counter', unit: 'none'}, + queryParams: new ReadableQueryParams({ + extrapolate: true, + mode: Mode.SAMPLES, + query: '', + cursor: '', + fields: ['id', 'timestamp'], + sortBys: [{field: 'timestamp', kind: 'desc'}], + aggregateCursor: '', + aggregateFields: [new VisualizeFunction('sum(value,metric_b,counter,none)')], + aggregateSortBys: [{field: 'sum(value,metric_b,counter,none)', kind: 'desc'}], + }), + }); + + const {result} = renderHookWithProviders( + () => ({ + references: useMetricReferences(), + queries: useMultiMetricsQueryParams(), + }), + { + additionalWrapper: Wrapper, + initialRouterConfig: { + location: { + pathname: '/', + query: { + metric: [metricWithFilter, metricWithoutFilter], + }, + }, + }, + } + ); + + // Metric A has a filter, so it gets the _if form + expect(result.current.references.A).toBe( + 'count_if(`status:ok`,value,metric_a,counter,none)' + ); + + // Metric B has no filter, so the yAxis is returned as-is + expect(result.current.references.B).toBe('sum(value,metric_b,counter,none)'); + }); +}); From 1e3ebc04b2c9c02d80c0f4f14dd79137f63922b1 Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Mon, 13 Apr 2026 17:25:39 -0400 Subject: [PATCH 07/11] fix merge conflict issue Accidentally deleted some conditional code --- static/app/views/explore/metrics/metricToolbar/index.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/static/app/views/explore/metrics/metricToolbar/index.tsx b/static/app/views/explore/metrics/metricToolbar/index.tsx index a547bcab6392ef..b2ee111da41939 100644 --- a/static/app/views/explore/metrics/metricToolbar/index.tsx +++ b/static/app/views/explore/metrics/metricToolbar/index.tsx @@ -105,9 +105,11 @@ export function MetricToolbar({ - - - + {!isNarrow && ( + + + + )} ) : isVisualizeEquation(visualize) ? ( Date: Mon, 13 Apr 2026 17:32:55 -0400 Subject: [PATCH 08/11] fix disabling for empty equations --- .../app/views/explore/metrics/metricPanel/index.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/static/app/views/explore/metrics/metricPanel/index.tsx b/static/app/views/explore/metrics/metricPanel/index.tsx index 19b9af089279db..61de8ab3ca997a 100644 --- a/static/app/views/explore/metrics/metricPanel/index.tsx +++ b/static/app/views/explore/metrics/metricPanel/index.tsx @@ -28,6 +28,7 @@ import { useQueryParamsMode, useQueryParamsSortBys, } from 'sentry/views/explore/queryParams/context'; +import {isVisualizeEquation} from 'sentry/views/explore/queryParams/visualize'; const RESULT_LIMIT = 50; const TWO_MINUTE_DELAY = 120; @@ -53,10 +54,6 @@ export function MetricPanel({ } = useTableOrientationControl(); const [infoContentHidden, setInfoContentHidden] = useState(false); const {isMetricOptionsEmpty} = useMetricOptions({enabled: Boolean(traceMetric.name)}); - const {result: timeseriesResult} = useMetricTimeseries({ - traceMetric, - enabled: !isMetricOptionsEmpty, - }); const hasMetricsUIRefresh = canUseMetricsUIRefresh(organization); const fields = getTraceSamplesTableFields(TraceSamplesTableColumns); @@ -82,6 +79,13 @@ export function MetricPanel({ const topEvents = useTopEvents(); const visualize = useMetricVisualize(); + const {result: timeseriesResult} = useMetricTimeseries({ + traceMetric, + enabled: + !isMetricOptionsEmpty || + (isVisualizeEquation(visualize) && Boolean(visualize.expression.text)), + }); + useMetricsPanelAnalytics({ interval, isTopN: !!topEvents, From 20cf69df99625353bbf99d0640bdd5013fce00b4 Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Mon, 13 Apr 2026 22:23:32 -0400 Subject: [PATCH 09/11] Address stale referenceMap --- .../views/explore/metrics/equationBuilder.tsx | 40 +++++++++++-------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/static/app/views/explore/metrics/equationBuilder.tsx b/static/app/views/explore/metrics/equationBuilder.tsx index c77684dc0f701d..074a6ac53f0660 100644 --- a/static/app/views/explore/metrics/equationBuilder.tsx +++ b/static/app/views/explore/metrics/equationBuilder.tsx @@ -1,4 +1,4 @@ -import {useCallback, useEffect, useMemo, useTransition} from 'react'; +import {useCallback, useEffect, useMemo, useRef, useTransition} from 'react'; import isEqual from 'lodash/isEqual'; import {ArithmeticBuilder} from 'sentry/components/arithmeticBuilder'; @@ -8,7 +8,6 @@ import { isTokenReference, } from 'sentry/components/arithmeticBuilder/token'; import {tokenizeExpression} from 'sentry/components/arithmeticBuilder/tokenizer'; -import {usePrevious} from 'sentry/utils/usePrevious'; /** * Takes an expression and map of references and returns the internal string representation that uses the references. @@ -79,12 +78,33 @@ export function EquationBuilder({ referenceMap?: Record; }) { const [_, startTransition] = useTransition(); - const prevReferenceMap = usePrevious(referenceMap); const references = useMemo( () => new Set(Object.keys(referenceMap ?? {})), [referenceMap] ); - const internalExpression = unresolveExpression(expression, 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 notify the parent. + useEffect(() => { + if (!isEqual(expressionMapRef.current, referenceMap)) { + const expr = new Expression( + internalExpression, + new Set(Object.keys(expressionMapRef.current ?? {})) + ); + handleExpressionChange(resolveExpression(expr, referenceMap)); + expressionMapRef.current = referenceMap; + } + }, [referenceMap, internalExpression, handleExpressionChange]); const handleInternalExpressionChange = useCallback( (newExpression: Expression) => { @@ -95,18 +115,6 @@ export function EquationBuilder({ [handleExpressionChange, referenceMap] ); - // Trigger the expression change when the reference map changes to ensure the query is showing the correct data - useEffect(() => { - if (!isEqual(prevReferenceMap, referenceMap)) { - const internalRepresentation = unresolveExpression(expression, prevReferenceMap); - const expr = new Expression( - internalRepresentation, - new Set(Object.keys(prevReferenceMap ?? {})) - ); - handleExpressionChange(resolveExpression(expr, referenceMap)); - } - }, [prevReferenceMap, referenceMap, expression, handleExpressionChange]); - return ( Date: Mon, 13 Apr 2026 23:28:33 -0400 Subject: [PATCH 10/11] Call setter if isValid --- static/app/views/explore/metrics/equationBuilder.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/static/app/views/explore/metrics/equationBuilder.tsx b/static/app/views/explore/metrics/equationBuilder.tsx index 074a6ac53f0660..c16a43c977344b 100644 --- a/static/app/views/explore/metrics/equationBuilder.tsx +++ b/static/app/views/explore/metrics/equationBuilder.tsx @@ -94,15 +94,18 @@ export function EquationBuilder({ mapChanged ? expressionMapRef.current : referenceMap ); - // When the reference map changes, re-resolve the expression and notify the parent. + // 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 ?? {})) ); - handleExpressionChange(resolveExpression(expr, referenceMap)); - expressionMapRef.current = referenceMap; + const resolved = resolveExpression(expr, referenceMap); + if (resolved.isValid) { + handleExpressionChange(resolved); + expressionMapRef.current = referenceMap; + } } }, [referenceMap, internalExpression, handleExpressionChange]); From bdb49208e8a1acc19d3b0bd699d5747fb82535af Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Tue, 14 Apr 2026 12:30:58 -0400 Subject: [PATCH 11/11] fix validity checks --- static/app/views/explore/metrics/equationBuilder.tsx | 10 ++++++++-- .../app/views/explore/metrics/metricToolbar/index.tsx | 4 ---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/static/app/views/explore/metrics/equationBuilder.tsx b/static/app/views/explore/metrics/equationBuilder.tsx index c16a43c977344b..fb6c6a17a42813 100644 --- a/static/app/views/explore/metrics/equationBuilder.tsx +++ b/static/app/views/explore/metrics/equationBuilder.tsx @@ -102,7 +102,11 @@ export function EquationBuilder({ new Set(Object.keys(expressionMapRef.current ?? {})) ); const resolved = resolveExpression(expr, referenceMap); - if (resolved.isValid) { + + // 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; } @@ -112,7 +116,9 @@ export function EquationBuilder({ const handleInternalExpressionChange = useCallback( (newExpression: Expression) => { startTransition(() => { - handleExpressionChange(resolveExpression(newExpression, referenceMap)); + if (newExpression.isValid) { + handleExpressionChange(resolveExpression(newExpression, referenceMap)); + } }); }, [handleExpressionChange, referenceMap] diff --git a/static/app/views/explore/metrics/metricToolbar/index.tsx b/static/app/views/explore/metrics/metricToolbar/index.tsx index 4623efbcee9f9e..ee53c099370062 100644 --- a/static/app/views/explore/metrics/metricToolbar/index.tsx +++ b/static/app/views/explore/metrics/metricToolbar/index.tsx @@ -60,10 +60,6 @@ export function MetricToolbar({ const handleExpressionChange = useCallback( (newExpression: Expression) => { - const isValid = newExpression.isValid; - if (!isValid) { - return; - } setVisualize(visualize.replace({yAxis: `${EQUATION_PREFIX}${newExpression.text}`})); }, [setVisualize, visualize]