fix(contrib/net/http): opt-in single server span for instrumented routers

contrib/gin-gonic/gin/gintrace.go

@@ -26,6 +26,7 @@ var instr *instrumentation.Instrumentation
 
 func init() {
 	instr = instrumentation.Load(instrumentation.PackageGin)
+	httptrace.RegisterRoutingHandlerType[*gin.Engine]()
 }
 
 // Middleware returns middleware that will trace incoming requests. If service is empty then the

contrib/go-chi/chi.v5/chi.go

@@ -27,6 +27,7 @@ var instr *instrumentation.Instrumentation
 
 func init() {
 	instr = instrumentation.Load(instrumentation.PackageChiV5)
+	httptrace.RegisterRoutingHandlerType[*chi.Mux]()
 }
 
 // Middleware returns middleware that will trace incoming requests.

contrib/go-chi/chi/chi.go

@@ -26,6 +26,7 @@ var instr *instrumentation.Instrumentation
 
 func init() {
 	instr = instrumentation.Load(instrumentation.PackageChi)
+	httptrace.RegisterRoutingHandlerType[*chi.Mux]()
 }
 
 // Middleware returns middleware that will trace incoming requests.

contrib/gorilla/mux/orchestrion.yml

@@ -21,6 +21,7 @@ aspects:
           imports:
             http: net/http
             instrumentation: github.com/DataDog/dd-trace-go/v2/instrumentation
+            instrhttptrace: github.com/DataDog/dd-trace-go/v2/instrumentation/httptrace
             sync: sync
             tracer: github.com/DataDog/dd-trace-go/v2/ddtrace/tracer
           lang: go1.18 # some parts of our codebase use generics, so ensure we can build if using old versions of gorilla/mux (e.g. if using a replace).
