Add collection-interval to metrics-generator user-configurable overri…

…des (#2899)

* Add collection-interval to metrics-generator user-configurable overrides

Signed-off-by: Robbie Lankford <robert.lankford@grafana.com>

* update CHANGELOG.md

* Update modules/overrides/userconfigurable/client/limits.go

Co-authored-by: Koenraad Verheyden <koenraad.verheyden@posteo.net>

* add collection interval validation

* implement json.Unmarshaler for duration type

* implement json.Marshaler for duration type

* fix tests/lint

* move duration code

---------

Signed-off-by: Robbie Lankford <robert.lankford@grafana.com>
Co-authored-by: Koenraad Verheyden <koenraad.verheyden@posteo.net>

CHANGELOG.md

@@ -19,6 +19,7 @@
 * [ENHANCEMENT] Add RootSpanName and RootServiceName to log about discarded spans [#2816](https://github.com/grafana/tempo/pull/2816) (@marcinginszt)
 * [ENHANCEMENT] Add `UserID` to log message about rate limiting [#2850](https://github.com/grafana/tempo/pull/2850) (@lshippy)
 * [ENHANCEMENT] Add span metrics filter policies to user-configurable overrides [#2906](https://github.com/grafana/tempo/pull/2906) (@rlankfo)
+* [ENHANCEMENT] Add collection-interval to metrics-generator config in user-configurable overrides [#2899](https://github.com/grafana/tempo/pull/2899) (@rlankfo)
 * [BUGFIX] Fix panic in metrics summary api [#2738](https://github.com/grafana/tempo/pull/2738) (@mdisibio)
 * [BUGFIX] Only search ingester blocks that fall within the request time range. [#2783](https://github.com/grafana/tempo/pull/2783) (@joe-elliott)
 * [BUGFIX] Align tempo_query_frontend_queries_total and tempo_query_frontend_queries_within_slo_total. [#2840](https://github.com/grafana/tempo/pull/2840) (@joe-elliott)

cmd/tempo/app/overrides_validation.go

@@ -2,6 +2,7 @@ package app
 
 import (
 	"fmt"
+	"time"
 
 	"golang.org/x/exp/slices"
 
@@ -57,5 +58,11 @@ func (v *overridesValidator) Validate(limits *client.Limits) error {
 		}
 	}
 
+	if collectionInterval, ok := limits.GetMetricsGenerator().GetCollectionInterval(); ok {
+		if collectionInterval < 15*time.Second || collectionInterval > 5*time.Minute {
+			return fmt.Errorf("metrics_generator.collection_interval \"%s\" is outside acceptable range of 15s to 5m", collectionInterval.String())
+		}
+	}
+
 	return nil
 }

cmd/tempo/app/overrides_validation_test.go

@@ -3,6 +3,7 @@ package app
 import (
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 
@@ -96,6 +97,35 @@ func Test_overridesValidator(t *testing.T) {
 			},
 			expErr: "invalid include policy: invalid match type: invalid",
 		},
+		{
+			name: "metrics_generator.collection_interval valid",
+			cfg:  Config{},
+			limits: client.Limits{
+				MetricsGenerator: &client.LimitsMetricsGenerator{
+					CollectionInterval: &client.Duration{Duration: 60 * time.Second},
+				},
+			},
+		},
+		{
+			name: "metrics_generator.collection_interval minimum",
+			cfg:  Config{},
+			limits: client.Limits{
+				MetricsGenerator: &client.LimitsMetricsGenerator{
+					CollectionInterval: &client.Duration{Duration: 1 * time.Second},
+				},
+			},
+			expErr: "metrics_generator.collection_interval \"1s\" is outside acceptable range of 15s to 5m",
+		},
+		{
+			name: "metrics_generator.collection_interval maximum",
+			cfg:  Config{},
+			limits: client.Limits{
+				MetricsGenerator: &client.LimitsMetricsGenerator{
+					CollectionInterval: &client.Duration{Duration: 10 * time.Minute},
+				},
+			},
+			expErr: "metrics_generator.collection_interval \"10m0s\" is outside acceptable range of 15s to 5m",
+		},
 	}
 
 	for _, tc := range testCases {

modules/overrides/user_configurable_overrides.go

@@ -215,6 +215,13 @@ func (o *userConfigurableOverridesManager) MetricsGeneratorDisableCollection(use
 	return o.Interface.MetricsGeneratorDisableCollection(userID)
 }
 
+func (o *userConfigurableOverridesManager) MetricsGeneratorCollectionInterval(userID string) time.Duration {
+	if collectionInterval, ok := o.getTenantLimits(userID).GetMetricsGenerator().GetCollectionInterval(); ok {
+		return collectionInterval
+	}
+	return o.Interface.MetricsGeneratorCollectionInterval(userID)
+}
+
 func (o *userConfigurableOverridesManager) MetricsGeneratorProcessorServiceGraphsDimensions(userID string) []string {
 	if dimensions, ok := o.getTenantLimits(userID).GetMetricsGenerator().GetProcessor().GetServiceGraphs().GetDimensions(); ok {
 		return dimensions

modules/overrides/user_configurable_overrides_test.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
+	"time"
 
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
@@ -72,6 +73,7 @@ func TestUserConfigOverridesManager_allFields(t *testing.T) {
 	assert.Empty(t, mgr.Forwarders(tenant1))
 	assert.Empty(t, mgr.MetricsGeneratorProcessors(tenant1))
 	assert.Equal(t, false, mgr.MetricsGeneratorDisableCollection(tenant1))
+	assert.Equal(t, 0*time.Second, mgr.MetricsGeneratorCollectionInterval(tenant1))
 	assert.Empty(t, mgr.MetricsGeneratorProcessorServiceGraphsDimensions(tenant1))
 	assert.Empty(t, false, mgr.MetricsGeneratorProcessorServiceGraphsEnableClientServerPrefix(tenant1))
 	assert.Empty(t, mgr.MetricsGeneratorProcessorServiceGraphsPeerAttributes(tenant1))
@@ -83,8 +85,9 @@ func TestUserConfigOverridesManager_allFields(t *testing.T) {
 	mgr.tenantLimits[tenant1] = &userconfigurableoverrides.Limits{
 		Forwarders: &[]string{"my-forwarder"},
 		MetricsGenerator: &userconfigurableoverrides.LimitsMetricsGenerator{
-			Processors:        map[string]struct{}{"service-graphs": {}},
-			DisableCollection: boolPtr(true),
+			Processors:         map[string]struct{}{"service-graphs": {}},
+			DisableCollection:  boolPtr(true),
+			CollectionInterval: &userconfigurableoverrides.Duration{Duration: 60 * time.Second},
 			Processor: &userconfigurableoverrides.LimitsMetricsGeneratorProcessor{
 				ServiceGraphs: &userconfigurableoverrides.LimitsMetricsGeneratorProcessorServiceGraphs{
 					Dimensions:               &[]string{"sg-dimension"},
@@ -126,6 +129,7 @@ func TestUserConfigOverridesManager_allFields(t *testing.T) {
 	assert.Equal(t, map[string]struct{}{"service-graphs": {}}, mgr.MetricsGeneratorProcessors(tenant1))
 	assert.Equal(t, true, mgr.MetricsGeneratorDisableCollection(tenant1))
 	assert.Equal(t, []string{"sg-dimension"}, mgr.MetricsGeneratorProcessorServiceGraphsDimensions(tenant1))
+	assert.Equal(t, 60*time.Second, mgr.MetricsGeneratorCollectionInterval(tenant1))
 	assert.Equal(t, true, mgr.MetricsGeneratorProcessorServiceGraphsEnableClientServerPrefix(tenant1))
 	assert.Equal(t, []string{"attribute"}, mgr.MetricsGeneratorProcessorServiceGraphsPeerAttributes(tenant1))
 	assert.Equal(t, []string{"sm-dimension"}, mgr.MetricsGeneratorProcessorSpanMetricsDimensions(tenant1))

modules/overrides/userconfigurable/client/duration.go

@@ -0,0 +1,45 @@
+package client
+
+import (
+	"encoding/json"
+	"fmt"
+	"time"
+)
+
+type Duration struct {
+	time.Duration
+}
+
+// MarshalJSON implements json.Marshaler.
+func (d *Duration) MarshalJSON() ([]byte, error) {
+	return []byte(fmt.Sprintf(`"%s"`, d.String())), nil
+}
+
+// UnmarshalJSON implements json.Unmarshaler.
+func (d *Duration) UnmarshalJSON(input []byte) error {
+	var unmarshalledJSON interface{}
+
+	err := json.Unmarshal(input, &unmarshalledJSON)
+	if err != nil {
+		return err
+	}
+
+	switch value := unmarshalledJSON.(type) {
+	case float64:
+		d.Duration = time.Duration(value)
+	case string:
+		d.Duration, err = time.ParseDuration(value)
+		if err != nil {
+			return err
+		}
+	default:
+		return fmt.Errorf("invalid duration: %#v", unmarshalledJSON)
+	}
+
+	return nil
+}
+
+var (
+	_ json.Marshaler   = (*Duration)(nil)
+	_ json.Unmarshaler = (*Duration)(nil)
+)

modules/overrides/userconfigurable/client/duration_test.go

@@ -0,0 +1,86 @@
+package client
+
+import (
+	"encoding/json"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+type mockStruct struct {
+	Duration *Duration `json:"duration"`
+}
+
+func TestDuration_MarshalJSON(t *testing.T) {
+	testCases := []struct {
+		name     string
+		input    mockStruct
+		expected []byte
+		expErr   string
+	}{
+		{
+			name:     "marshal duration",
+			input:    mockStruct{&Duration{60 * time.Second}},
+			expected: []byte("{\"duration\":\"1m0s\"}"),
+			expErr:   "",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			result, err := json.Marshal(tc.input)
+			if tc.expErr != "" {
+				assert.EqualError(t, err, tc.expErr)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tc.expected, result)
+			}
+		})
+	}
+}
+
+func TestDuration_UnmarshalJSON(t *testing.T) {
+	testCases := []struct {
+		name     string
+		input    []byte
+		expected mockStruct
+		expErr   string
+	}{
+		{
+			name:  "unmarshal duration nanoseconds",
+			input: []byte(`{"duration":60000000000}`),
+			expected: mockStruct{
+				Duration: &Duration{60 * time.Second},
+			},
+			expErr: "",
+		},
+		{
+			name:  "unmarshal duration string",
+			input: []byte(`{"duration":"60s"}`),
+			expected: mockStruct{
+				Duration: &Duration{60 * time.Second},
+			},
+			expErr: "",
+		},
+		{
+			name:     "unmarshal duration error",
+			input:    []byte(`{"duration":"foo"}`),
+			expected: mockStruct{},
+			expErr:   "time: invalid duration \"foo\"",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			var result mockStruct
+			err := json.Unmarshal(tc.input, &result)
+			if tc.expErr != "" {
+				assert.EqualError(t, err, tc.expErr)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, *tc.expected.Duration, *result.Duration)
+			}
+		})
+	}
+}

modules/overrides/userconfigurable/client/limits.go

@@ -1,6 +1,8 @@
 package client
 
 import (
+	"time"
+
 	filterconfig "github.com/grafana/tempo/pkg/spanfilter/config"
 	"github.com/grafana/tempo/pkg/util/listtomap"
 )
@@ -26,8 +28,9 @@ func (l *Limits) GetMetricsGenerator() *LimitsMetricsGenerator {
 }
 
 type LimitsMetricsGenerator struct {
-	Processors        listtomap.ListToMap `json:"processors,omitempty"`
-	DisableCollection *bool               `json:"disable_collection,omitempty"`
+	Processors         listtomap.ListToMap `json:"processors,omitempty"`
+	DisableCollection  *bool               `json:"disable_collection,omitempty"`
+	CollectionInterval *Duration           `json:"collection_interval,omitempty"`
 
 	Processor *LimitsMetricsGeneratorProcessor `json:"processor,omitempty"`
 }
@@ -53,6 +56,13 @@ func (l *LimitsMetricsGenerator) GetProcessor() *LimitsMetricsGeneratorProcessor
 	return nil
 }
 
+func (l *LimitsMetricsGenerator) GetCollectionInterval() (time.Duration, bool) {
+	if l != nil && l.CollectionInterval != nil {
+		return l.CollectionInterval.Duration, true
+	}
+	return 0, false
+}
+
 type LimitsMetricsGeneratorProcessor struct {
 	ServiceGraphs *LimitsMetricsGeneratorProcessorServiceGraphs `json:"service_graphs,omitempty"`
 	SpanMetrics   *LimitsMetricsGeneratorProcessorSpanMetrics   `json:"span_metrics,omitempty"`