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

[release/1.7] Fix support for OTLP config #10360

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

cpuguy83
Copy link
Member

This was broken due to 9360e37 (backport from main).
The backport should not have broken the toml config so this more or less restores that.

This treats the env vars as preferred.
It is not perfect since, as an example, Endpoint could be set on the config and the env to set the protocol could be set and these could conflict, however I think this is an unlikely scenario and people should use one or the other, not both.

This change converts the config into the relevant environment variables before initializing tracers.

Fixes #10358

@cpuguy83
Copy link
Member Author

cc @vvoland

@dmcgowan dmcgowan added the cherry-pick/1.6.x Change to be cherry picked to release/1.6 branch label Jun 19, 2024
@dmcgowan dmcgowan changed the title 1.7: Add back support for OTLP config from toml [release/1.7] Fix support for OTLP config Jun 19, 2024
tracing/plugin/otlp.go Outdated Show resolved Hide resolved
This was broken due to 9360e37
(backport from main).
The backport should not have broken the toml config so this more or less
restores that.

This treats the env vars as preferred.
It is not perfect since, as an example, `Endpoint` could be set on the
config and the env to set the protocol could be set and these could
conflict, however I think this is an unlikely scenario and people should
use one or the other, not both.

This change converts the config into the relevant environment variables
before initializing tracers.

Signed-off-by: Brian Goff <[email protected]>
@thaJeztah
Copy link
Member

@cpuguy83 may need a similar patch for 1.6 as well, or at least I see there was a backport to 1.6 as well;

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

@dmcgowan dmcgowan merged commit 88b4727 into containerd:release/1.7 Jun 20, 2024
57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick/1.6.x Change to be cherry picked to release/1.6 branch impact/changelog size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants