From fa2fc3892c41664daf75d991afe7d0eb97d4daee Mon Sep 17 00:00:00 2001 From: Piotr <17101802+thampiotr@users.noreply.github.com> Date: Mon, 9 Oct 2023 16:56:45 +0100 Subject: [PATCH] Sync loki.process: metric.coutners (#5415) * Sync loki.process: coutners * Add comments for other synced files --- CHANGELOG.md | 5 + component/loki/process/metric/counters.go | 4 +- .../loki/process/metric/counters_test.go | 96 +++++++++++++++++++ component/loki/process/metric/gauges.go | 2 + component/loki/process/metric/gauges_test.go | 2 + component/loki/process/metric/histograms.go | 2 + .../loki/process/metric/histograms_test.go | 2 + component/loki/process/metric/metricvec.go | 2 + component/loki/process/process_test.go | 2 + 9 files changed, 116 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45af044629e4..64815911e36a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,11 @@ internal API changes are not present. Main (unreleased) ----------------- +### Bugfixes + +- Fixed an issue where `loki.process` validation for stage `metric.counter` was + allowing invalid combination of configuration options. (@thampiotr) + v0.37.0-rc.1 (2023-10-06) ----------------- diff --git a/component/loki/process/metric/counters.go b/component/loki/process/metric/counters.go index f77f3998fcab..a289553510fa 100644 --- a/component/loki/process/metric/counters.go +++ b/component/loki/process/metric/counters.go @@ -1,5 +1,7 @@ package metric +// NOTE: This code is copied from Promtail (07cbef92268aecc0f20d1791a6df390c2df5c072) with changes kept to the minimum. + import ( "fmt" "time" @@ -55,7 +57,7 @@ func (c *CounterConfig) Validate() error { if c.MatchAll && c.Value != "" { return fmt.Errorf("a 'counter' metric supports either 'match_all' or a 'value', but not both") } - if c.CountEntryBytes && (!c.MatchAll && c.Action != "add") { + if c.CountEntryBytes && (!c.MatchAll || c.Action != "add") { return fmt.Errorf("the 'count_entry_bytes' counter field must be specified along with match_all set to true or action set to 'add'") } return nil diff --git a/component/loki/process/metric/counters_test.go b/component/loki/process/metric/counters_test.go index 45564b54f3fc..d66245e2986c 100644 --- a/component/loki/process/metric/counters_test.go +++ b/component/loki/process/metric/counters_test.go @@ -1,5 +1,7 @@ package metric +// NOTE: This code is copied from Promtail (07cbef92268aecc0f20d1791a6df390c2df5c072) with changes kept to the minimum. + import ( "testing" "time" @@ -9,6 +11,100 @@ import ( "github.com/stretchr/testify/assert" ) +var ( + counterTestTrue = true + counterTestFalse = false + counterTestVal = "some val" +) + +func Test_validateCounterConfig(t *testing.T) { + t.Parallel() + tests := []struct { + name string + config CounterConfig + err string + }{ + {"invalid action", + CounterConfig{ + Action: "del", + MaxIdle: 1 * time.Second, + }, + "the 'action' counter field must be either 'inc' or 'add'", + }, + {"invalid counter match all", + CounterConfig{ + MatchAll: counterTestTrue, + Value: counterTestVal, + Action: "inc", + MaxIdle: 1 * time.Second, + }, + "a 'counter' metric supports either 'match_all' or a 'value', but not both", + }, + {"invalid counter match bytes", + CounterConfig{ + MatchAll: counterTestFalse, + CountEntryBytes: counterTestTrue, + Action: "inc", + MaxIdle: 1 * time.Second, + }, + "the 'count_entry_bytes' counter field must be specified along with match_all set to true or action set to 'add'", + }, + {"invalid counter match bytes action", + CounterConfig{ + MatchAll: counterTestTrue, + CountEntryBytes: counterTestTrue, + Action: "inc", + MaxIdle: 1 * time.Second, + }, + "the 'count_entry_bytes' counter field must be specified along with match_all set to true or action set to 'add'", + }, + {"valid counter match bytes", + CounterConfig{ + MatchAll: counterTestTrue, + CountEntryBytes: counterTestTrue, + Action: "add", + MaxIdle: 1 * time.Second, + }, + "", + }, + {"valid", + CounterConfig{ + Value: counterTestVal, + Action: "inc", + MaxIdle: 1 * time.Second, + }, + "", + }, + {"valid match all is false", + CounterConfig{ + MatchAll: counterTestFalse, + Value: counterTestVal, + Action: "inc", + MaxIdle: 1 * time.Second, + }, + "", + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := tt.config.Validate() + + if err == nil { + if tt.err != "" { + t.Fatalf("Metrics stage validation error, expected error to cointain %q, but got no error", tt.err) + } + } else { + if tt.err == "" { + t.Fatalf("Metrics stage validation error, expected no error, but got %q", err) + } + assert.Contains(t, err.Error(), tt.err) + } + }) + } +} + func TestCounterExpiration(t *testing.T) { t.Parallel() cfg := &CounterConfig{ diff --git a/component/loki/process/metric/gauges.go b/component/loki/process/metric/gauges.go index f5c7eda18054..f3222af1615c 100644 --- a/component/loki/process/metric/gauges.go +++ b/component/loki/process/metric/gauges.go @@ -1,5 +1,7 @@ package metric +// NOTE: This code is copied from Promtail (07cbef92268aecc0f20d1791a6df390c2df5c072) with changes kept to the minimum. + import ( "fmt" "time" diff --git a/component/loki/process/metric/gauges_test.go b/component/loki/process/metric/gauges_test.go index a3044ca7be9b..789a51b74422 100644 --- a/component/loki/process/metric/gauges_test.go +++ b/component/loki/process/metric/gauges_test.go @@ -1,5 +1,7 @@ package metric +// NOTE: This code is copied from Promtail (07cbef92268aecc0f20d1791a6df390c2df5c072) with changes kept to the minimum. + import ( "testing" "time" diff --git a/component/loki/process/metric/histograms.go b/component/loki/process/metric/histograms.go index cd74625ba693..6ce2b2b5f3c9 100644 --- a/component/loki/process/metric/histograms.go +++ b/component/loki/process/metric/histograms.go @@ -1,5 +1,7 @@ package metric +// NOTE: This code is copied from Promtail (07cbef92268aecc0f20d1791a6df390c2df5c072) with changes kept to the minimum. + import ( "fmt" "time" diff --git a/component/loki/process/metric/histograms_test.go b/component/loki/process/metric/histograms_test.go index f60e4ac09935..70aaded856b4 100644 --- a/component/loki/process/metric/histograms_test.go +++ b/component/loki/process/metric/histograms_test.go @@ -1,5 +1,7 @@ package metric +// NOTE: This code is copied from Promtail (07cbef92268aecc0f20d1791a6df390c2df5c072) with changes kept to the minimum. + import ( "testing" "time" diff --git a/component/loki/process/metric/metricvec.go b/component/loki/process/metric/metricvec.go index ef698ff8ad30..d1a8d6939909 100644 --- a/component/loki/process/metric/metricvec.go +++ b/component/loki/process/metric/metricvec.go @@ -1,5 +1,7 @@ package metric +// NOTE: This code is copied from Promtail (07cbef92268aecc0f20d1791a6df390c2df5c072) with changes kept to the minimum. + import ( "strings" "sync" diff --git a/component/loki/process/process_test.go b/component/loki/process/process_test.go index 18f17b91b727..0fac72dfc3d9 100644 --- a/component/loki/process/process_test.go +++ b/component/loki/process/process_test.go @@ -1,5 +1,7 @@ package process +// NOTE: This code is copied from Promtail (07cbef92268aecc0f20d1791a6df390c2df5c072) with changes kept to the minimum. + import ( "context" "os"