Remove trace context from events

[nason]

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 NodeJS

We set the HASURA_GRAPHQL_STRINGIFY_NUMERIC_TYPES for a similar reason in order to deal with BigInt values correctly

Is 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:

  if (event?.trace_context) {
    Object.keys(event.trace_context).forEach((key) => {
      if (typeof event.trace_context[key].toString === 'function') {
         event.trace_context[key] = event.trace_context[key].toString()
      }
    })
  }

[tirumaraiselvan]

Dumb question probably: Why do you want trace_context to be stringified in the first place?

[paf31]

It's hard to give any concrete advice without knowing more about why you're consuming the whole event object this way. The event object has four possible fields: op, data, session_variables, and now also trace_context. Are you not able to extract what you need from those fields directly?

[nason]

Dumb question probably: Why do you want trace_context to be stringified in the first place?

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)

[nason]

It's hard to give any concrete advice without knowing more about why you're consuming the whole event object this way. The event object has four possible fields: op, data, session_variables, and now also trace_context. Are you not able to extract what you need from those fields directly?

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 op, data, and session_variables in our subscription patterns, so our event ingest endpoint had done no modification of the event, just passed it straight through -- until we updated to hasura v1.3.3.

As an alternative to the change I posed in the original comment, we could have certainly omitted trace_context. We decided to keep it because it seems like useful information to propagate on through our event bus, and would allow us to correlate all the way from hasura to our event subscribers (with some extra work on our end, of course)

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 😄

[paf31]

I wasn't understanding why numbers were an issue, but now I see it's because these numbers are too big for the JS number type, so they don't parse correctly?

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.

[brittanyroddy]

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 const eventBody = JSON.parse(event.body) if (eventBody.event.trace_context) { delete eventBody.event.trace_context } just another option for people needing a quick workaround. would love to see this bug resolved for tracing purposes as @nason mentioned!

[paf31]

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.

[andoks]

FYI: the field "trace_context" is not documented in https://hasura.io/docs/latest/event-triggers/payload/. Should it be?