move to otelhttptrace/test/clienttrace_test.go

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)
 
 <!-- Released section -->
 <!-- Don't change this section unless doing release -->
@@ -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
 

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
-}

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