feat(alerts): Skip metrics_layer when creating a PerformanceMetricsEntitySubscription using insights related aggregates/queries
For context, we want to enable alerting on metrics from the Insights modules. However, there is a blocker that this PR attempts to resolve:
Most metrics displayed in the Insights modules are based on the Discover SpansMetricsQueryBuilder without using the Metrics Layer, while Alerts uses AlertMetricsQueryBuilder with the Metrics Layer.
To support Insights metrics in Alerts, a possible solution involves the following two changes:
- Port over any fields and functions from the
SpansMetricsQueryBuilderconfig to theAlertMetricsQueryBuilderconfig - Disable using the Metrics Layer on the
AlertMetricsQueryBuilderwhen the alert is based on Insights metrics. (We cannot use the Metrics Layer because many of the Insights fields and functions cannot be reproduced with the Metrics Layer's capabilities)
This PR involves point 2 from the above. To detect whether an Alert query is Insights based, parse the aggregate and query. We do it this way because there are no other attributes on the Alert Rule that we can leverage to directly tell us if an Alert is based on an Insight metric (technically they're just generic metrics).
Codecov Report
Attention: Patch coverage is 89.18919% with 4 lines in your changes missing coverage. Please review.
Project coverage is 78.26%. Comparing base (
1eccd67) to head (5279b83). Report is 213 commits behind head on master.
:white_check_mark: All tests successful. No failed tests found.
| Files | Patch % | Lines |
|---|---|---|
| src/sentry/search/events/builder/metrics.py | 87.50% | 2 Missing and 2 partials :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #75765 +/- ##
==========================================
+ Coverage 78.25% 78.26% +0.01%
==========================================
Files 6833 6820 -13
Lines 303569 303808 +239
Branches 52248 52252 +4
==========================================
+ Hits 237546 237776 +230
Misses 59643 59643
- Partials 6380 6389 +9
| Files | Coverage Δ | |
|---|---|---|
| src/sentry/search/events/constants.py | 100.00% <100.00%> (ø) |
|
| src/sentry/search/events/types.py | 91.25% <100.00%> (+0.22%) |
:arrow_up: |
| src/sentry/snuba/entity_subscription.py | 90.51% <ø> (ø) |
|
| src/sentry/search/events/builder/metrics.py | 88.54% <87.50%> (-0.08%) |
:arrow_down: |
As discussed in slack, lets try to use the query parsing in the builder to make this determination instead of using regexes on the raw query and aggregate strings
Updated! I moved the logic into MetricsQueryBuilder and added a insights_metrics_override_metric_layer builder config option to control this
Fixed some issues with use_case_id not using SPAN when appropriate