[pull] main from SigNoz:main - #943
Merged
Merged
Conversation
… segfault (#12720) #### Description - A PromQL subquery without a step, for example `max_over_time(metric[5m:])`, segfaulted the whole query-service. The engine calls `NoStepSubqueryIntervalFn` for such subqueries, and we build the engine without it, so the call hits a nil function. - The bug is present on every PromQL surface, because all of them share the one engine constructor in `pkg/prometheus/engine.go`: v3 and v5 `query_range`, `/api/v1/query`, the clickhousev2 transpiler, and promql alert rules. A saved rule with such a subquery crash-loops the instance on its own schedule. - The fix sets the callback to 1m. This matches the Prometheus default global `evaluation_interval`, which upstream wires into this field. One place fixes every path. - This is the root cause of the SigNoz/platform-pod#3068 incident. The instance-hardening request from that incident is tracked in SigNoz/pulse-pod#308. #### Issues closed by this PR Closes SigNoz/platform-pod#3068 #### Additional Information We audited `EngineOpts` for more bugs of the same class. `NoStepSubqueryIntervalFn` is the only field the engine calls without a nil guard; `promql.NewEngine` defaults the other nil-able fields (`Parser`, `FeatureRegistry`). The remaining gaps against upstream wiring are not crashes, and we filed them separately: SigNoz/pulse-pod#305 (`@` modifier and negative offset disabled), SigNoz/pulse-pod#306 (engine self-metrics not registered), SigNoz/pulse-pod#307 (active query tracker startup panic risk), SigNoz/pulse-pod#309 (step guard in the v3 cache), SigNoz/pulse-pod#310 (upstream proposal to fail fast on the nil callback). Tests for the bug: - `pkg/prometheus/engine_test.go` — fails with the exact segfault when the fix is removed. - `tests/integration/tests/promqlconformance/04_no_step_subquery.py` — a step-less subquery through `/api/v5/query_range` returns correct values on both providers, and the service stays up. - `tests/integration/tests/alerts/04_promql_subquery_no_step.py` — a promql alert rule with a step-less subquery evaluates and fires. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )