DataDog / dd-trace-js

JavaScript APM Tracer
https://docs.datadoghq.com/tracing/
Other
639 stars 302 forks source link

hapi traces no longer respects x-datadog-trace-id and x-datadog-parent-id when updating from 2.3.0 to 2.33.0 #3121

Open jcjones1515 opened 1 year ago

jcjones1515 commented 1 year ago

Expected behaviour

Requests from the browser containing headers set by @datadog/browser-rum should link together requests with server side tracing like this:

image

Actual behaviour

After updating to 2.33.0 I now get nothing linked:

image

I observed that the headers are still being sent in dev tools, and no other changes were made.

Steps to reproduce

I see a test to cover this specific scenario, but literally I deployed the same code where the only difference is v2.3.0 -> v2.33.0.

Environment

jcjones1515 commented 1 year ago

if someone could point me in the right direction I'd be happy to contribute a fix

prescottprue commented 1 year ago

I am seeing a similar issue this with the graphql plugin in versions of dd-trace past v2.1.1 including v3 and v4. A few things I've noticed while looking around:

Here are the versions I've tried and what preserves trace_id from browser requests through a service running Apollo (graphql plugin) and what does not:

v2.1.1 - works
v2.2.0 - works
v3.0.0 - works
v3.10.0 - works
v3.11.0 - works
v3.12.0 - works
v3.13.0 - works
v3.14.0 - does not work
v3.16.0 - does not work
v3.19.0 - does not work
v4.0.0 - does not work
Operation system: Both mac and linux
Node.js version: ^18
Tracer version: 4
Agent version: latest
Relevant library versions: ^3.14.0, ^4.0.0
ESM: No
Libraries: apollo-server ^3 (internally uses express)
Plugins: graphql 

Happy to provide any more info if needed

prescottprue commented 1 year ago

Workaround

I was able to get things working again with ^3.19.0 and v4 by changing our front end config for RUM to include the datadog and tracecontext in the allowedTracingUrls.*.propagatorTypes like so:

const apiUrl = 'https://ourapi.example.com'

datadogRum.init({
  // other dd rum config
   allowedTracingUrls: [
-      apiUrl
+      {
+        match: apiUrl
+        propagatorTypes: ['datadog', 'tracecontext'],
+      },
    ],
])

I would still say this is a workaround since it isn't using defaults anymore (which don't work) and was "breaking" while being in a minor change

Theory

Since passing the W3C traceparent header gets things to work again I'm wondering if it is that the new logic in v3.14.0 for supporting this header is to blame for the datadog headers no longer being preserved 🤷‍♂️

Note On Previous Post

When posting that things were working above I forgot to include that having different versions of dd-trace in another service which is being called is still dropping the trace id unless all are on the older version

jcjones1515 commented 1 year ago

@prescottprue your change seems to have worked for me, I'm seeing it work on 4.1.0