From 1c6dbfed339523d3d683a9e49b4908539d3bfc84 Mon Sep 17 00:00:00 2001 From: Yusof Ganji Date: Thu, 13 Aug 2026 13:39:49 -0700 Subject: [PATCH] feat: sanitize "." in descriptor keys/values before publishing metrics Add an opt-in SANITIZE_DESCRIPTOR_METRIC_DOTS setting (default false, so behavior is unchanged for existing deployments). When enabled, "." in a descriptor key or value is replaced with "_" as the metric name is built, so dotted values (hostnames, gRPC paths, etc.) don't inject extra statsd hierarchy segments that break the Prometheus statsd_exporter metric-name -> label mapping. Sanitization is applied only to the fragments written into metric names in the config key-builders (descriptorKey, detailed_metric, value_to_metric, and share_threshold paths); the descriptor-map lookup key and the rate-limit cache key are left verbatim, so rate-limit matching is unaffected. This is distinct from the pre-existing SanitizeStatName, which only strips dots inside IPv4 addresses and runs unconditionally. The flag is threaded through the stats Manager (which already receives Settings), avoiding churn in the config loader chain and its ~40 call sites. Tests: config-layer unit tests for flag on/off across the value, non-IPv4 value, key, detailed_metric, value_to_metric, and matching-still-works cases; a SanitizeStatKeyValue helper unit test; a manager accessor test; and an end-to-end integration test that drives a dotted value through a real running server and asserts the sanitized metric name for both the value_to_metric and detailed_metric builders. Signed-off-by: Yusof Ganji Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 7 + src/config/config_impl.go | 40 +++-- src/settings/settings.go | 1 + src/stats/manager.go | 12 +- src/stats/manager_impl.go | 13 +- src/utils/utilities.go | 7 + test/config/config_test.go | 165 ++++++++++++++++++ test/integration/integration_test.go | 62 +++++++ .../current/ratelimit/config/sanitize.yaml | 12 ++ test/mocks/stats/manager.go | 13 +- test/stats/manager_impl_test.go | 8 + test/utils/utilities_test.go | 18 ++ 12 files changed, 337 insertions(+), 21 deletions(-) create mode 100644 test/integration/runtime/current/ratelimit/config/sanitize.yaml diff --git a/README.md b/README.md index 0485d0e7e..3ca468d74 100644 --- a/README.md +++ b/README.md @@ -1022,6 +1022,13 @@ Configure statistics output frequency with `STATS_FLUSH_INTERVAL`, where the typ To disable statistics entirely, set env var `DISABLE_STATS` to `true` +To sanitize `.` characters in descriptor keys and values before they are published as metrics, set env var +`SANITIZE_DESCRIPTOR_METRIC_DOTS` to `true` (default `false`). When enabled, each `.` in a descriptor key or value is +replaced with `_` in the emitted metric name. Because `.` is the statsd metric-hierarchy separator, a dotted value +(e.g. a gRPC path like `/helloworld.Greeter/SayHello`) would otherwise inject unintended extra hierarchy levels — +which breaks the Prometheus `statsd_exporter` metric-name-to-label mapping. This only affects metric names; rate limit +matching (which keys off the raw descriptor) is unchanged. It is disabled by default for backward compatibility. + Rate Limit Statistic Path: ``` diff --git a/src/config/config_impl.go b/src/config/config_impl.go index c2b48e3f9..82fffcdd8 100644 --- a/src/config/config_impl.go +++ b/src/config/config_impl.go @@ -12,6 +12,7 @@ import ( "gopkg.in/yaml.v2" "github.com/envoyproxy/ratelimit/src/stats" + "github.com/envoyproxy/ratelimit/src/utils" ) type yamlReplaces struct { @@ -443,6 +444,17 @@ func (this *rateLimitConfigImpl) Dump() string { return ret } +// maybeSanitize replaces '.' with '_' in a descriptor key or value fragment when the +// SANITIZE_DESCRIPTOR_METRIC_DOTS setting is enabled, so dotted values don't inject extra +// metric hierarchy levels. It must only be applied to fragments written into metric names, +// never to keys used for descriptor map lookups (that would break rate limit matching). +func (this *rateLimitConfigImpl) maybeSanitize(s string) string { + if this.statsManager.SanitizeDescriptorMetricDots() { + return utils.SanitizeStatKeyValue(s) + } + return s +} + func (this *rateLimitConfigImpl) GetLimit( ctx context.Context, domain string, descriptor *pb_struct.RateLimitDescriptor, ) *RateLimit { @@ -457,7 +469,7 @@ func (this *rateLimitConfigImpl) GetLimit( } if descriptor.GetLimit() != nil { - rateLimitKey := descriptorKey(domain, descriptor) + rateLimitKey := this.descriptorKey(domain, descriptor) rateLimitOverrideUnit := pb.RateLimitResponse_RateLimit_Unit(descriptor.GetLimit().GetUnit()) // When limit override is provided by envoy config, we don't want to enable shadow_mode rateLimit = NewRateLimit( @@ -494,8 +506,12 @@ func (this *rateLimitConfigImpl) GetLimit( // to check for a default value. finalKey := entry.Key + "_" + entry.Value + // Write the sanitized fragments (never finalKey itself, which is reused for the + // descriptor map lookup below and must stay verbatim for rate limit matching). detailedMetricFullKey.WriteString(".") - detailedMetricFullKey.WriteString(finalKey) + detailedMetricFullKey.WriteString(this.maybeSanitize(entry.Key)) + detailedMetricFullKey.WriteString("_") + detailedMetricFullKey.WriteString(this.maybeSanitize(entry.Value)) logger.Debugf("looking up key: %s", finalKey) nextDescriptor := descriptorsMap[finalKey] @@ -561,14 +577,14 @@ func (this *rateLimitConfigImpl) GetLimit( } // Write key and value (if any) - valueToMetricFullKey.WriteString(entry.Key) + valueToMetricFullKey.WriteString(this.maybeSanitize(entry.Key)) if valueToUse != "" { valueToMetricFullKey.WriteString("_") - valueToMetricFullKey.WriteString(valueToUse) + valueToMetricFullKey.WriteString(this.maybeSanitize(valueToUse)) } } else { // No next descriptor found; still append something deterministic - valueToMetricFullKey.WriteString(entry.Key) + valueToMetricFullKey.WriteString(this.maybeSanitize(entry.Key)) } if nextDescriptor != nil && nextDescriptor.limit != nil { @@ -635,15 +651,15 @@ func (this *rateLimitConfigImpl) GetLimit( for i, entry := range descriptor.Entries { shareThresholdMetricKey.WriteString(".") if i < len(rateLimit.ShareThresholdKeyPattern) && rateLimit.ShareThresholdKeyPattern[i] != "" { - shareThresholdMetricKey.WriteString(entry.Key) + shareThresholdMetricKey.WriteString(this.maybeSanitize(entry.Key)) shareThresholdMetricKey.WriteString("_") - shareThresholdMetricKey.WriteString(rateLimit.ShareThresholdKeyPattern[i]) + shareThresholdMetricKey.WriteString(this.maybeSanitize(rateLimit.ShareThresholdKeyPattern[i])) } else { // Include full key_value for entries without share_threshold - shareThresholdMetricKey.WriteString(entry.Key) + shareThresholdMetricKey.WriteString(this.maybeSanitize(entry.Key)) if entry.Value != "" { shareThresholdMetricKey.WriteString("_") - shareThresholdMetricKey.WriteString(entry.Value) + shareThresholdMetricKey.WriteString(this.maybeSanitize(entry.Value)) } } } @@ -676,15 +692,15 @@ func (this *rateLimitConfigImpl) IsEmptyDomains() bool { return len(this.domains) == 0 } -func descriptorKey(domain string, descriptor *pb_struct.RateLimitDescriptor) string { +func (this *rateLimitConfigImpl) descriptorKey(domain string, descriptor *pb_struct.RateLimitDescriptor) string { rateLimitKey := "" for _, entry := range descriptor.Entries { if rateLimitKey != "" { rateLimitKey += "." } - rateLimitKey += entry.Key + rateLimitKey += this.maybeSanitize(entry.Key) if entry.Value != "" { - rateLimitKey += "_" + entry.Value + rateLimitKey += "_" + this.maybeSanitize(entry.Value) } } return domain + "." + rateLimitKey diff --git a/src/settings/settings.go b/src/settings/settings.go index ffb547083..198f3a1e3 100644 --- a/src/settings/settings.go +++ b/src/settings/settings.go @@ -90,6 +90,7 @@ type Settings struct { ExtraTags map[string]string `envconfig:"EXTRA_TAGS" default:""` StatsFlushInterval time.Duration `envconfig:"STATS_FLUSH_INTERVAL" default:"10s"` DisableStats bool `envconfig:"DISABLE_STATS" default:"false"` + SanitizeDescriptorMetricDots bool `envconfig:"SANITIZE_DESCRIPTOR_METRIC_DOTS" default:"false"` UsePrometheus bool `envconfig:"USE_PROMETHEUS" default:"false"` PrometheusAddr string `envconfig:"PROMETHEUS_ADDR" default:":9090"` PrometheusPath string `envconfig:"PROMETHEUS_PATH" default:"/metrics"` diff --git a/src/stats/manager.go b/src/stats/manager.go index a63a7b5c9..688cfb8f7 100644 --- a/src/stats/manager.go +++ b/src/stats/manager.go @@ -20,13 +20,17 @@ type Manager interface { NewServiceStats() ServiceStats // Returns the stats.Store wrapped by the Manager. GetStatsStore() gostats.Store + // SanitizeDescriptorMetricDots reports whether '.' in descriptor keys/values should be + // replaced with '_' when building metric names (SANITIZE_DESCRIPTOR_METRIC_DOTS). + SanitizeDescriptorMetricDots() bool } type ManagerImpl struct { - store gostats.Store - rlStatsScope gostats.Scope - serviceStatsScope gostats.Scope - shouldRateLimitScope gostats.Scope + store gostats.Store + rlStatsScope gostats.Scope + serviceStatsScope gostats.Scope + shouldRateLimitScope gostats.Scope + sanitizeDescriptorMetricDots bool } // Stats for panic recoveries. diff --git a/src/stats/manager_impl.go b/src/stats/manager_impl.go index e629064c7..aa195f88c 100644 --- a/src/stats/manager_impl.go +++ b/src/stats/manager_impl.go @@ -11,10 +11,11 @@ import ( func NewStatManager(store gostats.Store, settings settings.Settings) *ManagerImpl { serviceScope := store.ScopeWithTags("ratelimit", settings.ExtraTags).Scope("service") return &ManagerImpl{ - store: store, - rlStatsScope: serviceScope.Scope("rate_limit"), - serviceStatsScope: serviceScope, - shouldRateLimitScope: serviceScope.Scope("call.should_rate_limit"), + store: store, + rlStatsScope: serviceScope.Scope("rate_limit"), + serviceStatsScope: serviceScope, + shouldRateLimitScope: serviceScope.Scope("call.should_rate_limit"), + sanitizeDescriptorMetricDots: settings.SanitizeDescriptorMetricDots, } } @@ -22,6 +23,10 @@ func (this *ManagerImpl) GetStatsStore() gostats.Store { return this.store } +func (this *ManagerImpl) SanitizeDescriptorMetricDots() bool { + return this.sanitizeDescriptorMetricDots +} + // Create new rate descriptor stats for a descriptor tuple. // @param key supplies the fully resolved descriptor tuple. // @return new stats. diff --git a/src/utils/utilities.go b/src/utils/utilities.go index 60aa516ce..1b8797346 100644 --- a/src/utils/utilities.go +++ b/src/utils/utilities.go @@ -87,6 +87,13 @@ func SanitizeStatName(s string) string { }) } +// SanitizeStatKeyValue replaces the statsd hierarchy separator '.' with '_' so that +// dots in a descriptor key or value do not create unintended metric hierarchy levels +// (which, for example, break the Prometheus statsd_exporter metric-name -> label mapping). +func SanitizeStatKeyValue(s string) string { + return strings.ReplaceAll(s, ".", "_") +} + type HitsAddend struct { Value uint64 IsNegative bool diff --git a/test/config/config_test.go b/test/config/config_test.go index 2bd009a99..19a9fe17c 100644 --- a/test/config/config_test.go +++ b/test/config/config_test.go @@ -2261,3 +2261,168 @@ func TestMetadata(t *testing.T) { // share_threshold parity (middle+trailing wildcards produce the same shared-counter // behaviour as trailing-only) is covered by TestShareThreshold Cases 5-7. + +// TestSanitizeDescriptorMetricDots verifies the SANITIZE_DESCRIPTOR_METRIC_DOTS behavior: +// when enabled, '.' in descriptor keys/values is replaced with '_' in emitted metric names, +// while rate-limit matching (which keys off the un-sanitized descriptor) is unaffected. +func TestSanitizeDescriptorMetricDots(t *testing.T) { + // A config whose leaf key/value are looked up by the un-sanitized descriptor. The + // descriptor value carries dots (an IP and a dotted host) to exercise the metric path. + basicCfg := func() []config.RateLimitConfigToLoad { + return []config.RateLimitConfigToLoad{{ + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "domain", + Descriptors: []config.YamlDescriptor{ + { + Key: "source_ip", + Value: "10.0.0.1", + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 5, + Unit: "minute", + }, + }, + }, + }, + }} + } + ipDescriptor := &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "source_ip", Value: "10.0.0.1"}}, + } + + t.Run("flag off keeps dots (backward compatible)", func(t *testing.T) { + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + rlConfig := config.NewRateLimitConfigImpl(basicCfg(), mockstats.NewMockStatManagerWithSanitize(store, false), false) + rl := rlConfig.GetLimit(context.TODO(), "domain", ipDescriptor) + asrt.NotNil(rl) + asrt.Equal("domain.source_ip_10.0.0.1", rl.Stats.Key) + }) + + t.Run("flag on sanitizes dotted value", func(t *testing.T) { + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + rlConfig := config.NewRateLimitConfigImpl(basicCfg(), mockstats.NewMockStatManagerWithSanitize(store, true), false) + rl := rlConfig.GetLimit(context.TODO(), "domain", ipDescriptor) + asrt.NotNil(rl) + expectedKey := "domain.source_ip_10_0_0_1" + asrt.Equal(expectedKey, rl.Stats.Key) + + // The sanitized key is the one actually registered as a counter. + rl.Stats.TotalHits.Inc() + asrt.EqualValues(1, store.NewCounter(expectedKey+".total_hits").Value()) + }) + + t.Run("flag on sanitizes non-ipv4 dotted value", func(t *testing.T) { + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + cfg := []config.RateLimitConfigToLoad{{ + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "domain", + Descriptors: []config.YamlDescriptor{ + { + Key: "host", + Value: "foo.bar", + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 5, + Unit: "minute", + }, + }, + }, + }, + }} + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManagerWithSanitize(store, true), false) + rl := rlConfig.GetLimit(context.TODO(), "domain", + &pb_struct.RateLimitDescriptor{Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "host", Value: "foo.bar"}}}) + asrt.NotNil(rl) + asrt.Equal("domain.host_foo_bar", rl.Stats.Key) + }) + + t.Run("flag on sanitizes dotted key", func(t *testing.T) { + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + cfg := []config.RateLimitConfigToLoad{{ + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "domain", + Descriptors: []config.YamlDescriptor{ + { + Key: "a.b", + Value: "v", + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 5, + Unit: "minute", + }, + }, + }, + }, + }} + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManagerWithSanitize(store, true), false) + rl := rlConfig.GetLimit(context.TODO(), "domain", + &pb_struct.RateLimitDescriptor{Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "a.b", Value: "v"}}}) + asrt.NotNil(rl) + asrt.Equal("domain.a_b_v", rl.Stats.Key) + }) + + t.Run("matching still works with dotted value when flag on", func(t *testing.T) { + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + rlConfig := config.NewRateLimitConfigImpl(basicCfg(), mockstats.NewMockStatManagerWithSanitize(store, true), false) + // The descriptor still carries the un-sanitized dotted value; if sanitization had + // leaked into the map lookup key, this would fail to match and return nil. + rl := rlConfig.GetLimit(context.TODO(), "domain", ipDescriptor) + asrt.NotNil(rl) + asrt.EqualValues(5, rl.Limit.RequestsPerUnit) + }) + + t.Run("flag on sanitizes detailed_metric path", func(t *testing.T) { + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + cfg := []config.RateLimitConfigToLoad{{ + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "domain", + Descriptors: []config.YamlDescriptor{ + { + Key: "source_ip", + DetailedMetric: true, + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 5, + Unit: "minute", + }, + }, + }, + }, + }} + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManagerWithSanitize(store, true), false) + rl := rlConfig.GetLimit(context.TODO(), "domain", ipDescriptor) + asrt.NotNil(rl) + asrt.Equal("domain.source_ip_10_0_0_1", rl.Stats.Key) + }) + + t.Run("flag on sanitizes value_to_metric path", func(t *testing.T) { + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + cfg := []config.RateLimitConfigToLoad{{ + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "domain", + Descriptors: []config.YamlDescriptor{ + { + Key: "source_ip", + ValueToMetric: true, + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 5, + Unit: "minute", + }, + }, + }, + }, + }} + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManagerWithSanitize(store, true), false) + rl := rlConfig.GetLimit(context.TODO(), "domain", ipDescriptor) + asrt.NotNil(rl) + asrt.Equal("domain.source_ip_10_0_0_1", rl.Stats.Key) + }) +} diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index d7d2c512d..705e40592 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -209,6 +209,68 @@ func TestBasicConfig_ExtraTags(t *testing.T) { }) } +// TestSanitizeDescriptorMetricDots exercises the SANITIZE_DESCRIPTOR_METRIC_DOTS setting +// end-to-end: a descriptor value carrying '.' characters (a gRPC path) is sent through the +// real gRPC server, and we assert the emitted metric name has the dots replaced with '_' +// while rate limiting still works (the request is served, not rejected). +func TestSanitizeDescriptorMetricDots(t *testing.T) { + common.WithMultiRedis(t, []common.RedisConfig{ + {Port: 6383}, + }, func() { + s := makeSimpleRedisSettings(6383, 6380, false, 0) + s.SanitizeDescriptorMetricDots = true + runner := startTestRunner(t, s) + defer runner.Stop() + + assert := assert.New(t) + conn, err := grpc.Dial(fmt.Sprintf("localhost:%v", s.GrpcPort), grpc.WithInsecure()) + assert.NoError(err) + defer conn.Close() + c := pb.NewRateLimitServiceClient(conn) + + // The sanitize domain has a value_to_metric descriptor on key "grpc_path", so the + // runtime value flows into the metric name. The value contains dots. + grpcPath := "/helloworld.Greeter/SayHello" + response, err := c.ShouldRateLimit( + context.Background(), + common.NewRateLimitRequest("sanitize", [][][2]string{{{"grpc_path", grpcPath}}}, 1)) + assert.NoError(err) + assert.Equal(pb.RateLimitResponse_OK, response.OverallCode) + + runner.GetStatsStore().Flush() + + // Dots in the value are replaced with '_' in the emitted metric name. + sanitizedValue := utils.SanitizeStatKeyValue(grpcPath) + sanitizedCounter := runner.GetStatsStore().NewCounter( + fmt.Sprintf("ratelimit.service.rate_limit.sanitize.grpc_path_%s.total_hits", sanitizedValue)) + assert.Equal(1, int(sanitizedCounter.Value())) + + // The un-sanitized (dotted) metric name is NOT emitted. + unsanitizedCounter := runner.GetStatsStore().NewCounter( + fmt.Sprintf("ratelimit.service.rate_limit.sanitize.grpc_path_%s.total_hits", grpcPath)) + assert.Equal(0, int(unsanitizedCounter.Value())) + + // The sanitize domain also has a detailed_metric descriptor on key "detailed_path". + // detailed_metric folds the runtime value into the metric name via a different builder + // than value_to_metric, so exercise that path too. + detailedResponse, err := c.ShouldRateLimit( + context.Background(), + common.NewRateLimitRequest("sanitize", [][][2]string{{{"detailed_path", grpcPath}}}, 1)) + assert.NoError(err) + assert.Equal(pb.RateLimitResponse_OK, detailedResponse.OverallCode) + + runner.GetStatsStore().Flush() + + detailedSanitizedCounter := runner.GetStatsStore().NewCounter( + fmt.Sprintf("ratelimit.service.rate_limit.sanitize.detailed_path_%s.total_hits", sanitizedValue)) + assert.Equal(1, int(detailedSanitizedCounter.Value())) + + detailedUnsanitizedCounter := runner.GetStatsStore().NewCounter( + fmt.Sprintf("ratelimit.service.rate_limit.sanitize.detailed_path_%s.total_hits", grpcPath)) + assert.Equal(0, int(detailedUnsanitizedCounter.Value())) + }) +} + func TestBasicTLSConfig(t *testing.T) { t.Run("WithoutPerSecondRedisTLS", testBasicConfigAuthTLS(false, 0)) t.Run("WithPerSecondRedisTLS", testBasicConfigAuthTLS(true, 0)) diff --git a/test/integration/runtime/current/ratelimit/config/sanitize.yaml b/test/integration/runtime/current/ratelimit/config/sanitize.yaml new file mode 100644 index 000000000..b04131d1b --- /dev/null +++ b/test/integration/runtime/current/ratelimit/config/sanitize.yaml @@ -0,0 +1,12 @@ +domain: sanitize +descriptors: + - key: grpc_path + value_to_metric: true + rate_limit: + unit: minute + requests_per_unit: 50 + - key: detailed_path + detailed_metric: true + rate_limit: + unit: minute + requests_per_unit: 50 diff --git a/test/mocks/stats/manager.go b/test/mocks/stats/manager.go index 253926410..82e7e3821 100644 --- a/test/mocks/stats/manager.go +++ b/test/mocks/stats/manager.go @@ -9,13 +9,18 @@ import ( ) type MockStatManager struct { - store gostats.Store + store gostats.Store + sanitizeDescriptorMetricDots bool } func (m *MockStatManager) GetStatsStore() gostats.Store { return m.store } +func (m *MockStatManager) SanitizeDescriptorMetricDots() bool { + return m.sanitizeDescriptorMetricDots +} + func (m *MockStatManager) NewShouldRateLimitStats() stats.ShouldRateLimitStats { s := m.store.Scope("call.should_rate_limit") ret := stats.ShouldRateLimitStats{} @@ -61,3 +66,9 @@ func (m *MockStatManager) NewDomainStats(key string) stats.DomainStats { func NewMockStatManager(store gostats.Store) stats.Manager { return &MockStatManager{store: store} } + +// NewMockStatManagerWithSanitize builds a mock stat manager with the +// SANITIZE_DESCRIPTOR_METRIC_DOTS behavior toggled on/off. +func NewMockStatManagerWithSanitize(store gostats.Store, sanitizeDescriptorMetricDots bool) stats.Manager { + return &MockStatManager{store: store, sanitizeDescriptorMetricDots: sanitizeDescriptorMetricDots} +} diff --git a/test/stats/manager_impl_test.go b/test/stats/manager_impl_test.go index 793ce835e..3e4b293a0 100644 --- a/test/stats/manager_impl_test.go +++ b/test/stats/manager_impl_test.go @@ -54,3 +54,11 @@ func TestEscapingInvalidChartersInMetricName(t *testing.T) { }) } } + +func TestSanitizeDescriptorMetricDotsAccessor(t *testing.T) { + statsStore := gostats.NewStore(gostatsMock.NewSink(), false) + + assert.False(t, stats.NewStatManager(statsStore, settings.Settings{}).SanitizeDescriptorMetricDots(), + "defaults to off for backward compatibility") + assert.True(t, stats.NewStatManager(statsStore, settings.Settings{SanitizeDescriptorMetricDots: true}).SanitizeDescriptorMetricDots()) +} diff --git a/test/utils/utilities_test.go b/test/utils/utilities_test.go index cadd2983f..efc735b71 100644 --- a/test/utils/utilities_test.go +++ b/test/utils/utilities_test.go @@ -138,3 +138,21 @@ func TestExpirationSecondsNonMonthDoesNotUseTimeSource(t *testing.T) { seconds := utils.ExpirationSeconds(pb.RateLimitResponse_RateLimit_DAY, timeSource, true) assert.EqualValues(t, 60*60*24, seconds) } + +func TestSanitizeStatKeyValue(t *testing.T) { + cases := []struct { + in string + want string + }{ + {"a.b.c", "a_b_c"}, + {"nodots", "nodots"}, + {"", ""}, + {"10.0.0.1", "10_0_0_1"}, + {"foo.bar", "foo_bar"}, + {".leading", "_leading"}, + {"trailing.", "trailing_"}, + } + for _, c := range cases { + assert.Equal(t, c.want, utils.SanitizeStatKeyValue(c.in)) + } +}