From 90b2ac5073a5ea8c85f128e2dffad1a2be340610 Mon Sep 17 00:00:00 2001 From: Martin Disibio Date: Fri, 13 Dec 2024 13:36:19 -0500 Subject: [PATCH] [TraceQL] Improve performance of select() queries (#4438) * Tighten up allconditions disable so that select expressions don't disable it * add benchmark * changelog * More test cases for scalar expressions --- CHANGELOG.md | 1 + pkg/traceql/ast.go | 8 ++------ pkg/traceql/ast_conditions_test.go | 19 +++++++++++++++++-- pkg/traceql/ast_test.go | 2 +- .../encoding/vparquet3/block_traceql_test.go | 1 + .../encoding/vparquet4/block_traceql_test.go | 1 + 6 files changed, 23 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 528ac8d3007..5dc66e5c94f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ querier: * [ENHANCEMENT] Reduce allocs related to marshalling dedicated columns repeatedly in the query frontend. [#4007](https://github.com/grafana/tempo/pull/4007) (@joe-elliott) * [ENHANCEMENT] Improve performance of TraceQL queries [#4114](https://github.com/grafana/tempo/pull/4114) (@mdisibio) * [ENHANCEMENT] Improve performance of TraceQL queries [#4163](https://github.com/grafana/tempo/pull/4163) (@mdisibio) +* [ENHANCEMENT] Improve performance of some TraceQL queries using select() operation [#4438](https://github.com/grafana/tempo/pull/4438) (@mdisibio) * [ENHANCEMENT] Reduce memory usage of classic histograms in the span-metrics and service-graphs processors [#4232](https://github.com/grafana/tempo/pull/4232) (@mdisibio) * [ENHANCEMENT] Implement simple Fetch by key for cache items [#4032](https://github.com/grafana/tempo/pull/4032) (@javiermolinar) * [ENHANCEMENT] Replace Grafana Agent example by Grafana Alloy[#4030](https://github.com/grafana/tempo/pull/4030) (@javiermolinar) diff --git a/pkg/traceql/ast.go b/pkg/traceql/ast.go index 278d8a5c54e..2b4f4b97567 100644 --- a/pkg/traceql/ast.go +++ b/pkg/traceql/ast.go @@ -159,10 +159,6 @@ func (p Pipeline) extractConditions(req *FetchSpansRequest) { for _, element := range p.Elements { element.extractConditions(req) } - // TODO this needs to be fine-tuned a bit, e.g. { .foo = "bar" } | by(.namespace), AllConditions can still be true - if len(p.Elements) > 1 { - req.AllConditions = false - } } func (p Pipeline) evaluate(input []*Spanset) (result []*Spanset, err error) { @@ -197,6 +193,7 @@ func newGroupOperation(e FieldExpression) GroupOperation { func (o GroupOperation) extractConditions(request *FetchSpansRequest) { o.Expression.extractConditions(request) + request.AllConditions = false } type CoalesceOperation struct{} @@ -265,7 +262,6 @@ func (o ScalarOperation) impliedType() StaticType { func (o ScalarOperation) extractConditions(request *FetchSpansRequest) { o.LHS.extractConditions(request) o.RHS.extractConditions(request) - request.AllConditions = false } type Aggregate struct { @@ -292,6 +288,7 @@ func (a Aggregate) impliedType() StaticType { } func (a Aggregate) extractConditions(request *FetchSpansRequest) { + request.AllConditions = false if a.e != nil { a.e.extractConditions(request) } @@ -421,7 +418,6 @@ func (ScalarFilter) __spansetExpression() {} func (f ScalarFilter) extractConditions(request *FetchSpansRequest) { f.lhs.extractConditions(request) f.rhs.extractConditions(request) - request.AllConditions = false } // ********************** diff --git a/pkg/traceql/ast_conditions_test.go b/pkg/traceql/ast_conditions_test.go index 1cd7ddf29bb..3a2be3ca3f9 100644 --- a/pkg/traceql/ast_conditions_test.go +++ b/pkg/traceql/ast_conditions_test.go @@ -147,6 +147,21 @@ func TestScalarFilter_extractConditions(t *testing.T) { }, allConditions: false, }, + { + query: `({ span.http.status_code = 200 } | count()) > ({ span.http.status_code = 500 } | count())`, + conditions: []Condition{ + newCondition(NewScopedAttribute(AttributeScopeSpan, false, "http.status_code"), OpEqual, NewStaticInt(200)), + newCondition(NewScopedAttribute(AttributeScopeSpan, false, "http.status_code"), OpEqual, NewStaticInt(500)), + }, + allConditions: false, + }, + { + query: `{ .foo = "a" } | 3 > 2`, + conditions: []Condition{ + newCondition(NewAttribute("foo"), OpEqual, NewStaticString("a")), + }, + allConditions: true, + }, } for _, tt := range tests { t.Run(tt.query, func(t *testing.T) { @@ -230,7 +245,7 @@ func TestSelect_extractConditions(t *testing.T) { secondPassConditions: []Condition{ newCondition(NewScopedAttribute(AttributeScopeResource, false, "service.name"), OpNone), }, - allConditions: false, + allConditions: true, }, { query: `{ } | select(.name,name)`, @@ -241,7 +256,7 @@ func TestSelect_extractConditions(t *testing.T) { newCondition(NewAttribute("name"), OpNone), newCondition(NewIntrinsic(IntrinsicName), OpNone), }, - allConditions: false, + allConditions: true, }, } for _, tt := range tests { diff --git a/pkg/traceql/ast_test.go b/pkg/traceql/ast_test.go index 0e8c271782c..557c1df81a8 100644 --- a/pkg/traceql/ast_test.go +++ b/pkg/traceql/ast_test.go @@ -610,7 +610,7 @@ func TestPipelineExtractConditions(t *testing.T) { newCondition(NewAttribute("foo1"), OpEqual, NewStaticString("a")), newCondition(NewAttribute("foo2"), OpEqual, NewStaticString("b")), }, - AllConditions: false, + AllConditions: true, }, }, { diff --git a/tempodb/encoding/vparquet3/block_traceql_test.go b/tempodb/encoding/vparquet3/block_traceql_test.go index 6b9f074ed5c..8ab94aff874 100644 --- a/tempodb/encoding/vparquet3/block_traceql_test.go +++ b/tempodb/encoding/vparquet3/block_traceql_test.go @@ -592,6 +592,7 @@ func BenchmarkBackendBlockTraceQL(b *testing.B) { {"||", "{ resource.service.name = `loki-querier` } || { resource.service.name = `loki-gateway` }"}, {"mixed", `{resource.namespace!="" && resource.service.name="cortex-gateway" && duration>50ms && resource.cluster=~"prod.*"}`}, {"complex", `{resource.cluster=~"prod.*" && resource.namespace = "tempo-prod" && resource.container="query-frontend" && name = "HTTP GET - tempo_api_v2_search_tags" && span.http.status_code = 200 && duration > 1s}`}, + {"select", `{resource.cluster=~"prod.*" && resource.namespace = "tempo-prod"} | select(resource.container)`}, } ctx := context.TODO() diff --git a/tempodb/encoding/vparquet4/block_traceql_test.go b/tempodb/encoding/vparquet4/block_traceql_test.go index 8c81e0d217a..b5c0701ebe5 100644 --- a/tempodb/encoding/vparquet4/block_traceql_test.go +++ b/tempodb/encoding/vparquet4/block_traceql_test.go @@ -943,6 +943,7 @@ func BenchmarkBackendBlockTraceQL(b *testing.B) { {"||", "{ resource.service.name = `loki-querier` } || { resource.service.name = `loki-gateway` }"}, {"mixed", `{resource.namespace!="" && resource.service.name="cortex-gateway" && duration>50ms && resource.cluster=~"prod.*"}`}, {"complex", `{resource.cluster=~"prod.*" && resource.namespace = "tempo-prod" && resource.container="query-frontend" && name = "HTTP GET - tempo_api_v2_search_tags" && span.http.status_code = 200 && duration > 1s}`}, + {"select", `{resource.cluster=~"prod.*" && resource.namespace = "tempo-prod"} | select(resource.container)`}, } ctx := context.TODO()