mirror of
https://github.com/SigNoz/signoz.git
synced 2026-09-27 13:50:41 +01:00
da9b4644df
Some checks failed
build-staging / prepare (push) Has been cancelled
cacheci / tests (push) Has been cancelled
Release Drafter / update_release_draft (push) Has been cancelled
build-staging / js-build (push) Has been cancelled
build-staging / go-build (push) Has been cancelled
build-staging / staging (push) Has been cancelled
#### 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)
35 lines
952 B
Go
35 lines
952 B
Go
package prometheus
|
|
|
|
import (
|
|
"log/slog"
|
|
"time"
|
|
|
|
"github.com/prometheus/prometheus/promql"
|
|
)
|
|
|
|
func NewEngine(logger *slog.Logger, cfg Config) *Engine {
|
|
var activeQueryTracker promql.QueryTracker
|
|
if cfg.ActiveQueryTrackerConfig.Enabled {
|
|
activeQueryTracker = promql.NewActiveQueryTracker(
|
|
cfg.ActiveQueryTrackerConfig.Path,
|
|
cfg.ActiveQueryTrackerConfig.MaxConcurrent,
|
|
logger,
|
|
)
|
|
}
|
|
|
|
return promql.NewEngine(promql.EngineOpts{
|
|
Logger: logger,
|
|
Reg: nil,
|
|
MaxSamples: 5_0000_000,
|
|
Timeout: cfg.Timeout,
|
|
ActiveQueryTracker: activeQueryTracker,
|
|
LookbackDelta: cfg.LookbackDelta,
|
|
// The engine calls this for subqueries that do not set a step, such as
|
|
// `metric[5m:]`, and segfaults if it is nil. 1m matches the default
|
|
// global evaluation_interval that Prometheus wires here.
|
|
NoStepSubqueryIntervalFn: func(int64) int64 {
|
|
return time.Minute.Milliseconds()
|
|
},
|
|
})
|
|
}
|