Skip to content

feat(occurrences on eap): Implement EAP query for tagstore group tag value count#111868

Open
shashjar wants to merge 1 commit intomasterfrom
shashjar/implement-tagstore-group-tag-value-count-eap-query
Open

feat(occurrences on eap): Implement EAP query for tagstore group tag value count#111868
shashjar wants to merge 1 commit intomasterfrom
shashjar/implement-tagstore-group-tag-value-count-eap-query

Conversation

@shashjar
Copy link
Copy Markdown
Member

Implements double reads of occurrences from EAP for get_group_tag_value_count in src/sentry/tagstore/snuba/backend.py.

@shashjar shashjar requested review from a team and thetruecpaul March 31, 2026 00:16
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 31, 2026
@shashjar shashjar marked this pull request as ready for review March 31, 2026 00:17
@shashjar shashjar requested a review from a team as a code owner March 31, 2026 00:17
@shashjar shashjar removed the request for review from a team March 31, 2026 00:17
Comment on lines +918 to +919
start=now - timedelta(days=90),
end=now,
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 EAP query uses a fixed 90-day time window, while the control Snuba query uses a dynamic window, causing systematic undercounting for older groups.
Severity: HIGH

Suggested Fix

Align the time windows between the control and experimental paths. The start and end datetime objects used for the main Snuba query should be passed to the count_occurrences call in the experimental path, replacing the hardcoded now - timedelta(days=90) value.

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/tagstore/snuba/backend.py#L918-L919

Potential issue: The experimental EAP query path in `get_group_tag_value_count` uses a
hardcoded 90-day time window for its `count_occurrences` call. However, the control path
(the existing Snuba query) uses a dynamic time window that can be much larger, often
extending back to the group's `first_seen` date. This discrepancy causes the
experimental path to systematically undercount events for any group older than 90 days.
While this currently only affects experiment telemetry, it would lead to incorrect data
being displayed in production if this experimental code path were to be promoted.

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

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.

1 participant