Skip to content

Commit a79085c

Browse files
afhassanafhassan
authored
Add holt_winters backwards compatibility in Cortex parser (#7223)
* add holt_winters to cortex parser Signed-off-by: Ahmed Hassan <afayekhassan@gmail.com> * refactor cortex parser setup Signed-off-by: Ahmed Hassan <afayekhassan@gmail.com> * fix holt winters backward compatibility test Signed-off-by: Ahmed Hassan <afayekhassan@gmail.com> * add query parsing validation in QFE Signed-off-by: Ahmed Hassan <afayekhassan@gmail.com> --------- Signed-off-by: Ahmed Hassan <afayekhassan@gmail.com>
1 parent cbd4c5f commit a79085c

5 files changed

Lines changed: 52 additions & 30 deletions

File tree

‎pkg/cortex/cortex.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040
"github.com/cortexproject/cortex/pkg/ingester/client"
4141
"github.com/cortexproject/cortex/pkg/overrides"
4242
"github.com/cortexproject/cortex/pkg/parquetconverter"
43+
cortexparser "github.com/cortexproject/cortex/pkg/parser"
4344
"github.com/cortexproject/cortex/pkg/querier"
4445
"github.com/cortexproject/cortex/pkg/querier/tenantfederation"
4546
"github.com/cortexproject/cortex/pkg/querier/tripperware"
@@ -553,7 +554,5 @@ func (t *Cortex) readyHandler(sm *services.Manager) http.HandlerFunc {
553554
}
554555

555556
func (t *Cortex) setupPromQLFunctions() {
556-
// The holt_winters function is renamed to double_exponential_smoothing and has been experimental since Prometheus v3. (https://github.com/prometheus/prometheus/pull/14930)
557-
// The cortex supports holt_winters for users using this function.
558-
querier.EnableExperimentalPromQLFunctions(t.Cfg.Querier.EnablePromQLExperimentalFunctions, true)
557+
cortexparser.Setup(t.Cfg.Querier.EnablePromQLExperimentalFunctions, true)
559558
}

‎pkg/parser/parser.go‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,41 @@ package parser
33
import (
44
"maps"
55

6+
"github.com/prometheus/prometheus/promql"
67
promqlparser "github.com/prometheus/prometheus/promql/parser"
78
"github.com/thanos-io/promql-engine/execution/parse"
89
)
910

10-
var functions = buildFunctions()
11+
var functions = buildFunctions(true)
1112

12-
func buildFunctions() map[string]*promqlparser.Function {
13+
func Setup(enableExperimentalFunctions, enableHoltWinters bool) {
14+
promqlparser.EnableExperimentalFunctions = enableExperimentalFunctions
15+
buildFunctions(enableHoltWinters)
16+
}
17+
18+
func buildFunctions(enableHoltWinters bool) map[string]*promqlparser.Function {
1319
fns := make(map[string]*promqlparser.Function, len(promqlparser.Functions))
1420
maps.Copy(fns, promqlparser.Functions)
1521
maps.Copy(fns, parse.XFunctions)
22+
23+
// The holt_winters function was renamed to double_exponential_smoothing and marked experimental in Prometheus v3.
24+
// Register holt_winters as an alias to maintain backward compatibility.
25+
if enableHoltWinters {
26+
if des, ok := fns["double_exponential_smoothing"]; ok {
27+
holtWinters := *des
28+
holtWinters.Experimental = false
29+
holtWinters.Name = "holt_winters"
30+
fns["holt_winters"] = &holtWinters
31+
32+
// Also register in global Prometheus parser for engine execution
33+
promqlparser.Functions["holt_winters"] = &holtWinters
34+
promql.FunctionCalls["holt_winters"] = promql.FunctionCalls["double_exponential_smoothing"]
35+
}
36+
} else {
37+
delete(promqlparser.Functions, "holt_winters")
38+
delete(promql.FunctionCalls, "holt_winters")
39+
}
40+
1641
return fns
1742
}
1843

‎pkg/querier/querier.go‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"github.com/prometheus/common/model"
1818
"github.com/prometheus/prometheus/model/labels"
1919
"github.com/prometheus/prometheus/promql"
20-
"github.com/prometheus/prometheus/promql/parser"
2120
"github.com/prometheus/prometheus/storage"
2221
"github.com/prometheus/prometheus/util/annotations"
2322
"github.com/thanos-io/thanos/pkg/strutil"
@@ -719,15 +718,3 @@ func validateQueryTimeRange(ctx context.Context, userID string, startMs, endMs i
719718

720719
return int64(startTime), int64(endTime), nil
721720
}
722-
723-
func EnableExperimentalPromQLFunctions(enablePromQLExperimentalFunctions, enableHoltWinters bool) {
724-
parser.EnableExperimentalFunctions = enablePromQLExperimentalFunctions
725-
726-
if enableHoltWinters {
727-
holtWinters := *parser.Functions["double_exponential_smoothing"]
728-
holtWinters.Experimental = false
729-
holtWinters.Name = "holt_winters"
730-
parser.Functions["holt_winters"] = &holtWinters
731-
promql.FunctionCalls["holt_winters"] = promql.FunctionCalls["double_exponential_smoothing"]
732-
}
733-
}

‎pkg/querier/querier_test.go‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
promchunk "github.com/cortexproject/cortex/pkg/chunk/encoding"
3535
"github.com/cortexproject/cortex/pkg/cortexpb"
3636
"github.com/cortexproject/cortex/pkg/ingester/client"
37+
cortexparser "github.com/cortexproject/cortex/pkg/parser"
3738
"github.com/cortexproject/cortex/pkg/querier/batch"
3839
"github.com/cortexproject/cortex/pkg/querier/series"
3940
"github.com/cortexproject/cortex/pkg/util"
@@ -1693,32 +1694,34 @@ func TestConfig_Validate(t *testing.T) {
16931694
}
16941695

16951696
func Test_EnableExperimentalPromQLFunctions(t *testing.T) {
1696-
EnableExperimentalPromQLFunctions(true, true)
1697+
cortexparser.Setup(true, true)
1698+
defer cortexparser.Setup(false, false)
16971699

1698-
// holt_winters function should exist
1699-
holtWintersFunc, ok := parser.Functions["holt_winters"]
1700-
require.True(t, ok)
1700+
// Verify experimental functions flag is set
1701+
require.True(t, parser.EnableExperimentalFunctions)
17011702

17021703
// double_exponential_smoothing function should exist
17031704
doubleExponentialSmoothingFunc, ok := parser.Functions["double_exponential_smoothing"]
17041705
require.True(t, ok)
1706+
require.Equal(t, "double_exponential_smoothing", doubleExponentialSmoothingFunc.Name)
1707+
require.True(t, parser.Functions["double_exponential_smoothing"].Experimental)
17051708

1706-
// holt_winters should not experimental function.
1709+
// holt_winters should exist
1710+
holtWintersFunc, ok := parser.Functions["holt_winters"]
1711+
require.True(t, ok)
1712+
require.Equal(t, "holt_winters", holtWintersFunc.Name)
17071713
require.False(t, holtWintersFunc.Experimental)
1708-
// holt_winters's Variadic, ReturnType, and ArgTypes are the same as the double_exponential_smoothing.
1714+
1715+
// holt_winters's Variadic, ReturnType, and ArgTypes are the same as the double_exponential_smoothing
17091716
require.Equal(t, doubleExponentialSmoothingFunc.Variadic, holtWintersFunc.Variadic)
17101717
require.Equal(t, doubleExponentialSmoothingFunc.ReturnType, holtWintersFunc.ReturnType)
17111718
require.Equal(t, doubleExponentialSmoothingFunc.ArgTypes, holtWintersFunc.ArgTypes)
17121719

1713-
// double_exponential_smoothing shouldn't be changed.
1714-
require.Equal(t, "double_exponential_smoothing", doubleExponentialSmoothingFunc.Name)
1715-
require.True(t, parser.Functions["double_exponential_smoothing"].Experimental)
1716-
1717-
// holt_winters function calls should exist.
1720+
// holt_winters function calls should exist
17181721
_, ok = promql.FunctionCalls["holt_winters"]
17191722
require.True(t, ok)
17201723

1721-
// double_exponential_smoothing function calls should exist.
1724+
// double_exponential_smoothing function calls should exist
17221725
_, ok = promql.FunctionCalls["double_exponential_smoothing"]
17231726
require.True(t, ok)
17241727
}

‎pkg/querier/tripperware/roundtrip.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"github.com/weaveworks/common/httpgrpc"
3232
"github.com/weaveworks/common/user"
3333

34+
cortexparser "github.com/cortexproject/cortex/pkg/parser"
3435
"github.com/cortexproject/cortex/pkg/util"
3536
"github.com/cortexproject/cortex/pkg/util/limiter"
3637
"github.com/cortexproject/cortex/pkg/util/requestmeta"
@@ -190,6 +191,13 @@ func NewQueryTripperware(
190191
source := GetSource(r)
191192
queriesPerTenant.WithLabelValues(op, source, userStr).Inc()
192193

194+
if isQuery || isQueryRange {
195+
query := r.FormValue("query")
196+
if _, err := cortexparser.ParseExpr(query); err != nil {
197+
return nil, httpgrpc.Errorf(http.StatusBadRequest, "%s", err.Error())
198+
}
199+
}
200+
193201
if maxSubQuerySteps > 0 && (isQuery || isQueryRange) {
194202
query := r.FormValue("query")
195203
// Check subquery step size.

0 commit comments

Comments
 (0)