fix(traces): Filter unreleased issue types from trace API#110356
fix(traces): Filter unreleased issue types from trace API#110356
Conversation
github-actions
bot
added
the
Scope: Backend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Empty visible types list skips filtering instead of blocking
- Changed logic to filter by impossible occurrence_type_id (-1) when visible types list is empty, ensuring no results are returned instead of all occurrences.
Or push these changes by commenting:
@cursor push a41492a9b0
Preview (a41492a9b0)
diff --git a/src/sentry/snuba/trace.py b/src/sentry/snuba/trace.py
--- a/src/sentry/snuba/trace.py
+++ b/src/sentry/snuba/trace.py
@@ -334,7 +334,9 @@
if organization:
visible_type_ids = [gt.type_id for gt in grouptype_registry.get_visible(organization)]
- if visible_type_ids:
+ if not visible_type_ids:
+ query = f"trace:{trace_id} occurrence_type_id:[-1]"
+ else:
query = f"trace:{trace_id} occurrence_type_id:[{','.join(map(str, visible_type_ids))}]"
occurrence_query = DiscoverQueryBuilder(This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
| if organization: | ||
| visible_type_ids = [gt.type_id for gt in grouptype_registry.get_visible(organization)] | ||
| if visible_type_ids: | ||
| query = f"trace:{trace_id} occurrence_type_id:[{','.join(map(str, visible_type_ids))}]" |
There was a problem hiding this comment.
Empty visible types list skips filtering instead of blocking
Medium Severity
When organization is provided but get_visible(organization) returns an empty list, the if visible_type_ids check is falsy, so the code falls back to the unfiltered query (trace:{trace_id}), returning all occurrences instead of none. The existing search code in src/sentry/issues/search.py handles this correctly by returning None (no results) when group_types is empty. This inverted fallback means that if no issue types are visible for an org, every occurrence is shown rather than hidden.
There was a problem hiding this comment.
This is true: if for some reason get_visible() is unavailable (should be very rare), we fallback to the original behavior, which is NO filter and "unreleased" occurrences would be included in the response. I thought this was better than returning None, but LMK if you disagree.
| # decide if this is released. | ||
| # decide if this is released. Add to HIDDEN_ISSUE_TYPES as well to prevent Events from this Group | ||
| # being displayed on frontend. | ||
| released: bool = False |
There was a problem hiding this comment.
Because released is stored in code (not db or options), the frontend cannot access it directly, which it needs to do for things like search autocomplete. Therefore, a copy of this needs to be maintained in a place in code that frontend can access:
sentry/static/app/types/group.tsx
Line 218 in 6f8fcaa
wmak
left a comment
wmak
left a comment
There was a problem hiding this comment.
one question about an arg change, but lgtm
| error_id: str | None = None, | ||
| additional_attributes: list[str] | None = None, | ||
| include_uptime: bool = False, | ||
| *, |
There was a problem hiding this comment.
What's the reason we're adding * as an arg?
There was a problem hiding this comment.
It forces the args after it to be a kwarg, so organization must be passed as a kwarg.
I like kwargs more than positional args, and it was easier to update the functions that call this one.
Additionally, organization is required but it comes after several optional args - without the *, I would have to give it a default value since params without a default cannot follow a param with default.
But, i do see how this breaks with current style, so happy to remove if you think that's best!
…110565) partially reverts #109929 - the issue type filtering is now done at the api (#110356) which makes the frontend filtering redundant. removes: - `addOccurrence()` method and its `HIDDEN_OCCURRENCE_TYPE_IDS` filtering - The test for `addOccurrence` filtering - All `addOccurrence()` calls → back to `occurrences.add()` keeps: - `EAPOccurrence.issue_type` field name (bug fix) - anrRootCause.tsx handling of both `type` and `issue_type` - Unfortunately, `TraceOccurrence` = `TracePerformanceIssue | EAPOccurrence`. `TracePerformanceIssue` uses `type` while `EAPOccurrence` now uses `issue_type` so both fields need to be checked on a `TraceOccurrence`.
2 participants
When developing a new performance issue type, we add it with
released = False, which allows us to test and calibrate without exposing the resulting "test issues" to users.Last week I observed events of unreleased issue types were appearing in multiple trace-related views in prod:
Users could see issues in traces that should have been hidden. Unreleased issue types are not returned in
/issues/api responses, but the same filtering was not applied to the trace API.I implemented these 2 prs: #109857 and #109929 (which can be reverted once this is landed, since the changes in this pr are a better and more broad solution).
Solution
Add backend filtering to the trace API (
/organizations/{org}/trace/{trace_id}/) using the same visibility system that the issues API uses. The trace query now callsgrouptype_registry.get_visible(organization)to filter performance occurrences by visible issue types, which:released=Trueissue typesorganizations:issue-{slug}-visiblefeature flag enabledChanges
src/sentry/snuba/trace.py: Addorganizationparam to_perf_issues_queryand filter by visible occurrence type IDssrc/sentry/api/endpoints/organization_trace.py: Thread organization through to the queryNotes
occurrence_type_idfield on the eventsget_visible()call is already used in the issues search endpoint, so performance characteristics are knownGroupTypeRegistry.get_visible)