WIP: Read EAP for tags #103242
Draft
WIP: Read EAP for tags #103242
+145
−15
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
http://localhost:8000/organizations/sentry/issues/17/distributions/?project=1&referrer=issue-stream
github-actions
bot
added
the
Scope: Backend
volokluev
reviewed
Nov 12, 2025
|
|
||
| @classmethod | ||
| @sentry_sdk.trace | ||
| def run_table_query( |
Member
There was a problem hiding this comment.
If this is hitting the EndpointTraceItemTable and you're trying to get tags, it's incorrect.
You want the attribute names endpoint:
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+764
to
+774
| query_string=f"group_id:{group.id}", | ||
| selected_columns=["id", "group_id"], | ||
| orderby=None, | ||
| offset=0, | ||
| limit=3, | ||
| referrer="tagstore._get_tag_keys_and_top_values", | ||
| config=SearchResolverConfig(auto_fields=True), | ||
| sampling_mode="NORMAL", | ||
| ) | ||
|
|
||
| raise Exception(str(eap_vals)) |
There was a problem hiding this comment.
Bug: Unconditional Exception raised in get_group_tag_keys_and_top_values() when fetching group tags.
Severity: CRITICAL | Confidence: 1.00
🔍 Detailed Analysis
The code unconditionally raises an Exception at line 774 within an if True: block, and also in the else branch at line 804. This occurs when GroupTagsEndpoint.get() calls backend.get_group_tag_keys_and_top_values(). The exception is not handled anywhere in the call chain, leading to a 500 error for any client attempting to retrieve group tag data.
💡 Suggested Fix
Remove the if True: block and the raise Exception(...) statements at lines 774 and 804. Implement the intended logic for eap_vals and values_by_key.
🤖 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#L756-L774
Potential issue: The code unconditionally raises an `Exception` at line 774 within an
`if True:` block, and also in the `else` branch at line 804. This occurs when
`GroupTagsEndpoint.get()` calls `backend.get_group_tag_keys_and_top_values()`. The
exception is not handled anywhere in the call chain, leading to a 500 error for any
client attempting to retrieve group tag data.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference_id: 2618570
| from sentry.models.releaseprojectenvironment import ReleaseProjectEnvironment | ||
| from sentry.models.releases.release_project import ReleaseProject | ||
| from sentry.replays.query import query_replays_dataset_tagkey_values | ||
| from sentry.search.eap.columns import SearchResolverConfig |
There was a problem hiding this comment.
Bug: Incorrect import path for SearchResolverConfig causing ImportError at module load time.
Severity: CRITICAL | Confidence: 1.00
🔍 Detailed Analysis
The import statement from sentry.search.eap.columns import SearchResolverConfig at line 29 is incorrect. SearchResolverConfig is defined in sentry.search.eap.types.py and imported into columns.py for internal use, but it is not re-exported. This will cause an ImportError: cannot import name 'SearchResolverConfig' when src/sentry/tagstore/snuba/backend.py is loaded.
💡 Suggested Fix
Change the import statement to from sentry.search.eap.types import SearchResolverConfig.
🤖 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#L29
Potential issue: The import statement `from sentry.search.eap.columns import
SearchResolverConfig` at line 29 is incorrect. `SearchResolverConfig` is defined in
`sentry.search.eap.types.py` and imported into `columns.py` for internal use, but it is
not re-exported. This will cause an `ImportError: cannot import name
'SearchResolverConfig'` when `src/sentry/tagstore/snuba/backend.py` is loaded.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference_id: 2618570
Comment on lines
+214
to
215
| if True: | ||
| self._forward_event_to_items(event, event_data, event_type, project) |
There was a problem hiding this comment.
Bug: Feature flag for EAP event forwarding is bypassed, enabling it unconditionally for all events.
Severity: CRITICAL | Confidence: 1.00
🔍 Detailed Analysis
The code unconditionally enables event forwarding to the EAP items system for all events from all projects by changing if in_rollout_group(...) to if True. This bypasses the intended gradual rollout mechanism controlled by the eventstream.eap_forwarding_rate option, causing the EAP data pipeline to receive all events instead of a controlled percentage. This could overwhelm the items/trace system, lead to data duplication, or degrade performance.
💡 Suggested Fix
Revert the if True condition to if in_rollout_group("eventstream.eap_forwarding_rate", event.project_id) and re-add the import for in_rollout_group.
🤖 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/eventstream/snuba.py#L214-L215
Potential issue: The code unconditionally enables event forwarding to the EAP items
system for all events from all projects by changing `if in_rollout_group(...)` to `if
True`. This bypasses the intended gradual rollout mechanism controlled by the
`eventstream.eap_forwarding_rate` option, causing the EAP data pipeline to receive all
events instead of a controlled percentage. This could overwhelm the items/trace system,
lead to data duplication, or degrade performance.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference_id: 2618570
❌ 75 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Not working, returning: