Skip to content

Standardize OkHttpHttpSender shutdown to await executor termination#8495

Draft
thswlsqls wants to merge 1 commit into
open-telemetry:mainfrom
thswlsqls:fix/standardize-sender-shutdown
Draft

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

Conversation

@thswlsqls

Copy link
Copy Markdown
Contributor

Fixes #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

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

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.

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.

Standardize sender shutdown implementations

1 participant