otelhttptrace: handle missing getconn hook without panic (#5965)

Fork of #5187 updated with main branch and tests, this PR adds nil dereference check for clientTracer.root in `end()` when span events are used instead of sub spans --------- Signed-off-by: Tonis Tiigi <[email protected]> Co-authored-by: Tonis Tiigi <[email protected]> Co-authored-by: Damien Mathieu <[email protected]>
open-telemetry · Sep 12, 2024 · 042f99b · 042f99b
1 parent 7d7d4c3
commit 042f99b
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ## [Unreleased]
 
+### Fixed
+
+- 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 -->
 

diff --git a/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go b/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go
@@ -215,6 +215,10 @@ func (ct *clientTracer) start(hook, spanName string, attrs ...attribute.KeyValue
 
 func (ct *clientTracer) end(hook string, err error, attrs ...attribute.KeyValue) {
 	if !ct.useSpans {
+		// sometimes end may be called without previous start
+		if ct.root == nil {
+			ct.root = trace.SpanFromContext(ct.Context)
+		}
 		if err != nil {
 			attrs = append(attrs, attribute.String(hook+".error", err.Error()))
 		}

diff --git 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 TestEndBeforeStartWithoutSubSpansDoesNotPanic(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