From 5f6bf93dd507e92378cce7c2e67add6ccf4bd345 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Fri, 1 Mar 2024 09:38:32 -0800 Subject: [PATCH] Expressions: Use enumerations rather than strings (#83741) --- pkg/expr/classic/classic.go | 15 +++++++-- pkg/expr/commands.go | 12 ++++--- pkg/expr/mathexp/resample.go | 33 ++++++++++++++----- pkg/expr/mathexp/resample_test.go | 4 +-- pkg/expr/models.go | 4 +-- pkg/expr/reader.go | 2 +- pkg/expr/threshold.go | 28 ++++++++++------ pkg/expr/threshold_test.go | 16 ++++----- pkg/services/ngalert/backtesting/eval_data.go | 4 +-- 9 files changed, 77 insertions(+), 41 deletions(-) diff --git a/pkg/expr/classic/classic.go b/pkg/expr/classic/classic.go index b920fc8a2f4..bcb99b24d7d 100644 --- a/pkg/expr/classic/classic.go +++ b/pkg/expr/classic/classic.go @@ -54,7 +54,7 @@ type condition struct { // Operator is the logical operator to use when there are two conditions in ConditionsCmd. // If there are more than two conditions in ConditionsCmd then operator is used to compare // the outcome of this condition with that of the condition before it. - Operator string + Operator ConditionOperatorType } // NeedsVars returns the variable names (refIds) that are dependencies @@ -216,7 +216,7 @@ func (cmd *ConditionsCmd) executeCond(_ context.Context, _ time.Time, cond condi return isCondFiring, isCondNoData, matches, nil } -func compareWithOperator(b1, b2 bool, operator string) bool { +func compareWithOperator(b1, b2 bool, operator ConditionOperatorType) bool { if operator == "or" { return b1 || b2 } else { @@ -262,8 +262,17 @@ type ConditionEvalJSON struct { Type string `json:"type"` // e.g. "gt" } +// The reducer function +// +enum +type ConditionOperatorType string + +const ( + ConditionOperatorAnd ConditionOperatorType = "and" + ConditionOperatorOr ConditionOperatorType = "or" +) + type ConditionOperatorJSON struct { - Type string `json:"type"` + Type ConditionOperatorType `json:"type"` } type ConditionQueryJSON struct { diff --git a/pkg/expr/commands.go b/pkg/expr/commands.go index 1f8926102c4..9e1c4958597 100644 --- a/pkg/expr/commands.go +++ b/pkg/expr/commands.go @@ -205,14 +205,14 @@ func (gr *ReduceCommand) Execute(ctx context.Context, _ time.Time, vars mathexp. type ResampleCommand struct { Window time.Duration VarToResample string - Downsampler string - Upsampler string + Downsampler mathexp.ReducerID + Upsampler mathexp.Upsampler TimeRange TimeRange refID string } // NewResampleCommand creates a new ResampleCMD. -func NewResampleCommand(refID, rawWindow, varToResample string, downsampler string, upsampler string, tr TimeRange) (*ResampleCommand, error) { +func NewResampleCommand(refID, rawWindow, varToResample string, downsampler mathexp.ReducerID, upsampler mathexp.Upsampler, tr TimeRange) (*ResampleCommand, error) { // TODO: validate reducer here, before execution window, err := gtime.ParseDuration(rawWindow) if err != nil { @@ -271,7 +271,11 @@ func UnmarshalResampleCommand(rn *rawNode) (*ResampleCommand, error) { return nil, fmt.Errorf("expected resample downsampler to be a string, got type %T", upsampler) } - return NewResampleCommand(rn.RefID, window, varToResample, downsampler, upsampler, rn.TimeRange) + return NewResampleCommand(rn.RefID, window, + varToResample, + mathexp.ReducerID(downsampler), + mathexp.Upsampler(upsampler), + rn.TimeRange) } // NeedsVars returns the variable names (refIds) that are dependencies diff --git a/pkg/expr/mathexp/resample.go b/pkg/expr/mathexp/resample.go index f6239be59f9..69c9a3db548 100644 --- a/pkg/expr/mathexp/resample.go +++ b/pkg/expr/mathexp/resample.go @@ -7,8 +7,23 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/data" ) +// The upsample function +// +enum +type Upsampler string + +const ( + // Use the last seen value + UpsamplerPad Upsampler = "pad" + + // backfill + UpsamplerBackfill Upsampler = "backfilling" + + // Do not fill values (nill) + UpsamplerFillNA Upsampler = "fillna" +) + // Resample turns the Series into a Number based on the given reduction function -func (s Series) Resample(refID string, interval time.Duration, downsampler string, upsampler string, from, to time.Time) (Series, error) { +func (s Series) Resample(refID string, interval time.Duration, downsampler ReducerID, upsampler Upsampler, from, to time.Time) (Series, error) { newSeriesLength := int(float64(to.Sub(from).Nanoseconds()) / float64(interval.Nanoseconds())) if newSeriesLength <= 0 { return s, fmt.Errorf("the series cannot be sampled further; the time range is shorter than the interval") @@ -37,19 +52,19 @@ func (s Series) Resample(refID string, interval time.Duration, downsampler strin var value *float64 if len(vals) == 0 { // upsampling switch upsampler { - case "pad": + case UpsamplerPad: if lastSeen != nil { value = lastSeen } else { value = nil } - case "backfilling": + case UpsamplerBackfill: if sIdx == s.Len() { // no vals left value = nil } else { _, value = s.GetPoint(sIdx) } - case "fillna": + case UpsamplerFillNA: value = nil default: return s, fmt.Errorf("upsampling %v not implemented", upsampler) @@ -61,15 +76,15 @@ func (s Series) Resample(refID string, interval time.Duration, downsampler strin ff := Float64Field(*fVec) var tmp *float64 switch downsampler { - case "sum": + case ReducerSum: tmp = Sum(&ff) - case "mean": + case ReducerMean: tmp = Avg(&ff) - case "min": + case ReducerMin: tmp = Min(&ff) - case "max": + case ReducerMax: tmp = Max(&ff) - case "last": + case ReducerLast: tmp = Last(&ff) default: return s, fmt.Errorf("downsampling %v not implemented", downsampler) diff --git a/pkg/expr/mathexp/resample_test.go b/pkg/expr/mathexp/resample_test.go index a09ae962266..afee71428b4 100644 --- a/pkg/expr/mathexp/resample_test.go +++ b/pkg/expr/mathexp/resample_test.go @@ -13,8 +13,8 @@ func TestResampleSeries(t *testing.T) { var tests = []struct { name string interval time.Duration - downsampler string - upsampler string + downsampler ReducerID + upsampler Upsampler timeRange backend.TimeRange seriesToResample Series series Series diff --git a/pkg/expr/models.go b/pkg/expr/models.go index dec7e3ce3a0..6e7f0919adf 100644 --- a/pkg/expr/models.go +++ b/pkg/expr/models.go @@ -54,10 +54,10 @@ type ResampleQuery struct { Window string `json:"window" jsonschema:"minLength=1,example=1w,example=10m"` // The downsample function - Downsampler string `json:"downsampler"` + Downsampler mathexp.ReducerID `json:"downsampler"` // The upsample function - Upsampler string `json:"upsampler"` + Upsampler mathexp.Upsampler `json:"upsampler"` } type ThresholdQuery struct { diff --git a/pkg/expr/reader.go b/pkg/expr/reader.go index b92e7044859..f4ca1cde5fd 100644 --- a/pkg/expr/reader.go +++ b/pkg/expr/reader.go @@ -156,7 +156,7 @@ func (h *ExpressionQueryReader) ReadQuery( } func getReferenceVar(exp string, refId string) (string, error) { - exp = strings.TrimPrefix(exp, "%") + exp = strings.TrimPrefix(exp, "$") if exp == "" { return "", fmt.Errorf("no variable specified to reference for refId %v", refId) } diff --git a/pkg/expr/threshold.go b/pkg/expr/threshold.go index f6fd093a27d..8a8f9ea5417 100644 --- a/pkg/expr/threshold.go +++ b/pkg/expr/threshold.go @@ -18,23 +18,31 @@ import ( type ThresholdCommand struct { ReferenceVar string RefID string - ThresholdFunc string + ThresholdFunc ThresholdType Conditions []float64 Invert bool } +// +enum +type ThresholdType string + const ( - ThresholdIsAbove = "gt" - ThresholdIsBelow = "lt" - ThresholdIsWithinRange = "within_range" - ThresholdIsOutsideRange = "outside_range" + ThresholdIsAbove ThresholdType = "gt" + ThresholdIsBelow ThresholdType = "lt" + ThresholdIsWithinRange ThresholdType = "within_range" + ThresholdIsOutsideRange ThresholdType = "outside_range" ) var ( - supportedThresholdFuncs = []string{ThresholdIsAbove, ThresholdIsBelow, ThresholdIsWithinRange, ThresholdIsOutsideRange} + supportedThresholdFuncs = []string{ + string(ThresholdIsAbove), + string(ThresholdIsBelow), + string(ThresholdIsWithinRange), + string(ThresholdIsOutsideRange), + } ) -func NewThresholdCommand(refID, referenceVar, thresholdFunc string, conditions []float64) (*ThresholdCommand, error) { +func NewThresholdCommand(refID, referenceVar string, thresholdFunc ThresholdType, conditions []float64) (*ThresholdCommand, error) { switch thresholdFunc { case ThresholdIsOutsideRange, ThresholdIsWithinRange: if len(conditions) < 2 { @@ -57,8 +65,8 @@ func NewThresholdCommand(refID, referenceVar, thresholdFunc string, conditions [ } type ConditionEvalJSON struct { - Params []float64 `json:"params"` - Type string `json:"type"` // e.g. "gt" + Params []float64 `json:"params"` + Type ThresholdType `json:"type"` // e.g. "gt" } // UnmarshalResampleCommand creates a ResampleCMD from Grafana's frontend query. @@ -121,7 +129,7 @@ func (tc *ThresholdCommand) Execute(ctx context.Context, now time.Time, vars mat } // createMathExpression converts all the info we have about a "threshold" expression in to a Math expression -func createMathExpression(referenceVar string, thresholdFunc string, args []float64, invert bool) (string, error) { +func createMathExpression(referenceVar string, thresholdFunc ThresholdType, args []float64, invert bool) (string, error) { var exp string switch thresholdFunc { case ThresholdIsAbove: diff --git a/pkg/expr/threshold_test.go b/pkg/expr/threshold_test.go index 18d1a19fb6f..c7e1c1f2eb6 100644 --- a/pkg/expr/threshold_test.go +++ b/pkg/expr/threshold_test.go @@ -14,7 +14,7 @@ import ( func TestNewThresholdCommand(t *testing.T) { type testCase struct { - fn string + fn ThresholdType args []float64 shouldError bool expectedError string @@ -107,7 +107,7 @@ func TestUnmarshalThresholdCommand(t *testing.T) { require.IsType(t, &ThresholdCommand{}, command) cmd := command.(*ThresholdCommand) require.Equal(t, []string{"A"}, cmd.NeedsVars()) - require.Equal(t, "gt", cmd.ThresholdFunc) + require.Equal(t, ThresholdIsAbove, cmd.ThresholdFunc) require.Equal(t, []float64{20.0, 80.0}, cmd.Conditions) }, }, @@ -172,10 +172,10 @@ func TestUnmarshalThresholdCommand(t *testing.T) { cmd := c.(*HysteresisCommand) require.Equal(t, []string{"B"}, cmd.NeedsVars()) require.Equal(t, []string{"B"}, cmd.LoadingThresholdFunc.NeedsVars()) - require.Equal(t, "gt", cmd.LoadingThresholdFunc.ThresholdFunc) + require.Equal(t, ThresholdIsAbove, cmd.LoadingThresholdFunc.ThresholdFunc) require.Equal(t, []float64{100.0}, cmd.LoadingThresholdFunc.Conditions) require.Equal(t, []string{"B"}, cmd.UnloadingThresholdFunc.NeedsVars()) - require.Equal(t, "lt", cmd.UnloadingThresholdFunc.ThresholdFunc) + require.Equal(t, ThresholdIsBelow, cmd.UnloadingThresholdFunc.ThresholdFunc) require.Equal(t, []float64{31.0}, cmd.UnloadingThresholdFunc.Conditions) require.True(t, cmd.UnloadingThresholdFunc.Invert) require.NotNil(t, cmd.LoadedDimensions) @@ -233,7 +233,7 @@ func TestCreateMathExpression(t *testing.T) { expected string ref string - function string + function ThresholdType params []float64 } @@ -297,7 +297,7 @@ func TestCreateMathExpression(t *testing.T) { func TestIsSupportedThresholdFunc(t *testing.T) { type testCase struct { - function string + function ThresholdType supported bool } @@ -325,8 +325,8 @@ func TestIsSupportedThresholdFunc(t *testing.T) { } for _, tc := range cases { - t.Run(tc.function, func(t *testing.T) { - supported := IsSupportedThresholdFunc(tc.function) + t.Run(string(tc.function), func(t *testing.T) { + supported := IsSupportedThresholdFunc(string(tc.function)) require.Equal(t, supported, tc.supported) }) } diff --git a/pkg/services/ngalert/backtesting/eval_data.go b/pkg/services/ngalert/backtesting/eval_data.go index cd0fecdccff..12b8b872bd5 100644 --- a/pkg/services/ngalert/backtesting/eval_data.go +++ b/pkg/services/ngalert/backtesting/eval_data.go @@ -16,8 +16,8 @@ import ( type dataEvaluator struct { refID string data []mathexp.Series - downsampleFunction string - upsampleFunction string + downsampleFunction mathexp.ReducerID + upsampleFunction mathexp.Upsampler } func newDataEvaluator(refID string, frame *data.Frame) (*dataEvaluator, error) {