feat(otel): add option to continue from otel

Author: costela

http/sentryhttp.go

@@ -91,17 +91,21 @@ func (h *Handler) handle(handler http.Handler) http.HandlerFunc {
 			sentry.ContinueFromRequest(r),
 			sentry.WithTransactionSource(sentry.SourceURL),
 		}
-		// We don't mind getting an existing transaction back so we don't need to
-		// check if it is.
-		transaction := sentry.StartTransaction(ctx,
-			fmt.Sprintf("%s %s", r.Method, r.URL.Path),
-			options...,
-		)
-		defer transaction.Finish()
+		// If a transaction was already started, whoever started is is responsible for finishing it.
+		transaction := sentry.TransactionFromContext(ctx)
+		if transaction == nil {
+			transaction = sentry.StartTransaction(ctx,
+				fmt.Sprintf("%s %s", r.Method, r.URL.Path),
+				options...,
+			)
+			defer transaction.Finish()
+			// We also avoid clobbering the request's context with an older version. If values were added after the
+			// original transaction's creation, they would be lost by indiscriminately overwriting the context.
+			r = r.WithContext(transaction.Context())
+		}
 		// TODO(tracing): if the next handler.ServeHTTP panics, store
 		// information on the transaction accordingly (status, tag,
 		// level?, ...).
-		r = r.WithContext(transaction.Context())
 		hub.Scope().SetRequest(r)
 		defer h.recoverWithSentry(hub, r)
 		// TODO(tracing): use custom response writer to intercept

otel/middleware_test.go

@@ -0,0 +1,124 @@
+package sentryotel
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/getsentry/sentry-go"
+	sentryhttp "github.com/getsentry/sentry-go/http"
+	"go.opentelemetry.io/otel"
+	otelSdkTrace "go.opentelemetry.io/otel/sdk/trace"
+	"go.opentelemetry.io/otel/trace"
+)
+
+func emptyContextWithSentryAndTracing(t *testing.T) (context.Context, map[string]*sentry.Span) {
+	t.Helper()
+
+	// we want to check sent events after they're finished, so sentrySpanMap cannot be used
+	spans := make(map[string]*sentry.Span)
+
+	client, err := sentry.NewClient(sentry.ClientOptions{
+		Debug:         true,
+		Dsn:           "https://[email protected]/123",
+		Environment:   "testing",
+		Release:       "1.2.3",
+		EnableTracing: true,
+		BeforeSendTransaction: func(event *sentry.Event, _ *sentry.EventHint) *sentry.Event {
+			for _, span := range event.Spans {
+				spans[span.SpanID.String()] = span
+			}
+			return event
+		},
+	})
+	if err != nil {
+		t.Fatalf("failed to create sentry client: %v", err)
+	}
+
+	hub := sentry.NewHub(client, sentry.NewScope())
+	return sentry.SetHubOnContext(context.Background(), hub), spans
+}
+
+func TestRespectOtelSampling(t *testing.T) {
+	spanProcessor := NewSentrySpanProcessor()
+
+	simulateOtelAndSentry := func(ctx context.Context) (root, inner trace.Span) {
+		handler := sentryhttp.New(sentryhttp.Options{}).Handle(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			_, inner = otel.Tracer("").Start(r.Context(), "test-inner-span")
+			defer inner.End()
+		}))
+		handler = ContinueFromOtel(handler)
+
+		tracer := otel.Tracer("")
+		// simulate an otel middleware creating the root span before sentry
+		ctx, root = tracer.Start(ctx, "test-root-span")
+		defer root.End()
+
+		handler.ServeHTTP(
+			httptest.NewRecorder(),
+			httptest.NewRequest(http.MethodGet, "/", nil).WithContext(ctx),
+		)
+
+		return root, inner
+	}
+
+	t.Run("always sample", func(t *testing.T) {
+		tp := otelSdkTrace.NewTracerProvider(
+			otelSdkTrace.WithSpanProcessor(spanProcessor),
+			otelSdkTrace.WithSampler(otelSdkTrace.AlwaysSample()),
+		)
+		otel.SetTracerProvider(tp)
+
+		ctx, spans := emptyContextWithSentryAndTracing(t)
+
+		root, inner := simulateOtelAndSentry(ctx)
+
+		if root.SpanContext().TraceID() != inner.SpanContext().TraceID() {
+			t.Errorf("otel root span and inner span should have the same trace id")
+		}
+
+		if len(spans) != 1 {
+			t.Errorf("got unexpected number of events sent to sentry: %d != 1", len(spans))
+		}
+
+		for _, span := range []trace.Span{root, inner} {
+			if !span.SpanContext().IsSampled() {
+				t.Errorf("otel span should be sampled")
+			}
+		}
+
+		// the root span is encoded into the event's context, not in sentry.Event.Spans
+		spanID := inner.SpanContext().SpanID().String()
+		sentrySpan, ok := spans[spanID]
+		if !ok {
+			t.Fatalf("sentry event could not be found from otel span %s", spanID)
+		}
+
+		if sentrySpan.Sampled != sentry.SampledTrue {
+			t.Errorf("sentry span should be sampled, not %v", sentrySpan.Sampled)
+		}
+	})
+
+	t.Run("never sample", func(t *testing.T) {
+		tp := otelSdkTrace.NewTracerProvider(
+			otelSdkTrace.WithSpanProcessor(spanProcessor),
+			otelSdkTrace.WithSampler(otelSdkTrace.NeverSample()),
+		)
+		otel.SetTracerProvider(tp)
+
+		ctx, spans := emptyContextWithSentryAndTracing(t)
+
+		root, inner := simulateOtelAndSentry(ctx)
+
+		if len(spans) != 0 {
+			t.Fatalf("sentry span should not have been sent to sentry")
+		}
+
+		for _, span := range []trace.Span{root, inner} {
+			if span.SpanContext().IsSampled() {
+				t.Errorf("otel span should not be sampled")
+			}
+		}
+	})
+}

otel/mittleware.go

@@ -0,0 +1,23 @@
+package sentryotel
+
+import (
+	"net/http"
+
+	"github.com/getsentry/sentry-go"
+	"go.opentelemetry.io/otel/trace"
+)
+
+// ContinueFromOtel is a HTTP middleware that can be used with [sentryhttp.Handler] to ensure an existing otel span is
+// used as the sentry transaction.
+// It should be used whenever the otel tracing is started before the sentry middleware (e.g. to ensure otel sampling
+// gets respected across service boundaries)
+func ContinueFromOtel(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if otelTrace := trace.SpanFromContext(r.Context()); otelTrace != nil && otelTrace.IsRecording() {
+			if transaction, ok := sentrySpanMap.Get(otelTrace.SpanContext().SpanID()); ok {
+				r = r.WithContext(sentry.SpanToContext(r.Context(), transaction))
+			}
+		}
+		next.ServeHTTP(w, r)
+	})
+}

otel/span_processor.go

@@ -46,11 +46,11 @@ func (ssp *sentrySpanProcessor) OnStart(parent context.Context, s otelSdkTrace.R
 
 		sentrySpanMap.Set(otelSpanID, span)
 	} else {
-		traceParentContext := getTraceParentContext(parent)
+		sampled := getSampled(parent, s)
 		transaction := sentry.StartTransaction(
 			parent,
 			s.Name(),
-			sentry.WithSpanSampled(traceParentContext.Sampled),
+			sentry.WithSpanSampled(sampled),
 		)
 		transaction.SpanID = sentry.SpanID(otelSpanID)
 		transaction.TraceID = sentry.TraceID(otelTraceID)
@@ -108,12 +108,17 @@ func flushSpanProcessor(ctx context.Context) error {
 	return nil
 }
 
-func getTraceParentContext(ctx context.Context) sentry.TraceParentContext {
+func getSampled(ctx context.Context, s otelSdkTrace.ReadWriteSpan) sentry.Sampled {
 	traceParentContext, ok := ctx.Value(sentryTraceParentContextKey{}).(sentry.TraceParentContext)
-	if !ok {
-		traceParentContext.Sampled = sentry.SampledUndefined
+	if ok {
+		return traceParentContext.Sampled
 	}
-	return traceParentContext
+
+	if s.SpanContext().IsSampled() {
+		return sentry.SampledTrue
+	}
+
+	return sentry.SampledFalse
 }
 
 func updateTransactionWithOtelData(transaction *sentry.Span, s otelSdkTrace.ReadOnlySpan) {

tracing.go

@@ -114,7 +114,7 @@ func StartSpan(ctx context.Context, operation string, options ...SpanOption) *Sp
 		StartTime: time.Now(),
 		Sampled:   SampledUndefined,
 
-		ctx:           context.WithValue(ctx, spanContextKey{}, &span),
+		ctx:           SpanToContext(ctx, &span),
 		parent:        parent,
 		isTransaction: !hasParent,
 	}
@@ -961,6 +961,12 @@ func SpanFromContext(ctx context.Context) *Span {
 	return nil
 }
 
+// SpanToContext stores a span in the provided context.
+// Usually this function does not need to be called directly; use [StartSpan] instead.
+func SpanToContext(ctx context.Context, span *Span) context.Context {
+	return context.WithValue(ctx, spanContextKey{}, span)
+}
+
 // StartTransaction will create a transaction (root span) if there's no existing
 // transaction in the context otherwise, it will return the existing transaction.
 func StartTransaction(ctx context.Context, name string, options ...SpanOption) *Span {