-
-
Notifications
You must be signed in to change notification settings - Fork 62
feat(eap): Allow arrays to be queried from EAP #7551
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(eap): Allow arrays to be queried from EAP #7551
Conversation
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a real bug, was this come across in tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
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.