fix(otelhttp): report correct read bytes in client instrumentation by Xio-Shark · Pull Request #8684 · open-telemetry/opentelemetry-go-contrib

Xio-Shark · 2026-03-17T11:15:04Z

Summary: 修复 otelhttp client instrumentation 中 read bytes 统计不准确问题

Changes:

新增回归测试覆盖 body read timing 问题
保证只在 body 最终状态确定后记录 read bytes
避免 EOF / Close 双路径重复记录

Motivation:

issue Potentially incorrect number of read bytes in otelhttp client instrumentation #5087 指出这是 logical race，不是单纯线程安全问题

Testing:

go test ./instrumentation/net/http/otelhttp/...

linux-foundation-easycla · 2026-03-17T11:15:14Z

The committers listed above are authorized under a signed CLA.

✅ login: Xio-Shark / name: Teng Yanxi (2aa72ff, 8757c34, e91b742, eec1f7d)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 14e286e8a8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-17T11:20:12Z

+		_ = wb.closeBody()
+		wb.recordMetricsOnce()


Avoid closing response body when Read returns EOF

wrappedBody.Read now calls closeBody() on every io.EOF, which changes behavior for upgraded/duplex responses (io.ReadWriteCloser) from transparent wrapping to forced shutdown. In protocol-switch or CONNECT-style streams, a read-side EOF can happen while the write side is still valid (TCP half-close); this automatic close tears down the underlying connection before user code calls Close, so subsequent writes can fail unexpectedly. The previous implementation finalized metrics/span on EOF without closing the body, which preserved caller control over stream lifetime.

Useful? React with 👍 / 👎.

Xio-Shark · 2026-04-18T01:34:30Z

Friendly follow-up: EasyCLA already reported success on March 17, 2026, but this PR still has the blocked: CLA label. @flc1125 could a maintainer please remove that label so the PR can proceed to normal review? Thanks!

Xio-Shark · 2026-04-18T19:55:26Z

Pushed follow-up commit 890ab4f34 to address the unresolved EOF/Close review concern.

What changed:

wrappedBody.Read() no longer calls closeBody() on io.EOF
metrics/span finalization still happens on EOF via recordMetricsOnce()
added a regression test to verify an io.ReadWriteCloser remains writable after EOF until the caller explicitly closes it

Local validation:

go test . -run 'TestWrappedBodyReadEOFThenCloseRecordsOnce|TestWrappedBodyReadEOFKeepsReadWriteCloserOpenUntilClose|TestTransportProtocolSwitch' -count=1
go test .

Both passed in instrumentation/net/http/otelhttp.

The blocked: CLA label is still present even though EasyCLA already reported success on March 17, 2026, so after this code-level review concern is out of the way the remaining blocker should still be maintainer-side label cleanup.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 890ab4f346

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-18T19:59:27Z

