diff --git a/CHANGELOG.md b/CHANGELOG.md index 74808781c13..f2120caf850 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Superfluous call to `WriteHeader` when flushing after setting a status code in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#6074) - Superfluous call to `WriteHeader` when writing the response body after setting a status code in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#6055) +- Possible nil dereference panic in `go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace`. (#5965) @@ -53,7 +54,6 @@ The next release will require at least [Go 1.22]. ### Fixed - Race condition when reading the HTTP body and writing the response in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#5916) -- Possible nil dereference panic in `go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace`. (#5965) ## [1.28.0/0.53.0/0.22.0/0.8.0/0.3.0/0.1.0] - 2024-07-02 diff --git a/instrumentation/net/http/httptrace/otelhttptrace/clienttrace_test.go b/instrumentation/net/http/httptrace/otelhttptrace/clienttrace_test.go index b6eebfe703e..966473dd0c3 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/clienttrace_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/clienttrace_test.go @@ -5,20 +5,9 @@ package otelhttptrace import ( "context" - "errors" "fmt" "net/http" "net/http/httptrace" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/attribute" - sdktrace "go.opentelemetry.io/otel/sdk/trace" - "go.opentelemetry.io/otel/sdk/trace/tracetest" - "go.opentelemetry.io/otel/trace" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" ) @@ -43,119 +32,3 @@ func ExampleNewClientTrace() { fmt.Println(resp.Status) } - -func Test_clientTracer_end(t *testing.T) { - t.Run("end called with no parent clientTracer span", func(t *testing.T) { - fixture := prepareClientTraceTest() - fixture.ct.end("http.getconn", nil, HTTPConnectionReused.Bool(true), HTTPConnectionWasIdle.Bool(true)) - assert.Len(t, fixture.spanRecorder.Ended(), 0) - }) - - t.Run("end called with no sub spans, no root span, and no errors", func(t *testing.T) { - fixture := prepareClientTraceTest() - WithoutSubSpans().apply(fixture.ct) - - ctx, span := fixture.tracer.Start(fixture.ct.Context, "client request") - fixture.ct.Context = ctx - - fixture.ct.end("http.getconn", nil, HTTPConnectionReused.Bool(true), HTTPConnectionWasIdle.Bool(true)) - span.End() - - require.Len(t, fixture.spanRecorder.Ended(), 1) - recSpan := fixture.spanRecorder.Ended()[0] - - require.Len(t, recSpan.Events(), 1) - gotEvent := recSpan.Events()[0] - assert.Equal(t, "http.getconn.done", gotEvent.Name) - - assert.Equal(t, - []attribute.KeyValue{HTTPConnectionReused.Bool(true), HTTPConnectionWasIdle.Bool(true)}, - gotEvent.Attributes, - ) - }) - - t.Run("end called with no sub spans, root span set, and no errors", func(t *testing.T) { - fixture := prepareClientTraceTest() - WithoutSubSpans().apply(fixture.ct) - - ctx, span := fixture.tracer.Start(fixture.ct.Context, "client request") - fixture.ct.Context = ctx - fixture.ct.root = span - - fixture.ct.end("http.getconn", nil, HTTPConnectionReused.Bool(true), HTTPConnectionWasIdle.Bool(true)) - span.End() - - require.Len(t, fixture.spanRecorder.Ended(), 1) - recSpan := fixture.spanRecorder.Ended()[0] - - require.Len(t, recSpan.Events(), 1) - gotEvent := recSpan.Events()[0] - assert.Equal(t, "http.getconn.done", gotEvent.Name) - - assert.Equal(t, - []attribute.KeyValue{ - HTTPConnectionReused.Bool(true), - HTTPConnectionWasIdle.Bool(true), - }, - gotEvent.Attributes, - ) - }) - - t.Run("end called with no sub spans, root span set, and error", func(t *testing.T) { - fixture := prepareClientTraceTest() - WithoutSubSpans().apply(fixture.ct) - - ctx, span := fixture.tracer.Start(fixture.ct.Context, "client request") - fixture.ct.Context = ctx - fixture.ct.root = span - - fixture.ct.end("http.getconn", errors.New("testError"), HTTPConnectionReused.Bool(true), HTTPConnectionWasIdle.Bool(true)) - span.End() - - require.Len(t, fixture.spanRecorder.Ended(), 1) - recSpan := fixture.spanRecorder.Ended()[0] - - require.Len(t, recSpan.Events(), 1) - gotEvent := recSpan.Events()[0] - assert.Equal(t, "http.getconn.done", gotEvent.Name) - - assert.Equal(t, - []attribute.KeyValue{ - HTTPConnectionReused.Bool(true), - HTTPConnectionWasIdle.Bool(true), - attribute.Key("http.getconn.error").String("testError"), - }, - gotEvent.Attributes, - ) - }) -} - -type clientTraceTestFixture struct { - spanRecorder *tracetest.SpanRecorder - tracer trace.Tracer - ct *clientTracer -} - -func prepareClientTraceTest() clientTraceTestFixture { - fixture := clientTraceTestFixture{} - fixture.spanRecorder = tracetest.NewSpanRecorder() - provider := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(fixture.spanRecorder)) - otel.SetTracerProvider(provider) - - fixture.tracer = provider.Tracer( - ScopeName, - trace.WithInstrumentationVersion(Version())) - - fixture.ct = &clientTracer{ - Context: context.Background(), - tracerProvider: otel.GetTracerProvider(), - root: nil, - tr: fixture.tracer, - activeHooks: make(map[string]context.Context), - redactedHeaders: map[string]struct{}{}, - addHeaders: true, - useSpans: true, - } - - return fixture -} diff --git a/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go b/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go index 846f4fd128f..3077c551cff 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go @@ -250,6 +250,22 @@ func TestEndBeforeStartCreatesSpan(t *testing.T) { require.Len(t, spans, 1) } +func TestEndBeforeStartWithSubSpansDoesNotPanic(t *testing.T) { + sr := tracetest.NewSpanRecorder() + tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) + otel.SetTracerProvider(tp) + + ct := otelhttptrace.NewClientTrace(context.Background(), otelhttptrace.WithoutSubSpans()) + + require.NotPanics(t, func() { + ct.DNSDone(httptrace.DNSDoneInfo{}) + }) + + // no spans created because we were just using background context without span + // and Start wasn't called which would have started a span + require.Len(t, sr.Ended(), 0) +} + type clientTraceTestFixture struct { Address string URL string