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
Remove trace context from events #6212
Comments
Dumb question probably: Why do you want trace_context to be stringified in the first place? |
It's hard to give any concrete advice without knowing more about why you're consuming the whole |
Not a dumb question at all @tirumaraiselvan! We have an ingest endpoint set up that that puts events onto an AWS EventBridge bus, which other services (mostly lambdas) subscribe to. This has been working really well for us, and I opened this issue after upgrading to v1.3.3 because the event payload we were receiving stopped being JSON.stringify-able, and our ingest started failing. We don't explicitly need the trace context, but its addition broke our ingest endpoint until we added the patch in the 1st comment. That said, it would be very cool one day to be able to use this trace context to correlate hasura action -> event -> ingest -> subscriber(s) |
Good question @paf31 -- we push the whole event onto an AWS EventBridge bus. We then subscribe to this bus using content-based filtering for selective actions. We use As an alternative to the change I posed in the original comment, we could have certainly omitted Having this trace context is great! My intent for opening this issue was less "we need this trace context to be JSON" and more "hey, this update added a really cool new feature, but broke the way we interact with hasura events, here's how we worked around it for context and in case anyone else runs into this. Is this the way it is now or do we have any other options?". If this is the way it is, so be it 😄 |
I wasn't understanding why numbers were an issue, but now I see it's because these numbers are too big for the JS Relevant: https://esdiscuss.org/topic/json-support-for-bigint-in-chrome-v8 I think we will probably remove the event context from the payload since it is available in HTTP headers anyway. |
We experienced this same issue after upgrading to v1.3.3. EventBridge suddenly rejected events created by Hasura. We stripped out the trace_context using |
In 2.0, this should no longer be an issue, since we have switched to using the B3 format for propagation. @tirumaraiselvan would it make sense to try to backport that change? It's unclear because it's technically a breaking change. |
FYI: the field "trace_context" is not documented in https://hasura.io/docs/latest/event-triggers/payload/. Should it be? |
We just updated to v1.3.3. Everything went smoothly, except some of our event handlers started failing because the new
trace_context
values don't stringify correctly in NodeJSWe set the
HASURA_GRAPHQL_STRINGIFY_NUMERIC_TYPES
for a similar reason in order to deal with BigInt values correctlyIs there any way the trace context in event payloads could respect this setting? What are some of our other options?
Right now we've added something like this to our event handlers, but its a bit of a hack:
The text was updated successfully, but these errors were encountered: