Sync loki.process: metric.coutners

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)
 -----------------
 

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

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{

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"

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"

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"

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"

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"

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"