diff --git a/CHANGELOG.md b/CHANGELOG.md index 83a3512d30f..1dc37a48021 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,6 +74,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed - Fix data race when writing log entries with `context.Context` fields in `go.opentelemetry.io/contrib/bridges/otelzap`. (#7368) +- Fix nil pointer dereference when `ClientTracer` did not have a span in `go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace`. (#7464) diff --git a/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go b/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go index 222b35871fa..942566e6550 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go @@ -11,11 +11,12 @@ import ( "strings" "sync" - "go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/trace" + + "go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv" ) // ScopeName is the instrumentation scope name. @@ -360,7 +361,10 @@ func (ct *clientTracer) got100Continue() { if ct.useSpans { span = ct.span("http.receive") } - span.AddEvent("GOT 100 - Continue") + // It's possible that Got100Continue is called before GotFirstResponseByte at which point span can be `nil`. + if span != nil { + span.AddEvent("GOT 100 - Continue") + } } func (ct *clientTracer) wait100Continue() { @@ -368,7 +372,10 @@ func (ct *clientTracer) wait100Continue() { if ct.useSpans { span = ct.span("http.send") } - span.AddEvent("GOT 100 - Wait") + // It's possible that Wait100Continue is called before GotFirstResponseByte at which point span can be `nil`. + if span != nil { + span.AddEvent("GOT 100 - Wait") + } } func (ct *clientTracer) got1xxResponse(code int, header textproto.MIMEHeader) error { @@ -376,10 +383,13 @@ func (ct *clientTracer) got1xxResponse(code int, header textproto.MIMEHeader) er if ct.useSpans { span = ct.span("http.receive") } - span.AddEvent("GOT 1xx", trace.WithAttributes( - HTTPStatus.Int(code), - HTTPHeaderMIME.String(sm2s(header)), - )) + // It's possible that Got1xxResponse is called before GotFirstResponseByte at which point span can be `nil`. + if span != nil { + span.AddEvent("GOT 1xx", trace.WithAttributes( + HTTPStatus.Int(code), + HTTPHeaderMIME.String(sm2s(header)), + )) + } return nil } diff --git a/instrumentation/net/http/httptrace/otelhttptrace/clienttracetest_test.go b/instrumentation/net/http/httptrace/otelhttptrace/clienttracetest_test.go index 4afe48d4f4d..f498c7ca640 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/clienttracetest_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/clienttracetest_test.go @@ -15,11 +15,12 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" + + "go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace" ) func getSpanFromRecorder(sr *tracetest.SpanRecorder, name string) (trace.ReadOnlySpan, bool) { @@ -266,6 +267,29 @@ func TestEndBeforeStartWithoutSubSpansDoesNotPanic(t *testing.T) { require.Empty(t, sr.Ended()) } +func TestNoClientTraceCallGuarantee(t *testing.T) { + t.Run("Got100Continue", func(t *testing.T) { + // It is possible that Got100Continue is called before GotFirstResponseByte. + // Also as there is no guarantee provided in the ClientTrace docs that GotFirstResponseByte should be called before + // Got100Continue this edge case should be covered. + assert.NotPanics(t, func() { + clientTrace := otelhttptrace.NewClientTrace(context.Background()) + clientTrace.Got100Continue() + }) + }) + t.Run("Got1xxResponse", func(t *testing.T) { + clientTrace := otelhttptrace.NewClientTrace(context.Background()) + err := clientTrace.Got1xxResponse(http.StatusNoContent, nil) + assert.NoError(t, err) + }) + t.Run("Wait100Continue", func(t *testing.T) { + assert.NotPanics(t, func() { + clientTrace := otelhttptrace.NewClientTrace(context.Background()) + clientTrace.Wait100Continue() + }) + }) +} + type clientTraceTestFixture struct { Address string URL string