@@ -29,6 +30,7 @@ aspects:
 
             func init() {
               __dd_instr = instrumentation.Load(instrumentation.PackageGorillaMux)
+              instrhttptrace.RegisterRoutingHandlerType[*Router]()
             }
 
             type ddRouterConfig struct {

contrib/julienschmidt/httprouter/orchestrion.yml

@@ -16,8 +16,13 @@ aspects:
       - inject-declarations:
           imports:
             tracing: "github.com/DataDog/dd-trace-go/contrib/julienschmidt/httprouter/v2/internal/tracing"
+            instrhttptrace: "github.com/DataDog/dd-trace-go/v2/instrumentation/httptrace"
           lang: go1.18
           template: |-
+            func init() {
+              instrhttptrace.RegisterRoutingHandlerType[*Router]()
+            }
+
             type __dd_wRouter struct {
               *Router
             }

contrib/labstack/echo.v4/echotrace.go

@@ -24,6 +24,7 @@ var instr *instrumentation.Instrumentation
 
 func init() {
 	instr = instrumentation.Load(instrumentation.PackageLabstackEchoV4)
+	httptrace.RegisterRoutingHandlerType[*echo.Echo]()
 }
 
 // Wrap configures the provided [echo.Echo] and returns it. This is a

contrib/labstack/echo.v5/echotrace.go

@@ -23,6 +23,7 @@ var instr *instrumentation.Instrumentation
 
 func init() {
 	instr = instrumentation.Load(instrumentation.PackageLabstackEchoV5)
+	httptrace.RegisterRoutingHandlerType[*echo.Echo]()
 }
 
 // Wrap configures the provided [echo.Echo] and returns it. It:

contrib/net/http/internal/orchestrion/wrap-handler.go

@@ -25,6 +25,8 @@ func WrapHandler(handler http.Handler) http.Handler {
 		tracedMux.ServeMux = handler
 		return tracedMux
 	default:
+		// wrap.Handler skips routers that already start their own server span
+		// (see DD_TRACE_HTTP_ROUTER_ROOT_SPAN), so no router check is needed here.
 		if options.GetBoolEnv("DD_TRACE_HTTP_HANDLER_RESOURCE_NAME_QUANTIZE", false) {
 			return wrap.Handler(handler, "", "", config.WithResourceNamer(quantizeResourceNamer))
 		}

contrib/net/http/internal/orchestrion/wrap-handler_test.go

@@ -0,0 +1,49 @@
+// Unless explicitly stated otherwise all files in this repository are licensed
+// under the Apache License Version 2.0.
+// This product includes software developed at Datadog (https://www.datadoghq.com/).
+// Copyright 2016 Datadog, Inc.
+
+package orchestrion
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/DataDog/dd-trace-go/contrib/net/http/v2/internal/wrap"
+
+	"github.com/DataDog/dd-trace-go/v2/instrumentation/httptrace"
+)
+
+type routerHandler struct{}
+
+func (routerHandler) ServeHTTP(http.ResponseWriter, *http.Request) {}
+
+type plainHandler struct{}
+
+func (plainHandler) ServeHTTP(http.ResponseWriter, *http.Request) {}
+
+func TestWrapHandlerRegisteredRouter(t *testing.T) {
+	httptrace.RegisterRoutingHandlerType[*routerHandler]()
+
+	t.Run("enabled skips wrapping", func(t *testing.T) {
+		t.Setenv("DD_TRACE_HTTP_ROUTER_ROOT_SPAN", "true")
+		router := &routerHandler{}
+		if got := WrapHandler(router); got != http.Handler(router) {
+			t.Errorf("WrapHandler(registered router) = %T, want the handler returned unchanged", got)
+		}
+	})
+
+	t.Run("disabled wraps as usual", func(t *testing.T) {
+		t.Setenv("DD_TRACE_HTTP_ROUTER_ROOT_SPAN", "false")
+		if _, ok := WrapHandler(&routerHandler{}).(wrap.WrappedHandler); !ok {
+			t.Error("WrapHandler with flag off should still wrap a registered router")
+		}
+	})
+
+	t.Run("unregistered handler always wrapped", func(t *testing.T) {
+		t.Setenv("DD_TRACE_HTTP_ROUTER_ROOT_SPAN", "true")
+		if _, ok := WrapHandler(&plainHandler{}).(wrap.WrappedHandler); !ok {
+			t.Error("WrapHandler(unregistered handler) should return a wrapped handler")
+		}
+	})
+}

contrib/net/http/internal/wrap/handler.go

@@ -14,19 +14,31 @@ import (
 	"github.com/DataDog/dd-trace-go/v2/ddtrace/ext"
 	"github.com/DataDog/dd-trace-go/v2/ddtrace/tracer"
 	"github.com/DataDog/dd-trace-go/v2/instrumentation/httptrace"
+	"github.com/DataDog/dd-trace-go/v2/instrumentation/options"
 )
 
 // serviceSourceWrapHandler is the service source value used when the service
 // name is explicitly set via the service parameter of WrapHandler.
 const serviceSourceWrapHandler = "opt.wrap_handler"
 
+// envRouterRootSpan, when true, makes both the Orchestrion and manual net/http
+// server instrumentation skip handlers that already start their own server span
+// (traced routers registered via httptrace.RegisterRoutingHandlerType), so the
+// router span becomes the trace root instead of a redundant net/http one.
+// Opt-in; the default preserves the existing span layout.
+// See https://github.com/DataDog/dd-trace-go/issues/3369.
+const envRouterRootSpan = "DD_TRACE_HTTP_ROUTER_ROOT_SPAN"
+
 type WrappedHandler struct {
 	http.HandlerFunc
 }
 
 // Handler wraps an [http.Handler] with tracing using the given service and resource.
 // If the WithResourceNamer option is provided as part of opts, it will take precedence over the resource argument.
 func Handler(h http.Handler, service, resource string, opts ...internal.Option) http.Handler {
+	if options.GetBoolEnv(envRouterRootSpan, false) && httptrace.IsRoutingHandler(h) {
+		return h
+	}
 	instr := internal.Instrumentation
 	cfg := internal.Default(instr)
 	cfg.ApplyOpts(opts...)

contrib/net/http/router_root_test.go

@@ -0,0 +1,39 @@
+// Unless explicitly stated otherwise all files in this repository are licensed
+// under the Apache License Version 2.0.
+// This product includes software developed at Datadog (https://www.datadoghq.com/).
+// Copyright 2016 Datadog, Inc.
+
+package http
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/DataDog/dd-trace-go/v2/instrumentation/httptrace"
+)
+
+type rootSpanRouter struct{}
+
+func (rootSpanRouter) ServeHTTP(http.ResponseWriter, *http.Request) {}
+
+// TestWrapHandlerRouterRootSpan covers the manual (non-Orchestrion) path of the
+// DD_TRACE_HTTP_ROUTER_ROOT_SPAN opt-in: a registered router is returned
+// unwrapped only when the flag is enabled.
+func TestWrapHandlerRouterRootSpan(t *testing.T) {
+	httptrace.RegisterRoutingHandlerType[*rootSpanRouter]()
+	r := &rootSpanRouter{}
+
+	t.Run("enabled skips wrapping", func(t *testing.T) {
+		t.Setenv("DD_TRACE_HTTP_ROUTER_ROOT_SPAN", "true")
+		if got := WrapHandler(r, "", ""); got != http.Handler(r) {
+			t.Errorf("WrapHandler(registered router) with flag on = %T, want handler unchanged", got)
+		}
+	})
+
+	t.Run("disabled wraps", func(t *testing.T) {
+		t.Setenv("DD_TRACE_HTTP_ROUTER_ROOT_SPAN", "false")
+		if got := WrapHandler(r, "", ""); got == http.Handler(r) {
+			t.Error("WrapHandler(registered router) with flag off should wrap the handler")
+		}
+	})
+}

instrumentation/httptrace/registry.go

@@ -0,0 +1,52 @@
+// Unless explicitly stated otherwise all files in this repository are licensed
+// under the Apache License Version 2.0.
+// This product includes software developed at Datadog (https://www.datadoghq.com/).
+// Copyright 2016 Datadog, Inc.
+
+package httptrace
+
+import (
+	"net/http"
+	"reflect"
+	"sync"
+)
+
+var (
+	routingHandlerTypesMu sync.RWMutex
+	routingHandlerTypes   = make(map[reflect.Type]struct{})
+)
+
+// RegisterRoutingHandlerType records that values of type T are HTTP handlers
+// that already start their own server span (typically a traced router). It is
+// meant to be called from a router integration's init function.
+//
+// When DD_TRACE_HTTP_ROUTER_ROOT_SPAN is enabled, the net/http server
+// instrumentation (both Orchestrion and manual WrapHandler) uses this registry
+// to avoid wrapping such handlers a second time, which would otherwise produce
+// a redundant server span above the router's own span. See
+// https://github.com/DataDog/dd-trace-go/issues/3369.
+//
+// Matching is by exact dynamic type: register the concrete type that is
+// assigned to http.Server.Handler (e.g. *chi.Mux). A router wrapped by other
+// middleware before reaching the server (h2c, http.TimeoutHandler, ...) will
+// not match, and the redundant span is kept.
+func RegisterRoutingHandlerType[T http.Handler]() {
+	t := reflect.TypeFor[T]()
+	routingHandlerTypesMu.Lock()
+	routingHandlerTypes[t] = struct{}{}
+	routingHandlerTypesMu.Unlock()
+}
+
+// IsRoutingHandler reports whether h's dynamic type was registered via
+// RegisterRoutingHandlerType. A typed-nil handler (e.g. (*chi.Mux)(nil)) still
+// matches on its type; only an untyped-nil handler returns false.
+func IsRoutingHandler(h http.Handler) bool {
+	if h == nil {
+		return false
+	}
+	t := reflect.TypeOf(h)
+	routingHandlerTypesMu.RLock()
+	_, ok := routingHandlerTypes[t]
+	routingHandlerTypesMu.RUnlock()
+	return ok
+}

instrumentation/httptrace/registry_test.go

@@ -0,0 +1,33 @@
+// Unless explicitly stated otherwise all files in this repository are licensed
+// under the Apache License Version 2.0.
+// This product includes software developed at Datadog (https://www.datadoghq.com/).
+// Copyright 2016 Datadog, Inc.
+
+package httptrace
+
+import (
+	"net/http"
+	"testing"
+)
+
+type fakeRoutingHandler struct{}
+
+func (fakeRoutingHandler) ServeHTTP(http.ResponseWriter, *http.Request) {}
+
+func TestRoutingHandlerRegistry(t *testing.T) {
+	RegisterRoutingHandlerType[*fakeRoutingHandler]()
+
+	if !IsRoutingHandler(&fakeRoutingHandler{}) {
+		t.Error("IsRoutingHandler(*fakeRoutingHandler) = false, want true")
+	}
+	if IsRoutingHandler(http.NewServeMux()) {
+		t.Error("IsRoutingHandler(*http.ServeMux) = true, want false")
+	}
+	if IsRoutingHandler(nil) {
+		t.Error("IsRoutingHandler(nil) = true, want false")
+	}
+	// A value receiver is a distinct type from its pointer; it must not match.
+	if IsRoutingHandler(fakeRoutingHandler{}) {
+		t.Error("IsRoutingHandler(fakeRoutingHandler{}) = true, want false")
+	}
+}

internal/env/supported_configurations.json

@@ -1332,6 +1332,13 @@
         "default": "false"
       }
     ],
