-
Notifications
You must be signed in to change notification settings - Fork 535
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] Implement structural operators #2625
Conversation
…cision. Fix edge case, fix flakey tests
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 great! obvious asks: changelog and docs for the new traceql features
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-2625-to-r103 origin/r103
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x ac63a44eb82520d01c42bab70cfab64e4328c3fb
# When the conflicts are resolved, stage and commit the changes
git add . && git cherry-pick --continue If you have the GitHub CLI installed: # Create the PR body template
gh pr view 2625 --json body --template 'Backport ac63a44eb82520d01c42bab70cfab64e4328c3fb from #2625{{ "\n\n---\n\n" }}{{ index . "body" }}' > .pr-body.txt
# Push the branch to GitHub and a PR
gh pr create --title "[r103] [TraceQL] Implement structural operators" --body-file .pr-body.txt --label "type/bug" --label "backport" --base r103 --milestone r103 --web Or, if you don't have the GitHub CLI installed (we recommend you install it!): # If you don't have the GitHub CLI installed: Push the branch to GitHub and manually create a PR:
git push --set-upstream origin backport-2625-to-r103
# Remove the local backport branch
git switch main
git branch -D backport-2625-to-r103 Unless you've used the GitHub CLI above, now create a pull request where the |
* Dynamic metrics group by multiple attributes * changelog * review feedback * trim whitespace on groupBy values * Working descendant operator * Add child and sibling operators * Split intrinsic into separate per operator to allow better column precision. Fix edge case, fix flakey tests * lint * changelog * Add missing default scope entries * Fix dual import of parquetquery * Fail spanset structural if multiple spansets present, add tests, use buffer * Document structural operators * Review feedback, comments * lint * lint (cherry picked from commit ac63a44)
* Dynamic metrics group by multiple attributes * changelog * review feedback * trim whitespace on groupBy values * Working descendant operator * Add child and sibling operators * Split intrinsic into separate per operator to allow better column precision. Fix edge case, fix flakey tests * lint * changelog * Add missing default scope entries * Fix dual import of parquetquery * Fail spanset structural if multiple spansets present, add tests, use buffer * Document structural operators * Review feedback, comments * lint * lint (cherry picked from commit ac63a44) Co-authored-by: Martin Disibio <[email protected]>
What this PR does:
Implements descendant (>>), child (>), and sibling (~) structural operators in vParquet2. The TraceQL engine now passes new intrinsics to storage so it can load the minimum columns (each operator uses a different set of columns). The
Span
interface was updated with generic methodsDescendantOf(span)
that the engine calls, so it is agnostic to how this data is represented in the backend. I.e. we are not exposing the raw left/right/parent integers to the engine. There are pros and cons to this approach, it is more flexible and as we implement TraceQL on other formats (live traces?), I think it will be better. The drawback is it means a very naive algorithm in the engine to detect the relationships O(n * m). If the engine had the integers it could do fancy things like put them in a map or sorted slice. I think for most cases n and m will be small.Note: My formatter is now gofumpt, so there is a little bit of noise from the fumpting.
Which issue(s) this PR fixes:
Fixes ?? <-- If we have an issue to track structural operators I couldn't find it.
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]