Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions static/app/views/explore/metrics/equationBuilder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -72,9 +79,11 @@ export function EquationBuilder({
expression,
referenceMap,
handleExpressionChange,
onReferenceLabelsChange,
}: {
expression: string;
handleExpressionChange: (expression: Expression) => void;
onReferenceLabelsChange?: (labels: string[]) => void;
referenceMap?: Record<string, string>;
}) {
const [_, startTransition] = useTransition();
Expand Down Expand Up @@ -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(() => {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<string, string[]>()
);

const onEquationLabelsChange = useCallback(
(equationLabel: string, labels: string[]) => {
onEquationLabelsChangeState(prev => new Map(prev).set(equationLabel, labels));
},
[]
);

const referencedMetricLabels = useMemo(() => {
const set = new Set<string>();
for (const labels of equationLabels.values()) {
for (const label of labels) {
set.add(label);
}
}
return set;
}, [equationLabels]);

return {referencedMetricLabels, onEquationLabelsChange};
}
6 changes: 6 additions & 0 deletions static/app/views/explore/metrics/metricPanel/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ interface MetricPanelProps extends React.HTMLAttributes<HTMLDivElement> {
dragListeners?: SyntheticListenerMap;
isAnyDragging?: boolean;
isDragging?: boolean;
onEquationLabelsChange?: (equationLabel: string, labels: string[]) => void;
ref?: React.Ref<HTMLDivElement>;
referenceMap?: Record<string, string>;
referencedMetricLabels?: Set<string>;
}

export function MetricPanel({
Expand All @@ -58,6 +60,8 @@ export function MetricPanel({
isDragging,
style,
ref,
referencedMetricLabels,
onEquationLabelsChange,
...rest
}: MetricPanelProps) {
const organization = useOrganization();
Expand Down Expand Up @@ -126,6 +130,8 @@ export function MetricPanel({
queryLabel={queryLabel}
referenceMap={referenceMap}
dragListeners={dragListeners}
referencedMetricLabels={referencedMetricLabels}
onEquationLabelsChange={onEquationLabelsChange}
/>
</Container>
{visualize.visible ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ interface SortableMetricPanelProps {
queryLabel: string;
sortableId: string;
traceMetric: TraceMetric;
onEquationLabelsChange?: (equationLabel: string, labels: string[]) => void;
referenceMap?: Record<string, string>;
referencedMetricLabels?: Set<string>;
}

export function SortableMetricPanel({
Expand All @@ -20,6 +22,8 @@ export function SortableMetricPanel({
queryIndex,
queryLabel,
referenceMap,
referencedMetricLabels,
onEquationLabelsChange,
isAnyDragging,
canDrag,
}: SortableMetricPanelProps) {
Expand All @@ -42,6 +46,8 @@ export function SortableMetricPanel({
dragListeners={canDrag ? listeners : undefined}
isAnyDragging={isAnyDragging}
isDragging={isDragging}
referencedMetricLabels={referencedMetricLabels}
onEquationLabelsChange={onEquationLabelsChange}
{...attributes}
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -17,6 +17,10 @@ export function DeleteMetricButton() {
icon={<IconDelete />}
size="zero"
onClick={removeMetric}
disabled={disabled}
tooltipProps={{
title: disabled ? t('This metric is used in an equation') : undefined,
}}
aria-label={t('Delete Metric')}
/>
);
Expand All @@ -28,6 +32,10 @@ export function DeleteMetricButton() {
icon={<IconDelete />}
aria-label={t('Delete metric')}
onClick={removeMetric}
disabled={disabled}
tooltipProps={{
title: disabled ? t('This metric is used in an equation') : undefined,
}}
/>
);
}
24 changes: 22 additions & 2 deletions static/app/views/explore/metrics/metricToolbar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,18 @@ interface MetricToolbarProps {
queryLabel: string;
traceMetric: TraceMetric;
dragListeners?: SyntheticListenerMap;
onEquationLabelsChange?: (equationLabel: string, labels: string[]) => void;
referenceMap?: Record<string, string>;
referencedMetricLabels?: Set<string>;
}

export function MetricToolbar({
traceMetric,
queryLabel,
referenceMap,
dragListeners,
referencedMetricLabels,
onEquationLabelsChange,
}: MetricToolbarProps) {
const organization = useOrganization();
const breakpoints = useBreakpoints();
Expand All @@ -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}`}));
Expand Down Expand Up @@ -115,9 +133,10 @@ export function MetricToolbar({
expression={visualize.expression.text}
referenceMap={referenceMap}
handleExpressionChange={handleExpressionChange}
onReferenceLabelsChange={handleReferenceLabelsChange}
/>
) : null}
{canRemoveMetric && <DeleteMetricButton />}
{canRemoveMetric && <DeleteMetricButton disabled={isReferencedByEquation} />}
</Grid>
{isNarrow && isVisualizeFunction(visualize) && (
<Filter traceMetric={traceMetric} />
Expand Down Expand Up @@ -165,9 +184,10 @@ export function MetricToolbar({
expression={visualize.expression.text}
referenceMap={referenceMap}
handleExpressionChange={handleExpressionChange}
onReferenceLabelsChange={handleReferenceLabelsChange}
/>
) : null}
{canRemoveMetric && <DeleteMetricButton />}
{canRemoveMetric && <DeleteMetricButton disabled={isReferencedByEquation} />}
</Grid>
);
}
75 changes: 75 additions & 0 deletions static/app/views/explore/metrics/metricsTab.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<ProviderWrapper>
<MetricsTabContent datePageFilterProps={datePageFilterProps} />
</ProviderWrapper>,
{
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();
});
});
Loading
Loading