-
Notifications
You must be signed in to change notification settings - Fork 4
Add promql support for the prequel product. #25
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
Conversation
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.
Pull Request Overview
This PR adds support for PromQL (Prometheus Query Language) expressions to the prequel compiler, enabling metric-based correlation rules alongside existing log-based rules.
- Introduces
NodeTypePromQLschema type and parsing infrastructure for PromQL expressions - Implements AST building and validation for PromQL nodes with interval and duration support
- Refactors node type checking into reusable helper methods (
IsMatcherNode,IsPromNode)
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/schema/schema.go | Adds NodeTypePromQL constant, replacing unused NodeTypeDesc |
| pkg/parser/parse.go | Adds ParsePromQL struct and updates ParseTermT to support PromQL terms |
| pkg/parser/tree.go | Implements PromQL parsing with nodeFromProm, adds type-checking helpers, refactors node assignment logic |
| pkg/parser/parse_test.go | Adds test case for PromQL parsing and fixes whitespace formatting |
| pkg/ast/ast.go | Updates AST builder to handle PromQL nodes and adds PromQL to valid node types |
| pkg/ast/ast_metrics.go | New file implementing PromQL AST node builder with event and timing support |
| pkg/ast/ast_machine.go | Adds PromQL case to machine node builder |
| pkg/ast/ast_log.go | Minor whitespace cleanup |
| pkg/ast/ast_test.go | Adds PromQL test case and updates line number for test after testdata changes |
| pkg/testdata/rules.go | Removes unnecessary event sources from error test cases and adds PromQL test data |
| pkg/datasrc/parse.go | Upgrades yaml dependency from v2 to v3 |
| go.mod / go.sum | Removes yaml.v2 dependency, retaining yaml.v3 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if promMatcher, err := b.buildPromQLNode(parserNode, machineAddress, nil); err != nil { | ||
| return nil, err | ||
| } else { | ||
| matchNode.Object = promMatcher |
Copilot
AI
Nov 18, 2025
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.
Inconsistent pattern: buildPromQLNode returns *AstNodeT while buildSeqMatcher and buildSetMatcher return *AstSeqMatcherT and *AstSetMatcherT respectively. Assigning the entire node (promMatcher) to matchNode.Object is incorrect - it should assign promMatcher.Object (which contains the *AstPromQL) to be consistent with the other cases. Alternatively, buildPromQLNode should be refactored to return *AstPromQL directly like the other builder functions.
| matchNode.Object = promMatcher | |
| matchNode.Object = promMatcher.Object |
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.
Not sure I did this right. @tonymeehan ?
|
|
||
| // Build children (either matcher children or nested machines) | ||
| if isMatcherNode(parserNode) { | ||
| if parserNode.IsMatcherNode() { |
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.
nice
No description provided.