Fix WithGroup metric option being silently ignored

internal/metric/collector_test.go

@@ -0,0 +1,90 @@
+package metric
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	operation "github.com/deckhouse/module-sdk/pkg/metric/operation"
+)
+
+// Set(..., WithGroup("g")) must land the group on the collected operation.
+// This is the end-to-end path a Go hook actually uses.
+func Test_Collector_Set_WithGroup(t *testing.T) {
+	mc := NewCollector()
+
+	mc.Set("d8_example_metric", 1, nil, operation.WithGroup("example_group"))
+
+	metrics := mc.CollectedMetrics()
+	require.Len(t, metrics, 1)
+	assert.Equal(t, "example_group", metrics[0].Group)
+	assert.Equal(t, "set", metrics[0].Action)
+}
+
+func Test_Collector_Add_WithGroup(t *testing.T) {
+	mc := NewCollector()
+
+	mc.Add("d8_example_metric", 2, nil, operation.WithGroup("example_group"))
+
+	metrics := mc.CollectedMetrics()
+	require.Len(t, metrics, 1)
+	assert.Equal(t, "example_group", metrics[0].Group)
+	assert.Equal(t, "add", metrics[0].Action)
+}
+
+func Test_Collector_Inc_WithGroup(t *testing.T) {
+	mc := NewCollector()
+
+	mc.Inc("d8_example_metric", nil, operation.WithGroup("example_group"))
+
+	metrics := mc.CollectedMetrics()
+	require.Len(t, metrics, 1)
+	assert.Equal(t, "example_group", metrics[0].Group)
+}
+
+// WithGroup must override the collector's default group.
+func Test_Collector_WithGroup_OverridesDefaultGroup(t *testing.T) {
+	mc := NewCollector(WithDefaultGroup("default_group"))
+
+	mc.Set("d8_example_metric", 1, nil, operation.WithGroup("explicit_group"))
+
+	metrics := mc.CollectedMetrics()
+	require.Len(t, metrics, 1)
+	assert.Equal(t, "explicit_group", metrics[0].Group)
+}
+
+// Without an explicit group the default group is used.
+func Test_Collector_DefaultGroup_Applied(t *testing.T) {
+	mc := NewCollector(WithDefaultGroup("default_group"))
+
+	mc.Set("d8_example_metric", 1, nil)
+
+	metrics := mc.CollectedMetrics()
+	require.Len(t, metrics, 1)
+	assert.Equal(t, "default_group", metrics[0].Group)
+}
+
+// Applying WithGroup to one operation must not leak into a later one.
+func Test_Collector_WithGroup_DoesNotLeakBetweenMetrics(t *testing.T) {
+	mc := NewCollector()
+
+	mc.Set("grouped", 1, nil, operation.WithGroup("g1"))
+	mc.Set("ungrouped", 1, nil)
+
+	metrics := mc.CollectedMetrics()
+	require.Len(t, metrics, 2)
+	assert.Equal(t, "g1", metrics[0].Group)
+	assert.Empty(t, metrics[1].Group)
+}
+
+func Test_Collector_Expire(t *testing.T) {
+	mc := NewCollector()
+
+	mc.Expire("some_group")
+
+	metrics := mc.CollectedMetrics()
+	require.Len(t, metrics, 1)
+	assert.Equal(t, "some_group", metrics[0].Group)
+	assert.Equal(t, "expire", metrics[0].Action)
+}

pkg/metric/operation/operation.go

@@ -36,8 +36,8 @@ type Operation struct {
 	Labels  map[string]string `json:"labels"`
 }
 
-func (op Operation) WithGroup(group string) {
-	op.Group = group //nolint: staticcheck
+func (op *Operation) WithGroup(group string) {
+	op.Group = group
 }
 
 func (op Operation) Validate() error {

pkg/metric/operation/operation_withgroup_test.go

@@ -0,0 +1,60 @@
+package operation
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/deckhouse/module-sdk/pkg"
+)
+
+// Regression test: WithGroup used to be declared on a value receiver, so the
+// assignment landed in a copy and the group was silently dropped.
+func Test_Operation_WithGroup_MutatesReceiver(t *testing.T) {
+	op := &Operation{Name: "d8_example_metric", Action: "set"}
+
+	op.WithGroup("example_group")
+
+	assert.Equal(t, "example_group", op.Group)
+}
+
+// The option produced by the package-level WithGroup must reach the operation
+// through the applier interface, exactly as the collector invokes it.
+func Test_WithGroup_Option_AppliesThroughApplier(t *testing.T) {
+	op := &Operation{Name: "d8_example_metric", Action: "set"}
+
+	var applier pkg.MetricCollectorOptionApplier = op
+	WithGroup("example_group").Apply(applier)
+
+	assert.Equal(t, "example_group", op.Group)
+}
+
+// *Operation must keep satisfying the applier interface after the receiver
+// change, otherwise the collector would fail to compile.
+func Test_Operation_ImplementsApplier(t *testing.T) {
+	var _ pkg.MetricCollectorOptionApplier = (*Operation)(nil)
+}
+
+func Test_MetricOperationsFromReader_RoundTrip(t *testing.T) {
+	data := []byte(`{"name":"m1","group":"g1","action":"set","value":1}` + "\n" +
+		`{"name":"m2","action":"add","value":2}` + "\n")
+
+	ops, err := MetricOperationsFromBytes(data)
+	require.NoError(t, err)
+	require.Len(t, ops, 2)
+
+	assert.Equal(t, "m1", ops[0].Name)
+	assert.Equal(t, "g1", ops[0].Group)
+	assert.Equal(t, "set", ops[0].Action)
+
+	assert.Equal(t, "m2", ops[1].Name)
+	assert.Empty(t, ops[1].Group)
+	assert.Equal(t, "add", ops[1].Action)
+}
+
+func Test_MetricOperationsFromBytes_Empty(t *testing.T) {
+	ops, err := MetricOperationsFromBytes(nil)
+	require.NoError(t, err)
+	assert.Empty(t, ops)
+}