Skip to content

Fix OBI being blocked on shutdown#1782

Merged
mariomac merged 4 commits into
open-telemetry:mainfrom
mariomac:dont-block-exit
Apr 10, 2026
Merged

Fix OBI being blocked on shutdown#1782
mariomac merged 4 commits into
open-telemetry:mainfrom
mariomac:dont-block-exit

Conversation

@mariomac

@mariomac mariomac commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Summary

OBI is taking long to exit, mainly because the OpenTelemetry metrics exporters got blocked during their Shutdown. This PR takes a best-effort approach to OTEL metrics shutdown: we try to shut it down in a background goroutine, as it is not a critical operation (the worst case is that we loose few last data points in an OBI instance we are anyway shutting down).

The approach that was taken to fix it in the integration tests was to wrap the OBI process into a shell script, but that was hiding the blocking in production, and its real cause.

It also broke the coverage reporting of the integration tests, as the OBI process was killed before it had time to exit.

This should also fix this issue: #781

Validation

@mariomac mariomac requested a review from a team as a code owner April 9, 2026 09:46
@codecov

codecov Bot commented Apr 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 44.82759% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.20%. Comparing base (7d92c2f) to head (7eb72e8).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...rnal/test/integration/components/docker/compose.go 0.00% 8 Missing ⚠️
pkg/internal/appolly/appolly.go 37.50% 4 Missing and 1 partial ⚠️
pkg/export/otel/metrics_svc_graph.go 66.66% 2 Missing ⚠️
pkg/export/otel/metrics.go 83.33% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1782       +/-   ##
===========================================
+ Coverage   45.06%   60.20%   +15.13%     
===========================================
  Files         339      339               
  Lines       37481    37489        +8     
===========================================
+ Hits        16892    22571     +5679     
+ Misses      19518    13749     -5769     
- Partials     1071     1169       +98     
Flag Coverage Δ
integration-test 57.16% <66.66%> (+35.92%) ⬆️
integration-test-arm 30.03% <42.85%> (+30.03%) ⬆️
integration-test-vm-x86_64-5.15.152 31.05% <44.44%> (+31.05%) ⬆️
integration-test-vm-x86_64-6.10.6 30.08% <44.44%> (+30.08%) ⬆️
k8s-integration-test 43.90% <66.66%> (+41.79%) ⬆️
oats-test 38.04% <66.66%> (+38.04%) ⬆️
unittests 48.58% <30.76%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@grcevski grcevski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Nice! I was seeing these a lot on my new hardware.

@rafaelroquetto rafaelroquetto requested a review from Copilot April 9, 2026 19:09
@mariomac mariomac force-pushed the dont-block-exit branch 2 times, most recently from 833fc94 to e09bbdf Compare April 10, 2026 10:30

@mmat11 mmat11 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thanks for fixing this!

@mariomac mariomac merged commit d25363d into open-telemetry:main Apr 10, 2026
78 checks passed
@mariomac mariomac deleted the dont-block-exit branch April 10, 2026 14:07
@MrAlias MrAlias added this to the v0.8.0 milestone Apr 16, 2026
@MrAlias MrAlias mentioned this pull request Apr 16, 2026
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.

4 participants