Skip to content

Commit

Permalink
otelhttp: get tracer from current context if not set in constructor
Browse files Browse the repository at this point in the history
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
  • Loading branch information
tonistiigi committed Sep 8, 2021
1 parent f650585 commit eced414
Show file tree
Hide file tree
Showing 8 changed files with 154 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Migrated EC2 resource detector support from root module `go.opentelemetry.io/contrib/detectors/aws` to a separate EC2 resource detector module `go.opentelemetry.io/contrib/detectors/aws/ec2` (#1017)
- Add `cloud.provider` and `cloud.platform` to AWS detectors. (#1043)
- `otelhttptrace.NewClientTrace` now redacts known sensitive headers by default. (#879)
- The `Transport`, `Handler`, and HTTP client convenience wrappers in the `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp` package now use the `TracerProvider` from the parent context if one exists and none was explicitly set when configuring the instrumentation. (#873)

### Fixed

Expand Down
6 changes: 6 additions & 0 deletions instrumentation/net/http/otelhttp/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ package otelhttp
import (
"net/http"

"go.opentelemetry.io/contrib"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
)

// Attribute keys that can be added to a span.
Expand All @@ -39,3 +41,7 @@ const (
// Filter is a predicate used to determine whether a given http.request should
// be traced. A Filter must return true if the request should be traced.
type Filter func(*http.Request) bool

func newTracer(tp trace.TracerProvider) trace.Tracer {
return tp.Tracer(instrumentationName, trace.WithInstrumentationVersion(contrib.SemVersion()))
}
13 changes: 6 additions & 7 deletions instrumentation/net/http/otelhttp/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,17 @@ func (o optionFunc) apply(c *config) {
// newConfig creates a new config struct and applies opts to it.
func newConfig(opts ...Option) *config {
c := &config{
Propagators: otel.GetTextMapPropagator(),
TracerProvider: otel.GetTracerProvider(),
MeterProvider: global.GetMeterProvider(),
Propagators: otel.GetTextMapPropagator(),
MeterProvider: global.GetMeterProvider(),
}
for _, opt := range opts {
opt.apply(c)
}

c.Tracer = c.TracerProvider.Tracer(
instrumentationName,
trace.WithInstrumentationVersion(contrib.SemVersion()),
)
// Tracer is only initialized if manually specified. Otherwise, can be passed with the tracing context.
if c.TracerProvider != nil {
c.Tracer = newTracer(c.TracerProvider)
}
c.Meter = c.MeterProvider.Meter(
instrumentationName,
metric.WithInstrumentationVersion(contrib.SemVersion()),
Expand Down
12 changes: 11 additions & 1 deletion instrumentation/net/http/otelhttp/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,18 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
trace.WithAttributes(semconv.HTTPServerAttributesFromHTTPRequest(h.operation, "", r)...),
}, h.spanStartOptions...) // start with the configured options

tracer := h.tracer

if tracer == nil {
if span := trace.SpanFromContext(r.Context()); span.SpanContext().IsValid() {
tracer = newTracer(span.TracerProvider())
} else {
tracer = newTracer(otel.GetTracerProvider())
}
}

ctx := h.propagators.Extract(r.Context(), propagation.HeaderCarrier(r.Header))
ctx, span := h.tracer.Start(ctx, h.spanNameFormatter(h.operation, r), opts...)
ctx, span := tracer.Start(ctx, h.spanNameFormatter(h.operation, r), opts...)
defer span.End()

readRecordFunc := func(int64) {}
Expand Down
32 changes: 32 additions & 0 deletions instrumentation/net/http/otelhttp/test/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,35 @@ func TestConvenienceWrappers(t *testing.T) {
assert.Equal(t, "HTTP POST", spans[2].Name())
assert.Equal(t, "HTTP POST", spans[3].Name())
}

func TestClientWithTraceContext(t *testing.T) {
sr := tracetest.NewSpanRecorder()
provider := trace.NewTracerProvider(trace.WithSpanProcessor(sr))

tracer := provider.Tracer("")
ctx, span := tracer.Start(context.Background(), "http requests")

content := []byte("Hello, world!")

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if _, err := w.Write(content); err != nil {
t.Fatal(err)
}
}))
defer ts.Close()

res, err := otelhttp.Get(ctx, ts.URL)
if err != nil {
t.Fatal(err)
}
res.Body.Close()

span.End()

spans := sr.Ended()
require.Equal(t, 2, len(spans))
assert.Equal(t, "HTTP GET", spans[0].Name())
assert.Equal(t, "http requests", spans[1].Name())
assert.NotEmpty(t, spans[0].Parent().SpanID())
assert.Equal(t, spans[1].SpanContext().SpanID(), spans[0].Parent().SpanID())
}
36 changes: 36 additions & 0 deletions instrumentation/net/http/otelhttp/test/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package test

import (
"context"
"fmt"
"io"
"io/ioutil"
Expand All @@ -24,6 +25,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
"go.opentelemetry.io/otel/attribute"
Expand Down Expand Up @@ -107,3 +109,37 @@ func TestHandlerBasics(t *testing.T) {
t.Fatalf("got %q, expected %q", got, expected)
}
}

func TestHandlerRequestWithTraceContext(t *testing.T) {
rr := httptest.NewRecorder()

h := otelhttp.NewHandler(
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_, err := w.Write([]byte("hello world"))
require.NoError(t, err)
}), "test_handler")

r, err := http.NewRequest(http.MethodGet, "http://localhost/", nil)
require.NoError(t, err)

spanRecorder := tracetest.NewSpanRecorder()
provider := sdktrace.NewTracerProvider(
sdktrace.WithSpanProcessor(spanRecorder),
)
tracer := provider.Tracer("")
ctx, span := tracer.Start(context.Background(), "test_request")
r = r.WithContext(ctx)

h.ServeHTTP(rr, r)
assert.Equal(t, 200, rr.Result().StatusCode)

span.End()

spans := spanRecorder.Ended()
require.Len(t, spans, 2)

assert.Equal(t, "test_handler", spans[0].Name())
assert.Equal(t, "test_request", spans[1].Name())
assert.NotEmpty(t, spans[0].Parent().SpanID())
assert.Equal(t, spans[1].SpanContext().SpanID(), spans[0].Parent().SpanID())
}
50 changes: 50 additions & 0 deletions instrumentation/net/http/otelhttp/test/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,16 @@
package test

import (
"context"
"io/ioutil"
"net/http"
"net/http/httptest"
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
"go.opentelemetry.io/otel/codes"
"go.opentelemetry.io/otel/propagation"
Expand Down Expand Up @@ -116,3 +121,48 @@ func TestTransportErrorStatus(t *testing.T) {
t.Errorf("expected error status message on span; got: %q", got)
}
}

func TestTransportRequestWithTraceContext(t *testing.T) {
spanRecorder := tracetest.NewSpanRecorder()
provider := sdktrace.NewTracerProvider(
sdktrace.WithSpanProcessor(spanRecorder),
)
content := []byte("Hello, world!")

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_, err := w.Write(content)
require.NoError(t, err)
}))
defer ts.Close()

tracer := provider.Tracer("")
ctx, span := tracer.Start(context.Background(), "test_span")

r, err := http.NewRequest(http.MethodGet, ts.URL, nil)
require.NoError(t, err)

r = r.WithContext(ctx)

tr := otelhttp.NewTransport(
http.DefaultTransport,
)

c := http.Client{Transport: tr}
res, err := c.Do(r)
require.NoError(t, err)

span.End()

body, err := ioutil.ReadAll(res.Body)
require.NoError(t, err)

require.Equal(t, content, body)

spans := spanRecorder.Ended()
require.Len(t, spans, 2)

assert.Equal(t, "test_span", spans[0].Name())
assert.Equal(t, "HTTP GET", spans[1].Name())
assert.NotEmpty(t, spans[1].Parent().SpanID())
assert.Equal(t, spans[0].SpanContext().SpanID(), spans[1].Parent().SpanID())
}
13 changes: 12 additions & 1 deletion instrumentation/net/http/otelhttp/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"io"
"net/http"

"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/codes"
"go.opentelemetry.io/otel/propagation"
semconv "go.opentelemetry.io/otel/semconv/v1.4.0"
Expand Down Expand Up @@ -87,9 +88,19 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) {
}
}

tracer := t.tracer

if tracer == nil {
if span := trace.SpanFromContext(r.Context()); span.SpanContext().IsValid() {
tracer = newTracer(span.TracerProvider())
} else {
tracer = newTracer(otel.GetTracerProvider())
}
}

opts := append([]trace.SpanStartOption{}, t.spanStartOptions...) // start with the configured options

ctx, span := t.tracer.Start(r.Context(), t.spanNameFormatter("", r), opts...)
ctx, span := tracer.Start(r.Context(), t.spanNameFormatter("", r), opts...)

r = r.WithContext(ctx)
span.SetAttributes(semconv.HTTPClientAttributesFromHTTPRequest(r)...)
Expand Down

0 comments on commit eced414

Please sign in to comment.