Supports ranger operators for strings in TraceQL (#2321)

* fix tests * strings compare * remove testing logs & changelog * changelog and docs * corrected test * lets goooo * update docs * words
grafana · Apr 21, 2023 · c6b269c · c6b269c
1 parent e2bc232
commit c6b269c
Show file tree

Hide file tree

Showing 13 changed files with 216 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -50,6 +50,7 @@
 * [BUGFIX] Correctly return unique spans when &&ing and ||ing spansets. [#2254](https://github.com/grafana/tempo/pull/2254) (@joe-elliott)
 * [BUGFIX] Support negative values on aggregate filters like `count() > -1`. [#2289](https://github.com/grafana/tempo/pull/2289) (@joe-elliott)
 * [BUGFIX] Support float as duration like `{duration > 1.5s}` [#2304]https://github.com/grafana/tempo/pull/2304 (@ie-pham)
+* [ENHANCEMENT] Supports ranger operators for strings in TraceQL [#2321]https://github.com/grafana/tempo/pull/2321 (@ie-pham)
 
 ## v2.0.1 / 2023-03-03
 

diff --git a/docs/sources/tempo/traceql/_index.md b/docs/sources/tempo/traceql/_index.md
@@ -129,6 +129,12 @@ For example, to find all traces where an `http.status_code` attribute in a span
 { span.http.status_code >= 400 && span.http.status_code < 500 }
 ```
 
+This works for `http.status_code` values that are strings as well using lexographic ordering:
+
+```
+{ span.http.status_code >= "400"}
+```
+
 Find all traces where the `http.method` attribute is either `GET` or `DELETE`:
 
 ```

diff --git a/docs/sources/tempo/traceql/query-editor.md b/docs/sources/tempo/traceql/query-editor.md
@@ -18,14 +18,14 @@ With Tempo 2.0, you can use the TraceQL viewer and query editor in the Tempo dat
 >**NOTE**: To use the TraceQL query editor, you need to enable the `traceqlEditor` feature flag. This feature is available starting in Grafana 9.3.2.
 
 
-<p align="center"><img src="../assets/query-editor-http-method.png" alt="Query editor showing request for http.method" /></p>
+<p align="center"><img src="./assets/query-editor-http-method.png" alt="Query editor showing request for http.method" /></p>
 
 Using the query editor, you can use the editor’s autocomplete suggestions to write queries. The editor detects span sets to provide relevant autocomplete options. It uses regular expressions (regex) to detect where it is inside a spanset and provide attribute names, scopes, intrinsic names, logic operators, or attribute values from Tempo's API, depending on what is expected for the current situation.
 
-<p align="center"><img src="../assets/query-editor-auto-complete.png" alt="Query editor showing the auto-complete feature" /></p>
+<p align="center"><img src="./assets/query-editor-auto-complete.png" alt="Query editor showing the auto-complete feature" /></p>
 
 Query results are returned in a table. Selecting the Trace ID or Span ID provides more detailed information.
 
-<p align="center"><img src="../assets/query-editor-results-span.png" alt="Query editor showing span results" /></p>
+<p align="center"><img src="./assets/query-editor-results-span.png" alt="Query editor showing span results" /></p>
 
 Selecting the trace ID from the returned results will open a trace diagram. Selecting a span from the returned results opens a trace diagram and reveals the relevant span in the trace diagram (above, the highlighted blue line).
diff --git a/pkg/traceql/ast_execute.go b/pkg/traceql/ast_execute.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"math"
 	"regexp"
+	"strings"
 )
 
 func (o SpansetOperation) evaluate(input []*Spanset) (output []*Spanset, err error) {
@@ -179,6 +180,20 @@ func (o BinaryOperation) execute(span Span) (Static, error) {
 		return NewStaticBool(false), nil
 	}
 
+	if lhsT == TypeString && rhsT == TypeString {
+		switch o.Op {
+		case OpGreater:
+			return NewStaticBool(strings.Compare(lhs.String(), rhs.String()) > 0), nil
+		case OpGreaterEqual:
+			return NewStaticBool(strings.Compare(lhs.String(), rhs.String()) >= 0), nil
+		case OpLess:
+			return NewStaticBool(strings.Compare(lhs.String(), rhs.String()) < 0), nil
+		case OpLessEqual:
+			return NewStaticBool(strings.Compare(lhs.String(), rhs.String()) <= 0), nil
+		default:
+		}
+	}
+
 	switch o.Op {
 	case OpAdd:
 		return NewStaticFloat(lhs.asFloat() + rhs.asFloat()), nil

diff --git a/pkg/traceql/ast_test.go b/pkg/traceql/ast_test.go
@@ -186,6 +186,72 @@ func TestSpansetFilterEvaluate(t *testing.T) {
 					&mockSpan{attributes: map[Attribute]Static{NewAttribute("foo"): NewStaticString("a")}}}},
 			},
 		},
+		{
+			"{ .http.status > `200` }",
+			[]*Spanset{
+				{Spans: []Span{
+					// First span should be dropped here
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticString("200")}},
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticString("201")}},
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticString("300")}},
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticString("301")}},
+				}},
+				{Spans: []Span{
+					// This entire spanset will be dropped
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticString("100")}},
+				}},
+			},
+			[]*Spanset{
+				{Spans: []Span{
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticString("201")}},
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticString("300")}},
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticString("301")}},
+				}},
+			},
+		},
+		{
+			"{ .http.status <= `300` }",
+			[]*Spanset{
+				{Spans: []Span{
+					// Last span should be dropped here
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticString("200")}},
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticString("201")}},
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticString("300")}},
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticString("301")}},
+				}},
+				{Spans: []Span{
+					// This entire spanset is valid
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticString("100")}},
+				}},
+			},
+			[]*Spanset{
+				{Spans: []Span{
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticString("200")}},
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticString("201")}},
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticString("300")}},
+				}},
+				{Spans: []Span{
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticString("100")}},
+				}},
+			},
+		},
+		{
+			"{ .http.status > `200` }",
+			[]*Spanset{
+				{Spans: []Span{
+					// This entire spanset will be dropped because mismatch type
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticInt(200)}},
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticInt(201)}},
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticInt(300)}},
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticInt(301)}},
+				}},
+				{Spans: []Span{
+					// This entire spanset will be dropped because mismatch type
+					&mockSpan{attributes: map[Attribute]Static{NewAttribute("http.status"): NewStaticInt(100)}},
+				}},
+			},
+			nil,
+		},
 		{
 			"{ .foo = 1 || (.foo >= 4 && .foo < 6) }",
 			[]*Spanset{

diff --git a/pkg/traceql/ast_validate.go b/pkg/traceql/ast_validate.go
@@ -168,6 +168,7 @@ func (o BinaryOperation) validate() error {
 
 	lhsT := o.LHS.impliedType()
 	rhsT := o.RHS.impliedType()
+
 	if !lhsT.isMatchingOperand(rhsT) {
 		return fmt.Errorf("binary operations must operate on the same type: %s", o.String())
 	}

diff --git a/pkg/traceql/enum_operators.go b/pkg/traceql/enum_operators.go
@@ -80,7 +80,11 @@ func binaryTypeValid(op Operator, t StaticType) bool {
 		return op == OpEqual ||
 			op == OpNotEqual ||
 			op == OpRegex ||
-			op == OpNotRegex
+			op == OpNotRegex ||
+			op == OpGreater ||
+			op == OpGreaterEqual ||
+			op == OpLess ||
+			op == OpLessEqual
 	case TypeNil:
 		fallthrough
 	case TypeStatus:

diff --git a/pkg/traceql/enum_operators_test.go b/pkg/traceql/enum_operators_test.go
@@ -85,7 +85,7 @@ func TestOperatorBinaryTypesValid(t *testing.T) {
 		{OpGreater, TypeStatus, false},
 		{OpLessEqual, TypeKind, false},
 		{OpGreaterEqual, TypeNil, false},
-		{OpLess, TypeString, false},
+		{OpLess, TypeString, true},
 		{OpLessEqual, TypeBoolean, false},
 		// string comparison
 		{OpRegex, TypeString, true},

diff --git a/pkg/traceql/test_examples.yaml b/pkg/traceql/test_examples.yaml
@@ -56,6 +56,7 @@ valid:
   - '{ duration > 1s * 2s }' 
   - '{ 1 * 1h = 1 }'     # combining float, int and duration can make sense, but can also be weird. we just accept it all
   - '{ 1 / 1.1 = 1 }'
+  - '{ .http.status >= "200" }'
   # spanset expressions
   - '{ true } && { true }'
   - '{ true } || { true }'

diff --git a/tempodb/encoding/vparquet/block_traceql.go b/tempodb/encoding/vparquet/block_traceql.go
@@ -6,6 +6,7 @@ import (
 	"math"
 	"reflect"
 	"sort"
+	"strings"
 	"sync"
 	"time"
 
@@ -813,9 +814,6 @@ func createStringPredicate(op traceql.Operator, operands traceql.Operands) (parq
 	s := operands[0].S
 
 	switch op {
-	case traceql.OpEqual:
-		return parquetquery.NewStringInPredicate([]string{s}), nil
-
 	case traceql.OpNotEqual:
 		return parquetquery.NewGenericPredicate(
 			func(v string) bool {
@@ -831,7 +829,56 @@ func createStringPredicate(op traceql.Operator, operands traceql.Operands) (parq
 
 	case traceql.OpRegex:
 		return parquetquery.NewRegexInPredicate([]string{s})
-
+	case traceql.OpEqual:
+		return parquetquery.NewStringInPredicate([]string{s}), nil
+	case traceql.OpGreater:
+		return parquetquery.NewGenericPredicate(
+			func(v string) bool {
+				return strings.Compare(v, s) > 0
+			},
+			func(min, max string) bool {
+				return strings.Compare(max, s) > 0
+			},
+			func(v parquet.Value) string {
+				return v.String()
+			},
+		), nil
+	case traceql.OpGreaterEqual:
+		return parquetquery.NewGenericPredicate(
+			func(v string) bool {
+				return strings.Compare(v, s) >= 0
+			},
+			func(min, max string) bool {
+				return strings.Compare(max, s) >= 0
+			},
+			func(v parquet.Value) string {
+				return v.String()
+			},
+		), nil
+	case traceql.OpLess:
+		return parquetquery.NewGenericPredicate(
+			func(v string) bool {
+				return strings.Compare(v, s) < 0
+			},
+			func(min, max string) bool {
+				return strings.Compare(min, s) < 0
+			},
+			func(v parquet.Value) string {
+				return v.String()
+			},
+		), nil
+	case traceql.OpLessEqual:
+		return parquetquery.NewGenericPredicate(
+			func(v string) bool {
+				return strings.Compare(v, s) <= 0
+			},
+			func(min, max string) bool {
+				return strings.Compare(min, s) <= 0
+			},
+			func(v parquet.Value) string {
+				return v.String()
+			},
+		), nil
 	default:
 		return nil, fmt.Errorf("operand not supported for strings: %+v", op)
 	}

diff --git a/tempodb/encoding/vparquet/block_traceql_test.go b/tempodb/encoding/vparquet/block_traceql_test.go
@@ -109,6 +109,11 @@ func TestBackendBlockSearchTraceQL(t *testing.T) {
 		traceql.MustExtractFetchSpansRequest(`{resource.` + LabelK8sClusterName + ` = "k8scluster"}`),
 		traceql.MustExtractFetchSpansRequest(`{resource.` + LabelK8sPodName + ` = "k8spod"}`),
 		traceql.MustExtractFetchSpansRequest(`{resource.` + LabelK8sContainerName + ` = "k8scontainer"}`),
+		// Comparing strings
+		traceql.MustExtractFetchSpansRequest(`{resource.` + LabelServiceName + ` > "myservic"}`),
+		traceql.MustExtractFetchSpansRequest(`{resource.` + LabelServiceName + ` >= "myservic"}`),
+		traceql.MustExtractFetchSpansRequest(`{resource.` + LabelServiceName + ` < "myservice1"}`),
+		traceql.MustExtractFetchSpansRequest(`{resource.` + LabelServiceName + ` <= "myservice1"}`),
 		// Span well-known attributes
 		traceql.MustExtractFetchSpansRequest(`{.` + LabelHTTPStatusCode + ` = 500}`),
 		traceql.MustExtractFetchSpansRequest(`{.` + LabelHTTPMethod + ` = "get"}`),

diff --git a/tempodb/encoding/vparquet2/block_traceql.go b/tempodb/encoding/vparquet2/block_traceql.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"math"
 	"reflect"
+	"strings"
 	"sync"
 	"time"
 
@@ -648,6 +649,7 @@ func createSpanIterator(makeIter makeIterFn, conditions []traceql.Condition, req
 // one batch each. It builds on top of the span iterator, and turns the groups of spans and resource-level values into
 // spansets.  Spansets are returned that match any of the given conditions.
 func createResourceIterator(makeIter makeIterFn, spanIterator parquetquery.Iterator, conditions []traceql.Condition, requireAtLeastOneMatch, requireAtLeastOneMatchOverall, allConditions bool) (parquetquery.Iterator, error) {
+
 	var (
 		columnSelectAs    = map[string]string{}
 		columnPredicates  = map[string][]parquetquery.Predicate{}
@@ -786,6 +788,7 @@ func createPredicate(op traceql.Operator, operands traceql.Operands) (parquetque
 }
 
 func createStringPredicate(op traceql.Operator, operands traceql.Operands) (parquetquery.Predicate, error) {
+
 	if op == traceql.OpNone {
 		return nil, nil
 	}
@@ -799,9 +802,6 @@ func createStringPredicate(op traceql.Operator, operands traceql.Operands) (parq
 	s := operands[0].S
 
 	switch op {
-	case traceql.OpEqual:
-		return parquetquery.NewStringInPredicate([]string{s}), nil
-
 	case traceql.OpNotEqual:
 		return parquetquery.NewGenericPredicate(
 			func(v string) bool {
@@ -818,6 +818,58 @@ func createStringPredicate(op traceql.Operator, operands traceql.Operands) (parq
 	case traceql.OpRegex:
 		return parquetquery.NewRegexInPredicate([]string{s})
 
+	case traceql.OpEqual:
+		return parquetquery.NewStringInPredicate([]string{s}), nil
+
+	case traceql.OpGreater:
+		return parquetquery.NewGenericPredicate(
+			func(v string) bool {
+				return strings.Compare(v, s) > 0
+			},
+			func(min, max string) bool {
+				return strings.Compare(max, s) > 0
+			},
+			func(v parquet.Value) string {
+				return v.String()
+			},
+		), nil
+	case traceql.OpGreaterEqual:
+		return parquetquery.NewGenericPredicate(
+			func(v string) bool {
+				return strings.Compare(v, s) >= 0
+			},
+			func(min, max string) bool {
+				return strings.Compare(max, s) >= 0
+			},
+			func(v parquet.Value) string {
+				return v.String()
+			},
+		), nil
+	case traceql.OpLess:
+		return parquetquery.NewGenericPredicate(
+			func(v string) bool {
+				return strings.Compare(v, s) < 0
+			},
+			func(min, max string) bool {
+				return strings.Compare(min, s) < 0
+			},
+			func(v parquet.Value) string {
+				return v.String()
+			},
+		), nil
+	case traceql.OpLessEqual:
+		return parquetquery.NewGenericPredicate(
+			func(v string) bool {
+				return strings.Compare(v, s) <= 0
+			},
+			func(min, max string) bool {
+				return strings.Compare(min, s) <= 0
+			},
+			func(v parquet.Value) string {
+				return v.String()
+			},
+		), nil
+
 	default:
 		return nil, fmt.Errorf("operand not supported for strings: %+v", op)
 	}

diff --git a/tempodb/encoding/vparquet2/block_traceql_test.go b/tempodb/encoding/vparquet2/block_traceql_test.go
@@ -109,6 +109,12 @@ func TestBackendBlockSearchTraceQL(t *testing.T) {
 		traceql.MustExtractFetchSpansRequest(`{resource.` + LabelK8sClusterName + ` = "k8scluster"}`),
 		traceql.MustExtractFetchSpansRequest(`{resource.` + LabelK8sPodName + ` = "k8spod"}`),
 		traceql.MustExtractFetchSpansRequest(`{resource.` + LabelK8sContainerName + ` = "k8scontainer"}`),
+		// Comparing strings
+
+		traceql.MustExtractFetchSpansRequest(`{resource.` + LabelServiceName + ` > "myservic"}`),
+		traceql.MustExtractFetchSpansRequest(`{resource.` + LabelServiceName + ` >= "myservic"}`),
+		traceql.MustExtractFetchSpansRequest(`{resource.` + LabelServiceName + ` < "myservice1"}`),
+		traceql.MustExtractFetchSpansRequest(`{resource.` + LabelServiceName + ` <= "myservice1"}`),
 		// Span well-known attributes
 		traceql.MustExtractFetchSpansRequest(`{.` + LabelHTTPStatusCode + ` = 500}`),
 		traceql.MustExtractFetchSpansRequest(`{.` + LabelHTTPMethod + ` = "get"}`),