Skip to content

command: runBuild, runBake: remove uses of DockerCLI.MeterProvider #3094

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thaJeztah
Copy link
Member

The DockerCli.MeterProvider (and DockerCli.TracerProvider) methods have no direct connection with the CLI itself and are wrappers for otel.GetMeterProvider() and otel.GetTracerProvider().

Remove the use of the CLI method, and use the otel functions directly.

The [DockerCli.MeterProvider] (and [DockerCli.TracerProvider]) methods
have no direct connection with the CLI itself and are wrappers for
`otel.GetMeterProvider()` and `otel.GetTracerProvider()`.

Remove the use of the CLI method, and use the otel functions directly.

[DockerCli.MeterProvider]: https://github.com/docker/cli/blob/91cbde67c509c8702417ee1cd64cd5d845ccd254/cli/command/telemetry.go#L62-L64
[DockerCli.TracerProvider]: https://github.com/docker/cli/blob/91cbde67c509c8702417ee1cd64cd5d845ccd254/cli/command/telemetry.go#L58-L60

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah marked this pull request as ready for review April 8, 2025 12:55
@thaJeztah thaJeztah requested a review from jsternberg April 8, 2025 12:55
@jsternberg
Copy link
Collaborator

I think the original intention of these methods was that they would construct the TraceProvider and the MeterProvider if they didn't already exist so I don't think this is as simple as just "use the otel function" for these. I also think it's still good if there's a CLI exposed way of accessing the providers outside of the otel library. This is partly because we know the otel library implementations can create memory leaks if they are used without being set.

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

Successfully merging this pull request may close these issues.

2 participants