Skip to content

[Prometheus.HttpListener] Fix shutdown hang - #7587

Merged
martincostello merged 2 commits into
open-telemetry:mainfrom
martincostello:fix-prometheus-flakiness
Aug 4, 2026
Merged

martincostello merged 2 commits into
open-telemetry:mainfrom
martincostello:fix-prometheus-flakiness

Conversation

@martincostello

Copy link
Copy Markdown
Member

Changes

Avoid disposal race in PrometheusHttpListener that can cause shutdown to hang indefinitely and flaky tests (1, 2, 3).

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

Avoid disposal race in `PrometheusHttpListener` that can cause shutdown to hang indefinitely.
@github-actions github-actions Bot added the pkg:OpenTelemetry.Exporter.Prometheus.HttpListener Issues related to OpenTelemetry.Exporter.Prometheus.HttpListener NuGet package label Aug 4, 2026
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Aug 4, 2026

Copy link
Copy Markdown

Pull request dashboard status

Merged · refreshed 2026-08-04 21:27 UTC

Status above doesn't look right?
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.23077% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.49%. Comparing base (2ae52ae) to head (5d86411).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
....Prometheus.HttpListener/PrometheusHttpListener.cs 69.23% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7587      +/-   ##
==========================================
- Coverage   90.51%   90.49%   -0.02%     
==========================================
  Files         290      290              
  Lines       16029    16039      +10     
==========================================
+ Hits        14509    14515       +6     
- Misses       1520     1524       +4     
Flag Coverage Δ
unittests-Project-Experimental 90.49% <69.23%> (-0.07%) ⬇️
unittests-Project-Stable 90.53% <69.23%> (-0.08%) ⬇️
unittests-UnstableCoreLibraries-Experimental 51.35% <69.23%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
....Prometheus.HttpListener/PrometheusHttpListener.cs 88.30% <69.23%> (-1.76%) ⬇️

... and 2 files with indirect coverage changes

@martincostello

Copy link
Copy Markdown
Member Author

6 runs with no Prometheus related failures.

There was one failure here on attempt 5, but that has no specific cause logged but appears to be within OpenTelemetry.Tests.

@martincostello
martincostello marked this pull request as ready for review August 4, 2026 15:56
@martincostello
martincostello requested a review from a team as a code owner August 4, 2026 15:56
Copilot AI lite review requested due to automatic review settings August 4, 2026 15:56

Copilot AI 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.

Pull request overview

This PR addresses a shutdown hang in the Prometheus HttpListener exporter by bounding shutdown waits and ensuring in-flight scrapes fail fast (HTTP 503) once disposal begins, with accompanying test hardening to avoid race-driven flakiness.

Changes:

  • Add bounded timeouts to listener shutdown/drain logic and log when the processing loop fails to stop promptly.
  • Ensure scrapes that are still collecting when disposal starts return HTTP 503 instead of completing with a successful response.
  • Harden integration/unit tests by adding explicit scrape/promtool timeouts and avoiding thread-pool scheduling races during disposal.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PromToolFixture.cs Adds scrape and overall promtool execution timeouts to prevent hangs in container-based validation.
test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs Makes the disposal-vs-scrape race test deterministic (thread-pool availability + dedicated disposal thread).
src/OpenTelemetry.Exporter.Prometheus.HttpListener/PrometheusHttpListener.cs Implements bounded shutdown waits and returns 503 when disposal begins mid-collection.
src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md Adds Unreleased entries describing the behavioral changes (needs PR link fixups).
Suppressed comments (1)

src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md:24

  • This CHANGELOG entry still uses a placeholder PR reference (#TODO). Please update it to the real PR link before merging.
  ([#TODO](https://github.com/open-telemetry/opentelemetry-dotnet/pull/TODO))

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md Outdated
Comment thread src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md Outdated
This was referenced Sep 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg:OpenTelemetry.Exporter.Prometheus.HttpListener Issues related to OpenTelemetry.Exporter.Prometheus.HttpListener NuGet package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants