Skip to content

lib/traceql: support unscoped attributes in TraceQL queries - #264

Open
niladrix719 wants to merge 1 commit into
VictoriaMetrics:masterfrom
niladrix719:unscoped-attr-traceql#198
Open

niladrix719 wants to merge 1 commit into
VictoriaMetrics:masterfrom
niladrix719:unscoped-attr-traceql#198

Conversation

@niladrix719

Copy link
Copy Markdown

Use the span attribute if the span has it, otherwise the resource attribute

.service.name is excluded so it keeps the stream filter fast path

Fixes #198

Signed-off-by: Niladri Adhikary <niladrix719@gmail.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="lib/traceql/filter_general.go">

<violation number="1" location="lib/traceql/filter_general.go:116">
P3: Dotted intrinsic names (`.name`, `.kind`, `.status`, `.duration`) now fall into this unscoped-attribute branch and are resolved as `span_attr:`/`resource_attr:` attribute lookups, while the same keywords without the dot resolve to the span name stream field (`name`), `status_code`, `kind`, or the `duration` field. `.status = "error"` will never match the status intrinsic, and a span whose literal attribute is named `status` would now match where it previously didn't. Extend the special-case beyond `.service.name` (or remap these dotted forms to the intrinsic fields) so unscoped intrinsics keep intrinsic semantics.</violation>

<violation number="2" location="lib/traceql/filter_general.go:120">
P3: For nil-value ops the generated expression contains a dead sub-filter: `{.foo != nil}` emits `("span_attr:foo":* and "span_attr:foo":* or ...)` (the presence guard is duplicated) and `{.foo = nil}` emits `("span_attr:foo":* and "span_attr:foo":"" or ...)`, where the first `and` branch is never satisfiable and the second branch already requires `"span_attr:foo":""`. Emit the span filter without re-adding the guard when the filter already encodes presence/absence (nil ops), e.g. `"span_attr:foo":"" and "resource_attr:foo":""` for `= nil`.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

spanFilter := filterCommon{fieldName: "span." + attr, op: fc.op, value: fc.value}
resourceFilter := filterCommon{fieldName: "resource." + attr, op: fc.op, value: fc.value}
spanField := quoteFieldNameIfNeeded(otelpb.SpanAttrPrefixField + attr)
return "(" + spanField + ":* and " + spanFilter.String() + " or " + spanField + `:"" and ` + resourceFilter.String() + ")"

@cubic-dev-ai cubic-dev-ai Bot Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: For nil-value ops the generated expression contains a dead sub-filter: {.foo != nil} emits ("span_attr:foo":* and "span_attr:foo":* or ...) (the presence guard is duplicated) and {.foo = nil} emits ("span_attr:foo":* and "span_attr:foo":"" or ...), where the first and branch is never satisfiable and the second branch already requires "span_attr:foo":"". Emit the span filter without re-adding the guard when the filter already encodes presence/absence (nil ops), e.g. "span_attr:foo":"" and "resource_attr:foo":"" for = nil.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At lib/traceql/filter_general.go, line 120:

<comment>For nil-value ops the generated expression contains a dead sub-filter: `{.foo != nil}` emits `("span_attr:foo":* and "span_attr:foo":* or ...)` (the presence guard is duplicated) and `{.foo = nil}` emits `("span_attr:foo":* and "span_attr:foo":"" or ...)`, where the first `and` branch is never satisfiable and the second branch already requires `"span_attr:foo":""`. Emit the span filter without re-adding the guard when the filter already encodes presence/absence (nil ops), e.g. `"span_attr:foo":"" and "resource_attr:foo":""` for `= nil`.</comment>

<file context>
@@ -111,6 +111,15 @@ func (fc *filterCommon) String() string {
+		spanFilter := filterCommon{fieldName: "span." + attr, op: fc.op, value: fc.value}
+		resourceFilter := filterCommon{fieldName: "resource." + attr, op: fc.op, value: fc.value}
+		spanField := quoteFieldNameIfNeeded(otelpb.SpanAttrPrefixField + attr)
+		return "(" + spanField + ":* and " + spanFilter.String() + " or " + spanField + `:"" and ` + resourceFilter.String() + ")"
+	}
+
</file context>
Fix with cubic

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's harmless


