From 7dcd61b4a8529cbe7c8f68aae9c05b18719c881b Mon Sep 17 00:00:00 2001 From: berce <75228263+bercedev@users.noreply.github.com> Date: Wed, 28 Oct 2026 17:23:53 +0300 Subject: [PATCH] client: end the otelhttp span after a successful hijack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit setupHijackConn only closed the response body when the upgrade failed. otelhttp ends its client span when the body is closed, so the span of every successful ContainerAttach, ContainerExecAttach and DialHijack call was never ended. A 101 response has no body, so closing it does not touch the hijacked connection. Signed-off-by: berce <75228263+bercedev@users.noreply.github.com> Signed-off-by: Paweł Gronowski --- client/hijack.go | 4 +- client/hijack_test.go | 39 +++++++++++++++++++- vendor/github.com/moby/moby/client/hijack.go | 4 +- 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/client/hijack.go b/client/hijack.go index fbb0402725..70dcf28699 100644 --- a/client/hijack.go +++ b/client/hijack.go @@ -100,8 +100,10 @@ func (cli *Client) setupHijackConn(req *http.Request, proto string) (retConn net if err != nil { return nil, "", err } + // Closing the body ends the otelhttp span. A 101 response has no body, + // so this does not close the connection. + _ = resp.Body.Close() if resp.StatusCode != http.StatusSwitchingProtocols { - _ = resp.Body.Close() return nil, "", fmt.Errorf("unable to upgrade to %s, received %d", proto, resp.StatusCode) } diff --git a/client/hijack_test.go b/client/hijack_test.go index a22511d679..154289a798 100644 --- a/client/hijack_test.go +++ b/client/hijack_test.go @@ -10,9 +10,12 @@ import ( "net/http/httptest" "net/url" "strings" + "sync/atomic" "testing" "time" + "go.opentelemetry.io/otel/trace" + "go.opentelemetry.io/otel/trace/noop" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" ) @@ -132,7 +135,7 @@ func TestHijackCancellationOwnership(t *testing.T) { } } -func TestHijackHooks(t *testing.T) { +func TestHijackHooksAndTracing(t *testing.T) { const ( reqHeaderKey = "X-Test-Request" reqHeaderValue = "request" @@ -162,9 +165,11 @@ func TestHijackHooks(t *testing.T) { serverURL, err := url.Parse(ts.URL) assert.NilError(t, err) + tp := &spanCounter{} var gotResponseHeader string c, err := New( WithHost("tcp://"+serverURL.Host), + WithTraceProvider(tp), WithHTTPRequestHook(func(req *http.Request) error { req.Header.Set(reqHeaderKey, reqHeaderValue) return nil @@ -180,6 +185,38 @@ func TestHijackHooks(t *testing.T) { defer func() { _ = conn.Close() }() assert.Equal(t, gotResponseHeader, respHeaderValue) + assert.Check(t, is.Equal(tp.started.Load(), int32(1))) + assert.Check(t, is.Equal(tp.ended.Load(), int32(1))) +} + +// spanCounter counts the spans started and ended through it. +type spanCounter struct { + noop.TracerProvider + started, ended atomic.Int32 +} + +func (p *spanCounter) Tracer(string, ...trace.TracerOption) trace.Tracer { + return countingTracer{p: p} +} + +type countingTracer struct { + noop.Tracer + p *spanCounter +} + +func (t countingTracer) Start(ctx context.Context, name string, opts ...trace.SpanStartOption) (context.Context, trace.Span) { + t.p.started.Add(1) + ctx, span := t.Tracer.Start(ctx, name, opts...) + return ctx, countingSpan{Span: span, p: t.p} +} + +type countingSpan struct { + trace.Span + p *spanCounter +} + +func (s countingSpan) End(...trace.SpanEndOption) { + s.p.ended.Add(1) } func TestTLSCloseWriter(t *testing.T) { diff --git a/vendor/github.com/moby/moby/client/hijack.go b/vendor/github.com/moby/moby/client/hijack.go index fbb0402725..70dcf28699 100644 --- a/vendor/github.com/moby/moby/client/hijack.go +++ b/vendor/github.com/moby/moby/client/hijack.go @@ -100,8 +100,10 @@ func (cli *Client) setupHijackConn(req *http.Request, proto string) (retConn net if err != nil { return nil, "", err } + // Closing the body ends the otelhttp span. A 101 response has no body, + // so this does not close the connection. + _ = resp.Body.Close() if resp.StatusCode != http.StatusSwitchingProtocols { - _ = resp.Body.Close() return nil, "", fmt.Errorf("unable to upgrade to %s, received %d", proto, resp.StatusCode) }