feat(nethttp): add httptrace join-points for otelhttptrace support#458
feat(nethttp): add httptrace join-points for otelhttptrace support#458gyanranjanpanda wants to merge 4 commits into
Conversation
Add compile-time instrumentation support for net/http/httptrace to automatically create sub-spans for HTTP client request lifecycle phases. This creates a new pkg/instrumentation/nethttp/httptrace package that implements a clientTracer producing sub-spans for: - DNS resolution (http.dns) - TCP connection (http.connect) - TLS handshake (http.tls) - Connection acquisition (http.getconn) - Request send (http.send) - Response receive (http.receive) The existing BeforeRoundTrip hook in the client package is enhanced to attach an httptrace.ClientTrace to the request context after creating the parent client span. No new YAML rules are needed — the httptrace functionality piggybacks on the existing RoundTrip join-point. Includes 14 unit tests covering all lifecycle phases, error scenarios, connection reuse attributes, and full request lifecycle verification. Closes open-telemetry#201 Assisted-by: Gemini Antigravity
gyanranjanpanda
force-pushed
the
feat/add-httptrace-join-points
branch
from
22984a6 to
cea4087
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22984a67f2
ℹ️ 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".
| func (ct *clientTracer) putIdleConn(err error) { | ||
| ct.end("http.receive", err) | ||
| } |
There was a problem hiding this comment.
http.receive is started in gotFirstResponseByte but only ended in putIdleConn; however, net/http/httptrace does not invoke PutIdleConn for HTTP/2 requests (and also skips it when keep-alives are disabled), so in those common paths the receive span never ends and is never exported. This causes missing sub-spans and incomplete trace data for successful requests over those transports.
Useful? React with 👍 / 👎.
| func (ct *clientTracer) getConn(host string) { | ||
| ct.start("http.getconn", "http.getconn", HTTPHostAttribute.String(host)) |
There was a problem hiding this comment.
GetConn receives a host:port string, but this code writes it directly to net.host.name, which should contain only the hostname. As a result, spans will carry invalid host-name values like example.com:443, reducing semantic correctness and potentially breaking grouping/queries that expect normalized host names.
Useful? React with 👍 / 👎.
1. Remove http.receive span entirely because PutIdleConn is not reliably invoked (e.g. for HTTP/2 or when keep-alives are disabled), preventing hanging spans. 2. Strip port from hostPort before setting net.host.name on getconn span.
|
could @kakkoyun @txabman42 review this |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #458 +/- ##
==========================================
+ Coverage 62.26% 62.94% +0.68%
==========================================
Files 62 63 +1
Lines 4717 4836 +119
==========================================
+ Hits 2937 3044 +107
- Misses 1535 1546 +11
- Partials 245 246 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
is there any thing i hve to change @kakkoyun i would like to change |
|
This PR is related to #202 Let's align first on a common approach for go-contrib join points. |
3 participants
Summary
Adds compile-time instrumentation support for
net/http/httptrace, implementing the join-points needed to supportotelhttptracefromopentelemetry-go-contrib(closes #201, part of umbrella #164).What Changed
New Package:
pkg/instrumentation/nethttp/httptrace/A
clientTracerimplementation that hooks into Go'snet/http/httptrace.ClientTracecallbacks to create detailed sub-spans for each HTTP client request lifecycle phase:http.getconnnet.host.name, connection reuse infohttp.dnsnet.host.name, resolved addresseshttp.connecthttp.tlshttp.sendhttp.receiveIntegration into Existing Client Hook
The existing
BeforeRoundTriphook inpkg/instrumentation/nethttp/client/is enhanced with 7 lines to attach anhttptrace.ClientTraceto the request context after creating the parent client span. No new YAML rules needed — the httptrace functionality piggybacks on the existingRoundTripjoin-point.Span Hierarchy
Testing
make format/go— ✅ PASSmake lint/go— ✅ 0 issuesmake lint/license-header— ✅ All headers presentmake build— ✅ Built successfullyFiles Changed
New files (3):
pkg/instrumentation/nethttp/httptrace/httptrace_hook.gopkg/instrumentation/nethttp/httptrace/httptrace_hook_test.gopkg/instrumentation/nethttp/httptrace/go.modModified files (2 meaningful):
pkg/instrumentation/nethttp/client/client_hook.go— wire httptrace intoBeforeRoundTrippkg/instrumentation/nethttp/client/go.mod— add httptrace dependencypkg/instrumentation/nethttp/server/go.mod— add httptrace replace directive (transitive)