diff --git a/app/vtselect/traces/jaeger/jaeger.go b/app/vtselect/traces/jaeger/jaeger.go index 8644d5249..958751e68 100644 --- a/app/vtselect/traces/jaeger/jaeger.go +++ b/app/vtselect/traces/jaeger/jaeger.go @@ -371,6 +371,17 @@ func parseJaegerTraceQueryParam(_ context.Context, r *http.Request) (*query.Trac // 1. retrieved from `span_attr:otel.status_description` field directly. // 2. converted from `status_message` field for Jaeger API. for k, v := range p.Attributes { + // Jaeger UI encodes key!=value as JSON {"key!": "value"}; strip "!" before field mapping. + negated := strings.HasSuffix(k, "!") + if negated { + k = strings.TrimSuffix(k, "!") + } + store := func(field string) { + if negated { + field = "!" + field + } + attributesFilter[field] = v + } // convert to OpenTelemetry field name in storage. if field, ok := spanAttributeMap[k]; ok { // 2 special cases that need to converted value as well. @@ -380,11 +391,11 @@ func parseJaegerTraceQueryParam(_ context.Context, r *http.Request) (*query.Trac case "span.kind": v = spanKindMap[v] } - attributesFilter[field] = v + store(field) } else if strings.HasPrefix(k, otelpb.InstrumentationScopeAttrPrefix) || strings.HasPrefix(k, otelpb.ResourceAttrPrefix) { - attributesFilter[k] = v + store(k) } else { - attributesFilter[otelpb.SpanAttrPrefixField+k] = v + store(otelpb.SpanAttrPrefixField + k) } } p.Attributes = attributesFilter diff --git a/app/vtselect/traces/query/query.go b/app/vtselect/traces/query/query.go index 3d1bd02e5..0528d3543 100644 --- a/app/vtselect/traces/query/query.go +++ b/app/vtselect/traces/query/query.go @@ -229,11 +229,24 @@ func getTraceIDList(ctx context.Context, cp *tracecommon.CommonParams, param *Tr } if len(param.Attributes) > 0 { for k, v := range param.Attributes { + negated := strings.HasPrefix(k, "!") + if negated { + k = strings.TrimPrefix(k, "!") + } if strings.HasPrefix(v, "~") { - // ~ prefix forces regex (e.g. tags={"key":"~value.*"}) - qStr += fmt.Sprintf(`AND %q:re(%s) `, k, strconv.Quote(v[1:])) + // ~ prefix forces regex (e.g. tags={"key":"~value.*"}); Jaeger uses {"key!":"~pat"} for key!=~pat. + pat := strconv.Quote(v[1:]) + if negated { + qStr += fmt.Sprintf(`AND !(%q:re(%s)) `, k, pat) + } else { + qStr += fmt.Sprintf(`AND %q:re(%s) `, k, pat) + } } else { - qStr += fmt.Sprintf(`AND %q:=%q `, k, v) + if negated { + qStr += fmt.Sprintf(`AND !(%q:=%q) `, k, v) + } else { + qStr += fmt.Sprintf(`AND %q:=%q `, k, v) + } } } } diff --git a/app/vtselect/traces/query/query_test.go b/app/vtselect/traces/query/query_test.go index f3e4ea201..94d8bc1eb 100644 --- a/app/vtselect/traces/query/query_test.go +++ b/app/vtselect/traces/query/query_test.go @@ -2,6 +2,9 @@ package query import ( "testing" + "time" + + "github.com/VictoriaMetrics/VictoriaLogs/lib/logstorage" ) func TestCheckTraceIDList(t *testing.T) { @@ -21,3 +24,16 @@ func TestCheckTraceIDList(t *testing.T) { f("abcd bcad", false) f("abcd\"", false) } + +func TestInvertedTagFilterQueryParses(t *testing.T) { + t.Parallel() + ts := time.Now().UnixNano() + for _, qStr := range []string{ + `* AND !("span_attr:foo":re("pat")) | last 1 by (_time) partition by (trace_id) | fields _time, trace_id | sort by (_time) desc`, + `* AND !("span_attr:foo":="bar") | last 1 by (_time) partition by (trace_id) | fields _time, trace_id | sort by (_time) desc`, + } { + if _, err := logstorage.ParseQueryAtTimestamp(qStr, ts); err != nil { + t.Fatalf("parse %q: %v", qStr, err) + } + } +} diff --git a/apptest/tests/otlp_ingestion_test.go b/apptest/tests/otlp_ingestion_test.go index 5355de4bf..481d9df0f 100644 --- a/apptest/tests/otlp_ingestion_test.go +++ b/apptest/tests/otlp_ingestion_test.go @@ -310,6 +310,52 @@ func getDefaultIngestRequestAndAssertFunc(tc *at.TestCase, sut at.VictoriaTraces cmpopts.IgnoreFields(at.JaegerAPITracesResponse{}, "Errors", "Limit", "Offset", "Total"), }, }) + + // inverted regex (Jaeger encodes key!=~pattern as {"key!": "~pattern"}): inner regex does not match tag -> trace included + tc.Assert(&at.AssertOptions{ + Msg: "unexpected /select/jaeger/api/traces response (inverted regex, non-matching pattern)", + Got: func() any { + return sut.JaegerAPITraces(t, at.JaegerQueryParam{ + TraceQueryParam: query.TraceQueryParam{ + ServiceName: serviceName, + StartTimeMin: spanTime.Add(-10 * time.Minute), + StartTimeMax: spanTime.Add(10 * time.Minute), + Attributes: map[string]string{ + "testTag!": "~other.*", + }, + }, + }, at.QueryOpts{}) + }, + Want: &at.JaegerAPITracesResponse{ + Data: expectTraceData, + }, + CmpOpts: []cmp.Option{ + cmpopts.IgnoreFields(at.JaegerAPITracesResponse{}, "Errors", "Limit", "Offset", "Total"), + }, + }) + + // inverted regex: inner regex matches tag -> trace excluded + tc.Assert(&at.AssertOptions{ + Msg: "unexpected /select/jaeger/api/traces response (inverted regex, matching pattern)", + Got: func() any { + return sut.JaegerAPITraces(t, at.JaegerQueryParam{ + TraceQueryParam: query.TraceQueryParam{ + ServiceName: serviceName, + StartTimeMin: spanTime.Add(-10 * time.Minute), + StartTimeMax: spanTime.Add(10 * time.Minute), + Attributes: map[string]string{ + "testTag!": "~test.*", + }, + }, + }, at.QueryOpts{}) + }, + Want: &at.JaegerAPITracesResponse{ + Data: []at.TracesResponseData{}, + }, + CmpOpts: []cmp.Option{ + cmpopts.IgnoreFields(at.JaegerAPITracesResponse{}, "Errors", "Limit", "Offset", "Total"), + }, + }) } return req, assertFunc } diff --git a/docs/victoriatraces/changelog/CHANGELOG.md b/docs/victoriatraces/changelog/CHANGELOG.md index 5b21d2873..800c36a1b 100644 --- a/docs/victoriatraces/changelog/CHANGELOG.md +++ b/docs/victoriatraces/changelog/CHANGELOG.md @@ -12,6 +12,8 @@ The following `tip` changes can be tested by building VictoriaTraces components ## tip +* FEATURE: [Single-node VictoriaTraces](https://docs.victoriametrics.com/victoriatraces/) and vtselect in [VictoriaTraces cluster](https://docs.victoriametrics.com/victoriatraces/cluster/): support inverted tag filters in the Jaeger query API. Jaeger UI encodes `key!=value` as JSON `{"key!": "value"}` (and regex with `~` as in positive filters). See [#120](https://github.com/VictoriaMetrics/VictoriaTraces/issues/120). + * BUGFIX: [Single-node VictoriaTraces](https://docs.victoriametrics.com/victoriatraces/) and vtinsert in [VictoriaTraces cluster](https://docs.victoriametrics.com/victoriatraces/cluster/): fix OTLP/gRPC failure with TLS enabled during HTTP/2 ALPN negotiation. See [#108](https://github.com/VictoriaMetrics/VictoriaTraces/issues/108) for details. Thank @hklhai for [the pull request #136](https://github.com/VictoriaMetrics/VictoriaTraces/pull/136). ## [v0.8.1](https://github.com/VictoriaMetrics/VictoriaTraces/releases/tag/v0.8.1)