feat(slack): Support unfurling Explore Metrics URLs in Slack#112706
Conversation
ce16f61 to
0407014
Compare
0407014 to
f500974
Compare
f500974 to
65d0a7d
Compare
|
@cursor review |
Add URL matching for /explore/metrics/ paths. The metrics URL uses a single "metric" JSON param with nested aggregateFields, unlike traces (visualize) and logs (aggregateField). The handler extracts yAxes from the metric param and uses the tracemetrics dataset. Refs LINEAR-DAIN-1477 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
65d0a7d to
26b61b5
Compare
narsaynorath
left a comment
There was a problem hiding this comment.
Nice. Just one comment about the wording of how to many metrics there are for clarity
Use getlist('metric') instead of get('metric') to handle multiple
metric params in the URL. Also replace raw 'tracemetrics' strings in
tests with SupportedTraceItemType.TRACEMETRICS for consistency with
other dataset assertions.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Each metric URL param is a self-contained chart config with its own query, yAxes, groupBy, and sort. Accumulating across all metric params would mix unrelated data and produce incorrect chart results. Since we only render one chart per unfurl, process only the first entry. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
json.loads can return non-dict values (null, number, array) which raise AttributeError on .get() calls. Add it to the except clause to match how the visualize parsing handles similar edge cases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1365696. Configure here.
The metric parsing loop extracted yAxes and groupBy but omitted chartType, causing explicitly chosen chart types to be silently ignored for metric unfurls. Now extracts it the same way the visualize_fields loop does. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| if metric_query: | ||
| query["query"] = metric_query | ||
| if metric_sort_bys: | ||
| query.setlist("sort", metric_sort_bys) |
There was a problem hiding this comment.
Bug: Metric parameter parsing and override logic runs for all explore URL types, not just metrics, potentially causing incorrect query and sort overrides for traces or logs URLs.
Severity: LOW
Suggested Fix
Wrap the metric query and sort override logic in a conditional check to ensure it only executes when explore_dataset is SupportedTraceItemType.TRACEMETRICS. This will prevent the overrides from being applied to non-metric URL unfurls.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/sentry/integrations/slack/unfurl/explore.py#L301-L304
Potential issue: The function `map_explore_query_args` unconditionally parses the
`metric` query parameter and applies its internal `query` and `aggregateSortBys` values.
This logic runs for all explore URL types, including traces and logs. If a traces or
logs URL contains a `metric` parameter (e.g., through URL manipulation or a frontend
bug), the `query` and `sort` values from the URL will be incorrectly overridden by the
values from the `metric` parameter. For example, a URL's `aggregateSort` parameter would
be ignored in favor of the `aggregateSortBys` from the `metric` JSON, leading to an
incorrect unfurl.
Did we get this right? 👍 / 👎 to inform future reviews.

Add Slack unfurl support for Explore Metrics URLs (
/explore/metrics/),alongside the existing traces and logs support.
The metrics URL structure differs from traces/logs — it uses a single
metricJSON query param with nested
aggregateFieldscontaining theyAxes, ratherthan separate
visualizeoraggregateFieldparams. The handler parses thisstructure and uses the
tracemetricsdataset for the events-timeseries API call.Supports both
/organizations/{slug}/explore/metrics/and customer domain{slug}.sentry.io/explore/metrics/URL formats.Depends on #112677
Fixes DAIN-1477