+    "DD_TRACE_HTTP_ROUTER_ROOT_SPAN": [
+      {
+        "implementation": "A",
+        "type": "boolean",
+        "default": "false"
+      }
+    ],
     "DD_TRACE_HTTP_SERVER_ERROR_STATUSES": [
       {
         "implementation": "B",

internal/orchestrion/_integration/chi.v5/router_root.go

@@ -0,0 +1,58 @@
+// Unless explicitly stated otherwise all files in this repository are licensed
+// under the Apache License Version 2.0.
+// This product includes software developed at Datadog (https://www.datadoghq.com/).
+// Copyright 2023-present Datadog, Inc.
+
+package chiv5
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/DataDog/dd-trace-go/v2/internal/orchestrion/_integration/internal/trace"
+)
+
+// TestCaseRouterRoot asserts the router span is the trace root when
+// DD_TRACE_HTTP_ROUTER_ROOT_SPAN is enabled (see issue #3369).
+type TestCaseRouterRoot struct {
+	TestCase
+}
+
+func (*TestCaseRouterRoot) PreBootstrap(_ context.Context, t *testing.T) {
+	t.Setenv("DD_TRACE_HTTP_ROUTER_ROOT_SPAN", "true")
+}
+
+func (tc *TestCaseRouterRoot) ExpectedTraces() trace.Traces {
+	return trace.Traces{
+		{
+			Tags: map[string]any{
+				"name":     "http.request",
+				"resource": "GET /",
+				"service":  "chi.v5.test",
+				"type":     "http",
+			},
+			Meta: map[string]string{
+				"http.url":  fmt.Sprintf("http://%s/", tc.Server.Addr),
+				"component": "net/http",
+				"span.kind": "client",
+			},
+			Children: trace.Traces{
+				{
+					Tags: map[string]any{
+						"name":     "http.request",
+						"resource": "GET /",
+						"service":  "chi.router",
+						"type":     "web",
+					},
+					Meta: map[string]string{
+						"http.url":  fmt.Sprintf("http://%s/", tc.Server.Addr),
+						"component": "go-chi/chi.v5",
+						"span.kind": "server",
+					},
+					Children: nil,
+				},
+			},
+		},
+	}
+}