-
Notifications
You must be signed in to change notification settings - Fork 544
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
Refactoring: implement arrays for traceql.Static with reused fields #3827
Conversation
I ran a couple of more benchmarks and the results look pretty good. Here are the latest benchmarks: Metrics:
TraceQL
|
a1c2083
to
8d8000e
Compare
The new implementation is meant to support static values representing []int, []float, []bool, and []string in the future without increasing the struct size
8d8000e
to
84553e4
Compare
84553e4
to
281eefc
Compare
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.
Looks nice, awesome work
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.
A couple of qs, otherwise it LGTM
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.
a great direction to take this. two small comments, but overall much prefer this to the interface approach
What this PR does:
This PR adds support for the new types
TypeIntArray
,TypeFloatArray
,TypeStringArray
, andTypeBooleanArray
totraceql.Static
. This is a perquisite for supporting arrays in TraceQL.The goal of this approach is to add support for array types to
traceql.Static
without increasing the size of the existing struct. It makes use of packed/reused struct fields that store values of multiple different types. This is accomplished through heavy use of casts andunsafe
.The new
traceql.Static
type is no longer comparable and can no longer be used as key in a map. This PR therefore introduces a new typetraceql.StaticMapKey
.For context: a previous approach based on a
traceql.Static
interface with multiple implementations was explored in this PR.Which issue(s) this PR fixes:
Is prerequisite for https://github.com/grafana/tempo-squad/issues/435
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]