From 469641f0c5b6cd43c9ca16e1534566d8c6777587 Mon Sep 17 00:00:00 2001 From: Nikodem Rafalski Date: Fri, 12 Jan 2024 15:46:01 +0100 Subject: [PATCH 1/5] sumologic timeslice validation --- manifest/v1alpha/slo/metrics_sumo_logic.go | 69 ++++++---- .../v1alpha/slo/metrics_sumo_logic_test.go | 126 ++++++++++++++++-- 2 files changed, 157 insertions(+), 38 deletions(-) diff --git a/manifest/v1alpha/slo/metrics_sumo_logic.go b/manifest/v1alpha/slo/metrics_sumo_logic.go index d878618e..cd1170f6 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,11 +45,11 @@ var sumoLogicCountMetricsLevelValidation = validation.New[CountMetricsSpec]( if *good.Type != "logs" || *total.Type != "logs" { return nil } - goodTS, err := getTimeSliceFromSumoLogicQuery(*good.Query) + goodTS, _, _, err := getTimeSliceFromSumoLogicQuery(*good.Query) if err != nil { return nil } - totalTS, err := getTimeSliceFromSumoLogicQuery(*total.Query) + totalTS, _, _, err := getTimeSliceFromSumoLogicQuery(*total.Query) if err != nil { return nil } @@ -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,49 @@ 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, timesliceString, containsAlias, 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.Seconds()); seconds != 15 && seconds != 30 && seconds != 60 || + strings.HasPrefix(timesliceString, "0") || // Sumo Logic doesn't support leading zeros in query body + strings.HasPrefix(timesliceString, "+") || + strings.HasPrefix(timesliceString, "-") || + strings.HasSuffix(timesliceString, "ms") { + return errors.Errorf("timeslice value must be 15, 30, or 60 seconds, got: [%s]", timesliceString) } - subMatch := subMatches[0] - if len(subMatch) != 2 { - return 0, fmt.Errorf("timeslice declaration must match regular expression: %s", sumoLogicTimeSliceRegexp) + + if !containsAlias { + return errors.New("timeslice operator requires an n9_time alias") } + return nil +} + +func getTimeSliceFromSumoLogicQuery(query string) (time.Duration, string, bool, error) { + r := regexp.MustCompile(`\stimeslice\s([-+]?(\d+[a-z]+\s?)+)\s(?:as n9_time)?`) + matchResults := r.FindStringSubmatch(query) + + if len(matchResults) == 0 { + return 0, "", false, fmt.Errorf("query must contain a 'timeslice' operator") + } + + if len(matchResults) > 3 { + return 0, "", false, fmt.Errorf("exactly one timeslice declaration is required in the query") + } + + if matchResults[1] != matchResults[2] { + return 0, "", false, fmt.Errorf("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 := matchResults[1] + containsAlias := strings.Contains(matchResults[0][1:], "as n9_time") + timeslice, err := time.ParseDuration(durationString) if err != nil { - return 0, fmt.Errorf("error parsing timeslice duration: %s", err.Error()) + return 0, "", containsAlias, fmt.Errorf("error parsing timeslice duration: %s", err.Error()) } - return timeslice, nil + + return timeslice, durationString, containsAlias, nil } diff --git a/manifest/v1alpha/slo/metrics_sumo_logic_test.go b/manifest/v1alpha/slo/metrics_sumo_logic_test.go index b56c23f4..cb1f7572 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,15 +187,15 @@ _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 @@ -205,6 +205,62 @@ _collector="n9-dev-tooling-cluster" _source="logs" Message: "exactly one timeslice declaration 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: "timeslice value must be 15, 30, or 60 seconds, got: [15000ms]", + }, + }, "invalid timeslice segment": { Query: ` _collector="n9-dev-tooling-cluster" _source="logs" @@ -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 @@ -265,7 +321,7 @@ _collector="n9-dev-tooling-cluster" _source="logs" 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) + }) + } } From 29e153361a79cb51220cbcb0372f3c8afbff5ecd Mon Sep 17 00:00:00 2001 From: Nikodem Rafalski Date: Mon, 15 Jan 2024 13:04:39 +0100 Subject: [PATCH 2/5] multiple timeslice case fix --- manifest/v1alpha/slo/metrics_sumo_logic.go | 14 ++++++++------ manifest/v1alpha/slo/metrics_sumo_logic_test.go | 8 ++++---- sdk/test_data/client/simple_module/go.mod | 6 +++--- sdk/test_data/client/simple_module/go.sum | 3 +++ 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/manifest/v1alpha/slo/metrics_sumo_logic.go b/manifest/v1alpha/slo/metrics_sumo_logic.go index cd1170f6..e49965aa 100644 --- a/manifest/v1alpha/slo/metrics_sumo_logic.go +++ b/manifest/v1alpha/slo/metrics_sumo_logic.go @@ -147,23 +147,25 @@ func validateSumoLogicTimeslice(query string) error { func getTimeSliceFromSumoLogicQuery(query string) (time.Duration, string, bool, error) { r := regexp.MustCompile(`\stimeslice\s([-+]?(\d+[a-z]+\s?)+)\s(?:as n9_time)?`) - matchResults := r.FindStringSubmatch(query) + matchResults := r.FindAllStringSubmatch(query, 2) if len(matchResults) == 0 { return 0, "", false, fmt.Errorf("query must contain a 'timeslice' operator") } - if len(matchResults) > 3 { - return 0, "", false, fmt.Errorf("exactly one timeslice declaration is required in the query") + if len(matchResults) > 1 { + return 0, "", false, fmt.Errorf("exactly one 'timeslice' declaration is required in the query") } - if matchResults[1] != matchResults[2] { + submatches := matchResults[0] + + if submatches[1] != submatches[2] { return 0, "", false, fmt.Errorf("timeslice interval must be in a NumberUnit form - for example '30s'") } // https://help.sumologic.com/05Search/Search-Query-Language/Search-Operators/timeslice#syntax - durationString := matchResults[1] - containsAlias := strings.Contains(matchResults[0][1:], "as n9_time") + durationString := strings.TrimSpace(submatches[1]) + containsAlias := strings.Contains(submatches[0][1:], "as n9_time") timeslice, err := time.ParseDuration(durationString) if err != nil { return 0, "", containsAlias, fmt.Errorf("error parsing timeslice duration: %s", err.Error()) diff --git a/manifest/v1alpha/slo/metrics_sumo_logic_test.go b/manifest/v1alpha/slo/metrics_sumo_logic_test.go index cb1f7572..5ba3df7d 100644 --- a/manifest/v1alpha/slo/metrics_sumo_logic_test.go +++ b/manifest/v1alpha/slo/metrics_sumo_logic_test.go @@ -202,7 +202,7 @@ _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: "exactly one 'timeslice' declaration is required in the query", }, }, "leading zeros in timeslice value": { @@ -303,18 +303,18 @@ _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: "imeslice operator requires an n9_time alias", }, }, "missing aggregation function": { diff --git a/sdk/test_data/client/simple_module/go.mod b/sdk/test_data/client/simple_module/go.mod index 4a17b03a..6947ad06 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.49.21 // 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..54fdc194 100644 --- a/sdk/test_data/client/simple_module/go.sum +++ b/sdk/test_data/client/simple_module/go.sum @@ -3,6 +3,7 @@ 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/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 +54,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 +95,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= From b41d4de2d104f167df7f9fe342e9c7ba75a4c82f Mon Sep 17 00:00:00 2001 From: Nikodem Rafalski Date: Mon, 15 Jan 2024 13:19:01 +0100 Subject: [PATCH 3/5] lints --- cspell.yaml | 1 + manifest/v1alpha/slo/metrics_sumo_logic.go | 15 +++++++-------- manifest/v1alpha/slo/metrics_sumo_logic_test.go | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) 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 e49965aa..f5bc6932 100644 --- a/manifest/v1alpha/slo/metrics_sumo_logic.go +++ b/manifest/v1alpha/slo/metrics_sumo_logic.go @@ -145,18 +145,17 @@ func validateSumoLogicTimeslice(query string) error { return nil } -func getTimeSliceFromSumoLogicQuery(query string) (time.Duration, string, bool, error) { +func getTimeSliceFromSumoLogicQuery(query string) ( + tsDuration time.Duration, durationString string, containsAlias bool, err error, +) { r := regexp.MustCompile(`\stimeslice\s([-+]?(\d+[a-z]+\s?)+)\s(?:as n9_time)?`) matchResults := r.FindAllStringSubmatch(query, 2) - if len(matchResults) == 0 { return 0, "", false, fmt.Errorf("query must contain a 'timeslice' operator") } - if len(matchResults) > 1 { return 0, "", false, fmt.Errorf("exactly one 'timeslice' declaration is required in the query") } - submatches := matchResults[0] if submatches[1] != submatches[2] { @@ -164,12 +163,12 @@ func getTimeSliceFromSumoLogicQuery(query string) (time.Duration, string, bool, } // https://help.sumologic.com/05Search/Search-Query-Language/Search-Operators/timeslice#syntax - durationString := strings.TrimSpace(submatches[1]) - containsAlias := strings.Contains(submatches[0][1:], "as n9_time") - timeslice, err := time.ParseDuration(durationString) + durationString = strings.TrimSpace(submatches[1]) + containsAlias = strings.Contains(submatches[0][1:], "as n9_time") + tsDuration, err = time.ParseDuration(durationString) if err != nil { return 0, "", containsAlias, fmt.Errorf("error parsing timeslice duration: %s", err.Error()) } - return timeslice, durationString, containsAlias, nil + return tsDuration, durationString, containsAlias, nil } diff --git a/manifest/v1alpha/slo/metrics_sumo_logic_test.go b/manifest/v1alpha/slo/metrics_sumo_logic_test.go index 5ba3df7d..466251c7 100644 --- a/manifest/v1alpha/slo/metrics_sumo_logic_test.go +++ b/manifest/v1alpha/slo/metrics_sumo_logic_test.go @@ -314,7 +314,7 @@ _collector="n9-dev-tooling-cluster" _source="logs" | sort by time asc`, Error: testutils.ExpectedError{ Prop: "spec.objectives[0].rawMetric.query.sumoLogic.query", - ContainsMessage: "imeslice operator requires an n9_time alias", + ContainsMessage: "timeslice operator requires an n9_time alias", }, }, "missing aggregation function": { From 288474fcc9595faa469e01bee57991659b8fcd6d Mon Sep 17 00:00:00 2001 From: Nikodem Rafalski Date: Wed, 24 Jan 2024 16:24:29 +0100 Subject: [PATCH 4/5] review --- manifest/v1alpha/slo/metrics_sumo_logic.go | 48 ++++++++++++---------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/manifest/v1alpha/slo/metrics_sumo_logic.go b/manifest/v1alpha/slo/metrics_sumo_logic.go index f5bc6932..465dbc25 100644 --- a/manifest/v1alpha/slo/metrics_sumo_logic.go +++ b/manifest/v1alpha/slo/metrics_sumo_logic.go @@ -45,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") @@ -126,49 +126,53 @@ var sumoLogicLogsTypeValidation = validation.New[SumoLogicMetric]( }) func validateSumoLogicTimeslice(query string) error { - timeslice, timesliceString, containsAlias, err := getTimeSliceFromSumoLogicQuery(query) + timeSlice, err := getTimeSliceFromSumoLogicQuery(query) if err != nil { return err } - if seconds := int(timeslice.Seconds()); seconds != 15 && seconds != 30 && seconds != 60 || - strings.HasPrefix(timesliceString, "0") || // Sumo Logic doesn't support leading zeros in query body - strings.HasPrefix(timesliceString, "+") || - strings.HasPrefix(timesliceString, "-") || - strings.HasSuffix(timesliceString, "ms") { - return errors.Errorf("timeslice value must be 15, 30, or 60 seconds, got: [%s]", timesliceString) + 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) } - if !containsAlias { + if !timeSlice.containsAlias { return errors.New("timeslice operator requires an n9_time alias") } return nil } -func getTimeSliceFromSumoLogicQuery(query string) ( - tsDuration time.Duration, durationString string, containsAlias bool, err error, -) { +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 0, "", false, fmt.Errorf("query must contain a 'timeslice' operator") + return parsedSumoLogicSlice{}, errors.New("query must contain a 'timeslice' operator") } if len(matchResults) > 1 { - return 0, "", false, fmt.Errorf("exactly one 'timeslice' declaration is required in the query") + return parsedSumoLogicSlice{}, errors.New("exactly one 'timeslice' usage is required in the query") } submatches := matchResults[0] if submatches[1] != submatches[2] { - return 0, "", false, fmt.Errorf("timeslice interval must be in a NumberUnit form - for example '30s'") + 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 - durationString = strings.TrimSpace(submatches[1]) - containsAlias = strings.Contains(submatches[0][1:], "as n9_time") - tsDuration, err = time.ParseDuration(durationString) + durationString := strings.TrimSpace(submatches[1]) + containsAlias := strings.Contains(submatches[0][1:], "as n9_time") + tsDuration, err := time.ParseDuration(durationString) if err != nil { - return 0, "", containsAlias, fmt.Errorf("error parsing timeslice duration: %s", err.Error()) + return parsedSumoLogicSlice{}, fmt.Errorf("error parsing timeslice duration: %s", err.Error()) } - return tsDuration, durationString, containsAlias, nil + return parsedSumoLogicSlice{duration: tsDuration, durationStr: durationString, containsAlias: containsAlias}, nil } From 57b2c5dc5de092675fc2084058f79d82cc21c4cf Mon Sep 17 00:00:00 2001 From: Nikodem Rafalski Date: Fri, 26 Jan 2024 09:52:13 +0100 Subject: [PATCH 5/5] test --- manifest/v1alpha/slo/metrics_sumo_logic_test.go | 2 +- sdk/test_data/client/simple_module/go.mod | 2 +- sdk/test_data/client/simple_module/go.sum | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/manifest/v1alpha/slo/metrics_sumo_logic_test.go b/manifest/v1alpha/slo/metrics_sumo_logic_test.go index 466251c7..13091ddf 100644 --- a/manifest/v1alpha/slo/metrics_sumo_logic_test.go +++ b/manifest/v1alpha/slo/metrics_sumo_logic_test.go @@ -202,7 +202,7 @@ _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: "exactly one 'timeslice' usage is required in the query", }, }, "leading zeros in timeslice value": { diff --git a/sdk/test_data/client/simple_module/go.mod b/sdk/test_data/client/simple_module/go.mod index 6947ad06..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.21 // 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 diff --git a/sdk/test_data/client/simple_module/go.sum b/sdk/test_data/client/simple_module/go.sum index 54fdc194..f92ddcf3 100644 --- a/sdk/test_data/client/simple_module/go.sum +++ b/sdk/test_data/client/simple_module/go.sum @@ -4,6 +4,7 @@ github.com/aws/aws-sdk-go v1.48.14 h1:nVLrp+F84SG+xGiFMfe1TE6ZV6smF+42tuuNgYGV30 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=