From 9e3e01bce3c888b0e6d759f23800379dd098b170 Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Tue, 28 May 2024 14:27:56 +1000 Subject: [PATCH] Upgrade dskit (#8190) * Upgrade dskit * Fix breaking change --- go.mod | 2 +- go.sum | 4 +- pkg/api/handlers.go | 3 +- .../grafana/dskit/middleware/grpc_stats.go | 1 + .../grafana/dskit/middleware/http_tracing.go | 13 +-- .../grafana/dskit/middleware/instrument.go | 51 +--------- .../dskit/middleware/route_injector.go | 97 +++++++++++++++++++ .../github.com/grafana/dskit/server/server.go | 7 +- vendor/modules.txt | 2 +- 9 files changed, 114 insertions(+), 66 deletions(-) create mode 100644 vendor/github.com/grafana/dskit/middleware/route_injector.go diff --git a/go.mod b/go.mod index 36ab3eb2d73..c6c5b776310 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( github.com/golang/snappy v0.0.4 github.com/google/gopacket v1.1.19 github.com/gorilla/mux v1.8.1 - github.com/grafana/dskit v0.0.0-20240509115328-a1bba1277f06 + github.com/grafana/dskit v0.0.0-20240528015923-27d7d41066d3 github.com/grafana/e2e v0.1.2-0.20240118170847-db90b84177fc github.com/hashicorp/golang-lru v1.0.2 // indirect github.com/json-iterator/go v1.1.12 diff --git a/go.sum b/go.sum index 137f5ed40df..dae1b4714d0 100644 --- a/go.sum +++ b/go.sum @@ -507,8 +507,8 @@ github.com/grafana-tools/sdk v0.0.0-20220919052116-6562121319fc h1:PXZQA2WCxe85T github.com/grafana-tools/sdk v0.0.0-20220919052116-6562121319fc/go.mod h1:AHHlOEv1+GGQ3ktHMlhuTUwo3zljV3QJbC0+8o2kn+4= github.com/grafana/alerting v0.0.0-20240516100902-0cf0ef264288 h1:cnaJxI2rhiJcyDxebP2zKGNlzK4KVpU1uznanv0VJlg= github.com/grafana/alerting v0.0.0-20240516100902-0cf0ef264288/go.mod h1:8nOsn7PWmttOmWiR7bvYIl3VLl+tIq72ZF+1y54w36M= -github.com/grafana/dskit v0.0.0-20240509115328-a1bba1277f06 h1:/QUlscuctksoAF335nhWJtNtDO2KB+p7I6C2GmI76lM= -github.com/grafana/dskit v0.0.0-20240509115328-a1bba1277f06/go.mod h1:HvSf3uf8Ps2vPpzHeAFyZTdUcbVr+Rxpq1xcx7J/muc= +github.com/grafana/dskit v0.0.0-20240528015923-27d7d41066d3 h1:k8vINlI4w+RYc37NRwQlRe/IHYoEbu6KAe2XdGDeV1U= +github.com/grafana/dskit v0.0.0-20240528015923-27d7d41066d3/go.mod h1:HvSf3uf8Ps2vPpzHeAFyZTdUcbVr+Rxpq1xcx7J/muc= github.com/grafana/e2e v0.1.2-0.20240118170847-db90b84177fc h1:BW+LjKJDz0So5LI8UZfW5neWeKpSkWqhmGjQFzcFfLM= github.com/grafana/e2e v0.1.2-0.20240118170847-db90b84177fc/go.mod h1:JVmqPBe8A/pZWwRoJW5ZjyALeY5OXMzPl7LrVXOdZAI= github.com/grafana/goautoneg v0.0.0-20231010094147-47ce5e72a9ae h1:Yxbw9jKGJVC6qAK5Ubzzb/qZwM6rRMMqaDc/d4Vp3pM= diff --git a/pkg/api/handlers.go b/pkg/api/handlers.go index e359c150b89..4c7e1e81e94 100644 --- a/pkg/api/handlers.go +++ b/pkg/api/handlers.go @@ -284,11 +284,12 @@ func NewQuerierHandler( api.InstallCodec(protobufCodec{}) router := mux.NewRouter() + routeInjector := middleware.RouteInjector{RouteMatcher: router} + router.Use(routeInjector.Wrap) // Use a separate metric for the querier in order to differentiate requests from the query-frontend when // running Mimir in monolithic mode. instrumentMiddleware := middleware.Instrument{ - RouteMatcher: router, Duration: querierRequestDuration, RequestBodySize: receivedMessageSize, ResponseBodySize: sentMessageSize, diff --git a/vendor/github.com/grafana/dskit/middleware/grpc_stats.go b/vendor/github.com/grafana/dskit/middleware/grpc_stats.go index 3d29d9baabb..ec766d640ac 100644 --- a/vendor/github.com/grafana/dskit/middleware/grpc_stats.go +++ b/vendor/github.com/grafana/dskit/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 { diff --git a/vendor/github.com/grafana/dskit/middleware/http_tracing.go b/vendor/github.com/grafana/dskit/middleware/http_tracing.go index 901970a4a6b..d75535ebe38 100644 --- a/vendor/github.com/grafana/dskit/middleware/http_tracing.go +++ b/vendor/github.com/grafana/dskit/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 { diff --git a/vendor/github.com/grafana/dskit/middleware/instrument.go b/vendor/github.com/grafana/dskit/middleware/instrument.go index 6e821e4c079..9813077ce6c 100644 --- a/vendor/github.com/grafana/dskit/middleware/instrument.go +++ b/vendor/github.com/grafana/dskit/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,7 +118,7 @@ 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" } @@ -128,53 +126,6 @@ func (i Instrument) getRouteName(r *http.Request) string { 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 diff --git a/vendor/github.com/grafana/dskit/middleware/route_injector.go b/vendor/github.com/grafana/dskit/middleware/route_injector.go new file mode 100644 index 00000000000..7b275f74f75 --- /dev/null +++ b/vendor/github.com/grafana/dskit/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 +} diff --git a/vendor/github.com/grafana/dskit/server/server.go b/vendor/github.com/grafana/dskit/server/server.go index c739228ddc3..a23eead3891 100644 --- a/vendor/github.com/grafana/dskit/server/server.go +++ b/vendor/github.com/grafana/dskit/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, diff --git a/vendor/modules.txt b/vendor/modules.txt index b41101ee4c3..bed72f53a1f 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -569,7 +569,7 @@ github.com/grafana-tools/sdk github.com/grafana/alerting/definition github.com/grafana/alerting/logging github.com/grafana/alerting/receivers -# github.com/grafana/dskit v0.0.0-20240509115328-a1bba1277f06 +# github.com/grafana/dskit v0.0.0-20240528015923-27d7d41066d3 ## explicit; go 1.20 github.com/grafana/dskit/backoff github.com/grafana/dskit/ballast