diff --git a/cspell.yaml b/cspell.yaml index 946a05c7..1cf41e7b 100644 --- a/cspell.yaml +++ b/cspell.yaml @@ -111,6 +111,7 @@ words: - stimeslice - strconv - structs + - submatches - sumologic - testutils - timeseries diff --git a/manifest/v1alpha/slo/metrics_sumo_logic.go b/manifest/v1alpha/slo/metrics_sumo_logic.go index d878618e..465dbc25 100644 --- a/manifest/v1alpha/slo/metrics_sumo_logic.go +++ b/manifest/v1alpha/slo/metrics_sumo_logic.go @@ -3,6 +3,7 @@ package slo import ( "fmt" "regexp" + "strings" "time" "github.com/pkg/errors" @@ -44,15 +45,15 @@ var sumoLogicCountMetricsLevelValidation = validation.New[CountMetricsSpec]( if *good.Type != "logs" || *total.Type != "logs" { return nil } - goodTS, err := getTimeSliceFromSumoLogicQuery(*good.Query) + goodTimeSlice, err := getTimeSliceFromSumoLogicQuery(*good.Query) if err != nil { return nil } - totalTS, err := getTimeSliceFromSumoLogicQuery(*total.Query) + totalTimeSlice, err := getTimeSliceFromSumoLogicQuery(*total.Query) if err != nil { return nil } - if goodTS != totalTS { + if goodTimeSlice.duration != totalTimeSlice.duration { return errors.Errorf( "'sumologic.query' with segment 'timeslice ${duration}', " + "${duration} must be the same for both 'good' and 'total' metrics") @@ -107,21 +108,9 @@ var sumoLogicLogsTypeValidation = validation.New[SumoLogicMetric]( WithName("query"). Required(). Rules( - validation.NewSingleRule(func(s string) error { - const minTimeSliceSeconds = 15 - timeslice, err := getTimeSliceFromSumoLogicQuery(s) - if err != nil { - return err - } - if timeslice.Seconds() < minTimeSliceSeconds { - return errors.Errorf("minimum timeslice value is [%ds], got: [%s]", minTimeSliceSeconds, timeslice) - } - return nil - }), + validation.NewSingleRule(validateSumoLogicTimeslice), validation.StringMatchRegexp(regexp.MustCompile(`(?m)\bn9_value\b`)). WithDetails("n9_value is required"), - validation.StringMatchRegexp(regexp.MustCompile(`(?m)\bn9_time\b`)). - WithDetails("n9_time is required"), validation.StringMatchRegexp(regexp.MustCompile(`(?m)\bby\b`)). WithDetails("aggregation function is required"), ), @@ -136,21 +125,54 @@ var sumoLogicLogsTypeValidation = validation.New[SumoLogicMetric]( return m.Type != nil && *m.Type == sumoLogicTypeLogs }) -var sumoLogicTimeSliceRegexp = regexp.MustCompile(`(?m)\stimeslice\s(\d+\w+)\s`) +func validateSumoLogicTimeslice(query string) error { + timeSlice, err := getTimeSliceFromSumoLogicQuery(query) + if err != nil { + return err + } -func getTimeSliceFromSumoLogicQuery(query string) (time.Duration, error) { - subMatches := sumoLogicTimeSliceRegexp.FindAllStringSubmatch(query, 2) - if len(subMatches) != 1 { - return 0, fmt.Errorf("exactly one timeslice declaration is required in the query") + if seconds := int(timeSlice.duration.Seconds()); seconds != 15 && seconds != 30 && seconds != 60 || + strings.HasPrefix(timeSlice.durationStr, "0") || // Sumo Logic doesn't support leading zeros in query body + strings.HasPrefix(timeSlice.durationStr, "+") || + strings.HasPrefix(timeSlice.durationStr, "-") || + strings.HasSuffix(timeSlice.durationStr, "ms") { + return errors.Errorf("timeslice value must be 15, 30, or 60 seconds, got: [%s]", timeSlice.durationStr) } - subMatch := subMatches[0] - if len(subMatch) != 2 { - return 0, fmt.Errorf("timeslice declaration must match regular expression: %s", sumoLogicTimeSliceRegexp) + + if !timeSlice.containsAlias { + return errors.New("timeslice operator requires an n9_time alias") } + return nil +} + +type parsedSumoLogicSlice struct { + containsAlias bool + duration time.Duration + durationStr string +} + +func getTimeSliceFromSumoLogicQuery(query string) (parsedSumoLogicSlice, error) { + r := regexp.MustCompile(`\stimeslice\s([-+]?(\d+[a-z]+\s?)+)\s(?:as n9_time)?`) + matchResults := r.FindAllStringSubmatch(query, 2) + if len(matchResults) == 0 { + return parsedSumoLogicSlice{}, errors.New("query must contain a 'timeslice' operator") + } + if len(matchResults) > 1 { + return parsedSumoLogicSlice{}, errors.New("exactly one 'timeslice' usage is required in the query") + } + submatches := matchResults[0] + + if submatches[1] != submatches[2] { + return parsedSumoLogicSlice{}, errors.New("timeslice interval must be in a NumberUnit form - for example '30s'") + } + // https://help.sumologic.com/05Search/Search-Query-Language/Search-Operators/timeslice#syntax - timeslice, err := time.ParseDuration(subMatch[1]) + durationString := strings.TrimSpace(submatches[1]) + containsAlias := strings.Contains(submatches[0][1:], "as n9_time") + tsDuration, err := time.ParseDuration(durationString) if err != nil { - return 0, fmt.Errorf("error parsing timeslice duration: %s", err.Error()) + return parsedSumoLogicSlice{}, fmt.Errorf("error parsing timeslice duration: %s", err.Error()) } - return timeslice, nil + + return parsedSumoLogicSlice{duration: tsDuration, durationStr: durationString, containsAlias: containsAlias}, nil } diff --git a/manifest/v1alpha/slo/metrics_sumo_logic_test.go b/manifest/v1alpha/slo/metrics_sumo_logic_test.go index b56c23f4..13091ddf 100644 --- a/manifest/v1alpha/slo/metrics_sumo_logic_test.go +++ b/manifest/v1alpha/slo/metrics_sumo_logic_test.go @@ -36,7 +36,7 @@ func TestSumoLogic_CountMetricsLevel(t *testing.T) { Query: ptr(` _collector="n9-dev-tooling-cluster" _source="logs" | json "log" - | timeslice 20s as n9_time + | timeslice 15s as n9_time | parse "level=* *" as (log_level, tail) | if (log_level matches "error" ,0,1) as log_level_not_error | sum(log_level_not_error) as n9_value by n9_time @@ -47,7 +47,7 @@ _collector="n9-dev-tooling-cluster" _source="logs" Query: ptr(` _collector="n9-dev-tooling-cluster" _source="logs" | json "log" - | timeslice 25s as n9_time + | timeslice 30s as n9_time | parse "level=* *" as (log_level, tail) | if (log_level matches "error" ,0,1) as log_level_not_error | sum(log_level_not_error) as n9_value by n9_time @@ -153,7 +153,7 @@ func TestSumoLogic_LogsType(t *testing.T) { Query: ptr(` _collector="n9-dev-tooling-cluster" _source="logs" | json "log" - | timeslice 20s as n9_time + | timeslice 15s as n9_time | parse "level=* *" as (log_level, tail) | if (log_level matches "error" ,0,1) as log_level_not_error | sum(log_level_not_error) as n9_value by n9_time @@ -173,7 +173,7 @@ _collector="n9-dev-tooling-cluster" _source="logs" }, ) }) - tests := map[string]struct { + invalidCases := map[string]struct { Query string Error testutils.ExpectedError }{ @@ -187,22 +187,78 @@ _collector="n9-dev-tooling-cluster" _source="logs" | sort by n9_time asc`, Error: testutils.ExpectedError{ Prop: "spec.objectives[0].rawMetric.query.sumoLogic.query", - Message: "exactly one timeslice declaration is required in the query", + Message: "query must contain a 'timeslice' operator", }, }, "two timeslice segments": { Query: ` _collector="n9-dev-tooling-cluster" _source="logs" | json "log" - | timeslice 30s - | timeslice 20s as n9_time + | timeslice 30s as n9_time + | timeslice 15s as n9_time + | parse "level=* *" as (log_level, tail) + | if (log_level matches "error" ,0,1) as log_level_not_error + | sum(log_level_not_error) as n9_value by n9_time + | sort by n9_time asc`, + Error: testutils.ExpectedError{ + Prop: "spec.objectives[0].rawMetric.query.sumoLogic.query", + Message: "exactly one 'timeslice' usage is required in the query", + }, + }, + "leading zeros in timeslice value": { + Query: ` +_collector="n9-dev-tooling-cluster" _source="logs" + | json "log" + | timeslice 015s as n9_time + | parse "level=* *" as (log_level, tail) + | if (log_level matches "error" ,0,1) as log_level_not_error + | sum(log_level_not_error) as n9_value by n9_time + | sort by n9_time asc`, + Error: testutils.ExpectedError{ + Prop: "spec.objectives[0].rawMetric.query.sumoLogic.query", + Message: "timeslice value must be 15, 30, or 60 seconds, got: [015s]", + }, + }, + "+ sign in timeslice value": { + Query: ` +_collector="n9-dev-tooling-cluster" _source="logs" + | json "log" + | timeslice +15s as n9_time + | parse "level=* *" as (log_level, tail) + | if (log_level matches "error" ,0,1) as log_level_not_error + | sum(log_level_not_error) as n9_value by n9_time + | sort by n9_time asc`, + Error: testutils.ExpectedError{ + Prop: "spec.objectives[0].rawMetric.query.sumoLogic.query", + Message: "timeslice interval must be in a NumberUnit form - for example '30s'", + }, + }, + "- sign in timeslice value": { + Query: ` +_collector="n9-dev-tooling-cluster" _source="logs" + | json "log" + | timeslice -15s as n9_time + | parse "level=* *" as (log_level, tail) + | if (log_level matches "error" ,0,1) as log_level_not_error + | sum(log_level_not_error) as n9_value by n9_time + | sort by n9_time asc`, + Error: testutils.ExpectedError{ + Prop: "spec.objectives[0].rawMetric.query.sumoLogic.query", + Message: "timeslice interval must be in a NumberUnit form - for example '30s'", + }, + }, + "milliseconds in timeslice value": { + Query: ` +_collector="n9-dev-tooling-cluster" _source="logs" + | json "log" + | timeslice 15000ms as n9_time | parse "level=* *" as (log_level, tail) | if (log_level matches "error" ,0,1) as log_level_not_error | sum(log_level_not_error) as n9_value by n9_time | sort by n9_time asc`, Error: testutils.ExpectedError{ Prop: "spec.objectives[0].rawMetric.query.sumoLogic.query", - Message: "exactly one timeslice declaration is required in the query", + Message: "timeslice value must be 15, 30, or 60 seconds, got: [15000ms]", }, }, "invalid timeslice segment": { @@ -219,7 +275,7 @@ _collector="n9-dev-tooling-cluster" _source="logs" Message: `error parsing timeslice duration: time: unknown unit "x" in duration "20x"`, }, }, - "minimum timeslice value": { + "unsupported timeslice value": { Query: ` _collector="n9-dev-tooling-cluster" _source="logs" | json "log" @@ -230,14 +286,14 @@ _collector="n9-dev-tooling-cluster" _source="logs" | sort by n9_time asc`, Error: testutils.ExpectedError{ Prop: "spec.objectives[0].rawMetric.query.sumoLogic.query", - Message: `minimum timeslice value is [15s], got: [14s]`, + Message: `timeslice value must be 15, 30, or 60 seconds, got: [14s]`, }, }, "missing n9_value": { Query: ` _collector="n9-dev-tooling-cluster" _source="logs" | json "log" - | timeslice 20s as n9_time + | timeslice 15s as n9_time | parse "level=* *" as (log_level, tail) | if (log_level matches "error" ,0,1) as log_level_not_error | sum(log_level_not_error) by n9_time @@ -247,25 +303,25 @@ _collector="n9-dev-tooling-cluster" _source="logs" ContainsMessage: "n9_value is required", }, }, - "missing n9_time": { + "missing n9_time alias": { Query: ` _collector="n9-dev-tooling-cluster" _source="logs" | json "log" - | timeslice 20s + | timeslice 30s | parse "level=* *" as (log_level, tail) | if (log_level matches "error" ,0,1) as log_level_not_error | sum(log_level_not_error) as n9_value by time | sort by time asc`, Error: testutils.ExpectedError{ Prop: "spec.objectives[0].rawMetric.query.sumoLogic.query", - ContainsMessage: "n9_time is required", + ContainsMessage: "timeslice operator requires an n9_time alias", }, }, "missing aggregation function": { Query: ` _collector="n9-dev-tooling-cluster" _source="logs" | json "log" - | timeslice 20s as n9_time + | timeslice 15s as n9_time | parse "level=* *" as (log_level, tail) | if (log_level matches "error" ,0,1) as log_level_not_error | sum(log_level_not_error) as n9_value`, @@ -275,7 +331,7 @@ _collector="n9-dev-tooling-cluster" _source="logs" }, }, } - for name, test := range tests { + for name, test := range invalidCases { t.Run(name, func(t *testing.T) { slo := validRawMetricSLO(v1alpha.SumoLogic) slo.Spec.Objectives[0].RawMetric.MetricQuery.SumoLogic = &SumoLogicMetric{ @@ -286,4 +342,50 @@ _collector="n9-dev-tooling-cluster" _source="logs" testutils.AssertContainsErrors(t, slo, err, 1, test.Error) }) } + validCases := map[string]struct { + Query string + Error testutils.ExpectedError + }{ + "valid timeslice [15s]": { + Query: ` +_collector="n9-dev-tooling-cluster" _source="logs" + | json "log" + | timeslice 15s as n9_time + | parse "level=* *" as (log_level, tail) + | if (log_level matches "error" ,0,1) as log_level_not_error + | sum(log_level_not_error) as n9_value by n9_time + | sort by n9_time asc`, + }, + "valid timeslice [30s]": { + Query: ` +_collector="n9-dev-tooling-cluster" _source="logs" + | json "log" + | timeslice 30s as n9_time + | parse "level=* *" as (log_level, tail) + | if (log_level matches "error" ,0,1) as log_level_not_error + | sum(log_level_not_error) as n9_value by n9_time + | sort by n9_time asc`, + }, + "valid timeslice [60s]": { + Query: ` +_collector="n9-dev-tooling-cluster" _source="logs" + | json "log" + | timeslice 60s as n9_time + | parse "level=* *" as (log_level, tail) + | if (log_level matches "error" ,0,1) as log_level_not_error + | sum(log_level_not_error) as n9_value by n9_time + | sort by n9_time asc`, + }, + } + for name, test := range validCases { + t.Run(name, func(t *testing.T) { + slo := validRawMetricSLO(v1alpha.SumoLogic) + slo.Spec.Objectives[0].RawMetric.MetricQuery.SumoLogic = &SumoLogicMetric{ + Type: ptr(sumoLogicTypeLogs), + Query: ptr(test.Query), + } + err := validate(slo) + testutils.AssertNoError(t, slo, err) + }) + } } diff --git a/sdk/test_data/client/simple_module/go.mod b/sdk/test_data/client/simple_module/go.mod index 4a17b03a..1afe4028 100644 --- a/sdk/test_data/client/simple_module/go.mod +++ b/sdk/test_data/client/simple_module/go.mod @@ -8,7 +8,7 @@ require github.com/nobl9/nobl9-go v0.73.0 require ( github.com/BurntSushi/toml v1.3.2 // indirect - github.com/aws/aws-sdk-go v1.49.16 // indirect + github.com/aws/aws-sdk-go v1.50.3 // indirect github.com/bmatcuk/doublestar/v4 v4.6.1 // indirect github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0 // indirect github.com/fatih/color v1.15.0 // indirect @@ -27,7 +27,7 @@ require ( github.com/lestrrat-go/blackmagic v1.0.2 // indirect github.com/lestrrat-go/httpcc v1.0.1 // indirect github.com/lestrrat-go/iter v1.0.2 // indirect - github.com/lestrrat-go/jwx v1.2.27 // indirect + github.com/lestrrat-go/jwx v1.2.28 // indirect github.com/lestrrat-go/option v1.0.1 // indirect github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-isatty v0.0.19 // indirect @@ -36,7 +36,7 @@ require ( github.com/pkg/errors v0.9.1 // indirect github.com/rs/zerolog v1.31.0 // indirect golang.org/x/crypto v0.17.0 // indirect - golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc // indirect + golang.org/x/exp v0.0.0-20240112132812-db7319d0e0e3 // indirect golang.org/x/net v0.17.0 // indirect golang.org/x/sys v0.15.0 // indirect golang.org/x/text v0.14.0 // indirect diff --git a/sdk/test_data/client/simple_module/go.sum b/sdk/test_data/client/simple_module/go.sum index ddc89eac..f92ddcf3 100644 --- a/sdk/test_data/client/simple_module/go.sum +++ b/sdk/test_data/client/simple_module/go.sum @@ -3,6 +3,8 @@ github.com/BurntSushi/toml v1.3.2/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbi github.com/aws/aws-sdk-go v1.48.14 h1:nVLrp+F84SG+xGiFMfe1TE6ZV6smF+42tuuNgYGV30s= github.com/aws/aws-sdk-go v1.48.14/go.mod h1:LF8svs817+Nz+DmiMQKTO3ubZ/6IaTpq3TjupRn3Eqk= github.com/aws/aws-sdk-go v1.49.16/go.mod h1:LF8svs817+Nz+DmiMQKTO3ubZ/6IaTpq3TjupRn3Eqk= +github.com/aws/aws-sdk-go v1.49.21/go.mod h1:LF8svs817+Nz+DmiMQKTO3ubZ/6IaTpq3TjupRn3Eqk= +github.com/aws/aws-sdk-go v1.50.3/go.mod h1:LF8svs817+Nz+DmiMQKTO3ubZ/6IaTpq3TjupRn3Eqk= github.com/bmatcuk/doublestar/v4 v4.6.1 h1:FH9SifrbvJhnlQpztAx++wlkk70QBf0iBWDwNy7PA4I= github.com/bmatcuk/doublestar/v4 v4.6.1/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc= github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= @@ -53,6 +55,7 @@ github.com/lestrrat-go/iter v1.0.2 h1:gMXo1q4c2pHmC3dn8LzRhJfP1ceCbgSiT9lUydIzlt github.com/lestrrat-go/iter v1.0.2/go.mod h1:Momfcq3AnRlRjI5b5O8/G5/BvpzrhoFTZcn06fEOPt4= github.com/lestrrat-go/jwx v1.2.27 h1:cvnTnda/YzdyFuWdEAMkI6BsLtItSrASEVCI3C/IUEQ= github.com/lestrrat-go/jwx v1.2.27/go.mod h1:Stob9LjSqR3lOmNdxF0/TvZo60V3hUGv8Fr7Bwzla3k= +github.com/lestrrat-go/jwx v1.2.28/go.mod h1:nF+91HEMh/MYFVwKPl5HHsBGMPscqbQb+8IDQdIazP8= github.com/lestrrat-go/option v1.0.0/go.mod h1:5ZHFbivi4xwXxhxY9XHDe2FHo6/Z7WWmtT7T5nBBp3I= github.com/lestrrat-go/option v1.0.1 h1:oAzP2fvZGQKWkvHa1/SAcFolBEca1oN+mQ7eooNBEYU= github.com/lestrrat-go/option v1.0.1/go.mod h1:5ZHFbivi4xwXxhxY9XHDe2FHo6/Z7WWmtT7T5nBBp3I= @@ -93,6 +96,7 @@ golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq golang.org/x/exp v0.0.0-20231206192017-f3f8817b8deb h1:c0vyKkb6yr3KR7jEfJaOSv4lG7xPkbN6r52aJz1d8a8= golang.org/x/exp v0.0.0-20231206192017-f3f8817b8deb/go.mod h1:iRJReGqOEeBhDZGkGbynYwcHlctCvnjTYIamk7uXpHI= golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc/go.mod h1:iRJReGqOEeBhDZGkGbynYwcHlctCvnjTYIamk7uXpHI= +golang.org/x/exp v0.0.0-20240112132812-db7319d0e0e3/go.mod h1:idGWGoKP1toJGkd5/ig9ZLuPcZBC3ewk7SzmH0uou08= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=