feat(alerts): Add support for trace metrics alert type #104901
Conversation
Add TRACE_ITEM_METRIC to valid event types for performance alerts. Update entity subscription to use TraceMetrics module for trace metric alerts.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #104901 +/- ##
===========================================
- Coverage 80.64% 80.56% -0.08%
===========================================
Files 9335 9343 +8
Lines 403011 402073 -938
Branches 25695 25695
===========================================
- Hits 325001 323944 -1057
- Misses 77544 77663 +119
Partials 466 466 |
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py
Outdated
Show resolved
Hide resolved
Add TRACE_ITEM_METRIC to valid event types for performance alerts. Update entity subscription to use TraceMetrics module for trace metric alerts.
…dd-alerts-backend
| logger.exception("Invalid trace metrics aggregate: %s %s", aggregate, e) | ||
| raise serializers.ValidationError( | ||
| {"aggregate": f"Invalid trace metrics aggregate: {aggregate}"} | ||
| ) |
There was a problem hiding this comment.
Validation error loses specific error message details
Low Severity
When validate_trace_metrics_aggregate catches an InvalidSearchQuery exception, it discards the specific error message from e and replaces it with a generic message. This includes cases where trace_metric is None (which raises an InvalidSearchQuery with "must specify metric name, type, and unit") and errors from resolve_function. The original error is only logged but not returned to the user, making it difficult to understand why a trace metrics aggregate is invalid.
| def test_extract_eap_metrics_alert(self) -> None: | ||
| assert ( | ||
| get_alert_type_from_aggregate_dataset( | ||
| "count(span.duration)", Dataset.EventsAnalyticsPlatform |
There was a problem hiding this comment.
should we leave this one in?
ddfd167 to
dc03815
Compare
Add TRACE_ITEM_METRIC to valid event types for performance alerts. Update entity subscription to use TraceMetrics module for trace metric alerts.
Additionally, this adds more thorough validation for EAP queries, specifically for
trace_metricsas they have a unique aggregate syntax (instead ofcount(column)it's almost alwayscount(column, <metric tuple>)) that we need to enforce. This should work on both sides of old and new alerts so we're forwards and backwards compatible during the workflow switch over.