open-telemetry / opentelemetry-js-contrib

OpenTelemetry instrumentation for JavaScript modules
https://opentelemetry.io
Apache License 2.0
664 stars 489 forks source link

[instrumentation-document-load] trace parent behaviour not as expected #2306

Open dmfallak opened 2 months ago

dmfallak commented 2 months ago

What version of OpenTelemetry are you using?

"@opentelemetry/core": "^1.25.0" "@opentelemetry/instrumentation-document-load": "^0.39.0" "@opentelemetry/api": "^1.9.0"

What version of Node are you using?

N/A (browser)

What did you do?

As per the readme:

Because the browser does not send a trace context header for the initial page navigation, the server needs to fake a trace context header in a middleware and then send that trace context header back to the client as a meta tag traceparent .

I followed this pattern on the backend when generating the page HTML:

<head>
  <!--
    https://www.w3.org/TR/trace-context/
    Set the `traceparent` in the server's HTML template code. It should be
    dynamically generated server side to have the server's request trace Id,
    a parent span Id that was set on the server's request span, and the trace
    flags to indicate the server's sampling decision
    (01 = sampled, 00 = notsampled).
    '{version}-{traceId}-{spanId}-{sampleDecision}'
  -->
  <meta name="traceparent" content="00-ab42124a3c573678d4d8b21ba52df3bf-d21f7bc17caa5aba-01">
</head>

I used the span ID here as the parent span ID for the server.

What did you expect to see?

The browser uses the span ID I provided and becomes the parent of the server span.

What did you see instead?

The browser uses the span ID as its own parent span ID, creates a new random span ID, and becomes the sibling of the server span. Both spans now refer to a nonexistent span ID.

Additional context

scheler commented 2 months ago

I looked at this some time ago and noticed that this was incorrectly implemented right from the beginning - see https://github.com/open-telemetry/opentelemetry-js-contrib/commit/4f48357b7d1dea246a67ef1a0e071fc64aa10e26 - the test case only checks for the server-sent trace id on the root span, but not the span id.

In fact, I don't think the trace api provides a way to specify the span id when starting a span. The idea mentioned in the html comment is hard to implement.