Skip to content

feat(issues): Split search.group_index referrer by UI vs API - #120404

Open
mrduncan wants to merge 2 commits into
masterfrom
mrduncan/split-index-referrer
Open

feat(issues): Split search.group_index referrer by UI vs API#120404
mrduncan wants to merge 2 commits into
masterfrom
mrduncan/split-index-referrer

Conversation

@mrduncan

Copy link
Copy Markdown
Member

Issue search emitted a single search.group_index Snuba referrer for every request, blending UI (browser session) traffic with public API / integration traffic. Emit a distinct search.group_index.api referrer for token-authenticated requests so the two can be measured separately.

Gated behind the off-by-default organizations:search-group-index-api-referrer flag for a gradual rollout.

Issue search emitted a single `search.group_index` Snuba referrer for
every request, blending UI (browser session) traffic with public API /
integration traffic. Emit a distinct `search.group_index.api` referrer
for token-authenticated requests so the two can be measured separately.

Gated behind the off-by-default `organizations:search-group-index-api-referrer`
flag for a gradual rollout.
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 22, 2026
Assign the private DRF `_authenticator` attribute via `setattr` so mypy does not flag it as [attr-defined], matching the existing `_wrap_request` helper.
@mrduncan
mrduncan marked this pull request as ready for review July 23, 2026 00:16
@mrduncan
mrduncan requested review from a team as code owners July 23, 2026 00:16
Comment on lines +799 to +800
SEARCH_GROUP_INDEX_API = "search.group_index.api"
SEARCH_GROUP_INDEX_API_SAMPLE = "search.group_index.api_sample"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The sampling code path in calculate_hits() bypasses get_search_referrer(), relying on fragile string concatenation to create the referrer, which is inconsistent with the intended API vs. UI split logic.
Severity: MEDIUM

Suggested Fix

Refactor the sampling code path in calculate_hits() to use the get_search_referrer() function to determine the base referrer before sampling is applied. This will ensure that both primary and sampling queries consistently use the same authentication-aware logic for splitting UI and API traffic.

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/snuba/referrer.py#L799-L800

Potential issue: The sampling code path in `calculate_hits()` bypasses the
`get_search_referrer()` function, which is intended to differentiate referrers for UI
vs. API requests. Instead, it relies on simple string concatenation to append `_sample`
to the original referrer. While this currently works by coincidence, it creates a
semantic inconsistency and makes the code fragile. This bypass of the intended
authentication-aware logic could lead to metrics pollution if the sampling logic or
feature flag behavior is modified in the future, as it violates the PR's goal of
explicitly separating API and UI traffic.

Did we get this right? 👍 / 👎 to inform future reviews.

SEARCH_GROUP_INDEX = "search.group_index"
SEER_NIGHT_SHIFT_FIXABILITY_SCORE_STRATEGY = "seer.night_shift.fixability_score_strategy"
SEARCH_GROUP_INDEX_SAMPLE = "search.group_index_sample"
SEARCH_GROUP_INDEX_API = "search.group_index.api"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we would also need to set this referrer in snuba like so: getsentry/snuba#7889, as otherwise it would have different rate limits than the original referrer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants