apollographql / router

A configurable, high-performance routing runtime for Apollo Federation 🚀
https://www.apollographql.com/docs/router/
Other
783 stars 255 forks source link

trace_id check to accepts dashes as UUID format #5086

Open juancarlosjr97 opened 2 months ago

juancarlosjr97 commented 2 months ago

Is your feature request related to a problem? Please describe. Long story short: trace_id to accept UUID format.

When adding the propagation and the custom header for the trace_id, aka correlation id from Kong, it fails with an error cannot generate custom trace_id: invalid digit found in the string as the Kong plugin for correlation id creates it in a format xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx, see Kong Correlation ID Plugin documentation: https://docs.konghq.com/hub/kong-inc/correlation-id/

Describe the solution you'd like The router checks for the trace_id to be a hexadecimal string https://github.com/apollographql/router/blob/dev/apollo-router/src/plugins/telemetry/mod.rs#L1850-L1856 which means do not accept dashes and it fails.

A clear and concise description of what you want to happen.

Describe alternatives you've considered

Raise an MR to accept any kind of string from trace_id.

A clear and concise description of any alternative solutions or features you've considered.

Additional context

Closely related to https://github.com/apollographql/router/pull/5071

Samjin commented 2 months ago

related to this too. https://github.com/apollographql/router/issues/5056

abernix commented 2 months ago

Do you think this is ultimately the same as #4892? If so, it should/could be fixed by https://github.com/apollographql/router/pull/5071? (It was recently opened).

Can you take a look?

juancarlosjr97 commented 2 months ago

Hey @abernix. Thanks for sharing. I had a quick look but from my understanding, if it is stripping the dashes, how does that provide continuity using the same trace_id on the request, or am I missing something?

abernix commented 2 months ago

@BrynCooke You're pretty connected to the work here - Can you advise? (Look carefully they're both different nuances of dashes in trace IDs)

BrynCooke commented 2 months ago

@juancarlosjr97 Trace ID in opentelemetry is represented as a 128 bit number. This is usually represented as a hex string without dashes and is based on w3c trace id format.

The UUID that is supplied by kong is a 128 but UUID. This has dashes in but is still 128 bits and so can be coerced into the Otel 128 bit number by removing the dashes and parsing as hex.

When sending to your APM, depending on protocol it will send the trace ID in different formats, but the APM should understand this and decode them to the same value. At least that's the theory.

I haven't used kong, but the docs seem to indicate that the otel plugin allows propagation of various formats: https://docs.konghq.com/hub/kong-inc/opentelemetry/#propagation. Is this something you have configured?

juancarlosjr97 commented 2 months ago

Hey @BrynCooke, if that is the case, then brilliant! Nothing else to do here, just wait until https://github.com/apollographql/router/pull/5071 is merged and I will report back this issue.

Regarding our kong setup, we have trace id enabled in traces. The format is something I will have to check when the MR gets in and I can test it on a live environment.