feat(slack): Unfurl Explore Traces URLs with chart previews#112020
feat(slack): Unfurl Explore Traces URLs with chart previews#112020DominikB2014 merged 9 commits intomasterfrom
Conversation
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
67dfd36 to
e7211bb
Compare
Check if any org has the data-browsing-widget-unfurl flag enabled before iterating through links. Returns empty immediately if no org qualifies, avoiding unnecessary per-link work. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Switch from calling Spans.run_timeseries_query() directly to using client.get() against the events-timeseries endpoint. This ensures the unfurl goes through the full API authorization flow, avoiding permission bypass concerns. Also fixes: - Remove sum from LINE_PLOT_FIELDS (frontend defines it as area, not line) - Add isinstance check on yAxes to prevent string iteration bug - start/end delta and environment handling now done by the endpoint Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The events-timeseries endpoint reads group-by from the groupBy param, not field. Also update timeseries_to_chart_data to produce the dict-keyed format that TOP5 chart types expect for grouped series. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When has_groups is true, always return the dict-keyed format that TOP5 chart types expect, even if only one group is present. Previously the check required len(matching) > 1, which caused single-group results to use the wrong single-series format. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Explore URL unfurls were bypassing the identity check that Discover unfurls enforce. Unlinked Slack users could trigger unfurls without a linked Sentry account. Add the same identity validation so users are prompted to link their account before explore charts are unfurled. Refs DAIN-1438 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
QueryDict.items() sends only the last value per key to the API, so charting y_axes[0] produced empty charts when multiple yAxes existed. Use y_axes[-1] to stay consistent with what the API actually receives. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| user=user, | ||
| path=f"/organizations/{org_slug}/events-timeseries/", | ||
| params=params, | ||
| ) |
There was a problem hiding this comment.
Multi-valued groupBy params silently dropped by API client
Medium Severity
map_explore_query_args collects multiple groupBy values from aggregateField JSON params into a QueryDict via setlist. However, when _unfurl_explore passes this QueryDict as params to client.get, the internal ApiClient.request iterates params.items(), which for a QueryDict returns only the last value per key. This silently drops all but the last groupBy, project, and environment value. The code acknowledges this limitation for yAxis (line 189-191) but doesn't handle it for groupBy — yet groupBy drives the topEvents and chart-type logic. A user URL with multiple group-by fields will produce an incorrectly grouped chart.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 07d7f7f. Configure here.
There was a problem hiding this comment.
Im planning to test a lot of cases in prod, its a bit annoying testing it all locally
edwardgou-sentry
left a comment
There was a problem hiding this comment.
the seer comment on deduplication looks like it might be valid but probably applies to the other datasets as well. I'll leave it up to you if you think they should be addressed, otherwise seems like a reasonable initial step to me
| DEFAULT_PERIOD = "14d" | ||
| DEFAULT_Y_AXIS = "count(span.duration)" | ||
|
|
||
| # All `multiPlotType: line` fields in /static/app/utils/discover/fields.tsx |
There was a problem hiding this comment.
typo? should be fields/functions from explore right?
There was a problem hiding this comment.
This does actually contain a list of aggregations and explore shares from this list so i think it's fine, but i'll make sure to double check in a follow up
The identity-linking prompt for explore links was gated on discover-basic, but the actual unfurl handler checks data-browsing-widget-unfurl. This caused unlinked users to be prompted to link their identity for unfurls that would silently produce nothing. Map each link type to its own feature flag. 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.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d11b0a1. Configure here.
| and features.has( | ||
| "organizations:discover-basic", organization, actor=request.user | ||
| ) | ||
| and features.has(feature_flag, organization, actor=request.user) |
There was a problem hiding this comment.
Explore unfurl triggers misleading "Discover" identity prompt
Medium Severity
The condition on lines 198–207 now triggers self.prompt_link(slack_request) for LinkType.EXPLORE links (in addition to DISCOVER). However, the prompt_link method hardcodes the user-facing text as "Link your Slack identity to Sentry to unfurl Discover charts." This means unlinked users who share an Explore Traces URL will see a prompt referencing "Discover charts," which is confusing and incorrect for the Explore context.
Reviewed by Cursor Bugbot for commit d11b0a1. Configure here.
There was a problem hiding this comment.
will fix this in a follow up, this is purely cosmetic
Add Slack URL unfurling support for `/explore/traces/` pages. When a user pastes an Explore Traces URL in Slack, it now renders a chart preview — the same way Discover URLs already do. The implementation parses `aggregateField` JSON query params from the URL to extract `yAxes` and `groupBy`, then calls the `events-stats` endpoint with `dataset=spans` and renders the chart using existing Discover chart types via Chartcuterie. Please note: Fully gated behind the existing `organizations:data-browsing-widget-unfurl` feature flag, with user-level actor checks so it can be safely tested per-user in production. Going to follow up with a bunch of cleanup, but wanted to test this behind a flag as it's much easier and more accurate in prod Refs DAIN-1438 --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>


Add Slack URL unfurling support for
/explore/traces/pages. When a user pastes an Explore Traces URL in Slack, it now renders a chart preview — the same way Discover URLs already do.The implementation parses
aggregateFieldJSON query params from the URL to extractyAxesandgroupBy, then calls theevents-statsendpoint withdataset=spansand renders the chart using existing Discover chart types via Chartcuterie.Please note: Fully gated behind the existing
organizations:data-browsing-widget-unfurlfeature flag, with user-level actor checks so it can be safely tested per-user in production. Going to follow up with a bunch of cleanup, but wanted to test this behind a flag as it's much easier and more accurate in prodRefs DAIN-1438