Skip to content

Go std net HTTP/1 clients: avoid injecting the traceparent twice#2752

Merged
mariomac merged 6 commits into
open-telemetry:mainfrom
mariomac:fix-double-tp-go-http1
Jul 23, 2026
Merged

Go std net HTTP/1 clients: avoid injecting the traceparent twice#2752
mariomac merged 6 commits into
open-telemetry:mainfrom
mariomac:fix-double-tp-go-http1

Conversation

@mariomac

@mariomac mariomac commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

This is the first PR of a series that would fix issue #2732 (duplicate traceparent in SDK instrumented workloads) at different levels/protocol.

This PR fixes the issue for Go standard net package (HTTP/1) clients. It adds a uprobe to the return address of header_writeSubset: when headers are written in the request, we check for a header containing Traceparent:. If it already exists, we abort the context propagation. Otherwise we inject the Traceparent: header.

Validation

@mariomac
mariomac requested a review from a team as a code owner July 22, 2026 14:53
@mmat11
mmat11 requested a review from Copilot July 22, 2026 17:31
@mmat11

mmat11 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

I see 5.10 is failing with

bpf_verifier_test.go:109: BPF verifier rejected program(s):
    load program: invalid argument:
        number of funcs in func_info doesn't match number of subprogs

Probably the new programs need to be added to FixupSpec() because of the non-inlined subprog passed to bpf_loop

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

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR addresses duplicate traceparent injection for Go net/http (HTTP/1.x) clients by adding a return uprobe that detects whether the application already serialized a Traceparent header, and skips eBPF injection when present.

Changes:

  • Add an entry/return uprobe pair around net/http.Header.writeSubset to detect an existing Traceparent and avoid double-injection.
  • Introduce a privileged integration test plus a small helper binary to validate both “header already present” and “header absent” scenarios.
  • Wire the new return probe into the gotracer probe descriptor list.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.

File Description
pkg/internal/ebpf/gotracer/tphttp_privileged_test.go Adds privileged end-to-end test that attaches the tracer to a live Go process and asserts Traceparent is not duplicated.
pkg/internal/ebpf/gotracer/testdata/tphttpclient/main.go Adds helper program that runs an HTTP/1 server/client loop and reports how many Traceparent values were received.
pkg/internal/ebpf/gotracer/gotracer.go Registers a return (“End”) probe for net/http.Header.writeSubset to support conditional injection.
bpf/gotracer/go_nethttp.c Implements the new return uprobe logic: stash writer state on entry, scan serialized headers on return, and inject only if missing.

Comment thread bpf/gotracer/go_nethttp.c Outdated
Comment thread bpf/gotracer/go_nethttp.c Outdated
Comment thread bpf/gotracer/go_nethttp.c Outdated
Comment thread bpf/gotracer/go_nethttp.c Outdated
Comment thread bpf/gotracer/go_nethttp.c Outdated
Comment thread bpf/gotracer/go_nethttp.c Outdated
Comment thread bpf/gotracer/go_nethttp.c Outdated
Comment thread bpf/gotracer/go_nethttp.c Outdated
Comment thread pkg/internal/ebpf/gotracer/tphttp_privileged_test.go Outdated
Comment thread pkg/internal/ebpf/gotracer/tphttp_privileged_test.go
@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@7204eac). Learn more about missing BASE report.
⚠️ Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
pkg/ebpf/common/common.go 85.71% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2752   +/-   ##
=======================================
  Coverage        ?   69.76%           
=======================================
  Files           ?      359           
  Lines           ?    51142           
  Branches        ?        0           
=======================================
  Hits            ?    35678           
  Misses          ?    13236           
  Partials        ?     2228           
Flag Coverage Δ
integration-test 50.21% <62.50%> (?)
integration-test-arm 25.31% <55.55%> (?)
integration-test-vm-5.15-lts 26.69% <100.00%> (?)
integration-test-vm-6.18-lts 26.15% <62.50%> (?)
k8s-integration-test 33.97% <62.50%> (?)
oats-test 34.77% <62.50%> (?)
unittests 67.85% <62.50%> (?)

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

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@grcevski grcevski 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.

LGTM! Nice this is the right approach. I'll let you deal with the copilot comments.

@mariomac
mariomac merged commit a13a746 into open-telemetry:main Jul 23, 2026
213 of 214 checks passed
@mariomac
mariomac deleted the fix-double-tp-go-http1 branch July 23, 2026 14:01
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.

4 participants