Skip to content
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

<!-- Released section -->
<!-- Don't change this section unless doing release -->
Expand Down
24 changes: 17 additions & 7 deletions instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@
"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.
Expand Down Expand Up @@ -360,26 +361,35 @@
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 {
Comment thread
pellared marked this conversation as resolved.
span.AddEvent("GOT 100 - Continue")
}

Check warning on line 367 in instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go

View check run for this annotation

Codecov / codecov/patch

instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go#L366-L367

Added lines #L366 - L367 were not covered by tests
}

func (ct *clientTracer) wait100Continue() {
span := ct.root
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 {
span := ct.root
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)),
))
}

Check warning on line 392 in instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go

View check run for this annotation

Codecov / codecov/patch

instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go#L388-L392

Added lines #L388 - L392 were not covered by tests
return nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down