Skip to content

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 changed the title 1.7: Add back support for OTLP config from toml [release/1.7] Fix support for OTLP config Jun 19, 2024
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 <cpuguy83@gmail.com>
@cpuguy83 cpuguy83 force-pushed the 1.7_fix_otlp_config branch from 178397b to 1ce1c8f Compare June 19, 2024 15:03
@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
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.

5 participants