From 576eacbaf27651d4446079ff45eb59641e65e5e2 Mon Sep 17 00:00:00 2001 From: Xenia Nisskhen Date: Fri, 27 Oct 2023 17:16:21 +0600 Subject: [PATCH 1/5] fix(filter): fix support custom retention for tagged metrics --- filter/cache_storage.go | 5 +++-- filter/cache_storage_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/filter/cache_storage.go b/filter/cache_storage.go index e80cca2e8..db38c6d9a 100644 --- a/filter/cache_storage.go +++ b/filter/cache_storage.go @@ -80,11 +80,12 @@ func (storage *Storage) buildRetentions(retentionScanner *bufio.Scanner) error { for retentionScanner.Scan() { line1 := retentionScanner.Text() - if strings.HasPrefix(line1, "#") || strings.Count(line1, "=") != 1 { + if strings.HasPrefix(line1, "#") || strings.Count(line1, "=") < 1 { continue } - patternString := strings.TrimSpace(strings.Split(line1, "=")[1]) + _, after, _ := strings.Cut(line1, "=") + patternString := strings.TrimSpace(after) pattern, err := regexp.Compile(patternString) if err != nil { return err diff --git a/filter/cache_storage_test.go b/filter/cache_storage_test.go index c0c3271e5..ad48469dc 100644 --- a/filter/cache_storage_test.go +++ b/filter/cache_storage_test.go @@ -35,6 +35,10 @@ var testRetentions = ` pattern = yearly$ retentions = 1y:100y + [tagged metrics] + pattern = ;tag1=val1(;.*)?$ + retentions = 10s:2d,1m:30d,15m:1y + [default] pattern = .* retentions = 120:7d @@ -229,4 +233,35 @@ func TestRetentions(t *testing.T) { So(metr.Retention, ShouldEqual, 120) So(metr.RetentionTimestamp, ShouldEqual, 120) }) + + Convey("Tagged metrics", t, func() { + Convey("should be 10sec", func() { + matchedMetric := moira.MatchedMetric{ + Metric: "my_super_metric;tag1=val1;tag2=val2", + Value: 12, + Timestamp: 151, + RetentionTimestamp: 0, + Retention: 10, + } + buffer := make(map[string]*moira.MatchedMetric) + storage.EnrichMatchedMetric(buffer, &matchedMetric) + So(len(buffer), ShouldEqual, 1) + So(matchedMetric.Retention, ShouldEqual, 10) + So(matchedMetric.RetentionTimestamp, ShouldEqual, 150) + }) + Convey("should be default 120sec", func() { + matchedMetric := moira.MatchedMetric{ + Metric: "my_super_metric;tag2=val2", + Value: 12, + Timestamp: 151, + RetentionTimestamp: 0, + Retention: 60, + } + buffer := make(map[string]*moira.MatchedMetric) + storage.EnrichMatchedMetric(buffer, &matchedMetric) + So(len(buffer), ShouldEqual, 1) + So(matchedMetric.Retention, ShouldEqual, 120) + So(matchedMetric.RetentionTimestamp, ShouldEqual, 120) + }) + }) } From 90cc50549fb60af3c0b49e075e8f2a85f8cb15be Mon Sep 17 00:00:00 2001 From: Xenia Nisskhen Date: Fri, 27 Oct 2023 17:27:51 +0600 Subject: [PATCH 2/5] fix(filter): fix support custom retention for tagged metrics --- filter/cache_storage_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filter/cache_storage_test.go b/filter/cache_storage_test.go index ad48469dc..b8af9a136 100644 --- a/filter/cache_storage_test.go +++ b/filter/cache_storage_test.go @@ -44,7 +44,7 @@ var testRetentions = ` retentions = 120:7d ` -var expectedRetentionIntervals = []int{60, 1200, 3600, 86400, 604800, 31536000, 120} +var expectedRetentionIntervals = []int{60, 1200, 3600, 86400, 604800, 31536000, 10, 120} var matchedMetrics = []moira.MatchedMetric{ { From ec217d7e82a50197dc2e40d32d53a9efe73f94da Mon Sep 17 00:00:00 2001 From: almostinf Date: Wed, 7 Aug 2024 12:03:31 +0300 Subject: [PATCH 3/5] added a more detailed description of errors in buildRetentions function --- filter/cache_storage.go | 20 +++++++++++++++----- filter/cache_storage_test.go | 1 + 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/filter/cache_storage.go b/filter/cache_storage.go index db38c6d9a..de92a235e 100644 --- a/filter/cache_storage.go +++ b/filter/cache_storage.go @@ -2,6 +2,7 @@ package filter import ( "bufio" + "fmt" "io" "regexp" "strconv" @@ -84,11 +85,18 @@ func (storage *Storage) buildRetentions(retentionScanner *bufio.Scanner) error { continue } - _, after, _ := strings.Cut(line1, "=") + _, after, found := strings.Cut(line1, "=") + if !found { + storage.logger.Error(). + String("pattern_line", line1). + Msg(`Invalid pattern format, it is correct to write in the format "pattern = regex"`) + continue + } + patternString := strings.TrimSpace(after) pattern, err := regexp.Compile(patternString) if err != nil { - return err + return fmt.Errorf("failed to compile regexp pattern '%s': %w", patternString, err) } retentionScanner.Scan() @@ -97,15 +105,16 @@ func (storage *Storage) buildRetentions(retentionScanner *bufio.Scanner) error { if len(splitted) < 2 { //nolint storage.logger.Error(). - String("pattern", patternString). - Msg("Invalid pattern found") + String("pattern_line", line1). + String("retentions_line", line2). + Msg(`Invalid retentions format, it is correct to write in the format "retentions = timePerPoint:timeToStore, timePerPoint:timeToStore, ..."`) continue } retentions := strings.TrimSpace(splitted[1]) retention, err := rawRetentionToSeconds(retentions[0:strings.Index(retentions, ":")]) if err != nil { - return err + return fmt.Errorf("failed to convert raw retentions '%s' to seconds: %w", retentions, err) } storage.retentions = append(storage.retentions, retentionMatcher{ @@ -113,6 +122,7 @@ func (storage *Storage) buildRetentions(retentionScanner *bufio.Scanner) error { retention: retention, }) } + return retentionScanner.Err() } diff --git a/filter/cache_storage_test.go b/filter/cache_storage_test.go index b8af9a136..d3b3904e8 100644 --- a/filter/cache_storage_test.go +++ b/filter/cache_storage_test.go @@ -249,6 +249,7 @@ func TestRetentions(t *testing.T) { So(matchedMetric.Retention, ShouldEqual, 10) So(matchedMetric.RetentionTimestamp, ShouldEqual, 150) }) + Convey("should be default 120sec", func() { matchedMetric := moira.MatchedMetric{ Metric: "my_super_metric;tag2=val2", From d1f36006f64af59b064e18de758946b9ab4d6fd9 Mon Sep 17 00:00:00 2001 From: almostinf Date: Wed, 7 Aug 2024 17:23:49 +0300 Subject: [PATCH 4/5] add custom error for invalid pattern and retentions --- filter/cache_storage.go | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/filter/cache_storage.go b/filter/cache_storage.go index 1b945798d..6e61741f5 100644 --- a/filter/cache_storage.go +++ b/filter/cache_storage.go @@ -2,6 +2,7 @@ package filter import ( "bufio" + "errors" "fmt" "io" "regexp" @@ -12,7 +13,12 @@ import ( "github.com/moira-alert/moira/metrics" ) -var defaultRetention = 60 +var ( + defaultRetention = 60 + + InvalidRetentionsFormatErr = errors.New("Invalid retentions format, it is correct to write in the format 'retentions = timePerPoint:timeToStore, timePerPoint:timeToStore, ...'") + InvalidPatternFormatErr = errors.New("Invalid pattern format, it is correct to write in the format 'pattern = regex'") +) type retentionMatcher struct { pattern *regexp.Regexp @@ -83,16 +89,17 @@ func (storage *Storage) buildRetentions(retentionScanner *bufio.Scanner) error { storage.retentions = make([]retentionMatcher, 0, 100) for retentionScanner.Scan() { - line1 := retentionScanner.Text() - if strings.HasPrefix(line1, "#") || strings.Count(line1, "=") < 1 { + patternLine := retentionScanner.Text() + if strings.HasPrefix(patternLine, "#") || strings.Count(patternLine, "=") < 1 { continue } - _, after, found := strings.Cut(line1, "=") + _, after, found := strings.Cut(patternLine, "=") if !found { storage.logger.Error(). - String("pattern_line", line1). - Msg(`Invalid pattern format, it is correct to write in the format "pattern = regex"`) + Error(InvalidPatternFormatErr). + String("pattern_line", patternLine). + Msg("Invalid pattern format") continue } @@ -103,14 +110,15 @@ func (storage *Storage) buildRetentions(retentionScanner *bufio.Scanner) error { } retentionScanner.Scan() - line2 := retentionScanner.Text() - splitted := strings.Split(line2, "=") + retentionsLine := retentionScanner.Text() + splitted := strings.Split(retentionsLine, "=") if len(splitted) < 2 { //nolint storage.logger.Error(). - String("pattern_line", line1). - String("retentions_line", line2). - Msg(`Invalid retentions format, it is correct to write in the format "retentions = timePerPoint:timeToStore, timePerPoint:timeToStore, ..."`) + Error(InvalidRetentionsFormatErr). + String("pattern_line", patternLine). + String("retentions_line", retentionsLine). + Msg("Invalid retentions format") continue } From de923748330528e9876168b82348fd558e936293 Mon Sep 17 00:00:00 2001 From: almostinf Date: Fri, 9 Aug 2024 12:08:38 +0300 Subject: [PATCH 5/5] make errors private and add const --- filter/cache_storage.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/filter/cache_storage.go b/filter/cache_storage.go index 6e61741f5..077a8c306 100644 --- a/filter/cache_storage.go +++ b/filter/cache_storage.go @@ -13,11 +13,11 @@ import ( "github.com/moira-alert/moira/metrics" ) -var ( - defaultRetention = 60 +const defaultRetention = 60 - InvalidRetentionsFormatErr = errors.New("Invalid retentions format, it is correct to write in the format 'retentions = timePerPoint:timeToStore, timePerPoint:timeToStore, ...'") - InvalidPatternFormatErr = errors.New("Invalid pattern format, it is correct to write in the format 'pattern = regex'") +var ( + invalidRetentionsFormatErr = errors.New("Invalid retentions format, it is correct to write in the format 'retentions = timePerPoint:timeToStore, timePerPoint:timeToStore, ...'") + invalidPatternFormatErr = errors.New("Invalid pattern format, it is correct to write in the format 'pattern = regex'") ) type retentionMatcher struct { @@ -97,7 +97,7 @@ func (storage *Storage) buildRetentions(retentionScanner *bufio.Scanner) error { _, after, found := strings.Cut(patternLine, "=") if !found { storage.logger.Error(). - Error(InvalidPatternFormatErr). + Error(invalidPatternFormatErr). String("pattern_line", patternLine). Msg("Invalid pattern format") continue @@ -115,7 +115,7 @@ func (storage *Storage) buildRetentions(retentionScanner *bufio.Scanner) error { if len(splitted) < 2 { //nolint storage.logger.Error(). - Error(InvalidRetentionsFormatErr). + Error(invalidRetentionsFormatErr). String("pattern_line", patternLine). String("retentions_line", retentionsLine). Msg("Invalid retentions format")