-
Notifications
You must be signed in to change notification settings - Fork 528
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 - Add support for scoped intrinsics using :
#3629
Conversation
:
docs/sources/tempo/traceql/_index.md
Outdated
@@ -95,6 +95,22 @@ these intrinsics are significantly more performant because they have to inspect | |||
possible to span-level intrinsics. | |||
{{% /admonition %}} | |||
|
|||
### Scoped Intrinsic Fields |
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.
Changing this to sentence case to match our style
### Scoped Intrinsic Fields | |
### Scoped intrinsic fields |
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.
looking good! a few comments but this is close to mergeable.
after this is merged please also file an issue in github.com/grafana/grafana requesting support for the new intrinsics. let me know and i will help you tag it correctly.
pkg/traceql/expr.y
Outdated
// trace: | ||
TRACE_COLON IDURATION { $$ = NewIntrinsic(IntrinsicTraceDuration) } | ||
| TRACE_COLON ROOTNAME { $$ = NewIntrinsic(IntrinsicTraceRootSpan) } | ||
| TRACE_COLON ROOTSERVICENAME { $$ = NewIntrinsic(IntrinsicTraceRootService) } |
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.
let's add a new token and do trace:rootService
like we spec'ed out here:
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.
asking for one small comment added, but otherwise lgtm.
great work!
@@ -167,8 +170,9 @@ selectOperation: | |||
; | |||
|
|||
attribute: | |||
intrinsicField { $$ = $1 } | |||
| attributeField { $$ = $1 } | |||
intrinsicField { $$ = $1 } |
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.
can we add a comment above the intrinsicField
definition that we are no longer adding these?
* add scoped intrinsics support : * lint * test & changelog * more test & changelog & doc * rename * add comment
What this PR does: Add support for scoped intrinsics using
:
. Users can now make queries like so:{span:duration > 100ms}
. Which is different from{span.duration > 100ms}
as the:
signifies that we are looking at the duration of the span where as the.
will signify that it is an attribute nameduration
of a span.This is the change will allow us to additionally features to Traceql like
trace:id
andlink:spanId
in the future.Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]