Skip to content
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

W3C span propagation id fix #1045

Merged

Conversation

SimunKaracic
Copy link
Contributor

@SimunKaracic SimunKaracic commented Jun 21, 2021

Propagate the current span id, not the parent span id, according to the spec.

When reading from headers, don't generate a new ID for the Remote span, but use the existing one.

This is the thread where @dguggemos reported the bug -> https://gitter.im/kamon-io/Kamon?at=60cc4e2cb31731135437e0aa

@pnerg If you have the time, check this out. You initially changed this to use the parentID instead of the spanID. According to the W3C spec, the current span ID should be propagated.
From the spec https://www.w3.org/TR/trace-context/#a-traceparent-is-received:
Update parent-id: The value of property parent-id MUST be set to a value representing the ID of the current operation.

#986 I believe this issue originated from the fact that akka-http was being double instrumented.

@the-overengineer
Copy link
Contributor

We talked about this a bit offline, looks good to me after looking at the spec. Please make sure this doesn't do something terribly wrong to Zipkin (or we at least need to check whether we need to adjust the joining of spans flag in the config/document it), though. The Span.Remote is just a reference with the same ID, so this should work, but issues might arise due to only one of the sides having the correct parent span ID.

@SimunKaracic
Copy link
Contributor Author

There's only one issue. When using w3c, we need to make sure that kamon.trace.identifier-scheme is set to double. This is already documented in the configuration, but we should probably warn if it's set to single. I'll try and get it done now, but if it's too complicated I'll create a task for it for the future

@SimunKaracic SimunKaracic merged commit b9316ef into kamon-io:master Jun 23, 2021
@SimunKaracic SimunKaracic deleted the w3c-span-propagation-id-fix branch June 23, 2021 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants