Skip to content

feat(tracemetrics): Add equation support in old tracemetric alerts#113665

Merged
narsaynorath merged 2 commits intomasterfrom
nar/feat/tracemetrics-add-equation-support-in-old-alerts
Apr 22, 2026
Merged

feat(tracemetrics): Add equation support in old tracemetric alerts#113665
narsaynorath merged 2 commits intomasterfrom
nar/feat/tracemetrics-add-equation-support-in-old-alerts

Conversation

@narsaynorath
Copy link
Copy Markdown
Member

Only adds the equation builder to old tracemetrics alerts

Best viewed with whitespace changes off since some component structures were indented to allow for the feature flag branch

Only adds the equation builder to old tracemetrics alerts
@narsaynorath narsaynorath requested a review from a team April 22, 2026 14:12
@narsaynorath narsaynorath requested review from a team as code owners April 22, 2026 14:12
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 22, 2026
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 89f37ac. Configure here.

return <MetricsEquationVisualize />;
return (
<MetricsEquationVisualize
projectIds={projectId ? [Number(projectId)] : undefined}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent projectIds fallback: undefined vs empty array

Medium Severity

The refactoring changed the projectIds fallback in the detector form path from [] (empty array, the prior behavior inside MetricToolbar) to undefined. The new legacy alert path in metricsEquationVisualizeField.tsx correctly uses [] as the fallback, matching the original behavior. This inconsistency means the MetricSelector in the detector form receives undefined instead of [] when no project is selected, which can change query behavior (e.g., fetching metrics for all projects rather than none).

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 89f37ac. Configure here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine for now, I can follow up in another PR to tighten up what the proper filter should be in this case. Since project selection is important for setting alerts I don't expect this to be common, only on init maybe but even then, I should just disable the request if we don't have a project where we expect one.

@narsaynorath narsaynorath merged commit e4b8d98 into master Apr 22, 2026
65 checks passed
@narsaynorath narsaynorath deleted the nar/feat/tracemetrics-add-equation-support-in-old-alerts branch April 22, 2026 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants