Instrumentation: Define handlers for requests that are not handled with named handlers (#50613)

Signed-off-by: bergquist <carl.bergquist@gmail.com>

Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com>
Co-authored-by: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com>

pkg/middleware/request_metrics.go

@@ -50,11 +50,6 @@ func RequestMetrics(features featuremgmt.FeatureToggles) web.Handler {
 	log := log.New("middleware.request-metrics")
 
 	return func(res http.ResponseWriter, req *http.Request, c *web.Context) {
-		if strings.HasPrefix(c.Req.URL.Path, "/public/") || c.Req.URL.Path == "robots.txt" || c.Req.URL.Path == "/metrics" {
-			c.Next()
-			return
-		}
-
 		rw := res.(web.ResponseWriter)
 		now := time.Now()
 		httpRequestsInFlight.Inc()
@@ -65,11 +60,17 @@ func RequestMetrics(features featuremgmt.FeatureToggles) web.Handler {
 		code := sanitizeCode(status)
 
 		handler := "unknown"
-		if routeOperation, exists := RouteOperationNameFromContext(c.Req.Context()); exists {
+		if routeOperation, exists := routeOperationName(c.Req); exists {
 			handler = routeOperation
 		} else {
-			if features.IsEnabled(featuremgmt.FlagLogRequestsInstrumentedAsUnknown) {
+			// if grafana does not recognize the handler and returns 404 we should register it as `notfound`
-				log.Warn("request instrumented as unknown", "path", c.Req.URL.Path, "status_code", status)
+			if status == http.StatusNotFound {
+				handler = "notfound"
+			} else {
+				// log requests where we could not identify handler so we can register them.
+				if features.IsEnabled(featuremgmt.FlagLogRequestsInstrumentedAsUnknown) {
+					log.Warn("request instrumented as unknown", "path", c.Req.URL.Path, "status_code", status)
+				}
 			}
 		}
 

pkg/middleware/request_test.go

@@ -0,0 +1,40 @@
+package middleware
+
+import (
+	"context"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCanGetRouteNameFromContext(t *testing.T) {
+	tcs := []struct {
+		namedHandler string
+		path         string
+
+		expected string
+	}{
+		{namedHandler: "", path: "/public/img/apple-touch-icon.png", expected: "public-assets"},
+		{namedHandler: "", path: "/favicon.ico", expected: "public-assets"},
+		{namedHandler: "", path: "/robots.txt", expected: "/robots.txt"},
+		{namedHandler: "", path: "/debug/pprof/heap", expected: "/debug/pprof-handlers"},
+		{namedHandler: "", path: "/debug/pprof/allocs", expected: "/debug/pprof-handlers"},
+		{namedHandler: "", path: "/debug/pprof/threadcreate", expected: "/debug/pprof-handlers"},
+		{namedHandler: "/api/dashboard/:uid", path: "/api/dashboard/ddfgeasdfr", expected: "/api/dashboard/:uid"},
+		{namedHandler: "", path: "/metrics", expected: "/metrics"},
+	}
+
+	for _, tc := range tcs {
+		req, err := http.NewRequest(http.MethodPost, "https://ops.grafana.net"+tc.path, nil)
+
+		if tc.namedHandler != "" {
+			req = req.WithContext(context.WithValue(context.Background(), routeOperationNameKey, tc.namedHandler))
+		}
+
+		assert.NoError(t, err)
+
+		handler, _ := routeOperationName(req)
+		assert.Equal(t, tc.expected, handler)
+	}
+}

pkg/middleware/request_tracing.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"regexp"
 	"strconv"
 	"strings"
 
@@ -31,20 +32,40 @@ func ProvideRouteOperationName(name string) web.Handler {
 	}
 }
 
-// RouteOperationNameFromContext receives the route operation name from context, if set.
+var unnamedHandlers = []struct {
-func RouteOperationNameFromContext(ctx context.Context) (string, bool) {
+	pathPattern *regexp.Regexp
-	if val := ctx.Value(routeOperationNameKey); val != nil {
+	handler     string
+}{
+	{handler: "public-assets", pathPattern: regexp.MustCompile("^/favicon.ico")},
+	{handler: "public-assets", pathPattern: regexp.MustCompile("^/public/")},
+	{handler: "/metrics", pathPattern: regexp.MustCompile("^/metrics")},
+	{handler: "/healthz", pathPattern: regexp.MustCompile("^/healthz")},
+	{handler: "/robots.txt", pathPattern: regexp.MustCompile("^/robots.txt$")},
+	// bundle all pprof endpoints under the same handler name
+	{handler: "/debug/pprof-handlers", pathPattern: regexp.MustCompile("^/debug/pprof")},
+}
+
+// routeOperationName receives the route operation name from context, if set.
+func routeOperationName(req *http.Request) (string, bool) {
+	if val := req.Context().Value(routeOperationNameKey); val != nil {
 		op, ok := val.(string)
 		return op, ok
 	}
 
+	for _, hp := range unnamedHandlers {
+		if hp.pathPattern.Match([]byte(req.URL.Path)) {
+			return hp.handler, true
+		}
+	}
+
 	return "", false
 }
 
 func RequestTracing(tracer tracing.Tracer) web.Handler {
 	return func(res http.ResponseWriter, req *http.Request, c *web.Context) {
 		if strings.HasPrefix(c.Req.URL.Path, "/public/") ||
-			c.Req.URL.Path == "robots.txt" {
+			c.Req.URL.Path == "/robots.txt" ||
+			c.Req.URL.Path == "/favicon.ico" {
 			c.Next()
 			return
 		}
@@ -59,7 +80,7 @@ func RequestTracing(tracer tracing.Tracer) web.Handler {
 
 		// Only call span.Finish when a route operation name have been set,
 		// meaning that not set the span would not be reported.
-		if routeOperation, exists := RouteOperationNameFromContext(c.Req.Context()); exists {
+		if routeOperation, exists := routeOperationName(c.Req); exists {
 			defer span.End()
 			span.SetName(fmt.Sprintf("HTTP %s %s", req.Method, routeOperation))
 		}