chore: Add has_labelfilter_before_parser to metrics.go log line

The value of this field indicates that the query could potentially benefit from bloom filters on structured metadata, _if_ the queried field is a structured metadata field. Signed-off-by: Christian Haudum <[email protected]>
grafana · Sep 24, 2024 · 8f94681 · 8f94681
1 parent b6e9945
commit 8f94681
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 32 deletions.
diff --git a/pkg/logql/metrics.go b/pkg/logql/metrics.go
@@ -219,6 +219,13 @@ func RecordRangeAndInstantQueryMetrics(
 		logValues = append(logValues, "disable_pipeline_wrappers", "false")
 	}
 
+	// Query is eligible for bloom filtering
+	if hasMatchEqualLabelFilterBeforeParser(p) {
+		logValues = append(logValues, "has_labelfilter_before_parser", "true")
+	} else {
+		logValues = append(logValues, "has_labelfilter_before_parser", "false")
+	}
+
 	level.Info(logger).Log(
 		logValues...,
 	)
@@ -242,6 +249,19 @@ func RecordRangeAndInstantQueryMetrics(
 	recordUsageStats(queryType, stats)
 }
 
+func hasMatchEqualLabelFilterBeforeParser(p Params) bool {
+	filters := syntax.ExtractLabelFiltersBeforeParser(p.GetExpression())
+	if len(filters) == 0 {
+		return false
+	}
+	for _, f := range filters {
+		if !syntax.IsMatchEqualFilterer(f.LabelFilterer) {
+			return false
+		}
+	}
+	return true
+}
+
 func RecordLabelQueryMetrics(
 	ctx context.Context,
 	log log.Logger,

diff --git a/pkg/logql/syntax/ast.go b/pkg/logql/syntax/ast.go
@@ -70,6 +70,55 @@ func ExtractLineFilters(e Expr) []LineFilterExpr {
 	return filters
 }
 
+func ExtractLabelFiltersBeforeParser(e Expr) []*LabelFilterExpr {
+	if e == nil {
+		return nil
+	}
+	var (
+		filters         []*LabelFilterExpr
+		foundParseStage bool
+	)
+
+	visitor := &DepthFirstTraversal{
+		VisitLabelFilterFn: func(v RootVisitor, e *LabelFilterExpr) {
+			if !foundParseStage {
+				filters = append(filters, e)
+			}
+		},
+
+		// TODO(rfratto): Find a way to generically represent or test for an
+		// expression that modifies extracted labels (parsers, keep, drop, etc.).
+		//
+		// As the AST is now, we can't prove at compile time that the list of
+		// visitors below is complete. For example, if a new parser stage
+		// expression is added without updating this list, blooms can silently
+		// misbehave.
+
+		VisitLogfmtParserFn:           func(v RootVisitor, e *LogfmtParserExpr) { foundParseStage = true },
+		VisitLabelParserFn:            func(v RootVisitor, e *LabelParserExpr) { foundParseStage = true },
+		VisitJSONExpressionParserFn:   func(v RootVisitor, e *JSONExpressionParser) { foundParseStage = true },
+		VisitLogfmtExpressionParserFn: func(v RootVisitor, e *LogfmtExpressionParser) { foundParseStage = true },
+		VisitLabelFmtFn:               func(v RootVisitor, e *LabelFmtExpr) { foundParseStage = true },
+		VisitKeepLabelFn:              func(v RootVisitor, e *KeepLabelsExpr) { foundParseStage = true },
+		VisitDropLabelsFn:             func(v RootVisitor, e *DropLabelsExpr) { foundParseStage = true },
+	}
+	e.Accept(visitor)
+	return filters
+}
+
+func IsMatchEqualFilterer(filterer log.LabelFilterer) bool {
+	switch filter := filterer.(type) {
+	case *log.LineFilterLabelFilter:
+		return filter.Type == labels.MatchEqual
+	case *log.StringLabelFilter:
+		return filter.Type == labels.MatchEqual
+	case *log.BinaryLabelFilter:
+		return IsMatchEqualFilterer(filter.Left) && IsMatchEqualFilterer(filter.Right)
+	default:
+		return false
+	}
+}
+
 // implicit holds default implementations
 type implicit struct{}
 

diff --git a/pkg/storage/bloom/v1/ast_extractor.go b/pkg/storage/bloom/v1/ast_extractor.go
@@ -38,38 +38,8 @@ func ExtractTestableLabelMatchers(expr syntax.Expr) []LabelMatcher {
 	if expr == nil {
 		return nil
 	}
-
-	var (
-		exprs           []*syntax.LabelFilterExpr
-		foundParseStage bool
-	)
-
-	visitor := &syntax.DepthFirstTraversal{
-		VisitLabelFilterFn: func(v syntax.RootVisitor, e *syntax.LabelFilterExpr) {
-			if !foundParseStage {
-				exprs = append(exprs, e)
-			}
-		},
-
-		// TODO(rfratto): Find a way to generically represent or test for an
-		// expression that modifies extracted labels (parsers, keep, drop, etc.).
-		//
-		// As the AST is now, we can't prove at compile time that the list of
-		// visitors below is complete. For example, if a new parser stage
-		// expression is added without updating this list, blooms can silently
-		// misbehave.
-
-		VisitLogfmtParserFn:           func(v syntax.RootVisitor, e *syntax.LogfmtParserExpr) { foundParseStage = true },
-		VisitLabelParserFn:            func(v syntax.RootVisitor, e *syntax.LabelParserExpr) { foundParseStage = true },
-		VisitJSONExpressionParserFn:   func(v syntax.RootVisitor, e *syntax.JSONExpressionParser) { foundParseStage = true },
-		VisitLogfmtExpressionParserFn: func(v syntax.RootVisitor, e *syntax.LogfmtExpressionParser) { foundParseStage = true },
-		VisitLabelFmtFn:               func(v syntax.RootVisitor, e *syntax.LabelFmtExpr) { foundParseStage = true },
-		VisitKeepLabelFn:              func(v syntax.RootVisitor, e *syntax.KeepLabelsExpr) { foundParseStage = true },
-		VisitDropLabelsFn:             func(v syntax.RootVisitor, e *syntax.DropLabelsExpr) { foundParseStage = true },
-	}
-	expr.Accept(visitor)
-
-	return buildLabelMatchers(exprs)
+	filters := syntax.ExtractLabelFiltersBeforeParser(expr)
+	return buildLabelMatchers(filters)
 }
 
 func buildLabelMatchers(exprs []*syntax.LabelFilterExpr) []LabelMatcher {