diff --git a/static/app/views/explore/metrics/equationBuilder.tsx b/static/app/views/explore/metrics/equationBuilder.tsx index fb6c6a17a42813..a2a5793df59c2e 100644 --- a/static/app/views/explore/metrics/equationBuilder.tsx +++ b/static/app/views/explore/metrics/equationBuilder.tsx @@ -9,6 +9,13 @@ import { } 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. */ @@ -72,9 +79,11 @@ export function EquationBuilder({ expression, referenceMap, handleExpressionChange, + onReferenceLabelsChange, }: { expression: string; handleExpressionChange: (expression: Expression) => void; + onReferenceLabelsChange?: (labels: string[]) => void; referenceMap?: Record; }) { const [_, startTransition] = useTransition(); @@ -113,6 +122,16 @@ export function EquationBuilder({ } }, [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(() => { diff --git a/static/app/views/explore/metrics/hooks/useEquationReferencedLabels.tsx b/static/app/views/explore/metrics/hooks/useEquationReferencedLabels.tsx new file mode 100644 index 00000000000000..15a96564c5e284 --- /dev/null +++ b/static/app/views/explore/metrics/hooks/useEquationReferencedLabels.tsx @@ -0,0 +1,31 @@ +import {useCallback, useMemo, useState} from 'react'; + +/** + * Tracks which metric labels (A, B, etc.) are referenced by equations. + * Each EquationBuilder reports its labels via onEquationLabelsChange after + * unresolving its expression. The aggregate set is derived from state. + */ +export function useEquationReferencedLabels() { + const [equationLabels, onEquationLabelsChangeState] = useState( + new Map() + ); + + const onEquationLabelsChange = useCallback( + (equationLabel: string, labels: string[]) => { + onEquationLabelsChangeState(prev => new Map(prev).set(equationLabel, labels)); + }, + [] + ); + + const referencedMetricLabels = useMemo(() => { + const set = new Set(); + for (const labels of equationLabels.values()) { + for (const label of labels) { + set.add(label); + } + } + return set; + }, [equationLabels]); + + return {referencedMetricLabels, onEquationLabelsChange}; +} diff --git a/static/app/views/explore/metrics/metricPanel/index.tsx b/static/app/views/explore/metrics/metricPanel/index.tsx index 61a03da9a1d686..f400e3dd51c459 100644 --- a/static/app/views/explore/metrics/metricPanel/index.tsx +++ b/static/app/views/explore/metrics/metricPanel/index.tsx @@ -44,8 +44,10 @@ interface MetricPanelProps extends React.HTMLAttributes { dragListeners?: SyntheticListenerMap; isAnyDragging?: boolean; isDragging?: boolean; + onEquationLabelsChange?: (equationLabel: string, labels: string[]) => void; ref?: React.Ref; referenceMap?: Record; + referencedMetricLabels?: Set; } export function MetricPanel({ @@ -58,6 +60,8 @@ export function MetricPanel({ isDragging, style, ref, + referencedMetricLabels, + onEquationLabelsChange, ...rest }: MetricPanelProps) { const organization = useOrganization(); @@ -126,6 +130,8 @@ export function MetricPanel({ queryLabel={queryLabel} referenceMap={referenceMap} dragListeners={dragListeners} + referencedMetricLabels={referencedMetricLabels} + onEquationLabelsChange={onEquationLabelsChange} /> {visualize.visible ? ( diff --git a/static/app/views/explore/metrics/metricPanel/sortableMetricPanel.tsx b/static/app/views/explore/metrics/metricPanel/sortableMetricPanel.tsx index a1468aedec56a2..0c211d5e25c82d 100644 --- a/static/app/views/explore/metrics/metricPanel/sortableMetricPanel.tsx +++ b/static/app/views/explore/metrics/metricPanel/sortableMetricPanel.tsx @@ -11,7 +11,9 @@ interface SortableMetricPanelProps { queryLabel: string; sortableId: string; traceMetric: TraceMetric; + onEquationLabelsChange?: (equationLabel: string, labels: string[]) => void; referenceMap?: Record; + referencedMetricLabels?: Set; } export function SortableMetricPanel({ @@ -20,6 +22,8 @@ export function SortableMetricPanel({ queryIndex, queryLabel, referenceMap, + referencedMetricLabels, + onEquationLabelsChange, isAnyDragging, canDrag, }: SortableMetricPanelProps) { @@ -42,6 +46,8 @@ export function SortableMetricPanel({ dragListeners={canDrag ? listeners : undefined} isAnyDragging={isAnyDragging} isDragging={isDragging} + referencedMetricLabels={referencedMetricLabels} + onEquationLabelsChange={onEquationLabelsChange} {...attributes} /> ); diff --git a/static/app/views/explore/metrics/metricToolbar/deleteMetricButton.tsx b/static/app/views/explore/metrics/metricToolbar/deleteMetricButton.tsx index daf5c333dda725..a58160f16d049e 100644 --- a/static/app/views/explore/metrics/metricToolbar/deleteMetricButton.tsx +++ b/static/app/views/explore/metrics/metricToolbar/deleteMetricButton.tsx @@ -6,7 +6,7 @@ import {useOrganization} from 'sentry/utils/useOrganization'; import {canUseMetricsUIRefresh} from 'sentry/views/explore/metrics/metricsFlags'; import {useRemoveMetric} from 'sentry/views/explore/metrics/metricsQueryParams'; -export function DeleteMetricButton() { +export function DeleteMetricButton({disabled}: {disabled?: boolean}) { const organization = useOrganization(); const removeMetric = useRemoveMetric(); @@ -17,6 +17,10 @@ export function DeleteMetricButton() { icon={} size="zero" onClick={removeMetric} + disabled={disabled} + tooltipProps={{ + title: disabled ? t('This metric is used in an equation') : undefined, + }} aria-label={t('Delete Metric')} /> ); @@ -28,6 +32,10 @@ export function DeleteMetricButton() { icon={} aria-label={t('Delete metric')} onClick={removeMetric} + disabled={disabled} + tooltipProps={{ + title: disabled ? t('This metric is used in an equation') : undefined, + }} /> ); } diff --git a/static/app/views/explore/metrics/metricToolbar/index.tsx b/static/app/views/explore/metrics/metricToolbar/index.tsx index ee53c099370062..ec888f45ebb9d2 100644 --- a/static/app/views/explore/metrics/metricToolbar/index.tsx +++ b/static/app/views/explore/metrics/metricToolbar/index.tsx @@ -32,7 +32,9 @@ interface MetricToolbarProps { queryLabel: string; traceMetric: TraceMetric; dragListeners?: SyntheticListenerMap; + onEquationLabelsChange?: (equationLabel: string, labels: string[]) => void; referenceMap?: Record; + referencedMetricLabels?: Set; } export function MetricToolbar({ @@ -40,6 +42,8 @@ export function MetricToolbar({ queryLabel, referenceMap, dragListeners, + referencedMetricLabels, + onEquationLabelsChange, }: MetricToolbarProps) { const organization = useOrganization(); const breakpoints = useBreakpoints(); @@ -58,6 +62,20 @@ export function MetricToolbar({ metricQueries.filter(q => isVisualizeFunction(q.queryParams.visualizes[0]!)).length > 1 || isVisualizeEquation(visualize); + // A metric function cannot be deleted if it is referenced by any equation. + // referencedMetricLabels is precomputed from the stored equations and + // overridden with exact labels when the user edits an equation, so that + // duplicate metrics only block deletion of the specific label used. + const isReferencedByEquation = + isVisualizeFunction(visualize) && (referencedMetricLabels?.has(queryLabel) ?? false); + + const handleReferenceLabelsChange = useCallback( + (labels: string[]) => { + onEquationLabelsChange?.(queryLabel, labels); + }, + [onEquationLabelsChange, queryLabel] + ); + const handleExpressionChange = useCallback( (newExpression: Expression) => { setVisualize(visualize.replace({yAxis: `${EQUATION_PREFIX}${newExpression.text}`})); @@ -115,9 +133,10 @@ export function MetricToolbar({ expression={visualize.expression.text} referenceMap={referenceMap} handleExpressionChange={handleExpressionChange} + onReferenceLabelsChange={handleReferenceLabelsChange} /> ) : null} - {canRemoveMetric && } + {canRemoveMetric && } {isNarrow && isVisualizeFunction(visualize) && ( @@ -165,9 +184,10 @@ export function MetricToolbar({ expression={visualize.expression.text} referenceMap={referenceMap} handleExpressionChange={handleExpressionChange} + onReferenceLabelsChange={handleReferenceLabelsChange} /> ) : null} - {canRemoveMetric && } + {canRemoveMetric && } ); } diff --git a/static/app/views/explore/metrics/metricsTab.spec.tsx b/static/app/views/explore/metrics/metricsTab.spec.tsx index 12f7168d8156d8..d451e9a336cc6e 100644 --- a/static/app/views/explore/metrics/metricsTab.spec.tsx +++ b/static/app/views/explore/metrics/metricsTab.spec.tsx @@ -731,4 +731,79 @@ describe('MetricsTabContent', () => { expect(screen.getByRole('button', {name: 'Add Metric'})).toBeDisabled(); expect(screen.getByRole('button', {name: 'Add Equation'})).toBeDisabled(); }); + + it('disables delete button for metrics referenced by an equation', async () => { + const orgWithEquations = OrganizationFixture({ + features: ['tracemetrics-enabled', 'tracemetrics-equations-in-explore'], + }); + + const metricA = JSON.stringify({ + metric: {name: 'metricA', type: 'distribution', unit: 'none'}, + query: '', + aggregateFields: [{yAxes: ['sum(value,metricA,distribution,none)']}], + aggregateSortBys: [], + mode: 'samples', + }); + + const metricB = JSON.stringify({ + metric: {name: 'metricB', type: 'distribution', unit: 'none'}, + query: '', + aggregateFields: [{yAxes: ['sum(value,metricB,distribution,none)']}], + aggregateSortBys: [], + mode: 'samples', + }); + + const equation = JSON.stringify({ + metric: {name: '', type: ''}, + query: '', + aggregateFields: [{yAxes: ['equation|sum(value,metricA,distribution,none)']}], + aggregateSortBys: [], + mode: 'samples', + }); + + render( + + + , + { + organization: orgWithEquations, + initialRouterConfig: { + location: { + pathname: '/organizations/:orgId/explore/metrics/', + query: { + start: '2025-04-10T14%3A37%3A55', + end: '2025-04-10T20%3A04%3A51', + metric: [metricA, metricB, equation], + title: 'Test Title', + }, + }, + route: '/organizations/:orgId/explore/metrics/', + }, + } + ); + + const toolbars = screen.getAllByTestId('metric-toolbar'); + expect(toolbars).toHaveLength(3); + + await waitFor(() => { + expect( + within(toolbars[0]!).getByRole('button', {name: 'metricA'}) + ).toBeInTheDocument(); + }); + + // Metric A should be disabled because it is referenced by the equation + expect( + within(toolbars[0]!).getByRole('button', {name: 'Delete metric'}) + ).toBeDisabled(); + + // Metric B should be enabled because it is not referenced by the equation + expect( + within(toolbars[1]!).getByRole('button', {name: 'Delete metric'}) + ).toBeEnabled(); + + // Equation deletion should always be enabled + expect( + within(toolbars[2]!).getByRole('button', {name: 'Delete metric'}) + ).toBeEnabled(); + }); }); diff --git a/static/app/views/explore/metrics/metricsTab.tsx b/static/app/views/explore/metrics/metricsTab.tsx index 205cb21e0e2da9..d5fbbf0bf4d613 100644 --- a/static/app/views/explore/metrics/metricsTab.tsx +++ b/static/app/views/explore/metrics/metricsTab.tsx @@ -1,3 +1,4 @@ +import {Fragment} from 'react'; import {closestCenter, DndContext} from '@dnd-kit/core'; import {SortableContext, verticalListSortingStrategy} from '@dnd-kit/sortable'; import styled from '@emotion/styled'; @@ -22,6 +23,7 @@ import { import {ToolbarVisualizeAddChart} from 'sentry/views/explore/components/toolbar/toolbarVisualize'; import {useMetricsAnalytics} from 'sentry/views/explore/hooks/useAnalytics'; import {useMetricOptions} from 'sentry/views/explore/hooks/useMetricOptions'; +import {useEquationReferencedLabels} from 'sentry/views/explore/metrics/hooks/useEquationReferencedLabels'; import {useMetricReferences} from 'sentry/views/explore/metrics/hooks/useMetricReferences'; import {useSortableMetricQueries} from 'sentry/views/explore/metrics/hooks/useSortableMetricQueries'; import {MetricPanel} from 'sentry/views/explore/metrics/metricPanel'; @@ -53,12 +55,28 @@ type MetricsTabProps = { function MetricsTabContentRefreshLayout({datePageFilterProps}: MetricsTabProps) { return ( + + + ); +} + +function MetricsTabContentRefreshInner({datePageFilterProps}: MetricsTabProps) { + const {referencedMetricLabels, onEquationLabelsChange} = useEquationReferencedLabels(); + + return ( + - - + + - + ); } @@ -69,15 +87,35 @@ export function MetricsTabContent({datePageFilterProps}: MetricsTabProps) { return ; } + return ; +} + +function MetricsTabContentDefaultLayout({datePageFilterProps}: MetricsTabProps) { return ( - - - + ); } +function MetricsTabContentDefaultInner({datePageFilterProps}: MetricsTabProps) { + const {referencedMetricLabels, onEquationLabelsChange} = useEquationReferencedLabels(); + + return ( + + + + + + ); +} + function MetricsTabFilterSection({datePageFilterProps}: MetricsTabProps) { const organization = useOrganization(); const metricQueries = useMultiMetricsQueryParams(); @@ -146,7 +184,15 @@ function MetricsTabFilterSection({datePageFilterProps}: MetricsTabProps) { ); } -function MetricsQueryBuilderSection() { +interface SectionProps { + onEquationLabelsChange: (equationLabel: string, labels: string[]) => void; + referencedMetricLabels: Set; +} + +function MetricsQueryBuilderSection({ + referencedMetricLabels, + onEquationLabelsChange, +}: SectionProps) { const organization = useOrganization(); const metricQueries = useMultiMetricsQueryParams(); const addMetricQuery = useAddMetricQuery(); @@ -180,6 +226,8 @@ function MetricsQueryBuilderSection() { traceMetric={metricQuery.metric} queryLabel={metricQuery.label ?? ''} referenceMap={referenceMap} + referencedMetricLabels={referencedMetricLabels} + onEquationLabelsChange={onEquationLabelsChange} /> ); @@ -203,7 +251,10 @@ function MetricsQueryBuilderSection() { ); } -function MetricsTabBodySection() { +function MetricsTabBodySection({ + referencedMetricLabels, + onEquationLabelsChange, +}: SectionProps) { const organization = useOrganization(); const metricQueries = useMultiMetricsQueryParams(); const addMetricQuery = useAddMetricQuery(); @@ -249,6 +300,8 @@ function MetricsTabBodySection() { sortableQueries={aggregateMetricQueries} referenceMap={referenceMap} isAnyDragging={isDragging} + referencedMetricLabels={referencedMetricLabels} + onEquationLabelsChange={onEquationLabelsChange} /> {showSectionSeparator ? ( @@ -264,6 +317,8 @@ function MetricsTabBodySection() { sortableQueries={equationMetricQueries} referenceMap={referenceMap} isAnyDragging={isDragging} + referencedMetricLabels={referencedMetricLabels} + onEquationLabelsChange={onEquationLabelsChange} /> ); @@ -321,12 +378,16 @@ function MetricsTabBodySection() { interface SortableMetricPanelSectionProps { dataTestId: string; isAnyDragging: boolean; + onEquationLabelsChange: (equationLabel: string, labels: string[]) => void; referenceMap: Record; + referencedMetricLabels: Set; sortableQueries: ReturnType; } function SortableMetricPanelSection({ dataTestId, + referencedMetricLabels, + onEquationLabelsChange, sortableQueries, isAnyDragging, referenceMap, @@ -358,6 +419,8 @@ function SortableMetricPanelSection({ removeMetric={metricQuery.removeMetric} >