Extract route name logic to a single middleware and use it everywhere

CHANGELOG.md

@@ -209,6 +209,7 @@
   * `operation_duration_seconds`
 * [ENHANCEMENT] Add `outcome` label to `gate_duration_seconds` metric. Possible values are `rejected_canceled`, `rejected_deadline_exceeded`, `rejected_other`, and `permitted`. #512
 * [ENHANCEMENT] Expose `InstancesWithTokensCount` and `InstancesWithTokensInZoneCount` in `ring.ReadRing` interface. #516
+* [ENHANCEMENT] Middleware: determine route name in a single place, and add `middleware.ExtractRouteName()` method to allow consuming applications to retrieve the route name. #527
 * [BUGFIX] spanlogger: Support multiple tenant IDs. #59
 * [BUGFIX] Memberlist: fixed corrupted packets when sending compound messages with more than 255 messages or messages bigger than 64KB. #85
 * [BUGFIX] Ring: `ring_member_ownership_percent` and `ring_tokens_owned` metrics are not updated on scale down. #109

middleware/grpc_stats.go

@@ -31,6 +31,7 @@ type contextKey int
 
 const (
 	contextKeyMethodName contextKey = 1
+	contextKeyRouteName  contextKey = 2
 )
 
 func (g *grpcStatsHandler) TagRPC(ctx context.Context, info *stats.RPCTagInfo) context.Context {

middleware/http_tracing.go

@@ -24,14 +24,13 @@ var _ = nethttp.MWURLTagFunc
 
 // Tracer is a middleware which traces incoming requests.
 type Tracer struct {
-	RouteMatcher RouteMatcher
-	SourceIPs    *SourceIPExtractor
+	SourceIPs *SourceIPExtractor
 }
 
 // Wrap implements Interface
 func (t Tracer) Wrap(next http.Handler) http.Handler {
 	options := []nethttp.MWOption{
-		nethttp.OperationNameFunc(makeHTTPOperationNameFunc(t.RouteMatcher)),
+		nethttp.OperationNameFunc(httpOperationNameFunc),
 		nethttp.MWSpanObserver(func(sp opentracing.Span, r *http.Request) {
 			// add a tag with the client's user agent to the span
 			userAgent := r.Header.Get("User-Agent")
@@ -130,11 +129,9 @@ func HTTPGRPCTracingInterceptor(router *mux.Router) grpc.UnaryServerInterceptor
 	}
 }
 
-func makeHTTPOperationNameFunc(routeMatcher RouteMatcher) func(r *http.Request) string {
-	return func(r *http.Request) string {
-		routeName := getRouteName(routeMatcher, r)
-		return getOperationName(routeName, r)
-	}
+func httpOperationNameFunc(r *http.Request) string {
+	routeName := ExtractRouteName(r.Context())
+	return getOperationName(routeName, r)
 }
 
 func getOperationName(routeName string, r *http.Request) string {

middleware/instrument.go

@@ -8,7 +8,6 @@ import (
 	"context"
 	"io"
 	"net/http"
-	"regexp"
 	"strconv"
 	"strings"
 
@@ -45,7 +44,6 @@ func (f PerTenantCallback) shouldInstrument(ctx context.Context) (string, bool)
 
 // Instrument is a Middleware which records timings for every HTTP request
 type Instrument struct {
-	RouteMatcher      RouteMatcher
 	Duration          *prometheus.HistogramVec
 	PerTenantDuration *prometheus.HistogramVec
 	PerTenantCallback PerTenantCallback
@@ -120,61 +118,14 @@ func (i Instrument) Wrap(next http.Handler) http.Handler {
 // We do all this as we do not wish to emit high cardinality labels to
 // prometheus.
 func (i Instrument) getRouteName(r *http.Request) string {
-	route := getRouteName(i.RouteMatcher, r)
+	route := ExtractRouteName(r.Context())
 	if route == "" {
 		route = "other"
 	}
 
 	return route
 }
 
-func getRouteName(routeMatcher RouteMatcher, r *http.Request) string {
-	var routeMatch mux.RouteMatch
-	if routeMatcher == nil || !routeMatcher.Match(r, &routeMatch) {
-		return ""
-	}
-
-	if routeMatch.MatchErr == mux.ErrNotFound {
-		return "notfound"
-	}
-
-	if routeMatch.Route == nil {
-		return ""
-	}
-
-	if name := routeMatch.Route.GetName(); name != "" {
-		return name
-	}
-
-	tmpl, err := routeMatch.Route.GetPathTemplate()
-	if err == nil {
-		return MakeLabelValue(tmpl)
-	}
-
-	return ""
-}
-
-var invalidChars = regexp.MustCompile(`[^a-zA-Z0-9]+`)
-
-// MakeLabelValue converts a Gorilla mux path to a string suitable for use in
-// a Prometheus label value.
-func MakeLabelValue(path string) string {
-	// Convert non-alnums to underscores.
-	result := invalidChars.ReplaceAllString(path, "_")
-
-	// Trim leading and trailing underscores.
-	result = strings.Trim(result, "_")
-
-	// Make it all lowercase
-	result = strings.ToLower(result)
-
-	// Special case.
-	if result == "" {
-		result = "root"
-	}
-	return result
-}
-
 type reqBody struct {
 	b    io.ReadCloser
 	read int64

middleware/route_injector.go

@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: AGPL-3.0-only
+
+package middleware
+
+import (
+	"context"
+	"net/http"
+	"regexp"
+	"strings"
+
+	"github.com/gorilla/mux"
+)
+
+// RouteInjector is a middleware that injects the route name for the current request into the request context.
+//
+// The route name can be retrieved by calling ExtractRouteName.
+type RouteInjector struct {
+	RouteMatcher RouteMatcher
+}
+
+func (i RouteInjector) Wrap(handler http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		routeName := getRouteName(i.RouteMatcher, r)
+		handler.ServeHTTP(w, WithRouteName(r, routeName))
+	})
+}
+
+// WithRouteName annotates r's context with the provided route name.
+//
+// The provided value must be suitable to use as a Prometheus label value.
+//
+// This method should generally only be used in tests: in production code, use RouteInjector instead.
+func WithRouteName(r *http.Request, routeName string) *http.Request {
+	ctx := context.WithValue(r.Context(), contextKeyRouteName, routeName)
+	return r.WithContext(ctx)
+}
+
+// ExtractRouteName returns the route name associated with this request that was previously injected by the
+// RouteInjector middleware or WithRouteName.
+//
+// This is the same route name used for trace and metric names, and is already suitable for use as a Prometheus label
+// value.
+func ExtractRouteName(ctx context.Context) string {
+	routeName, ok := ctx.Value(contextKeyRouteName).(string)
+	if !ok {
+		return ""
+	}
+
+	return routeName
+}
+
+func getRouteName(routeMatcher RouteMatcher, r *http.Request) string {
+	var routeMatch mux.RouteMatch
+	if routeMatcher == nil || !routeMatcher.Match(r, &routeMatch) {
+		return ""
+	}
+
+	if routeMatch.MatchErr == mux.ErrNotFound {
+		return "notfound"
+	}
+
+	if routeMatch.Route == nil {
+		return ""
+	}
+
+	if name := routeMatch.Route.GetName(); name != "" {
+		return name
+	}
+
+	tmpl, err := routeMatch.Route.GetPathTemplate()
+	if err == nil {
+		return MakeLabelValue(tmpl)
+	}
+
+	return ""
+}
+
+var invalidChars = regexp.MustCompile(`[^a-zA-Z0-9]+`)
+
+// MakeLabelValue converts a Gorilla mux path to a string suitable for use in
+// a Prometheus label value.
+func MakeLabelValue(path string) string {
+	// Convert non-alnums to underscores.
+	result := invalidChars.ReplaceAllString(path, "_")
+
+	// Trim leading and trailing underscores.
+	result = strings.Trim(result, "_")
+
+	// Make it all lowercase
+	result = strings.ToLower(result)
+
+	// Special case.
+	if result == "" {
+		result = "root"
+	}
+	return result
+}

middleware/route_injector_test.go

@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: AGPL-3.0-only
+// Provenance-includes-location: https://github.com/weaveworks/common/blob/main/middleware/instrument_test.go
+// Provenance-includes-license: Apache-2.0
+// Provenance-includes-copyright: Weaveworks Ltd.
+
+package middleware
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gorilla/mux"
+	"github.com/stretchr/testify/require"
+)
+
+func TestRouteInjector(t *testing.T) {
+	testCases := map[string]string{
+		"/":                       "root",
+		"/foo/bar/blah":           "foo_bar_blah",
+		"/templated/name-1/thing": "templated_name_thing",
+		"/named":                  "my-named-route",
+		"/does-not-exist":         "notfound",
+	}
+
+	for url, expectedRouteName := range testCases {
+		t.Run(url, func(t *testing.T) {
+			actualRouteName := ""
+
+			handler := func(_ http.ResponseWriter, r *http.Request) {
+				actualRouteName = ExtractRouteName(r.Context())
+			}
+
+			router := mux.NewRouter()
+			router.HandleFunc("/", handler)
+			router.HandleFunc("/foo/bar/blah", handler)
+			router.HandleFunc("/templated/{name}/thing", handler)
+			router.HandleFunc("/named", handler).Name("my-named-route")
+			router.NotFoundHandler = http.HandlerFunc(handler)
+
+			endpoint := RouteInjector{router}.Wrap(router)
+			endpoint.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, url, nil))
+
+			require.Equal(t, expectedRouteName, actualRouteName)
+		})
+	}
+
+}
+
+func TestMakeLabelValue(t *testing.T) {
+	for input, want := range map[string]string{
+		"/":                      "root", // special case
+		"//":                     "root", // unintended consequence of special case
+		"a":                      "a",
+		"/foo":                   "foo",
+		"foo/":                   "foo",
+		"/foo/":                  "foo",
+		"/foo/bar":               "foo_bar",
+		"foo/bar/":               "foo_bar",
+		"/foo/bar/":              "foo_bar",
+		"/foo/{orgName}/Bar":     "foo_orgname_bar",
+		"/foo/{org_name}/Bar":    "foo_org_name_bar",
+		"/foo/{org__name}/Bar":   "foo_org_name_bar",
+		"/foo/{org___name}/_Bar": "foo_org_name_bar",
+		"/foo.bar/baz.qux/":      "foo_bar_baz_qux",
+	} {
+		if have := MakeLabelValue(input); want != have {
+			t.Errorf("%q: want %q, have %q", input, want, have)
+		}
+	}
+}

server/server.go

@@ -517,13 +517,14 @@ func BuildHTTPMiddleware(cfg Config, router *mux.Router, metrics *Metrics, logge
 	defaultLogMiddleware.DisableRequestSuccessLog = cfg.DisableRequestSuccessLog
 
 	defaultHTTPMiddleware := []middleware.Interface{
-		middleware.Tracer{
+		middleware.RouteInjector{
 			RouteMatcher: router,
-			SourceIPs:    sourceIPs,
+		},
+		middleware.Tracer{
+			SourceIPs: sourceIPs,
 		},
 		defaultLogMiddleware,
 		middleware.Instrument{
-			RouteMatcher:      router,
 			Duration:          metrics.RequestDuration,
 			PerTenantDuration: metrics.PerTenantRequestDuration,
 			PerTenantCallback: cfg.PerTenantDurationInstrumentation,