feat: detect and validate inner queries in default_zero() functions #23
Conversation
- Add query parsing to detect default_zero() usage and extract inner queries - Validate inner queries separately to catch metrics masked by default_zero() - Support nested default_zero() calls with proper parentheses matching - Fail linting when default_zero() masks invalid metrics or syntax errors - Add comprehensive test suite with edge cases and nested scenarios - Enhance logging with debug info for query analysis and validation BREAKING CHANGE: Linter now fails for invalid metrics wrapped in default_zero() that previously passed validation Resolves issue where default_zero() masked invalid metrics in CI pipelines
| @@ -0,0 +1,67 @@ | |||
| # Enhanced default_zero() Validation | |||
There was a problem hiding this comment.
Goose added this file. Not sure we like it? Up to you @kuzmik.
There was a problem hiding this comment.
Does it do anything with the agent/LLMs performance or is this for humans?
There was a problem hiding this comment.
i don't think it helps the LLM at all
|
Process I used:
It was on auto-pilot mode (I think they call it "smart approval") and did not stop to ask me for anything until it was done generating the code, the tests, running the tests, and writing a summary of what it had done. I was using |
…tion
- Add MetricInfo struct to track individual metrics with positioning and nesting info
- Enhance QueryAnalysis to support complex queries with IsComplexQuery flag and Metrics slice
- Implement isComplexQuery() to detect mathematical operations and multiple metrics
- Add extractAllMetrics() to parse and extract all metrics from complex expressions
- Add extractDefaultZeroMetrics() with overlap prevention for nested default_zero() calls
- Add extractRemainingMetrics() to find metrics not wrapped in default_zero()
- Enhance main validation logic to validate each metric individually in complex queries
- Add comprehensive test coverage for multi-metric scenarios
- Support complex queries like: (default_zero(default_zero(avg:metric1{tag}))+default_zero(avg:metric2{tag}))/default_zero(avg:metric3{tag})
- Maintain full backward compatibility with existing single-metric functionality
- Prevent invalid metrics from hiding behind default_zero() in mathematical expressions
Test files added:
- datadogmetric-multi-metric-complex.yaml: nested default_zero with multiple metrics
- datadogmetric-multi-metric-simple.yaml: simple addition of two metrics
- datadogmetric-mixed-metrics.yaml: mixed default_zero and non-default_zero metrics
All tests pass via 'make dockertest' with real Datadog API validation.
|
@kuzmik @4ndypanda the second commit to find multiple instances really blew up the complexity of the go code. I didn't read it very closely TBH. Would definitely not trust it without a good read from one of you. The first commit to catch the simpler cases seems good to me though. |
Looks like the agent is solving the problem by extracting all the individual metrics in the query, removing the Seems like an easier implementation is just replacing all |
|
@kuzmik @4ndypanda would you like me to have it do the |
|
Going to try this with Claude instead. |
BREAKING CHANGE: Linter now fails for invalid metrics wrapped in default_zero()
that previously passed validation
Resolves issue where default_zero() masked invalid metrics in CI pipelines