feat(rpc): Add more query logging #104480
Conversation
wmak
commented
Dec 5, 2025
wmak
commented
- Move query logging so we have it in even more places
- add some logging to the timeseries/table responses too
- Move query logging so we have it in even more places - add some logging to the timeseries/table responses too
github-actions
bot
added
the
Scope: Backend
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| from sentry.utils import json, metrics | ||
| from sentry.utils.snuba import SnubaError, _snuba_pool | ||
|
|
||
| logger = logging.getLogger(__name__) |
There was a problem hiding this comment.
Bug: Removed log_rpc_request() function but left an orphaned call site in spans_rpc.py, causing an AttributeError on timeout.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The pull request removed the log_rpc_request() function from rpc_dataset_common.py, but a call site in spans_rpc.py (around line 323) was not removed. When a trace query times out during pagination, this orphaned call will attempt to execute rpc_dataset_common.log_rpc_request(), leading to an AttributeError: module 'sentry.snuba.rpc_dataset_common' has no attribute 'log_rpc_request'. This unhandled exception will crash the request handler.
💡 Suggested Fix
Remove the orphaned call to rpc_dataset_common.log_rpc_request() in spans_rpc.py (around line 323) or migrate its logging logic to the new snuba_rpc.py.
🤖 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/utils/snuba_rpc.py#L52
Potential issue: The pull request removed the `log_rpc_request()` function from
`rpc_dataset_common.py`, but a call site in `spans_rpc.py` (around line 323) was not
removed. When a trace query times out during pagination, this orphaned call will attempt
to execute `rpc_dataset_common.log_rpc_request()`, leading to an `AttributeError: module
'sentry.snuba.rpc_dataset_common' has no attribute 'log_rpc_request'`. This unhandled
exception will crash the request handler.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 5900602
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| "rpc_rows": rpc_rows, | ||
| "meta": timeseries_response.meta, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Bug: Incorrect log message for timeseries response
The log message for timeseries responses incorrectly states "Table RPC query response" when it should indicate a timeseries response. This appears to be a copy-paste error from the table response logging block above. This will make it difficult to distinguish between table and timeseries query responses in logs, hindering debugging and monitoring efforts.
