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

fix(opentelemetry): set default propagation type to w3c #9457

Merged
merged 2 commits into from
Oct 28, 2022

Conversation

mayocream
Copy link
Contributor

@mayocream mayocream commented Sep 22, 2022

According to the original design, the default propagation header type should be w3c, described in https://docs.konghq.com/hub/kong-inc/opentelemetry/#propagation.

The plugin detects the propagation format from the headers and will use the appropriate format to propagate the span context. If no appropriate format is found, the plugin will fail back to the default format, which is w3c.

The previous implementation didn't specify the default header type successfully, it was failback to the b3 default.

@mayocream mayocream added this to the 3.0.1 milestone Sep 22, 2022
@mayocream mayocream added pr/wip A work in progress PR opened to receive feedback plugins/opentelemetry and removed plugins/opentelemetry labels Sep 22, 2022
@mayocream mayocream changed the title fix(opentelemetry) set default propagation type to w3c fix(opentelemetry): set default propagation type to w3c Sep 22, 2022
@fffonion fffonion removed this from the 3.0.1 milestone Oct 19, 2022
@fffonion
Copy link
Contributor

discussed with @mayocream and moving this out of 3.0.1 scope

According to the original design, the default propagation header type should be `w3c`, described in https://docs.konghq.com/hub/kong-inc/opentelemetry/#propagation.

> The plugin detects the propagation format from the headers and will use the appropriate format to propagate the span context. If no appropriate format is found, the plugin will failback to the default format, which is w3c.

The previous implementation didn't specify the default header type successfully, it was failbacked to the `b3` default.

WIP: add tests.
@mayocream mayocream marked this pull request as ready for review October 26, 2022 06:31
@mayocream mayocream requested a review from a team as a code owner October 26, 2022 06:31
@mayocream mayocream added pr/please review size/XS and removed pr/wip A work in progress PR opened to receive feedback size/S labels Oct 26, 2022
@fffonion fffonion merged commit 5e3b9e7 into master Oct 28, 2022
@fffonion fffonion deleted the fix/otel-default-header-type branch October 28, 2022 03:56
mayocream added a commit that referenced this pull request Nov 2, 2022
fffonion pushed a commit that referenced this pull request Nov 2, 2022
@dhanutalari
Copy link

i am using kong 3.4.2 and still i am getting
propagation.lua:540 [opentelemetry] Mismatched header types. conf: w3c. found: b3
propagation.lua:540 [opentelemetry] Mismatched header types. conf: w3c. found: was

@mayocream cloud you help me to resolve this ??

@samugi
Copy link
Member

samugi commented May 26, 2024

hi @dhanutalari, are the requests that generated those logs passing any b3 and/or AWS formatted tracing headers?

If that is the case, in 3.4.x, that was an expected behavior, as mentioned in the docs, the "default format", that this PR modified, is only effective when no incoming tracing header is present in the requests, otherwise the format used depends on what is detected in the incoming request, based on an hardcoded order of precedence.

This obviously did not offer a lot of flexibility, the propagation module has recently been rewritten to enable better configurability and complete freedom in deciding how headers propagation behaves (i.e. how Kong extracts/injects tracing headers). The new propagation module is unreleased at the moment but will soon be available in 3.7. You can find documentation about how the new propagation module works in our unreleased docs for the OTel plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants