From 2544f003cf334c55150827b7be43bed2a7477c75 Mon Sep 17 00:00:00 2001 From: Salva Corts Date: Fri, 15 Mar 2024 11:21:48 +0100 Subject: [PATCH] chore: Add summary for received filters in bloom gateway (#12158) --- pkg/bloomgateway/bloomgateway.go | 1 + pkg/bloomgateway/metrics.go | 8 +++ pkg/querier/queryrange/metrics.go | 56 +++++++++++++++++++ pkg/querier/queryrange/roundtrip.go | 2 + .../indexshipper/indexgateway/gateway.go | 1 - 5 files changed, 67 insertions(+), 1 deletion(-) diff --git a/pkg/bloomgateway/bloomgateway.go b/pkg/bloomgateway/bloomgateway.go index 515b27b01b2cf..4138ff4c1beb2 100644 --- a/pkg/bloomgateway/bloomgateway.go +++ b/pkg/bloomgateway/bloomgateway.go @@ -218,6 +218,7 @@ func (g *Gateway) FilterChunkRefs(ctx context.Context, req *logproto.FilterChunk } filters := syntax.ExtractLineFilters(req.Plan.AST) + g.metrics.receivedFilters.Observe(float64(len(filters))) // Shortcut if request does not contain filters if len(filters) == 0 { diff --git a/pkg/bloomgateway/metrics.go b/pkg/bloomgateway/metrics.go index 9058a90078ac5..0e428b248d1e3 100644 --- a/pkg/bloomgateway/metrics.go +++ b/pkg/bloomgateway/metrics.go @@ -18,6 +18,7 @@ type serverMetrics struct { filteredSeries prometheus.Histogram requestedChunks prometheus.Histogram filteredChunks prometheus.Histogram + receivedFilters prometheus.Histogram } func newMetrics(registerer prometheus.Registerer, namespace, subsystem string) *metrics { @@ -66,6 +67,13 @@ func newServerMetrics(registerer prometheus.Registerer, namespace, subsystem str Help: "Total amount of chunk refs filtered by bloom-gateway", Buckets: prometheus.ExponentialBucketsRange(1, 100e3, 10), }), + receivedFilters: promauto.With(registerer).NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "request_filters", + Help: "Number of filters per request.", + Buckets: prometheus.ExponentialBuckets(1, 2, 9), // 1 -> 256 + }), } } diff --git a/pkg/querier/queryrange/metrics.go b/pkg/querier/queryrange/metrics.go index 390f2c81d771e..9482becf98817 100644 --- a/pkg/querier/queryrange/metrics.go +++ b/pkg/querier/queryrange/metrics.go @@ -1,9 +1,13 @@ package queryrange import ( + "context" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" "github.com/grafana/loki/pkg/logql" + "github.com/grafana/loki/pkg/logql/syntax" "github.com/grafana/loki/pkg/querier/queryrange/queryrangebase" ) @@ -13,6 +17,7 @@ type Metrics struct { *MiddlewareMapperMetrics *SplitByMetrics *LogResultCacheMetrics + *QueryMetrics *queryrangebase.ResultsCacheMetrics } @@ -35,6 +40,57 @@ func NewMetrics(registerer prometheus.Registerer, metricsNamespace string) *Metr MiddlewareMapperMetrics: NewMiddlewareMapperMetrics(registerer), SplitByMetrics: NewSplitByMetrics(registerer), LogResultCacheMetrics: NewLogResultCacheMetrics(registerer), + QueryMetrics: NewMiddlewareQueryMetrics(registerer, metricsNamespace), ResultsCacheMetrics: queryrangebase.NewResultsCacheMetrics(registerer), } } + +type QueryMetrics struct { + receivedFilters prometheus.Histogram +} + +func NewMiddlewareQueryMetrics(registerer prometheus.Registerer, metricsNamespace string) *QueryMetrics { + return &QueryMetrics{ + receivedFilters: promauto.With(registerer).NewHistogram(prometheus.HistogramOpts{ + Namespace: metricsNamespace, + Name: "query_frontend_query_filters", + Help: "Number of filters per query.", + Buckets: prometheus.ExponentialBuckets(1, 2, 9), // 1 -> 256 + }), + } +} + +// QueryMetricsMiddleware can be inserted into the middleware chain to expose timing information. +func QueryMetricsMiddleware(metrics *QueryMetrics) queryrangebase.Middleware { + return queryrangebase.MiddlewareFunc(func(next queryrangebase.Handler) queryrangebase.Handler { + return queryrangebase.HandlerFunc(func(ctx context.Context, req queryrangebase.Request) (queryrangebase.Response, error) { + var expr syntax.Expr + switch r := req.(type) { + case *LokiRequest: + if r.Plan != nil { + expr = r.Plan.AST + } + case *LokiInstantRequest: + if r.Plan != nil { + expr = r.Plan.AST + } + default: + return next.Do(ctx, req) + } + + // The plan should always be present, but if it's not, we'll parse the query to get the filters. + if expr == nil { + var err error + expr, err = syntax.ParseExpr(req.GetQuery()) + if err != nil { + return nil, err + } + } + + filters := syntax.ExtractLineFilters(expr) + metrics.receivedFilters.Observe(float64(len(filters))) + + return next.Do(ctx, req) + }) + }) +} diff --git a/pkg/querier/queryrange/roundtrip.go b/pkg/querier/queryrange/roundtrip.go index 5532eab989c1e..5184ef62bb13c 100644 --- a/pkg/querier/queryrange/roundtrip.go +++ b/pkg/querier/queryrange/roundtrip.go @@ -426,6 +426,7 @@ func NewLogFilterTripperware(cfg Config, engineOpts logql.EngineOpts, log log.Lo statsHandler := indexStatsTripperware.Wrap(next) queryRangeMiddleware := []base.Middleware{ + QueryMetricsMiddleware(metrics.QueryMetrics), StatsCollectorMiddleware(), NewLimitsMiddleware(limits), NewQuerySizeLimiterMiddleware(schema.Configs, engineOpts, log, limits, statsHandler), @@ -703,6 +704,7 @@ func NewMetricTripperware(cfg Config, engineOpts logql.EngineOpts, log log.Logge statsHandler := indexStatsTripperware.Wrap(next) queryRangeMiddleware := []base.Middleware{ + QueryMetricsMiddleware(metrics.QueryMetrics), StatsCollectorMiddleware(), NewLimitsMiddleware(limits), } diff --git a/pkg/storage/stores/shipper/indexshipper/indexgateway/gateway.go b/pkg/storage/stores/shipper/indexshipper/indexgateway/gateway.go index e5aa0bcc73524..a2325bd5c51bb 100644 --- a/pkg/storage/stores/shipper/indexshipper/indexgateway/gateway.go +++ b/pkg/storage/stores/shipper/indexshipper/indexgateway/gateway.go @@ -229,7 +229,6 @@ func (g *Gateway) GetChunkRef(ctx context.Context, req *logproto.GetChunkRefRequ // Extract LineFiltersExpr from the plan. If there is none, we can short-circuit and return before making a req // to the bloom-gateway (through the g.bloomQuerier) - // TODO(owen-d): metrics for number of filters seen, but probably do elsewhere (in query-frontend?) if len(syntax.ExtractLineFilters(req.Plan.AST)) == 0 { return result, nil }