Skip to content

Standardize OkHttpHttpSender shutdown to await executor termination#8495

Merged
jack-berg merged 1 commit into
open-telemetry:mainfrom
thswlsqls:fix/standardize-sender-shutdown
Jul 13, 2026
Merged

Standardize OkHttpHttpSender shutdown to await executor termination#8495
jack-berg merged 1 commit into
open-telemetry:mainfrom
thswlsqls:fix/standardize-sender-shutdown

Conversation

@thswlsqls

@thswlsqls thswlsqls commented Jun 21, 2026 •
edited by jack-berg
Loading

Copy link
Copy Markdown
Contributor

Related to #8280

Description

  • Align OkHttpHttpSender.shutdown() with the sibling OkHttpGrpcSender.shutdown() in the same package, which Standardize sender shutdown implementations #8280 designates as the reference implementation.
  • Order the shutdown steps consistently: dispatcher().cancelAll(), then connectionPool().evictAll(), then (for a managed executor) executorService.shutdownNow().
  • For a managed executor, await termination instead of returning immediately: a daemon okhttp-shutdown thread calls awaitTermination(5, SECONDS), logs a WARNING on timeout, and completes the returned CompletableResultCode. A non-managed executor still returns CompletableResultCode.ofSuccess().
  • Internal transport package (io.opentelemetry.exporter.sender.okhttp.internal); no public API change, no apidiff, no CHANGELOG entry.

Testing done

  • Added OkHttpHttpSenderTest shutdown tests mirroring OkHttpGrpcSenderTest: shutdown_CompletableResultCodeShouldWaitForThreads, shutdown_NonManagedExecutor_ReturnsImmediately, shutdown_ExecutorDoesNotTerminateInTime_LogsWarningButSucceeds, shutdown_InterruptedWhileWaiting_StillSucceeds.
  • ./gradlew :exporters:sender:okhttp:check passed (5 tests, 0 failures).

@codecov

codecov Bot commented Jun 21, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.76%. Comparing base (824334c) to head (23ac0b3).
⚠️ Report is 42 commits behind head on main.

Files with missing lines Patch % Lines
...orter/sender/okhttp/internal/OkHttpHttpSender.java 73.33% 3 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (73.33%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #8495      +/-   ##
============================================
- Coverage     78.77%   78.76%   -0.01%     
- Complexity     8579     8580       +1     
============================================
  Files          1009     1009              
  Lines         28993    29006      +13     
  Branches       3599     3600       +1     
============================================
+ Hits          22839    22848       +9     
- Misses         5311     5314       +3     
- Partials        843      844       +1

☔ 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.

@opentelemetry-pr-dashboard opentelemetry-pr-dashboard Bot mentioned this pull request Jun 25, 2026
Open
@jack-berg jack-berg added the needs author feedback Waiting for additional feedback from the author label Jul 2, 2026
@thswlsqls
thswlsqls marked this pull request as ready for review July 4, 2026 09:06
@thswlsqls
thswlsqls requested a review from a team as a code owner July 4, 2026 09:07
@thswlsqls

thswlsqls commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Ready for review. The failing codecov/patch check is only the timeout/interrupt log branches in shutdown(); happy to add coverage if you'd prefer.

@github-actions github-actions Bot removed the needs author feedback Waiting for additional feedback from the author label Jul 5, 2026
@jack-berg
jack-berg merged commit bf29609 into open-telemetry:main Jul 13, 2026
28 of 29 checks passed
@otelbot

otelbot Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution @thswlsqls! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.

This was referenced Jul 17, 2026
Draft
Draft
@thswlsqls
thswlsqls deleted the fix/standardize-sender-shutdown branch July 17, 2026 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jack-berg jack-berg jack-berg approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@thswlsqls @jack-berg