feat(eap): Allow arrays to be queried from EAP #7551
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| attributes_array = _process_arrays(arrays) | ||
| for key, value in attributes_array.items(): | ||
| add_attribute(k, v) | ||
|
|
||
| item = GetTraceResponse.Item( |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
This is a real bug, was this come across in tests?
There was a problem hiding this comment.
No, it was due to a late refactor before I commit (rename k/v to key/value) and didn't run the tests in-between.
|
Sentry needed to have some compatibility work done: getsentry/sentry#104063 |
| expression=FunctionCall( | ||
| "attributes_array", | ||
| "toJSONString", | ||
| (column("attributes_array"),), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Bug: FunctionCall is instantiated with 3 positional arguments instead of the expected 2, leading to a TypeError at runtime.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The FunctionCall dataclass is defined with two fields: function_name and parameters. However, the code attempts to instantiate FunctionCall with three positional arguments: "attributes_array", "toJSONString", and (column("attributes_array"),). This mismatch in the number of arguments will lead to a TypeError at runtime when the toJSONString function call is constructed, causing an unexpected server crash for any request querying the attributes_array column.
💡 Suggested Fix
Remove the extraneous first argument "attributes_array" from the FunctionCall constructor. The correct call should be FunctionCall("toJSONString", (column("attributes_array"),),).
🤖 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: snuba/web/rpc/v1/endpoint_get_trace.py#L231-L236
Potential issue: The `FunctionCall` dataclass is defined with two fields:
`function_name` and `parameters`. However, the code attempts to instantiate
`FunctionCall` with three positional arguments: `"attributes_array"`, `"toJSONString"`,
and `(column("attributes_array"),)`. This mismatch in the number of arguments will lead
to a `TypeError` at runtime when the `toJSONString` function call is constructed,
causing an unexpected server crash for any request querying the `attributes_array`
column.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 3924818
This will allow us to return array values stored in
attributes_arraywhen fetching a trace. Support to return arrays with containing different types were added tosentry-protos(getsentry/sentry-protos#153).One caveat right now is
clickhouse-driverdoesn't support theJSONorVariantcolumn types. The workaround for this right now is to encode as JSON and send the value back to Snuba as a string.There is a PR opened which adds support but there's not a lot of activity. Perhaps we should fork
clickhouse-driverand add support for theJSONtype for a mid term solution, then upsrtream the result.