-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat: Use extrapolation mode in alerts #103731
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Use extrapolation mode in alerts #103731
Conversation
| metricExtractionRules: null, | ||
| triggers: triggersClone, | ||
| resolveThreshold: rule.resolveThreshold, | ||
| extrapolationMode: rule.extrapolationMode, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Duplicate alert rules retain original extrapolation mode in chart
When duplicating a metric alert rule, the form initializes extrapolationMode from the original rule's data. This causes the chart to be queried with the original rule's extrapolation mode, but duplicated rules should be treated as new rules without backend-set extrapolation mode. The extrapolation mode is correctly cleared when saving (line 820), but not during the initial state setup for the chart display.
| ? SAMPLING_MODE.NORMAL | ||
| ? rule.extrapolationMode === ExtrapolationMode.NONE | ||
| ? SAMPLING_MODE.HIGH_ACCURACY | ||
| : SAMPLING_MODE.NORMAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Inconsistent sampling mode logic between chart components
The sampling mode is being applied based only on dataset === Dataset.EVENTS_ANALYTICS_PLATFORM, but in TriggersChart the same logic checks both dataset === Dataset.EVENTS_ANALYTICS_PLATFORM AND traceItemType === TraceItemDataset.SPANS. This inconsistency means sampling mode is applied to non-span trace items in the details view but not in the form view, leading to different API calls for the same alert configuration across different pages.
scttcper
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good for metric detectors, hard to say on metric alerts
The backend returns extrapolation mode, a field used for transaction -> span
migration. Use this value to call events-stats/ endpoint with the correct extrapolation
mode and sampling mode.
Alerts with SERVER_WEIGHTED call events-stats/ with 'serverOnly' extrapolation mode and SAMPLING_MODE.NORMAL
Alerts with NONE call events-stats/ with 'none' extrapolation mode and SAMPLING_MODE.HIGH_ACCURACY
Extrapolation mode will be set in the backend and not a field we'd expose to the user.
We're going to have an extrapolation mode called "serverOnly" which extrapolates based on server sampling (which will match generic_metric dataset numbers)
and disable extrapolation (ie extrapolation mode none) to match transaction dataset numbers
This PR takes care of both legacy alerts and monitors