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

cli: make initializing the global meter- and tracing providers optional #5188

Merged
merged 1 commit into from
Jun 24, 2024

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Jun 24, 2024

- Description for the changelog

Add WithEnableGlobalMeterProvider and WithEnableGlobalTracerProvider options

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah added status/2-code-review area/metrics area/metrics/otel area/go-sdk Changes affecting the Go SDK impact/go-sdk Noteworthy (compatibility changes) in the Go SDK labels Jun 24, 2024
@thaJeztah thaJeztah added this to the 27.0.0 milestone Jun 24, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jun 24, 2024

Codecov Report

Attention: Patch coverage is 0% with 13 lines in your changes missing coverage. Please review.

Project coverage is 61.42%. Comparing base (50acbb0) to head (617eb52).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5188      +/-   ##
==========================================
- Coverage   61.74%   61.42%   -0.32%     
==========================================
  Files         297      298       +1     
  Lines       20787    20797      +10     
==========================================
- Hits        12835    12775      -60     
- Misses       7037     7109      +72     
+ Partials      915      913       -2     

Copy link
Member

@laurazard laurazard left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@krissetto krissetto left a comment

Choose a reason for hiding this comment

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

Tiny nit but LGTM

@@ -92,6 +92,8 @@ type DockerCli struct {
// this may be replaced by explicitly passing a context to functions that
// need it.
baseCtx context.Context

enableGlobalMeter, enableGlobalTracer bool
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we should probably call these Meter/TracerProvider

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, they were already far too long names for an un-exported field 😂

cli/cobra.go Outdated Show resolved Hide resolved
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah merged commit 7fafd33 into docker:master Jun 24, 2024
97 checks passed
@thaJeztah thaJeztah deleted the tracer_meter_optional branch June 24, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/go-sdk Changes affecting the Go SDK area/metrics/otel area/metrics impact/go-sdk Noteworthy (compatibility changes) in the Go SDK process/cherry-pick/26.1 status/2-code-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants