From 9e9d3d5894d051b71c334119b8f5a94f219677c6 Mon Sep 17 00:00:00 2001 From: xuannam230201 Date: Fri, 31 Oct 2025 17:17:08 +0700 Subject: [PATCH 1/4] Add field to add unspecified value to metric Signed-off-by: xuannam230201 Signed-off-by: Nam Dang --- README.md | 62 ++++++ src/config/config_impl.go | 65 +++++- test/config/config_test.go | 401 +++++++++++++++++++++++++++++++++++++ 3 files changed, 522 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index eee69cb45..5413b412b 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,7 @@ - [Example 7](#example-7) - [Example 8](#example-8) - [Example 9](#example-9) + - [Example 10](#example-10) - [Loading Configuration](#loading-configuration) - [File Based Configuration Loading](#file-based-configuration-loading) - [xDS Management Server Based Configuration Loading](#xds-management-server-based-configuration-loading) @@ -282,6 +283,7 @@ descriptors: requests_per_unit: shadow_mode: (optional) detailed_metric: (optional) + value_to_metric: (optional) descriptors: (optional block) - ... (nested repetition of above) ``` @@ -336,6 +338,14 @@ Setting the `detailed_metric: true` for a descriptor will extend the metrics tha NB! This should only be enabled in situations where the potentially large cardinality of metrics that this can lead to is acceptable. +### Including descriptor values in metrics + +Setting `value_to_metric: true` (default: `false`) for a descriptor will include the descriptor's runtime value in the metric key, even when the descriptor value is not explicitly defined in the configuration. This allows you to track metrics per descriptor value when the value comes from the runtime request, providing visibility into different rate limit scenarios without needing to pre-define every possible value. + +**Note:** If a value is explicitly specified in a descriptor (e.g., `value: "GET"`), that value is always included in the metric key regardless of the `value_to_metric` setting. The `value_to_metric` flag only affects descriptors where the value is not explicitly defined in the configuration. + +When combined with wildcard matching, the full runtime value is included in the metric key, not just the wildcard prefix. This feature works independently of `detailed_metric` - when `detailed_metric` is set, it takes precedence and `value_to_metric` is ignored. + ### Examples #### Example 1 @@ -629,6 +639,58 @@ descriptors: requests_per_unit: 20 ``` +#### Example 10 + +Using `value_to_metric: true` to include descriptor values in metrics when values are not explicitly defined in the configuration: + +```yaml +domain: example10 +descriptors: + - key: route + value_to_metric: true + descriptors: + - key: http_method + value_to_metric: true + descriptors: + - key: subject_id + rate_limit: + unit: minute + requests_per_unit: 60 +``` + +With this configuration, requests with different runtime values for `route` and `http_method` will generate separate metrics: + +- Request: `route=api`, `http_method=GET`, `subject_id=123` +- Metric key: `example10.route_api.http_method_GET.subject_id` + +- Request: `route=web`, `http_method=POST`, `subject_id=456` +- Metric key: `example10.route_web.http_method_POST.subject_id` + +Without `value_to_metric: true`, both requests would use the same metric key: `example10.route.http_method.subject_id`. + +When combined with wildcard matching, the full runtime value is included: + +```yaml +domain: example10_wildcard +descriptors: + - key: user + value_to_metric: true + descriptors: + - key: action + value: read* + value_to_metric: true + descriptors: + - key: resource + rate_limit: + unit: minute + requests_per_unit: 100 +``` + +- Request: `user=alice`, `action=readfile`, `resource=documents` +- Metric key: `example10_wildcard.user_alice.action_readfile.resource` + +Note: When `detailed_metric: true` is set on a descriptor, it takes precedence and `value_to_metric` is ignored for that descriptor. + ## Loading Configuration Rate limit service supports following configuration loading methods. You can define which methods to use by configuring environment variable `CONFIG_TYPE`. diff --git a/src/config/config_impl.go b/src/config/config_impl.go index 45c276b43..4d1d1bb4c 100644 --- a/src/config/config_impl.go +++ b/src/config/config_impl.go @@ -32,6 +32,7 @@ type YamlDescriptor struct { Descriptors []YamlDescriptor ShadowMode bool `yaml:"shadow_mode"` DetailedMetric bool `yaml:"detailed_metric"` + ValueToMetric bool `yaml:"value_to_metric"` } type YamlRoot struct { @@ -43,6 +44,7 @@ type rateLimitDescriptor struct { descriptors map[string]*rateLimitDescriptor limit *RateLimit wildcardKeys []string + valueToMetric bool } type rateLimitDomain struct { @@ -68,6 +70,7 @@ var validKeys = map[string]bool{ "name": true, "replaces": true, "detailed_metric": true, + "value_to_metric": true, } // Create a new rate limit config entry. @@ -183,10 +186,10 @@ func (this *rateLimitDescriptor) loadDescriptors(config RateLimitConfigToLoad, p } } - logger.Debugf( - "loading descriptor: key=%s%s", newParentKey, rateLimitDebugString) - newDescriptor := &rateLimitDescriptor{map[string]*rateLimitDescriptor{}, rateLimit, nil} - newDescriptor.loadDescriptors(config, newParentKey+".", descriptorConfig.Descriptors, statsManager) + logger.Debugf( + "loading descriptor: key=%s%s", newParentKey, rateLimitDebugString) + newDescriptor := &rateLimitDescriptor{map[string]*rateLimitDescriptor{}, rateLimit, nil, descriptorConfig.ValueToMetric} + newDescriptor.loadDescriptors(config, newParentKey+".", descriptorConfig.Descriptors, statsManager) this.descriptors[finalKey] = newDescriptor // Preload keys ending with "*" symbol. @@ -262,8 +265,8 @@ func (this *rateLimitConfigImpl) loadConfig(config RateLimitConfigToLoad) { } logger.Debugf("loading domain: %s", root.Domain) - newDomain := &rateLimitDomain{rateLimitDescriptor{map[string]*rateLimitDescriptor{}, nil, nil}} - newDomain.loadDescriptors(config, root.Domain+".", root.Descriptors, this.statsManager) + newDomain := &rateLimitDomain{rateLimitDescriptor{map[string]*rateLimitDescriptor{}, nil, nil, false}} + newDomain.loadDescriptors(config, root.Domain+".", root.Descriptors, this.statsManager) this.domains[root.Domain] = newDomain } @@ -313,6 +316,10 @@ func (this *rateLimitConfigImpl) GetLimit( var detailedMetricFullKey strings.Builder detailedMetricFullKey.WriteString(domain) + // Build value_to_metric-enhanced metric key as we traverse + var valueToMetricFullKey strings.Builder + valueToMetricFullKey.WriteString(domain) + for i, entry := range descriptor.Entries { // First see if key_value is in the map. If that isn't in the map we look for just key // to check for a default value. @@ -323,20 +330,55 @@ func (this *rateLimitConfigImpl) GetLimit( logger.Debugf("looking up key: %s", finalKey) nextDescriptor := descriptorsMap[finalKey] + matchedViaWildcard := false if nextDescriptor == nil && len(prevDescriptor.wildcardKeys) > 0 { for _, wildcardKey := range prevDescriptor.wildcardKeys { if strings.HasPrefix(finalKey, strings.TrimSuffix(wildcardKey, "*")) { nextDescriptor = descriptorsMap[wildcardKey] + matchedViaWildcard = true break } } } + matchedUsingValue := nextDescriptor != nil if nextDescriptor == nil { finalKey = entry.Key logger.Debugf("looking up key: %s", finalKey) nextDescriptor = descriptorsMap[finalKey] + matchedUsingValue = false + } + + // Build value_to_metric metrics path for this level + valueToMetricFullKey.WriteString(".") + if nextDescriptor != nil { + if matchedViaWildcard { + if nextDescriptor.valueToMetric { + valueToMetricFullKey.WriteString(entry.Key) + valueToMetricFullKey.WriteString("_") + valueToMetricFullKey.WriteString(entry.Value) + } else { + valueToMetricFullKey.WriteString(entry.Key) + } + } else if matchedUsingValue { + // Matched explicit key+value in config + valueToMetricFullKey.WriteString(entry.Key) + valueToMetricFullKey.WriteString("_") + valueToMetricFullKey.WriteString(entry.Value) + } else { + // Matched default key (no value) in config + if nextDescriptor.valueToMetric { + valueToMetricFullKey.WriteString(entry.Key) + valueToMetricFullKey.WriteString("_") + valueToMetricFullKey.WriteString(entry.Value) + } else { + valueToMetricFullKey.WriteString(entry.Key) + } + } + } else { + // No next descriptor found; still append something deterministic + valueToMetricFullKey.WriteString(entry.Key) } if nextDescriptor != nil && nextDescriptor.limit != nil { @@ -367,6 +409,17 @@ func (this *rateLimitConfigImpl) GetLimit( rateLimit.Stats = this.statsManager.NewStats(detailedMetricFullKey.String()) } + // If not using detailed metric, but any value_to_metric path produced a different key, + // override stats to use the value_to_metric-enhanced key + if rateLimit != nil && !rateLimit.DetailedMetric { + enhancedKey := valueToMetricFullKey.String() + if enhancedKey != rateLimit.FullKey { + // Recreate to ensure a clean stats struct, then set to enhanced stats + rateLimit = NewRateLimit(rateLimit.Limit.RequestsPerUnit, rateLimit.Limit.Unit, this.statsManager.NewStats(rateLimit.FullKey), rateLimit.Unlimited, rateLimit.ShadowMode, rateLimit.Name, rateLimit.Replaces, rateLimit.DetailedMetric) + rateLimit.Stats = this.statsManager.NewStats(enhancedKey) + } + } + return rateLimit } diff --git a/test/config/config_test.go b/test/config/config_test.go index 3c06b0889..fecf7fe5c 100644 --- a/test/config/config_test.go +++ b/test/config/config_test.go @@ -952,3 +952,404 @@ func TestDetailedMetric(t *testing.T) { }) } } + +func TestValueToMetric_UsesRuntimeValuesInStats(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: "route", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "http_method", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "subject_id", + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 60, + Unit: "minute", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) + + rl := rlConfig.GetLimit( + context.TODO(), "domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "route", Value: "draw"}, + {Key: "http_method", Value: "GET"}, + {Key: "subject_id", Value: "123"}, + }, + }, + ) + asrt.NotNil(rl) + + // Should include actual runtime values for keys that set value_to_metric: true + expectedKey := "domain.route_draw.http_method_GET.subject_id" + asrt.Equal(expectedKey, rl.Stats.Key) + + // Increment a couple of counters to ensure the key is actually used in stats + rl.Stats.TotalHits.Inc() + rl.Stats.WithinLimit.Inc() + + asrt.EqualValues(1, store.NewCounter(expectedKey+".total_hits").Value()) + asrt.EqualValues(1, store.NewCounter(expectedKey+".within_limit").Value()) +} + +func TestValueToMetric_DefaultKeyIncludesValueAtThatLevel(t *testing.T) { + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + + cfg := []config.RateLimitConfigToLoad{ + { + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "d", + Descriptors: []config.YamlDescriptor{ + { + Key: "k1", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "k2", + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 1, + Unit: "second", + }, + }, + }, + }, + }, + }, + }, + } + + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) + rl := rlConfig.GetLimit( + context.TODO(), "d", + &pb_struct.RateLimitDescriptor{Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "k1", Value: "A"}, + {Key: "k2", Value: "foo"}, + }}, + ) + asrt.NotNil(rl) + asrt.Equal("d.k1_A.k2", rl.Stats.Key) +} + +func TestValueToMetric_MidLevelOnly(t *testing.T) { + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + + cfg := []config.RateLimitConfigToLoad{ + { + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "d", + Descriptors: []config.YamlDescriptor{ + { + Key: "k1", + Descriptors: []config.YamlDescriptor{ + { + Key: "k2", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "k3", + RateLimit: &config.YamlRateLimit{RequestsPerUnit: 1, Unit: "second"}, + }, + }, + }, + }, + }, + }, + }, + }, + } + + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) + rl := rlConfig.GetLimit( + context.TODO(), "d", + &pb_struct.RateLimitDescriptor{Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "k1", Value: "X"}, + {Key: "k2", Value: "Y"}, + {Key: "k3", Value: "Z"}, + }}, + ) + asrt.NotNil(rl) + // k1 has no flag -> just key; k2 has flag -> include value + asrt.Equal("d.k1.k2_Y.k3", rl.Stats.Key) +} + +func TestValueToMetric_NoFlag_Unchanged(t *testing.T) { + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + + cfg := []config.RateLimitConfigToLoad{ + { + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "d", + Descriptors: []config.YamlDescriptor{ + { + Key: "k1", + Descriptors: []config.YamlDescriptor{ + { + Key: "k2", + RateLimit: &config.YamlRateLimit{RequestsPerUnit: 1, Unit: "second"}, + }, + }, + }, + }, + }, + }, + } + + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) + rl := rlConfig.GetLimit( + context.TODO(), "d", + &pb_struct.RateLimitDescriptor{Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "k1", Value: "X"}, + {Key: "k2", Value: "Y"}, + }}, + ) + asrt.NotNil(rl) + // No flags anywhere -> same as old behavior when default matched at k1 + asrt.Equal("d.k1.k2", rl.Stats.Key) +} + +func TestValueToMetric_DoesNotOverrideDetailedMetric(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: "route", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "http_method", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "subject_id", + DetailedMetric: true, + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 60, + Unit: "minute", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) + + rl := rlConfig.GetLimit( + context.TODO(), "domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "route", Value: "draw"}, + {Key: "http_method", Value: "GET"}, + {Key: "subject_id", Value: "123"}, + }, + }, + ) + asrt.NotNil(rl) + + // With detailed_metric at the leaf, the detailed metric key should be used, regardless of value_to_metric flags + expectedKey := "domain.route_draw.http_method_GET.subject_id_123" + asrt.Equal(expectedKey, rl.Stats.Key) + + rl.Stats.TotalHits.Inc() + asrt.EqualValues(1, store.NewCounter(expectedKey+".total_hits").Value()) +} + +func TestValueToMetric_WithConfiguredValues(t *testing.T) { + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + + cfg := []config.RateLimitConfigToLoad{ + { + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "test-domain", + Descriptors: []config.YamlDescriptor{ + { + Key: "route", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "http_method", + Value: "GET", // Configured value in descriptor + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "subject_id", + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 60, + Unit: "minute", + }, + }, + }, + }, + { + Key: "http_method", + Value: "POST", // Another configured value + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "subject_id", + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 30, + Unit: "minute", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) + + // Test GET path - should include runtime value for route, but use configured value for http_method + rl := rlConfig.GetLimit( + context.TODO(), "test-domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "route", Value: "api"}, + {Key: "http_method", Value: "GET"}, + {Key: "subject_id", Value: "user123"}, + }, + }, + ) + asrt.NotNil(rl) + asrt.EqualValues(60, rl.Limit.RequestsPerUnit) + // route has value_to_metric=true, so includes runtime value; http_method has configured value, so uses that + expectedKey := "test-domain.route_api.http_method_GET.subject_id" + asrt.Equal(expectedKey, rl.Stats.Key) + + // Test POST path - should include runtime value for route, but use configured value for http_method + rl = rlConfig.GetLimit( + context.TODO(), "test-domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "route", Value: "api"}, + {Key: "http_method", Value: "POST"}, + {Key: "subject_id", Value: "user456"}, + }, + }, + ) + asrt.NotNil(rl) + asrt.EqualValues(30, rl.Limit.RequestsPerUnit) + expectedKey = "test-domain.route_api.http_method_POST.subject_id" + asrt.Equal(expectedKey, rl.Stats.Key) + + // Test that stats are actually created with the correct keys + rl.Stats.TotalHits.Inc() + asrt.EqualValues(1, store.NewCounter(expectedKey+".total_hits").Value()) +} + +func TestValueToMetric_WithWildcard(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: "user", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "action", + Value: "read*", // Wildcard pattern + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "resource", + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 100, + Unit: "minute", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) + + // Test wildcard matching with value_to_metric - should include full runtime value + rl := rlConfig.GetLimit( + context.TODO(), "domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "user", Value: "alice"}, + {Key: "action", Value: "readfile"}, // Matches "read*" wildcard + {Key: "resource", Value: "documents"}, + }, + }, + ) + asrt.NotNil(rl) + asrt.EqualValues(100, rl.Limit.RequestsPerUnit) + // Both user and action should include their full runtime values due to value_to_metric + expectedKey := "domain.user_alice.action_readfile.resource" + asrt.Equal(expectedKey, rl.Stats.Key) + + // Test another wildcard match + rl = rlConfig.GetLimit( + context.TODO(), "domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "user", Value: "bob"}, + {Key: "action", Value: "readdata"}, // Also matches "read*" wildcard + {Key: "resource", Value: "database"}, + }, + }, + ) + asrt.NotNil(rl) + expectedKey = "domain.user_bob.action_readdata.resource" + asrt.Equal(expectedKey, rl.Stats.Key) + + // Test that stats are actually created with the correct keys + rl.Stats.TotalHits.Inc() + asrt.EqualValues(1, store.NewCounter(expectedKey+".total_hits").Value()) +} From 17dbaf6a4d1977a614a3efac87ac7c021a6ba9ec Mon Sep 17 00:00:00 2001 From: Nam Dang Date: Wed, 12 Nov 2025 02:35:04 +0700 Subject: [PATCH 2/4] Update README.md to pass docs_check_format check Signed-off-by: Nam Dang --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 5413b412b..7e2346519 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,7 @@ - [Replaces](#replaces) - [ShadowMode](#shadowmode) - [Including detailed metrics for unspecified values](#including-detailed-metrics-for-unspecified-values) + - [Including descriptor values in metrics](#including-descriptor-values-in-metrics) - [Examples](#examples) - [Example 1](#example-1) - [Example 2](#example-2) From 10d33893473f01f655a232ff90b688a613d3cd31 Mon Sep 17 00:00:00 2001 From: Nam Dang Date: Wed, 12 Nov 2025 02:53:19 +0700 Subject: [PATCH 3/4] Update format to pass pre-commit check Signed-off-by: Nam Dang --- src/config/config_impl.go | 24 +- test/config/config_test.go | 614 ++++++++++++++++++------------------- 2 files changed, 319 insertions(+), 319 deletions(-) diff --git a/src/config/config_impl.go b/src/config/config_impl.go index 4d1d1bb4c..59a650494 100644 --- a/src/config/config_impl.go +++ b/src/config/config_impl.go @@ -32,7 +32,7 @@ type YamlDescriptor struct { Descriptors []YamlDescriptor ShadowMode bool `yaml:"shadow_mode"` DetailedMetric bool `yaml:"detailed_metric"` - ValueToMetric bool `yaml:"value_to_metric"` + ValueToMetric bool `yaml:"value_to_metric"` } type YamlRoot struct { @@ -41,10 +41,10 @@ type YamlRoot struct { } type rateLimitDescriptor struct { - descriptors map[string]*rateLimitDescriptor - limit *RateLimit - wildcardKeys []string - valueToMetric bool + descriptors map[string]*rateLimitDescriptor + limit *RateLimit + wildcardKeys []string + valueToMetric bool } type rateLimitDomain struct { @@ -70,7 +70,7 @@ var validKeys = map[string]bool{ "name": true, "replaces": true, "detailed_metric": true, - "value_to_metric": true, + "value_to_metric": true, } // Create a new rate limit config entry. @@ -186,10 +186,10 @@ func (this *rateLimitDescriptor) loadDescriptors(config RateLimitConfigToLoad, p } } - logger.Debugf( - "loading descriptor: key=%s%s", newParentKey, rateLimitDebugString) - newDescriptor := &rateLimitDescriptor{map[string]*rateLimitDescriptor{}, rateLimit, nil, descriptorConfig.ValueToMetric} - newDescriptor.loadDescriptors(config, newParentKey+".", descriptorConfig.Descriptors, statsManager) + logger.Debugf( + "loading descriptor: key=%s%s", newParentKey, rateLimitDebugString) + newDescriptor := &rateLimitDescriptor{map[string]*rateLimitDescriptor{}, rateLimit, nil, descriptorConfig.ValueToMetric} + newDescriptor.loadDescriptors(config, newParentKey+".", descriptorConfig.Descriptors, statsManager) this.descriptors[finalKey] = newDescriptor // Preload keys ending with "*" symbol. @@ -265,8 +265,8 @@ func (this *rateLimitConfigImpl) loadConfig(config RateLimitConfigToLoad) { } logger.Debugf("loading domain: %s", root.Domain) - newDomain := &rateLimitDomain{rateLimitDescriptor{map[string]*rateLimitDescriptor{}, nil, nil, false}} - newDomain.loadDescriptors(config, root.Domain+".", root.Descriptors, this.statsManager) + newDomain := &rateLimitDomain{rateLimitDescriptor{map[string]*rateLimitDescriptor{}, nil, nil, false}} + newDomain.loadDescriptors(config, root.Domain+".", root.Descriptors, this.statsManager) this.domains[root.Domain] = newDomain } diff --git a/test/config/config_test.go b/test/config/config_test.go index fecf7fe5c..20563e699 100644 --- a/test/config/config_test.go +++ b/test/config/config_test.go @@ -954,326 +954,326 @@ func TestDetailedMetric(t *testing.T) { } func TestValueToMetric_UsesRuntimeValuesInStats(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: "route", - ValueToMetric: true, - Descriptors: []config.YamlDescriptor{ - { - Key: "http_method", - ValueToMetric: true, - Descriptors: []config.YamlDescriptor{ - { - Key: "subject_id", - RateLimit: &config.YamlRateLimit{ - RequestsPerUnit: 60, - Unit: "minute", - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) - - rl := rlConfig.GetLimit( - context.TODO(), "domain", - &pb_struct.RateLimitDescriptor{ - Entries: []*pb_struct.RateLimitDescriptor_Entry{ - {Key: "route", Value: "draw"}, - {Key: "http_method", Value: "GET"}, - {Key: "subject_id", Value: "123"}, - }, - }, - ) - asrt.NotNil(rl) - - // Should include actual runtime values for keys that set value_to_metric: true - expectedKey := "domain.route_draw.http_method_GET.subject_id" - asrt.Equal(expectedKey, rl.Stats.Key) - - // Increment a couple of counters to ensure the key is actually used in stats - rl.Stats.TotalHits.Inc() - rl.Stats.WithinLimit.Inc() - - asrt.EqualValues(1, store.NewCounter(expectedKey+".total_hits").Value()) - asrt.EqualValues(1, store.NewCounter(expectedKey+".within_limit").Value()) + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + + cfg := []config.RateLimitConfigToLoad{ + { + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "domain", + Descriptors: []config.YamlDescriptor{ + { + Key: "route", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "http_method", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "subject_id", + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 60, + Unit: "minute", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) + + rl := rlConfig.GetLimit( + context.TODO(), "domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "route", Value: "draw"}, + {Key: "http_method", Value: "GET"}, + {Key: "subject_id", Value: "123"}, + }, + }, + ) + asrt.NotNil(rl) + + // Should include actual runtime values for keys that set value_to_metric: true + expectedKey := "domain.route_draw.http_method_GET.subject_id" + asrt.Equal(expectedKey, rl.Stats.Key) + + // Increment a couple of counters to ensure the key is actually used in stats + rl.Stats.TotalHits.Inc() + rl.Stats.WithinLimit.Inc() + + asrt.EqualValues(1, store.NewCounter(expectedKey+".total_hits").Value()) + asrt.EqualValues(1, store.NewCounter(expectedKey+".within_limit").Value()) } func TestValueToMetric_DefaultKeyIncludesValueAtThatLevel(t *testing.T) { - asrt := assert.New(t) - store := stats.NewStore(stats.NewNullSink(), false) - - cfg := []config.RateLimitConfigToLoad{ - { - Name: "inline", - ConfigYaml: &config.YamlRoot{ - Domain: "d", - Descriptors: []config.YamlDescriptor{ - { - Key: "k1", - ValueToMetric: true, - Descriptors: []config.YamlDescriptor{ - { - Key: "k2", - RateLimit: &config.YamlRateLimit{ - RequestsPerUnit: 1, - Unit: "second", - }, - }, - }, - }, - }, - }, - }, - } - - rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) - rl := rlConfig.GetLimit( - context.TODO(), "d", - &pb_struct.RateLimitDescriptor{Entries: []*pb_struct.RateLimitDescriptor_Entry{ - {Key: "k1", Value: "A"}, - {Key: "k2", Value: "foo"}, - }}, - ) - asrt.NotNil(rl) - asrt.Equal("d.k1_A.k2", rl.Stats.Key) + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + + cfg := []config.RateLimitConfigToLoad{ + { + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "d", + Descriptors: []config.YamlDescriptor{ + { + Key: "k1", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "k2", + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 1, + Unit: "second", + }, + }, + }, + }, + }, + }, + }, + } + + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) + rl := rlConfig.GetLimit( + context.TODO(), "d", + &pb_struct.RateLimitDescriptor{Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "k1", Value: "A"}, + {Key: "k2", Value: "foo"}, + }}, + ) + asrt.NotNil(rl) + asrt.Equal("d.k1_A.k2", rl.Stats.Key) } func TestValueToMetric_MidLevelOnly(t *testing.T) { - asrt := assert.New(t) - store := stats.NewStore(stats.NewNullSink(), false) - - cfg := []config.RateLimitConfigToLoad{ - { - Name: "inline", - ConfigYaml: &config.YamlRoot{ - Domain: "d", - Descriptors: []config.YamlDescriptor{ - { - Key: "k1", - Descriptors: []config.YamlDescriptor{ - { - Key: "k2", - ValueToMetric: true, - Descriptors: []config.YamlDescriptor{ - { - Key: "k3", - RateLimit: &config.YamlRateLimit{RequestsPerUnit: 1, Unit: "second"}, - }, - }, - }, - }, - }, - }, - }, - }, - } - - rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) - rl := rlConfig.GetLimit( - context.TODO(), "d", - &pb_struct.RateLimitDescriptor{Entries: []*pb_struct.RateLimitDescriptor_Entry{ - {Key: "k1", Value: "X"}, - {Key: "k2", Value: "Y"}, - {Key: "k3", Value: "Z"}, - }}, - ) - asrt.NotNil(rl) - // k1 has no flag -> just key; k2 has flag -> include value - asrt.Equal("d.k1.k2_Y.k3", rl.Stats.Key) + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + + cfg := []config.RateLimitConfigToLoad{ + { + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "d", + Descriptors: []config.YamlDescriptor{ + { + Key: "k1", + Descriptors: []config.YamlDescriptor{ + { + Key: "k2", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "k3", + RateLimit: &config.YamlRateLimit{RequestsPerUnit: 1, Unit: "second"}, + }, + }, + }, + }, + }, + }, + }, + }, + } + + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) + rl := rlConfig.GetLimit( + context.TODO(), "d", + &pb_struct.RateLimitDescriptor{Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "k1", Value: "X"}, + {Key: "k2", Value: "Y"}, + {Key: "k3", Value: "Z"}, + }}, + ) + asrt.NotNil(rl) + // k1 has no flag -> just key; k2 has flag -> include value + asrt.Equal("d.k1.k2_Y.k3", rl.Stats.Key) } func TestValueToMetric_NoFlag_Unchanged(t *testing.T) { - asrt := assert.New(t) - store := stats.NewStore(stats.NewNullSink(), false) - - cfg := []config.RateLimitConfigToLoad{ - { - Name: "inline", - ConfigYaml: &config.YamlRoot{ - Domain: "d", - Descriptors: []config.YamlDescriptor{ - { - Key: "k1", - Descriptors: []config.YamlDescriptor{ - { - Key: "k2", - RateLimit: &config.YamlRateLimit{RequestsPerUnit: 1, Unit: "second"}, - }, - }, - }, - }, - }, - }, - } - - rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) - rl := rlConfig.GetLimit( - context.TODO(), "d", - &pb_struct.RateLimitDescriptor{Entries: []*pb_struct.RateLimitDescriptor_Entry{ - {Key: "k1", Value: "X"}, - {Key: "k2", Value: "Y"}, - }}, - ) - asrt.NotNil(rl) - // No flags anywhere -> same as old behavior when default matched at k1 - asrt.Equal("d.k1.k2", rl.Stats.Key) + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + + cfg := []config.RateLimitConfigToLoad{ + { + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "d", + Descriptors: []config.YamlDescriptor{ + { + Key: "k1", + Descriptors: []config.YamlDescriptor{ + { + Key: "k2", + RateLimit: &config.YamlRateLimit{RequestsPerUnit: 1, Unit: "second"}, + }, + }, + }, + }, + }, + }, + } + + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) + rl := rlConfig.GetLimit( + context.TODO(), "d", + &pb_struct.RateLimitDescriptor{Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "k1", Value: "X"}, + {Key: "k2", Value: "Y"}, + }}, + ) + asrt.NotNil(rl) + // No flags anywhere -> same as old behavior when default matched at k1 + asrt.Equal("d.k1.k2", rl.Stats.Key) } func TestValueToMetric_DoesNotOverrideDetailedMetric(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: "route", - ValueToMetric: true, - Descriptors: []config.YamlDescriptor{ - { - Key: "http_method", - ValueToMetric: true, - Descriptors: []config.YamlDescriptor{ - { - Key: "subject_id", - DetailedMetric: true, - RateLimit: &config.YamlRateLimit{ - RequestsPerUnit: 60, - Unit: "minute", - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) - - rl := rlConfig.GetLimit( - context.TODO(), "domain", - &pb_struct.RateLimitDescriptor{ - Entries: []*pb_struct.RateLimitDescriptor_Entry{ - {Key: "route", Value: "draw"}, - {Key: "http_method", Value: "GET"}, - {Key: "subject_id", Value: "123"}, - }, - }, - ) - asrt.NotNil(rl) - - // With detailed_metric at the leaf, the detailed metric key should be used, regardless of value_to_metric flags - expectedKey := "domain.route_draw.http_method_GET.subject_id_123" - asrt.Equal(expectedKey, rl.Stats.Key) - - rl.Stats.TotalHits.Inc() - asrt.EqualValues(1, store.NewCounter(expectedKey+".total_hits").Value()) + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + + cfg := []config.RateLimitConfigToLoad{ + { + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "domain", + Descriptors: []config.YamlDescriptor{ + { + Key: "route", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "http_method", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "subject_id", + DetailedMetric: true, + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 60, + Unit: "minute", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) + + rl := rlConfig.GetLimit( + context.TODO(), "domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "route", Value: "draw"}, + {Key: "http_method", Value: "GET"}, + {Key: "subject_id", Value: "123"}, + }, + }, + ) + asrt.NotNil(rl) + + // With detailed_metric at the leaf, the detailed metric key should be used, regardless of value_to_metric flags + expectedKey := "domain.route_draw.http_method_GET.subject_id_123" + asrt.Equal(expectedKey, rl.Stats.Key) + + rl.Stats.TotalHits.Inc() + asrt.EqualValues(1, store.NewCounter(expectedKey+".total_hits").Value()) } func TestValueToMetric_WithConfiguredValues(t *testing.T) { - asrt := assert.New(t) - store := stats.NewStore(stats.NewNullSink(), false) - - cfg := []config.RateLimitConfigToLoad{ - { - Name: "inline", - ConfigYaml: &config.YamlRoot{ - Domain: "test-domain", - Descriptors: []config.YamlDescriptor{ - { - Key: "route", - ValueToMetric: true, - Descriptors: []config.YamlDescriptor{ - { - Key: "http_method", - Value: "GET", // Configured value in descriptor - ValueToMetric: true, - Descriptors: []config.YamlDescriptor{ - { - Key: "subject_id", - RateLimit: &config.YamlRateLimit{ - RequestsPerUnit: 60, - Unit: "minute", - }, - }, - }, - }, - { - Key: "http_method", - Value: "POST", // Another configured value - ValueToMetric: true, - Descriptors: []config.YamlDescriptor{ - { - Key: "subject_id", - RateLimit: &config.YamlRateLimit{ - RequestsPerUnit: 30, - Unit: "minute", - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) - - // Test GET path - should include runtime value for route, but use configured value for http_method - rl := rlConfig.GetLimit( - context.TODO(), "test-domain", - &pb_struct.RateLimitDescriptor{ - Entries: []*pb_struct.RateLimitDescriptor_Entry{ - {Key: "route", Value: "api"}, - {Key: "http_method", Value: "GET"}, - {Key: "subject_id", Value: "user123"}, - }, - }, - ) - asrt.NotNil(rl) - asrt.EqualValues(60, rl.Limit.RequestsPerUnit) - // route has value_to_metric=true, so includes runtime value; http_method has configured value, so uses that - expectedKey := "test-domain.route_api.http_method_GET.subject_id" - asrt.Equal(expectedKey, rl.Stats.Key) - - // Test POST path - should include runtime value for route, but use configured value for http_method - rl = rlConfig.GetLimit( - context.TODO(), "test-domain", - &pb_struct.RateLimitDescriptor{ - Entries: []*pb_struct.RateLimitDescriptor_Entry{ - {Key: "route", Value: "api"}, - {Key: "http_method", Value: "POST"}, - {Key: "subject_id", Value: "user456"}, - }, - }, - ) - asrt.NotNil(rl) - asrt.EqualValues(30, rl.Limit.RequestsPerUnit) - expectedKey = "test-domain.route_api.http_method_POST.subject_id" - asrt.Equal(expectedKey, rl.Stats.Key) + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + + cfg := []config.RateLimitConfigToLoad{ + { + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "test-domain", + Descriptors: []config.YamlDescriptor{ + { + Key: "route", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "http_method", + Value: "GET", // Configured value in descriptor + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "subject_id", + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 60, + Unit: "minute", + }, + }, + }, + }, + { + Key: "http_method", + Value: "POST", // Another configured value + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "subject_id", + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 30, + Unit: "minute", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) + + // Test GET path - should include runtime value for route, but use configured value for http_method + rl := rlConfig.GetLimit( + context.TODO(), "test-domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "route", Value: "api"}, + {Key: "http_method", Value: "GET"}, + {Key: "subject_id", Value: "user123"}, + }, + }, + ) + asrt.NotNil(rl) + asrt.EqualValues(60, rl.Limit.RequestsPerUnit) + // route has value_to_metric=true, so includes runtime value; http_method has configured value, so uses that + expectedKey := "test-domain.route_api.http_method_GET.subject_id" + asrt.Equal(expectedKey, rl.Stats.Key) + + // Test POST path - should include runtime value for route, but use configured value for http_method + rl = rlConfig.GetLimit( + context.TODO(), "test-domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "route", Value: "api"}, + {Key: "http_method", Value: "POST"}, + {Key: "subject_id", Value: "user456"}, + }, + }, + ) + asrt.NotNil(rl) + asrt.EqualValues(30, rl.Limit.RequestsPerUnit) + expectedKey = "test-domain.route_api.http_method_POST.subject_id" + asrt.Equal(expectedKey, rl.Stats.Key) // Test that stats are actually created with the correct keys rl.Stats.TotalHits.Inc() @@ -1295,8 +1295,8 @@ func TestValueToMetric_WithWildcard(t *testing.T) { ValueToMetric: true, Descriptors: []config.YamlDescriptor{ { - Key: "action", - Value: "read*", // Wildcard pattern + Key: "action", + Value: "read*", // Wildcard pattern ValueToMetric: true, Descriptors: []config.YamlDescriptor{ { From f60829f7c09266a6ef6d48122400b52c3c6dcb87 Mon Sep 17 00:00:00 2001 From: Nam Dang Date: Fri, 28 Nov 2025 18:58:46 +0700 Subject: [PATCH 4/4] Update based on comments and add more unit tests Signed-off-by: Nam Dang --- src/config/config_impl.go | 23 ++-- test/config/config_test.go | 239 +++++++++++++++++++++++++++++++++++++ 2 files changed, 255 insertions(+), 7 deletions(-) diff --git a/src/config/config_impl.go b/src/config/config_impl.go index 59a650494..b2ccd10a8 100644 --- a/src/config/config_impl.go +++ b/src/config/config_impl.go @@ -356,22 +356,28 @@ func (this *rateLimitConfigImpl) GetLimit( if matchedViaWildcard { if nextDescriptor.valueToMetric { valueToMetricFullKey.WriteString(entry.Key) - valueToMetricFullKey.WriteString("_") - valueToMetricFullKey.WriteString(entry.Value) + if entry.Value != "" { + valueToMetricFullKey.WriteString("_") + valueToMetricFullKey.WriteString(entry.Value) + } } else { valueToMetricFullKey.WriteString(entry.Key) } } else if matchedUsingValue { // Matched explicit key+value in config valueToMetricFullKey.WriteString(entry.Key) - valueToMetricFullKey.WriteString("_") - valueToMetricFullKey.WriteString(entry.Value) + if entry.Value != "" { + valueToMetricFullKey.WriteString("_") + valueToMetricFullKey.WriteString(entry.Value) + } } else { // Matched default key (no value) in config if nextDescriptor.valueToMetric { valueToMetricFullKey.WriteString(entry.Key) - valueToMetricFullKey.WriteString("_") - valueToMetricFullKey.WriteString(entry.Value) + if entry.Value != "" { + valueToMetricFullKey.WriteString("_") + valueToMetricFullKey.WriteString(entry.Value) + } } else { valueToMetricFullKey.WriteString(entry.Key) } @@ -406,7 +412,9 @@ func (this *rateLimitConfigImpl) GetLimit( // Replace metric with detailed metric, if leaf descriptor is detailed. if rateLimit != nil && rateLimit.DetailedMetric { - rateLimit.Stats = this.statsManager.NewStats(detailedMetricFullKey.String()) + detailedKey := detailedMetricFullKey.String() + rateLimit.Stats = this.statsManager.NewStats(detailedKey) + rateLimit.FullKey = detailedKey } // If not using detailed metric, but any value_to_metric path produced a different key, @@ -417,6 +425,7 @@ func (this *rateLimitConfigImpl) GetLimit( // Recreate to ensure a clean stats struct, then set to enhanced stats rateLimit = NewRateLimit(rateLimit.Limit.RequestsPerUnit, rateLimit.Limit.Unit, this.statsManager.NewStats(rateLimit.FullKey), rateLimit.Unlimited, rateLimit.ShadowMode, rateLimit.Name, rateLimit.Replaces, rateLimit.DetailedMetric) rateLimit.Stats = this.statsManager.NewStats(enhancedKey) + rateLimit.FullKey = enhancedKey } } diff --git a/test/config/config_test.go b/test/config/config_test.go index 20563e699..bd87a0c5c 100644 --- a/test/config/config_test.go +++ b/test/config/config_test.go @@ -1353,3 +1353,242 @@ func TestValueToMetric_WithWildcard(t *testing.T) { rl.Stats.TotalHits.Inc() asrt.EqualValues(1, store.NewCounter(expectedKey+".total_hits").Value()) } + +func TestValueToMetric_WithEmptyValue(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: "route", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "http_method", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "subject_id", + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 60, + Unit: "minute", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) + + // Test with empty value for route - should not include underscore and empty value + rl := rlConfig.GetLimit( + context.TODO(), "domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "route", Value: ""}, // Empty value + {Key: "http_method", Value: "GET"}, + {Key: "subject_id", Value: "123"}, + }, + }, + ) + asrt.NotNil(rl) + + // Should not include underscore and empty value for route + expectedKey := "domain.route.http_method_GET.subject_id" + asrt.Equal(expectedKey, rl.Stats.Key) + + // Test with empty value for http_method - should not include underscore and empty value + rl = rlConfig.GetLimit( + context.TODO(), "domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "route", Value: "draw"}, + {Key: "http_method", Value: ""}, // Empty value + {Key: "subject_id", Value: "123"}, + }, + }, + ) + asrt.NotNil(rl) + + // Should not include underscore and empty value for http_method + expectedKey = "domain.route_draw.http_method.subject_id" + asrt.Equal(expectedKey, rl.Stats.Key) + + // Test with empty value for both - should not include underscores and empty values + rl = rlConfig.GetLimit( + context.TODO(), "domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "route", Value: ""}, // Empty value + {Key: "http_method", Value: ""}, // Empty value + {Key: "subject_id", Value: "123"}, + }, + }, + ) + asrt.NotNil(rl) + + // Should not include underscores and empty values + expectedKey = "domain.route.http_method.subject_id" + asrt.Equal(expectedKey, rl.Stats.Key) + + // Increment counters to ensure the keys are actually used in stats + rl.Stats.TotalHits.Inc() + rl.Stats.WithinLimit.Inc() + + asrt.EqualValues(1, store.NewCounter(expectedKey+".total_hits").Value()) + asrt.EqualValues(1, store.NewCounter(expectedKey+".within_limit").Value()) +} + +// TestValueToMetric_FullKeyMatchesStatsKey verifies that rateLimit.FullKey always matches +// rateLimit.Stats.Key. This is important for debugging and log/metric correlation. +// FullKey is used in debug logs, while Stats.Key is used for actual metrics. +func TestValueToMetric_FullKeyMatchesStatsKey(t *testing.T) { + asrt := assert.New(t) + store := stats.NewStore(stats.NewNullSink(), false) + + cfg := []config.RateLimitConfigToLoad{ + { + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "test-domain", + Descriptors: []config.YamlDescriptor{ + { + Key: "route", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "http_method", + ValueToMetric: true, + Descriptors: []config.YamlDescriptor{ + { + Key: "subject_id", + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 60, + Unit: "minute", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + rlConfig := config.NewRateLimitConfigImpl(cfg, mockstats.NewMockStatManager(store), false) + + // Test case 1: value_to_metric enabled - FullKey should match Stats.Key + rl := rlConfig.GetLimit( + context.TODO(), "test-domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "route", Value: "api"}, + {Key: "http_method", Value: "GET"}, + {Key: "subject_id", Value: "user123"}, + }, + }, + ) + asrt.NotNil(rl) + asrt.Equal(rl.FullKey, rl.Stats.Key, "FullKey should match Stats.Key when value_to_metric is enabled") + expectedKey := "test-domain.route_api.http_method_GET.subject_id" + asrt.Equal(expectedKey, rl.FullKey) + asrt.Equal(expectedKey, rl.Stats.Key) + + // Test case 2: value_to_metric disabled - FullKey should match Stats.Key + cfgNoValueToMetric := []config.RateLimitConfigToLoad{ + { + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "test-domain-2", + Descriptors: []config.YamlDescriptor{ + { + Key: "route", + Descriptors: []config.YamlDescriptor{ + { + Key: "http_method", + Descriptors: []config.YamlDescriptor{ + { + Key: "subject_id", + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 60, + Unit: "minute", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + rlConfig2 := config.NewRateLimitConfigImpl(cfgNoValueToMetric, mockstats.NewMockStatManager(store), false) + rl2 := rlConfig2.GetLimit( + context.TODO(), "test-domain-2", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "route", Value: "api"}, + {Key: "http_method", Value: "GET"}, + {Key: "subject_id", Value: "user123"}, + }, + }, + ) + asrt.NotNil(rl2) + asrt.Equal(rl2.FullKey, rl2.Stats.Key, "FullKey should match Stats.Key even when value_to_metric is disabled") + + // Test case 3: detailed_metric enabled - FullKey should match Stats.Key + cfgDetailedMetric := []config.RateLimitConfigToLoad{ + { + Name: "inline", + ConfigYaml: &config.YamlRoot{ + Domain: "test-domain-3", + Descriptors: []config.YamlDescriptor{ + { + Key: "route", + Descriptors: []config.YamlDescriptor{ + { + Key: "http_method", + Descriptors: []config.YamlDescriptor{ + { + Key: "subject_id", + DetailedMetric: true, + RateLimit: &config.YamlRateLimit{ + RequestsPerUnit: 60, + Unit: "minute", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + rlConfig3 := config.NewRateLimitConfigImpl(cfgDetailedMetric, mockstats.NewMockStatManager(store), false) + rl3 := rlConfig3.GetLimit( + context.TODO(), "test-domain-3", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "route", Value: "api"}, + {Key: "http_method", Value: "GET"}, + {Key: "subject_id", Value: "user123"}, + }, + }, + ) + asrt.NotNil(rl3) + asrt.Equal(rl3.FullKey, rl3.Stats.Key, "FullKey should match Stats.Key when detailed_metric is enabled") +}