Skip to content

fix(metrics): honor context cancellation when collecting metrics - #211

Open
skyc1e wants to merge 1 commit into
base:mainfrom
skyc1e:fix/metrics-request-context
Open

fix(metrics): honor context cancellation when collecting metrics#211
skyc1e wants to merge 1 commit into
base:mainfrom
skyc1e:fix/metrics-request-context

Conversation

@skyc1e

@skyc1e skyc1e commented Sep 7, 2026

Copy link
Copy Markdown

Description

A metrics endpoint that stalls before sending headers or while streaming its response leaves Collect(ctx, ...) blocked even after the caller cancels the context. All four collectors currently use http.Get, which does not receive that context.

Create the HTTP request with the caller's context in the builder, reth, geth, and base-reth-node collectors. Cancellation now interrupts both the request and response-body reads, while preserving the existing client, parsing, body cleanup, and wrapped errors.

Add local HTTP-server tests for each collector covering successful collection and cancellation before headers and during a partial response body. Canceled scrapes must return context.Canceled and must not record a block.

Testing

  • All 12 new scenarios pass. The eight cancellation scenarios failed on the original code because Collect did not return after cancellation.
  • go test ./... -timeout=5m: all packages pass except the existing TestResolveConfigFilePath failure on Windows (backslash paths versus slash expectations). Reproduced that failure in a separate, unchanged checkout of 52f715f.
  • go vet ./...: passes.
  • Changed Go files formatted with gofmt; git diff --check passes.
  • Tested with Go 1.24.11 on Windows/amd64. Node-backed benchmark examples were not run locally.

@cb-heimdall

Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

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.

2 participants