// Unscoped attribute `.foo` reads the span attribute if the span has it, and the resource attribute otherwise,
// `.service.name` is mapped to the resource stream field below.
if attr, ok := strings.CutPrefix(fc.fieldName, "."); ok && fc.fieldName != ".service.name" {

@cubic-dev-ai cubic-dev-ai Bot Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Dotted intrinsic names (.name, .kind, .status, .duration) now fall into this unscoped-attribute branch and are resolved as span_attr:/resource_attr: attribute lookups, while the same keywords without the dot resolve to the span name stream field (name), status_code, kind, or the duration field. .status = "error" will never match the status intrinsic, and a span whose literal attribute is named status would now match where it previously didn't. Extend the special-case beyond .service.name (or remap these dotted forms to the intrinsic fields) so unscoped intrinsics keep intrinsic semantics.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At lib/traceql/filter_general.go, line 116:

<comment>Dotted intrinsic names (`.name`, `.kind`, `.status`, `.duration`) now fall into this unscoped-attribute branch and are resolved as `span_attr:`/`resource_attr:` attribute lookups, while the same keywords without the dot resolve to the span name stream field (`name`), `status_code`, `kind`, or the `duration` field. `.status = "error"` will never match the status intrinsic, and a span whose literal attribute is named `status` would now match where it previously didn't. Extend the special-case beyond `.service.name` (or remap these dotted forms to the intrinsic fields) so unscoped intrinsics keep intrinsic semantics.</comment>

<file context>
@@ -111,6 +111,15 @@ func (fc *filterCommon) String() string {
 
+	// Unscoped attribute `.foo` reads the span attribute if the span has it, and the resource attribute otherwise,
+	// `.service.name` is mapped to the resource stream field below.
+	if attr, ok := strings.CutPrefix(fc.fieldName, "."); ok && fc.fieldName != ".service.name" {
+		spanFilter := filterCommon{fieldName: "span." + attr, op: fc.op, value: fc.value}
+		resourceFilter := filterCommon{fieldName: "resource." + attr, op: fc.op, value: fc.value}
</file context>
Fix with cubic

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Tempo, a leading . means an attribute rather than an intrinsic. I checked this against Tempo 3.1.0: {.status = "error"} matches an attribute named status, while bare {status = error} matches the built-in status field, which is the same behavior we have here

@Vandit1604 Vandit1604 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the case in the issue. I tried it with spans that have istio.cluster_id on the span, on the resource, or on both. Each one came back as expected, with the span value taking priority like in Tempo.

The Kiali query from the issue, with | select(...) at the end, works too. TempoAPISearch in apptest/vtsingle.go on master could turn these cases into a test of the returned spans.

I left one note on line 118 about the instrumentation scope.

// `.service.name` is mapped to the resource stream field below.
if attr, ok := strings.CutPrefix(fc.fieldName, "."); ok && fc.fieldName != ".service.name" {
spanFilter := filterCommon{fieldName: "span." + attr, op: fc.op, value: fc.value}
resourceFilter := filterCommon{fieldName: "resource." + attr, op: fc.op, value: fc.value}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tempo also looks at the instrumentation scope after the resource (protospan.go). So a span that has istio.cluster_id = "c1" only on its scope is missed by =. It also shows up for = nil and != "c1".

The issue only asks for span and resource, so this is fine as a follow-up. If you want to add it here, a third branch on scope_attr: should work, since {instrumentation.istio.cluster_id = "c1"} already returns that span.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wrong here, sorry. I ran Tempo 3.1.0 with the same spans, and {.istio.cluster_id = "c1"} returns only the span and resource matches. The scope-only span comes back only for {instrumentation.istio.cluster_id = "c1"}, so the PR already matches Tempo and you can ignore this note.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tempo API: unscoped TraceQL attribute selectors (.attr) return no results — breaks Kiali tracing integration

2 participants