feat(occurrences on eap): Implement double reads from EAP for error counts timeseries #107308
Conversation
…_timeseries_by_project_and_release`
github-actions
bot
added
the
Scope: Backend
|
bugbot review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
src/sentry/api/endpoints/release_thresholds/utils/get_errors_counts_timeseries.py
Show resolved
Hide resolved
src/sentry/api/endpoints/release_thresholds/utils/get_errors_counts_timeseries.py
Show resolved
Hide resolved
src/sentry/api/endpoints/release_thresholds/utils/get_errors_counts_timeseries.py
Show resolved
Hide resolved
…errors-counts-timeseries
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| query_string += f' release:"{release_value_list[0]}"' | ||
| else: | ||
| release_filter = " OR ".join([f'release:"{r}"' for r in release_value_list]) | ||
| query_string += f" ({release_filter})" | ||
|
|
||
| if environments_list: | ||
| if len(environments_list) == 1: | ||
| query_string += f' environment:"{environments_list[0]}"' | ||
| else: | ||
| env_filter = " OR ".join([f'environment:"{e}"' for e in environments_list]) | ||
| query_string += f" ({env_filter})" |
There was a problem hiding this comment.
Bug: Release and environment names containing quotes are not escaped when building the EAP query string, which can lead to query syntax errors and silent data loss.
Severity: MEDIUM
Suggested Fix
Instead of building the query via f-string interpolation, use a structured query format or properly escape the release and environment values before inserting them into the query string. This will prevent special characters like quotes from breaking the query syntax.
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/api/endpoints/release_thresholds/utils/get_errors_counts_timeseries.py#L141-L151
Potential issue: The function `get_errors_counts_timeseries` constructs a query string
for an external service using f-string interpolation. Release and environment names are
directly embedded into this string. Sentry does not prevent release or environment names
from containing quote characters. If a name includes a quote, it will break the query
syntax, causing an exception. A broad exception handler then catches this error and
silently returns an empty list, leading to data loss and creating a discrepancy between
the primary Snuba results and the results from this function.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| ) | ||
| return [] | ||
|
|
||
| projects = list(Project.objects.filter(id__in=project_id_list, organization_id=organization_id)) |
There was a problem hiding this comment.
EAP database errors crash request despite available Snuba results
Medium Severity
The _get_errors_counts_timeseries_eap function has incomplete exception handling. The Organization.objects.get() call only catches DoesNotExist, not other database errors (e.g., connection timeout). The Project.objects.filter() call at line 131 has no exception handling at all. If either database operation fails, the exception propagates to the caller, crashing the API request even though Snuba results were already successfully obtained. This defeats the purpose of the double-read pattern where EAP failures should be gracefully handled without affecting production.
Implements double reads of occurrences from EAP for
get_errors_counts_timeseries_by_project_and_releaseinsrc/sentry/api/endpoints/release_thresholds/utils/get_errors_counts_timeseries.py.