-
Notifications
You must be signed in to change notification settings - Fork 521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TraceQL link intrinsic link:traceID and link:spanID #3741
TraceQL link intrinsic link:traceID and link:spanID #3741
Conversation
1de8379
to
18440f2
Compare
Will we need doc for this? |
@@ -781,10 +785,13 @@ func checkConditions(conditions []traceql.Condition) error { | |||
} | |||
|
|||
// Check for conditions that are not supported in vParquet2 | |||
if cond.Attribute.Intrinsic == traceql.IntrinsicEventName { | |||
if cond.Attribute.Intrinsic == traceql.IntrinsicEventName || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At some point I would like to swap checkConditions to look for supported things, instead of unsupported things. Which is less brittle going forward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, it would be safer to check what is supported. I've made a note for myself to refactor this in a separate PR
@@ -1458,43 +1505,59 @@ func createAllIterator(ctx context.Context, primaryIter parquetquery.Iterator, c | |||
var ( | |||
// If there are only span conditions, then don't return a span upstream | |||
// unless it matches at least 1 span-level condition. | |||
spanRequireAtLeastOneMatch = len(spanConditions) > 0 && len(resourceConditions) == 0 && len(traceConditions) == 0 | |||
spanRequireAtLeastOneMatch = len(catConditions.span) > 0 && len(catConditions.resource) == 0 && len(catConditions.trace) == 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just fyi that these are removed in: #3734
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the heads up. I've resolved the conflicts 👍
Yes, we should have documentation for all TraceQL extensions. I added a documentation sub-task to the TraceQL additions epic |
What this PR does:
Add
link
scope and the intrinsicslink:traceID
andlink:spanID
.Autocomplete is currently not working, but is also part of a separate issue.
Which issue(s) this PR fixes:
Contributes to grafana/tempo-squad#433)
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]