+		trackedBody = newRequestBodyTracker(body, func() {
+			if currentTrackedBody() == trackedBody {
+				recordMetrics()


Synchronize recordMetrics callback publication

The request-body onClose callback can run on the transport write goroutine while RoundTrip later reassigns recordMetrics after t.rt.RoundTrip(r) returns, but this read/write of the captured function variable is unsynchronized. In practice (notably with net/http's concurrent write loop), this is a data race and can cause nondeterministic behavior, including invoking the stale no-op callback and dropping http.client.request.body.size/duration emission for that request. Please publish/access the callback through synchronization (e.g., mutex/atomic or a stable closure that reads synchronized state) before any concurrent close path can observe it.

Useful? React with 👍 / 👎.

flc1125 · 2026-04-19T01:36:25Z

This file is not necessary, please remove it.

Additionally, please read first: https://github.com/open-telemetry/community/blob/main/policies/genai.md#frequently-asked-questions---contributors

Xio-Shark · 2026-04-20T02:12:09Z

Hi @flc1125, I've pushed the follow-up on top of the rebased branch.

Changes included:

kept upgraded bodies open after EOF
serialized metrics callback access to avoid the callback/assignment race around recordMetrics
preserved error metrics on transport failures

Validation:

in instrumentation/net/http/otelhttp:
- go build ./...
- go test . -run 'TestWrappedBodyReadEOFKeepsReadWriteCloserOpenUntilClose|TestTransportMetricsReportFinalRequestBodySize|TestTransportErrorTypeMetricAttribute' -count=1

chatgpt-codex-connector · 2026-04-21T10:58:33Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Xio-Shark · 2026-04-21T11:01:08Z

Hi @flc1125, thanks for the review. I've addressed all three points:

Agent-run doc removed — The rebased branch no longer contains docs/agent-runs/2026-03-17-otelhttp-read-bytes.md.
P1 EOF/Close behavior — Fixed in eec1f7d19: wrappedBody.Read() no longer calls closeBody() on io.EOF. A regression test verifies that upgraded bodies (io.ReadWriteCloser) remain writable after EOF until the caller explicitly closes them.
P1 recordMetrics callback race — Fixed in 2aa72ff1f: the callback publication path is now serialized with a mutex so the transport write goroutine and RoundTrip cannot race on the captured function variable.
Error metrics preservation — Added in e91b7421d: ensures body-read errors are still reflected in http.client.request.body.size even when EOF does not trigger close.

I've also read the OpenTelemetry GenAI policy you linked.

Could you please take another look when you have a moment? The blocked: CLA label is still attached even though EasyCLA reported success on March 17—would appreciate it if a maintainer could clear that as well. Thanks!

Xio-Shark · 2026-04-21T11:51:01Z

Hi @flc1125, friendly reminder that the EasyCLA check was already passed on 2026-03-17 ("The committers listed above are authorized under a signed CLA."). Could you please help remove the "blocked: CLA" label so this PR can move forward to review? Thank you!

codecov · 2026-04-21T21:02:05Z

Codecov Report

❌ Patch coverage is 98.92473% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.4%. Comparing base (c5160bb) to head (e91b742).
⚠️ Report is 13 commits behind head on main.

Files with missing lines	Patch %	Lines
instrumentation/net/http/otelhttp/transport.go	98.9%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #8684     +/-   ##
=======================================
- Coverage   83.3%   82.4%   -1.0%     
=======================================
  Files        193     193             
  Lines      15826   19794   +3968     
=======================================
+ Hits       13191   16314   +3123     
- Misses      2152    2997    +845     
  Partials     483     483

Files with missing lines	Coverage Δ
instrumentation/net/http/otelhttp/transport.go	`94.5% <98.9%> (+1.7%)`	⬆️

... and 186 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Xio-Shark requested review from a team and dmathieu as code owners March 17, 2026 11:15

flc1125 added the blocked: CLA Waiting on CLA to be signed before progress can be made label Mar 17, 2026

chatgpt-codex-connector Bot reviewed Mar 17, 2026

View reviewed changes

github-actions Bot requested a review from sonalgaud12 April 18, 2026 19:55

chatgpt-codex-connector Bot reviewed Apr 18, 2026

View reviewed changes

flc1125 removed the blocked: CLA Waiting on CLA to be signed before progress can be made label Apr 19, 2026

flc1125 requested changes Apr 19, 2026

View reviewed changes

fix(otelhttp): report correct read bytes in client instrumentation

8757c34

Xio-Shark force-pushed the fix-otelhttp-read-bytes branch from 890ab4f to 8757c34 Compare April 19, 2026 15:06

Xio-Shark added 3 commits April 20, 2026 00:04

fix(otelhttp): keep upgraded bodies open after EOF

eec1f7d

fix(otelhttp): serialize metrics callback access

2aa72ff

fix(otelhttp): preserve error metrics on failures

e91b742

Conversation

Xio-Shark commented Mar 17, 2026

Uh oh!

linux-foundation-easycla Bot commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

Xio-Shark commented Apr 18, 2026

Uh oh!

Xio-Shark commented Apr 18, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

flc1125 Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Xio-Shark commented Apr 20, 2026

Uh oh!

chatgpt-codex-connector Bot commented Apr 21, 2026

Uh oh!

Xio-Shark commented Apr 21, 2026

Uh oh!

Xio-Shark commented Apr 21, 2026

Uh oh!

codecov Bot commented Apr 21, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

linux-foundation-easycla Bot commented Mar 17, 2026 •

edited

Loading