Repository: open-telemetry/opentelemetry-go-contrib (go)

Issue:
otelmux: Add config option for a custom span name formatter
Currently, spans created in the [`otelmux`](https://pkg.go.dev/go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux) instrumentation module are named after the route template  (or the route regexp if no template is available). For example, given a route of `/users/{id}`, spans will be named `/users/{id}`.

This is fine in general, but it's impossible to differentiate between handlers for different methods. For example, if there are `GET /users/{id}` and `DELETE /users/{id}` handlers, the only way to differentiate spans is to visually inspect the `http.method` attribute.

In the [`otelhttp`](https://pkg.go.dev/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp) module, a [`WithSpanNameFormatter`](https://pkg.go.dev/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp#WithSpanNameFormatter) option function is available, which allows users to generate custom names on a per-request basis.

I propose adding a similar `WithSpanNameFormatter` option function to the `otelmux` module, with the difference that where `otelhttp`'s function provides the handler's `operation` name, the `otelmux` version would provide the route name (to avoid needing to re-compute it on every request).

I have a PR already in progress that I will link to this issue.

Pull request:
otelmux: Add new WithSpanNameFormatter option
Fixes #3040

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2d6f0c3dd..226875eba 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ### Added
 
 - Add the new `go.opentelemetry.io/contrib/instrgen` package to provide auto-generated source code instrumentation. (#3068)
+- `otelmux`: Add new `WithSpanNameFormatter` option to `go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux` to allow customizing span names. (#3041)
 
 ## [1.12.0/0.37.0/0.6.0]
 
diff --git a/instrumentation/github.com/gorilla/mux/otelmux/config.go b/instrumentation/github.com/gorilla/mux/otelmux/config.go
index 2e4d60b67..f0376123c 100644
--- a/instrumentation/github.com/gorilla/mux/otelmux/config.go
+++ b/instrumentation/github.com/gorilla/mux/otelmux/config.go
@@ -15,14 +15,17 @@
 package otelmux // import "go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux"
 
 import (
+	"net/http"
+
 	"go.opentelemetry.io/otel/propagation"
 	oteltrace "go.opentelemetry.io/otel/trace"
 )
 
 // config is used to configure the mux middleware.
 type config struct {
-	TracerProvider oteltrace.TracerProvider
-	Propagators    propagation.TextMapPropagator
+	TracerProvider    oteltrace.TracerProvider
+	Propagators       propagation.TextMapPropagator
+	spanNameFormatter func(string, *http.Request) string
 }
 
 // Option specifies instrumentation configuration options.
@@ -56,3 +59,13 @@ func WithTracerProvider(provider oteltrace.TracerProvider) Option {
 		}
 	})
 }
+
+// WithSpanNameFormatter specifies a function to use for generating a custom span
+// name. By default, the route name (path template or regexp) is used. The route
+// name is provided so you can use it in the span name without needing to
+// duplicate the logic for extracting it from the request.
+func WithSpanNameFormatter(fn func(routeName string, r *http.Request) string) Option {
+	return optionFunc(func(cfg *config) {
+		cfg.spanNameFormatter = fn
+	})
+}
diff --git a/instrumentation/github.com/gorilla/mux/otelmux/go.mod b/instrumentation/github.com/gorilla/mux/otelmux/go.mod
index af215e8b7..099f22836 100644
--- a/instrumentation/github.com/gorilla/mux/otelmux/go.mod
+++ b/instrumentation/github.com/gorilla/mux/otelmux/go.mod
@@ -7,6 +7,7 @@ require (
 	github.com/gorilla/mux v1.8.0
 	github.com/stretchr/testify v1.8.1
 	go.opentelemetry.io/otel v1.11.2
+	go.opentelemetry.io/otel/sdk v1.11.2
 	go.opentelemetry.io/otel/trace v1.11.2
 )
 
@@ -15,5 +16,6 @@ require (
 	github.com/go-logr/logr v1.2.3 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
+	golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
diff --git a/instrumentation/github.com/gorilla/mux/otelmux/go.sum b/instrumentation/github.com/gorilla/mux/otelmux/go.sum
index 3238d8dd0..a5d16bfa3 100644
--- a/instrumentation/github.com/gorilla/mux/otelmux/go.sum
+++ b/instrumentation/github.com/gorilla/mux/otelmux/go.sum
@@ -22,8 +22,12 @@ github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKs
 github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
 go.opentelemetry.io/otel v1.11.2 h1:YBZcQlsVekzFsFbjygXMOXSs6pialIZxcjfO/mBDmR0=
 go.opentelemetry.io/otel v1.11.2/go.mod h1:7p4EUV+AqgdlNV9gL97IgUZiVR3yrFXYo53f9BM3tRI=
+go.opentelemetry.io/otel/sdk v1.11.2 h1:GF4JoaEx7iihdMFu30sOyRx52HDHOkl9xQ8SMqNXUiU=
+go.opentelemetry.io/otel/sdk v1.11.2/go.mod h1:wZ1WxImwpq+lVRo4vsmSOxdd+xwoUJ6rqyLc3SyX9aU=
 go.opentelemetry.io/otel/trace v1.11.2 h1:Xf7hWSF2Glv0DE3MH7fBHvtpSBsjcBUe5MYAmZM/+y0=
 go.opentelemetry.io/otel/trace v1.11.2/go.mod h1:4N+yC7QEz7TTsG9BSRLNAa63eg5E06ObSbKPmxQ/pKA=
+golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8 h1:h+EGohizhe9XlX18rfpa8k8RAc5XyaeamM+0VHRd4lc=
+golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
diff --git a/instrumentation/github.com/gorilla/mux/otelmux/mux.go b/instrumentation/github.com/gorilla/mux/otelmux/mux.go
index acd9f0e11..03fbd02f2 100644
--- a/instrumentation/github.com/gorilla/mux/otelmux/mux.go
+++ b/instrumentation/github.com/gorilla/mux/otelmux/mux.go
@@ -51,21 +51,26 @@ func Middleware(service string, opts ...Option) mux.MiddlewareFunc {
 	if cfg.Propagators == nil {
 		cfg.Propagators = otel.GetTextMapPropagator()
 	}
+	if cfg.spanNameFormatter == nil {
+		cfg.spanNameFormatter = defaultSpanNameFunc
+	}
 	return func(handler http.Handler) http.Handler {
 		return traceware{
-			service:     service,
-			tracer:      tracer,
-			propagators: cfg.Propagators,
-			handler:     handler,
+			service:           service,
+			tracer:            tracer,
+			propagators:       cfg.Propagators,
+			handler:           handler,
+			spanNameFormatter: cfg.spanNameFormatter,
 		}
 	}
 }
 
 type traceware struct {
-	service     string
-	tracer      oteltrace.Tracer
-	propagators propagation.TextMapPropagator
-	handler     http.Handler
+	service           string
+	tracer            oteltrace.Tracer
+	propagators       propagation.TextMapPropagator
+	handler           http.Handler
+	spanNameFormatter func(string, *http.Request) string
 }
 
 type recordingResponseWriter struct {
@@ -111,25 +116,27 @@ func putRRW(rrw *recordingResponseWriter) {
 	rrwPool.Put(rrw)
 }
 
+// defaultSpanNameFunc just reuses the route name as the span name.
+func defaultSpanNameFunc(routeName string, _ *http.Request) string { return routeName }
+
 // ServeHTTP implements the http.Handler interface. It does the actual
 // tracing of the request.
 func (tw traceware) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	ctx := tw.propagators.Extract(r.Context(), propagation.HeaderCarrier(r.Header))
-	spanName := ""
+	routeStr := ""
 	route := mux.CurrentRoute(r)
 	if route != nil {
 		var err error
-		spanName, err = route.GetPathTemplate()
+		routeStr, err = route.GetPathTemplate()
 		if err != nil {
-			spanName, err = route.GetPathRegexp()
+			routeStr, err = route.GetPathRegexp()
 			if err != nil {
-				spanName = ""
+				routeStr = ""
 			}
 		}
 	}
-	routeStr := spanName
-	if spanName == "" {
-		spanName = fmt.Sprintf("HTTP %s route not found", r.Method)
+	if routeStr == "" {
+		routeStr = fmt.Sprintf("HTTP %s route not found", r.Method)
 	}
 	opts := []oteltrace.SpanStartOption{
 		oteltrace.WithAttributes(semconv.NetAttributesFromHTTPRequest("tcp", r)...),
@@ -137,6 +144,7 @@ func (tw traceware) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		oteltrace.WithAttributes(semconv.HTTPServerAttributesFromHTTPRequest(tw.service, routeStr, r)...),
 		oteltrace.WithSpanKind(oteltrace.SpanKindServer),
 	}
+	spanName := tw.spanNameFormatter(routeStr, r)
 	ctx, span := tw.tracer.Start(ctx, spanName, opts...)
 	defer span.End()
 	r2 := r.WithContext(ctx)


Test patch:
diff --git a/instrumentation/github.com/gorilla/mux/otelmux/mux_test.go b/instrumentation/github.com/gorilla/mux/otelmux/mux_test.go
index b9e9a3eb4..308b6880a 100644
--- a/instrumentation/github.com/gorilla/mux/otelmux/mux_test.go
+++ b/instrumentation/github.com/gorilla/mux/otelmux/mux_test.go
@@ -17,6 +17,7 @@ package otelmux
 import (
 	"bufio"
 	"context"
+	"fmt"
 	"io"
 	"net"
 	"net/http"
@@ -25,9 +26,12 @@ import (
 
 	"github.com/gorilla/mux"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/propagation"
+	sdktrace "go.opentelemetry.io/otel/sdk/trace"
+	"go.opentelemetry.io/otel/sdk/trace/tracetest"
 	"go.opentelemetry.io/otel/trace"
 )
 
@@ -162,3 +166,47 @@ func TestResponseWriterInterfaces(t *testing.T) {
 
 	router.ServeHTTP(w, r)
 }
+
+func TestCustomSpanNameFormatter(t *testing.T) {
+	exporter := tracetest.NewInMemoryExporter()
+
+	tp := sdktrace.NewTracerProvider(sdktrace.WithSyncer(exporter))
+
+	routeTpl := "/user/{id}"
+
+	testdata := []struct {
+		spanNameFormatter func(string, *http.Request) string
+		expected          string
+	}{
+		{nil, routeTpl},
+		{
+			func(string, *http.Request) string { return "custom" },
+			"custom",
+		},
+		{
+			func(name string, r *http.Request) string {
+				return fmt.Sprintf("%s %s", r.Method, name)
+			},
+			"GET " + routeTpl,
+		},
+	}
+
+	for i, d := range testdata {
+		t.Run(fmt.Sprintf("%d_%s", i, d.expected), func(t *testing.T) {
+			router := mux.NewRouter()
+			router.Use(Middleware("foobar", WithTracerProvider(tp), WithSpanNameFormatter(d.spanNameFormatter)))
+			router.HandleFunc(routeTpl, func(w http.ResponseWriter, r *http.Request) {})
+
+			r := httptest.NewRequest("GET", "/user/123", nil)
+			w := httptest.NewRecorder()
+
+			router.ServeHTTP(w, r)
+
+			spans := exporter.GetSpans()
+			require.Len(t, spans, 1)
+			assert.Equal(t, d.expected, spans[0].Name)
+
+			exporter.Reset()
+		})
+	}
+}