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

telemetry: fix early meterprovider shutdown #5456

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

laurazard
Copy link
Member

- What I did

In 4b5a196, we changed the CLI global meter provider shutdown in order to handle any error returned by the metric export.

Unfortunately, we dropped a defer during the fix, which causes the meter provider to be immediately shutdown after being created and metrics to not be collected/exporter.

- How I did it

Re-defer the new Meter Provider shutdown call.

- How to verify it

- Description for the changelog

Fix issue causing CLI OTel metrics to not be collected.

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

Screenshot 2024-09-20 at 00 53 09

In 4b5a196, we changed the CLI global
meter provider shutdown in order to handle any error returned by the
metric export.

Unfortunately, we dropped a `defer` during the fix, which
causes the meter provider to be immediately shutdown after being created
and metrics to not be collected/exporter.

Signed-off-by: Laura Brehm <laurabrehm@hey.com>
@codecov-commenter
Copy link

codecov-commenter commented Sep 20, 2024

Codecov Report

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

Please upload report for BASE (master@a5fb752). Learn more about missing BASE report.
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #5456   +/-   ##
=========================================
  Coverage          ?   59.74%           
=========================================
  Files             ?      345           
  Lines             ?    23431           
  Branches          ?        0           
=========================================
  Hits              ?    13999           
  Misses            ?     8458           
  Partials          ?      974           

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.

LGTM 🙈🤦‍♂️

@thaJeztah thaJeztah added this to the 28.0.0 milestone Sep 20, 2024
@thaJeztah thaJeztah merged commit 48cfe21 into docker:master Sep 20, 2024
110 checks passed
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.

3 participants