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

metricutil: switch to using the cli meter provider #2373

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

jsternberg
Copy link
Collaborator

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.

Copy link
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

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

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 <jonathan.sternberg@docker.com>
@jsternberg
Copy link
Collaborator Author

Yea it's a follow up to that PR. Rebased to include the vendor update for docker/cli.

@crazy-max crazy-max merged commit a7d59ae into docker:master Apr 4, 2024
63 checks passed
@jsternberg jsternberg deleted the docker-cli-meter-provider branch April 4, 2024 14:17
github.com/docker/cli v26.0.0+incompatible
github.com/docker/cli v26.0.1-0.20240401150816-155dc5e4e406+incompatible
Copy link
Member

Choose a reason for hiding this comment

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

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)

cc @vvoland @laurazard

Copy link
Member

Choose a reason for hiding this comment

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

If this is possible to backport docker/cli#4889, I think so yes

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

Successfully merging this pull request may close these issues.

None yet

4 participants