Skip to content

Fix/sampler shutdown lifecycle#8574

Open
Debashismitra01 wants to merge 4 commits into
open-telemetry:mainfrom
Debashismitra01:fix/sampler-shutdown-lifecycle
Open

Fix/sampler shutdown lifecycle#8574
Debashismitra01 wants to merge 4 commits into
open-telemetry:mainfrom
Debashismitra01:fix/sampler-shutdown-lifecycle

Conversation

@Debashismitra01

Copy link
Copy Markdown

Changes

Adds shutdown lifecycle support to Samplers.

Previously Sampler implementations had no way to release resources when
SdkTracerProvider was shut down. Resource-owning samplers such as
JaegerRemoteSampler could leave background executors and connections running.

This change:

  • Adds default Sampler.shutdown()
  • Invokes sampler shutdown from SdkTracerProvider shutdown
  • Updates JaegerRemoteSampler to release polling and sender resources
  • Adds lifecycle tests

Fixes #8535

@Debashismitra01 Debashismitra01 requested a review from a team as a code owner July 6, 2026 21:34
@Debashismitra01 Debashismitra01 force-pushed the fix/sampler-shutdown-lifecycle branch 5 times, most recently from 230786a to 333c143 Compare July 6, 2026 22:35
@Debashismitra01 Debashismitra01 force-pushed the fix/sampler-shutdown-lifecycle branch from 333c143 to 83b70e5 Compare July 6, 2026 22:53
@otelbot otelbot Bot added the api-change Changes to public API surface area label Jul 6, 2026
@otelbot

otelbot Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

⚠️ API changes detected — additional maintainer review required

@jack-berg @jkwatson

This PR modifies the public API surface area of the following module(s):

  • opentelemetry-sdk-extension-jaeger-remote-sampler
  • opentelemetry-sdk-trace

Please review the changes in docs/apidiffs/current_vs_latest/ carefully before approving.

@Debashismitra01 Debashismitra01 force-pushed the fix/sampler-shutdown-lifecycle branch from d8eb3fa to f246d3e Compare July 7, 2026 00:07
@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.56%. Comparing base (2626179) to head (f246d3e).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8574      +/-   ##
============================================
+ Coverage     90.96%   91.56%   +0.59%     
- Complexity    10207    10266      +59     
============================================
  Files          1013     1013              
  Lines         27160    27115      -45     
  Branches       3182     3184       +2     
============================================
+ Hits          24706    24827     +121     
+ Misses         1730     1564     -166     
  Partials        724      724              

☔ View full report in Codecov by Harness.
📢 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.

@jkwatson jkwatson 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! I'll let @jack-berg also approve before merging, as there's an API change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-change Changes to public API surface area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add close / shutdown method to samplers

2 participants