metricutil: switch to using the cli meter provider#2373
Merged
crazy-max merged 1 commit intodocker:masterfrom Apr 4, 2024
Merged
metricutil: switch to using the cli meter provider#2373crazy-max merged 1 commit intodocker:masterfrom
crazy-max merged 1 commit intodocker:masterfrom
Conversation
crazy-max
reviewed
Apr 2, 2024
Member
crazy-max
left a comment
crazy-max
left a comment
There was a problem hiding this comment.
This a follow-up of docker/cli#4889 right? Can you rebase as well?
The meter provider initialization that was located here has now been moved to a common area in the docker cli. This upgrades our CLI version and then uses this common code instead of our own version. As a piece of additional functionality, the docker OTEL endpoint can now be overwritten with `DOCKER_CLI_OTEL_EXPORTER_OTLP_ENDPOINT` for testing. This removes the OTLP exporter from the CLI that was previously locked behind `BUILDX_EXPERIMENTAL`. I do plan for this to return, but as a proper part of the `docker/cli` implementation rather than something special with `buildx`. Signed-off-by: Jonathan A. Sternberg <[email protected]>
jsternberg
force-pushed
the
docker-cli-meter-provider
branch
from
b449881 to
b4799f9
Compare
Collaborator
Author
|
Yea it's a follow up to that PR. Rebased to include the vendor update for |
crazy-max
approved these changes
Apr 2, 2024
tonistiigi
approved these changes
Apr 2, 2024
thaJeztah
reviewed
Apr 9, 2024
| github.com/creack/pty v1.1.18 | ||
| github.com/distribution/reference v0.5.0 | ||
| github.com/docker/cli v26.0.0+incompatible | ||
| github.com/docker/cli v26.0.1-0.20240401150816-155dc5e4e406+incompatible |
Member
There was a problem hiding this comment.
Do we want the change backported to the 26.0 branch so that we don't have to force consumers to depend on master? (Haven't checked if we can)
Member
There was a problem hiding this comment.
If this is possible to backport docker/cli#4889, I think so yes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The meter provider initialization that was located here has now been
moved to a common area in the docker cli. This upgrades our CLI version
and then uses this common code instead of our own version.
As a piece of additional functionality, the docker OTEL endpoint can now
be overwritten with
DOCKER_CLI_OTEL_EXPORTER_OTLP_ENDPOINTfortesting.
This removes the OTLP exporter from the CLI that was previously locked
behind
BUILDX_EXPERIMENTAL. I do plan for this to return, but as aproper part of the
docker/cliimplementation rather than somethingspecial with
buildx.