diff --git a/README.md b/README.md index 7e2346519..3fb807767 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ - [ShadowMode](#shadowmode) - [Including detailed metrics for unspecified values](#including-detailed-metrics-for-unspecified-values) - [Including descriptor values in metrics](#including-descriptor-values-in-metrics) + - [Sharing thresholds for wildcard matches](#sharing-thresholds-for-wildcard-matches) - [Examples](#examples) - [Example 1](#example-1) - [Example 2](#example-2) @@ -33,6 +34,7 @@ - [Example 8](#example-8) - [Example 9](#example-9) - [Example 10](#example-10) + - [Example 11](#example-11) - [Loading Configuration](#loading-configuration) - [File Based Configuration Loading](#file-based-configuration-loading) - [xDS Management Server Based Configuration Loading](#xds-management-server-based-configuration-loading) @@ -285,6 +287,7 @@ descriptors: shadow_mode: (optional) detailed_metric: (optional) value_to_metric: (optional) + share_threshold: (optional) descriptors: (optional block) - ... (nested repetition of above) ``` @@ -347,6 +350,20 @@ Setting `value_to_metric: true` (default: `false`) for a descriptor will include 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. +### Sharing thresholds for wildcard matches + +Setting `share_threshold: true` (default: `false`) for a descriptor with a wildcard value (ending with `*`) allows all values matching that wildcard to share the same rate limit threshold, instead of using isolated thresholds for each matching value. + +This is useful when you want to apply a single rate limit across multiple resources that match a wildcard pattern. For example, if you have a rule for `files/*`, both `files/a.pdf` and `files/b.csv` will share the same threshold when `share_threshold: true` is set. + +**Important notes:** + +- `share_threshold` can only be used with wildcard values (values ending with `*`) +- When `share_threshold: true` is enabled, all matching values share the same cache key and rate limit counter +- When `share_threshold: false` (or not set), each matching value has its own isolated threshold +- When combined with `value_to_metric: true`, the metric key includes the wildcard prefix (the part before `*`) instead of the full runtime value, to reflect that values are sharing a threshold +- When combined with `detailed_metric: true`, the metric key also includes the wildcard prefix for entries with `share_threshold` enabled + ### Examples #### Example 1 @@ -692,6 +709,58 @@ descriptors: Note: When `detailed_metric: true` is set on a descriptor, it takes precedence and `value_to_metric` is ignored for that descriptor. +#### Example 11 + +Using `share_threshold: true` to share rate limits across wildcard matches: + +```yaml +domain: example11 +descriptors: + # With share_threshold: true, all files/* matches share the same threshold + - key: files + value: files/* + share_threshold: true + rate_limit: + unit: hour + requests_per_unit: 10 + + # Without share_threshold, each files_no_share/* match has its own isolated threshold + - key: files_no_share + value: files_no_share/* + share_threshold: false + rate_limit: + unit: hour + requests_per_unit: 10 +``` + +With this configuration: + +- Requests for `files/a.pdf`, `files/b.csv`, and `files/c.txt` all share the same threshold of 10 requests per hour +- If 5 requests are made for `files/a.pdf` and 5 requests for `files/b.csv`, a request for `files/c.txt` will be rate limited (OVER_LIMIT) because the shared threshold of 10 has been reached +- Requests for `files_no_share/a.pdf` and `files_no_share/b.csv` each have their own isolated threshold of 10 requests per hour +- If 10 requests are made for `files_no_share/a.pdf` (exhausting its quota), requests for `files_no_share/b.csv` will still be allowed (up to 10 requests) + +Combining `share_threshold` with `value_to_metric`: + +```yaml +domain: example11_metrics +descriptors: + - key: route + value: api/* + share_threshold: true + value_to_metric: true + descriptors: + - key: method + rate_limit: + unit: minute + requests_per_unit: 60 +``` + +- Request: `route=api/v1`, `method=GET` +- Metric key: `example11_metrics.route_api.method_GET` (includes the wildcard prefix `api` instead of the full value `api/v1`) + +This reflects that all `api/*` routes share the same threshold, while still providing visibility into which API routes are being accessed. + ## 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.go b/src/config/config.go index d18860339..b469bcf45 100644 --- a/src/config/config.go +++ b/src/config/config.go @@ -25,6 +25,9 @@ type RateLimit struct { Name string Replaces []string DetailedMetric bool + // ShareThresholdKeyPattern is a slice of wildcard patterns for descriptor entries + // The slice index corresponds to the descriptor entry index. + ShareThresholdKeyPattern []string } // Interface for interacting with a loaded rate limit config. diff --git a/src/config/config_impl.go b/src/config/config_impl.go index b2ccd10a8..8663d5a01 100644 --- a/src/config/config_impl.go +++ b/src/config/config_impl.go @@ -33,6 +33,7 @@ type YamlDescriptor struct { ShadowMode bool `yaml:"shadow_mode"` DetailedMetric bool `yaml:"detailed_metric"` ValueToMetric bool `yaml:"value_to_metric"` + ShareThreshold bool `yaml:"share_threshold"` } type YamlRoot struct { @@ -41,10 +42,12 @@ 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 + shareThreshold bool + wildcardPattern string // stores the wildcard pattern when share_threshold is true } type rateLimitDomain struct { @@ -71,6 +74,7 @@ var validKeys = map[string]bool{ "replaces": true, "detailed_metric": true, "value_to_metric": true, + "share_threshold": true, } // Create a new rate limit config entry. @@ -90,11 +94,12 @@ func NewRateLimit(requestsPerUnit uint32, unit pb.RateLimitResponse_RateLimit_Un Unit: unit, Name: name, }, - Unlimited: unlimited, - ShadowMode: shadowMode, - Name: name, - Replaces: replaces, - DetailedMetric: detailedMetric, + Unlimited: unlimited, + ShadowMode: shadowMode, + Name: name, + Replaces: replaces, + DetailedMetric: detailedMetric, + ShareThresholdKeyPattern: nil, } } @@ -186,16 +191,38 @@ 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) - this.descriptors[finalKey] = newDescriptor + // Validate share_threshold can only be used with wildcards + if descriptorConfig.ShareThreshold { + if len(finalKey) == 0 || finalKey[len(finalKey)-1:] != "*" { + panic(newRateLimitConfigError( + config.Name, + fmt.Sprintf("share_threshold can only be used with wildcard values (ending with '*'), but found key '%s'", finalKey))) + } + } + + // Store wildcard pattern if share_threshold is enabled + var wildcardPattern string = "" + if descriptorConfig.ShareThreshold && len(finalKey) > 0 && finalKey[len(finalKey)-1:] == "*" { + wildcardPattern = finalKey + } // Preload keys ending with "*" symbol. if finalKey[len(finalKey)-1:] == "*" { this.wildcardKeys = append(this.wildcardKeys, finalKey) } + + logger.Debugf( + "loading descriptor: key=%s%s", newParentKey, rateLimitDebugString) + newDescriptor := &rateLimitDescriptor{ + descriptors: map[string]*rateLimitDescriptor{}, + limit: rateLimit, + wildcardKeys: nil, + valueToMetric: descriptorConfig.ValueToMetric, + shareThreshold: descriptorConfig.ShareThreshold, + wildcardPattern: wildcardPattern, + } + newDescriptor.loadDescriptors(config, newParentKey+".", descriptorConfig.Descriptors, statsManager) + this.descriptors[finalKey] = newDescriptor } } @@ -265,7 +292,14 @@ func (this *rateLimitConfigImpl) loadConfig(config RateLimitConfigToLoad) { } logger.Debugf("loading domain: %s", root.Domain) - newDomain := &rateLimitDomain{rateLimitDescriptor{map[string]*rateLimitDescriptor{}, nil, nil, false}} + newDomain := &rateLimitDomain{rateLimitDescriptor{ + descriptors: map[string]*rateLimitDescriptor{}, + limit: nil, + wildcardKeys: nil, + valueToMetric: false, + shareThreshold: false, + wildcardPattern: "", + }} newDomain.loadDescriptors(config, root.Domain+".", root.Descriptors, this.statsManager) this.domains[root.Domain] = newDomain } @@ -320,6 +354,10 @@ func (this *rateLimitConfigImpl) GetLimit( var valueToMetricFullKey strings.Builder valueToMetricFullKey.WriteString(domain) + // Track share_threshold patterns for entries matched via wildcard (using indexes) + // This allows share_threshold to work when wildcard has nested descriptors + var shareThresholdPatterns map[int]string + 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. @@ -350,11 +388,31 @@ func (this *rateLimitConfigImpl) GetLimit( matchedUsingValue = false } + // Track share_threshold pattern when matching via wildcard, even if no rate_limit at this level + if matchedViaWildcard && nextDescriptor != nil && nextDescriptor.shareThreshold && nextDescriptor.wildcardPattern != "" { + // Extract the value part from the wildcard pattern (e.g., "key_files*" -> "files*") + if shareThresholdPatterns == nil { + shareThresholdPatterns = make(map[int]string) + } + + wildcardValue := strings.TrimPrefix(nextDescriptor.wildcardPattern, entry.Key+"_") + shareThresholdPatterns[i] = wildcardValue + logger.Debugf("tracking share_threshold for entry index %d (key %s), wildcard pattern %s", i, entry.Key, wildcardValue) + } + // Build value_to_metric metrics path for this level valueToMetricFullKey.WriteString(".") if nextDescriptor != nil { + // Check if share_threshold is enabled for this entry + hasShareThreshold := shareThresholdPatterns[i] != "" if matchedViaWildcard { - if nextDescriptor.valueToMetric { + // When share_threshold is enabled AND value_to_metric is enabled, use the prefix of the wildcard pattern + if hasShareThreshold && nextDescriptor.valueToMetric { + wildcardPrefix := strings.TrimSuffix(shareThresholdPatterns[i], "*") + valueToMetricFullKey.WriteString(entry.Key) + valueToMetricFullKey.WriteString("_") + valueToMetricFullKey.WriteString(wildcardPrefix) + } else if nextDescriptor.valueToMetric { valueToMetricFullKey.WriteString(entry.Key) if entry.Value != "" { valueToMetricFullKey.WriteString("_") @@ -365,14 +423,28 @@ func (this *rateLimitConfigImpl) GetLimit( } } else if matchedUsingValue { // Matched explicit key+value in config - valueToMetricFullKey.WriteString(entry.Key) - if entry.Value != "" { + // When share_threshold is enabled AND value_to_metric is enabled, use the prefix of the wildcard pattern + if hasShareThreshold && nextDescriptor.valueToMetric { + wildcardPrefix := strings.TrimSuffix(shareThresholdPatterns[i], "*") + valueToMetricFullKey.WriteString(entry.Key) valueToMetricFullKey.WriteString("_") - valueToMetricFullKey.WriteString(entry.Value) + valueToMetricFullKey.WriteString(wildcardPrefix) + } else { + valueToMetricFullKey.WriteString(entry.Key) + if entry.Value != "" { + valueToMetricFullKey.WriteString("_") + valueToMetricFullKey.WriteString(entry.Value) + } } } else { // Matched default key (no value) in config - if nextDescriptor.valueToMetric { + // When share_threshold is enabled AND value_to_metric is enabled, use the prefix of the wildcard pattern + if hasShareThreshold && nextDescriptor.valueToMetric { + wildcardPrefix := strings.TrimSuffix(shareThresholdPatterns[i], "*") + valueToMetricFullKey.WriteString(entry.Key) + valueToMetricFullKey.WriteString("_") + valueToMetricFullKey.WriteString(wildcardPrefix) + } else if nextDescriptor.valueToMetric { valueToMetricFullKey.WriteString(entry.Key) if entry.Value != "" { valueToMetricFullKey.WriteString("_") @@ -391,7 +463,31 @@ func (this *rateLimitConfigImpl) GetLimit( logger.Debugf("found rate limit: %s", finalKey) if i == len(descriptor.Entries)-1 { - rateLimit = nextDescriptor.limit + // Create a copy of the rate limit to avoid modifying the shared object + originalLimit := nextDescriptor.limit + rateLimit = &RateLimit{ + FullKey: originalLimit.FullKey, + Stats: originalLimit.Stats, + Limit: originalLimit.Limit, + Unlimited: originalLimit.Unlimited, + ShadowMode: originalLimit.ShadowMode, + Name: originalLimit.Name, + Replaces: originalLimit.Replaces, + DetailedMetric: originalLimit.DetailedMetric, + // Initialize ShareThresholdKeyPattern with correct length, empty strings for entries without share_threshold + ShareThresholdKeyPattern: nil, + } + // Apply all tracked share_threshold patterns when we find the rate_limit + // This works whether the rate_limit is at the wildcard level or deeper + // Only entries with share_threshold will have non-empty patterns + if len(shareThresholdPatterns) > 0 { + rateLimit.ShareThresholdKeyPattern = make([]string, len(descriptor.Entries)) + } + + for idx, pattern := range shareThresholdPatterns { + rateLimit.ShareThresholdKeyPattern[idx] = pattern + logger.Debugf("share_threshold enabled for entry index %d, using wildcard pattern %s", idx, pattern) + } } else { logger.Debugf("request depth does not match config depth, there are more entries in the request's descriptor") } @@ -402,7 +498,10 @@ func (this *rateLimitConfigImpl) GetLimit( descriptorsMap = nextDescriptor.descriptors } else { if rateLimit != nil && rateLimit.DetailedMetric { + // Preserve ShareThresholdKeyPattern when recreating rate limit + originalShareThresholdKeyPattern := rateLimit.ShareThresholdKeyPattern rateLimit = NewRateLimit(rateLimit.Limit.RequestsPerUnit, rateLimit.Limit.Unit, this.statsManager.NewStats(rateLimit.FullKey), rateLimit.Unlimited, rateLimit.ShadowMode, rateLimit.Name, rateLimit.Replaces, rateLimit.DetailedMetric) + rateLimit.ShareThresholdKeyPattern = originalShareThresholdKeyPattern } break @@ -411,10 +510,39 @@ func (this *rateLimitConfigImpl) GetLimit( } // Replace metric with detailed metric, if leaf descriptor is detailed. + // When share_threshold is enabled, expose the prefix (before *) of the wildcard pattern if rateLimit != nil && rateLimit.DetailedMetric { - detailedKey := detailedMetricFullKey.String() - rateLimit.Stats = this.statsManager.NewStats(detailedKey) - rateLimit.FullKey = detailedKey + // Check if any entry has share_threshold enabled + hasShareThreshold := rateLimit.ShareThresholdKeyPattern != nil && len(rateLimit.ShareThresholdKeyPattern) > 0 + if hasShareThreshold { + // Build metric key with wildcard prefix for entries with share_threshold + var shareThresholdMetricKey strings.Builder + shareThresholdMetricKey.WriteString(domain) + for i, entry := range descriptor.Entries { + shareThresholdMetricKey.WriteString(".") + if i < len(rateLimit.ShareThresholdKeyPattern) && rateLimit.ShareThresholdKeyPattern[i] != "" { + // Use the prefix of the wildcard pattern (before *) + wildcardPrefix := strings.TrimSuffix(rateLimit.ShareThresholdKeyPattern[i], "*") + shareThresholdMetricKey.WriteString(entry.Key) + shareThresholdMetricKey.WriteString("_") + shareThresholdMetricKey.WriteString(wildcardPrefix) + } else { + // Include full key_value for entries without share_threshold + shareThresholdMetricKey.WriteString(entry.Key) + if entry.Value != "" { + shareThresholdMetricKey.WriteString("_") + shareThresholdMetricKey.WriteString(entry.Value) + } + } + } + shareThresholdKey := shareThresholdMetricKey.String() + rateLimit.FullKey = shareThresholdKey + rateLimit.Stats = this.statsManager.NewStats(shareThresholdKey) + } else { + detailedKey := detailedMetricFullKey.String() + rateLimit.FullKey = detailedKey + rateLimit.Stats = this.statsManager.NewStats(detailedKey) + } } // If not using detailed metric, but any value_to_metric path produced a different key, @@ -423,7 +551,9 @@ func (this *rateLimitConfigImpl) GetLimit( enhancedKey := valueToMetricFullKey.String() if enhancedKey != rateLimit.FullKey { // Recreate to ensure a clean stats struct, then set to enhanced stats + originalShareThresholdKeyPattern := rateLimit.ShareThresholdKeyPattern rateLimit = NewRateLimit(rateLimit.Limit.RequestsPerUnit, rateLimit.Limit.Unit, this.statsManager.NewStats(rateLimit.FullKey), rateLimit.Unlimited, rateLimit.ShadowMode, rateLimit.Name, rateLimit.Replaces, rateLimit.DetailedMetric) + rateLimit.ShareThresholdKeyPattern = originalShareThresholdKeyPattern rateLimit.Stats = this.statsManager.NewStats(enhancedKey) rateLimit.FullKey = enhancedKey } diff --git a/src/limiter/cache_key.go b/src/limiter/cache_key.go index 7beadd54a..8b2056fc5 100644 --- a/src/limiter/cache_key.go +++ b/src/limiter/cache_key.go @@ -63,10 +63,18 @@ func (this *CacheKeyGenerator) GenerateCacheKey( b.WriteString(domain) b.WriteByte('_') - for _, entry := range descriptor.Entries { + for i, entry := range descriptor.Entries { b.WriteString(entry.Key) b.WriteByte('_') - b.WriteString(entry.Value) + // If share_threshold is enabled for this entry index, use the wildcard pattern instead of the actual value + // Use entry index instead of key name to handle nested descriptors with same key names + valueToUse := entry.Value + if limit != nil && limit.ShareThresholdKeyPattern != nil && i < len(limit.ShareThresholdKeyPattern) { + if wildcardPattern := limit.ShareThresholdKeyPattern[i]; wildcardPattern != "" { + valueToUse = wildcardPattern + } + } + b.WriteString(valueToUse) b.WriteByte('_') } diff --git a/test/config/config_test.go b/test/config/config_test.go index bd87a0c5c..19b4a6832 100644 --- a/test/config/config_test.go +++ b/test/config/config_test.go @@ -1592,3 +1592,187 @@ func TestValueToMetric_FullKeyMatchesStatsKey(t *testing.T) { asrt.NotNil(rl3) asrt.Equal(rl3.FullKey, rl3.Stats.Key, "FullKey should match Stats.Key when detailed_metric is enabled") } + +// TestShareThreshold tests config (ShareThresholdKeyPattern) and metrics (Stats.Key) +// Cache key generation is tested in base_limiter_test.go +func TestShareThreshold(t *testing.T) { + asrt := assert.New(t) + stats := stats.NewStore(stats.NewNullSink(), false) + rlConfig := config.NewRateLimitConfigImpl(loadFile("share_threshold.yaml"), mockstats.NewMockStatManager(stats), false) + + // Test Case 1: Basic share_threshold functionality + t.Run("Basic share_threshold", func(t *testing.T) { + testValues := []string{"files/a.pdf", "files/b.csv", "files/c.txt"} + var rateLimits []*config.RateLimit + + for _, value := range testValues { + rl := rlConfig.GetLimit( + context.TODO(), "test-domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "files", Value: value}}, + }) + asrt.NotNil(rl) + // Verify config: ShareThresholdKeyPattern is set correctly + asrt.Equal("files/*", rl.ShareThresholdKeyPattern[0]) + asrt.EqualValues(100, rl.Limit.RequestsPerUnit) + // Verify stats: All should have the same stats key + asrt.Equal("test-domain.files", rl.Stats.Key) + rateLimits = append(rateLimits, rl) + } + + // Verify all have same stats key + for i := 1; i < len(rateLimits); i++ { + asrt.Equal(rateLimits[0].Stats.Key, rateLimits[i].Stats.Key, "All values should have same stats key") + } + }) + + // Test Case 2: share_threshold with metrics (value_to_metric and detailed_metric) + // Tests that metrics correctly include wildcard prefix when share_threshold is enabled + t.Run("share_threshold with metrics", func(t *testing.T) { + // Test value_to_metric + rl1 := rlConfig.GetLimit( + context.TODO(), "test-domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "route", Value: "api/v1"}, + {Key: "method", Value: "GET"}, + }, + }) + asrt.NotNil(rl1) + asrt.Equal("api/*", rl1.ShareThresholdKeyPattern[0]) + asrt.Equal("test-domain.route_api/.method", rl1.Stats.Key) + + // Test detailed_metric + rl2 := rlConfig.GetLimit( + context.TODO(), "test-domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "service", Value: "svc/user"}, + {Key: "endpoint", Value: "get"}, + }, + }) + asrt.NotNil(rl2) + asrt.Equal("svc/*", rl2.ShareThresholdKeyPattern[0]) + asrt.True(rl2.DetailedMetric) + asrt.Equal("test-domain.service_svc/.endpoint_get", rl2.Stats.Key) + }) + + // Test Case 3: Nested wildcards with share_threshold + t.Run("Nested wildcards", func(t *testing.T) { + // Nested share_threshold + rl1 := rlConfig.GetLimit( + context.TODO(), "test-domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "nested", Value: "parent"}, + {Key: "files", Value: "nested/file1.txt"}, + }, + }) + asrt.NotNil(rl1) + asrt.Equal("nested/*", rl1.ShareThresholdKeyPattern[1]) + asrt.EqualValues(200, rl1.Limit.RequestsPerUnit) + + // Multiple wildcards + rl2 := rlConfig.GetLimit( + context.TODO(), "test-domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "files", Value: "top/file1.txt"}, + {Key: "files", Value: "nested/file1.txt"}, + }, + }) + asrt.NotNil(rl2) + asrt.Equal("top/*", rl2.ShareThresholdKeyPattern[0]) + asrt.Equal("nested/*", rl2.ShareThresholdKeyPattern[1]) + + // Parent has share_threshold, child does not + rl3 := rlConfig.GetLimit( + context.TODO(), "test-domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "path", Value: "path/foo"}, + {Key: "file", Value: "file/doc1"}, + {Key: "type", Value: "pdf"}, + }, + }) + asrt.NotNil(rl3) + asrt.Equal("path/*", rl3.ShareThresholdKeyPattern[0]) + asrt.Equal("", rl3.ShareThresholdKeyPattern[1]) + + // Both parent and child have share_threshold + rl4 := rlConfig.GetLimit( + context.TODO(), "test-domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "user", Value: "user/alice"}, + {Key: "resource", Value: "res/file1"}, + {Key: "action", Value: "read"}, + }, + }) + asrt.NotNil(rl4) + asrt.Equal("user/*", rl4.ShareThresholdKeyPattern[0]) + asrt.Equal("res/*", rl4.ShareThresholdKeyPattern[1]) + }) + + // Test Case 4: Explicit value takes precedence over wildcard + // Verify: file_test1 uses isolated threshold, file_test2/test23/test123 share threshold + t.Run("Explicit value precedence over wildcard", func(t *testing.T) { + // Test explicit match: file_test1 should use isolated threshold (50/min) + rlTest1 := rlConfig.GetLimit( + context.TODO(), "test-domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "file", Value: "test1"}, + }, + }) + asrt.NotNil(rlTest1, "Should find rate limit for file=test1") + asrt.EqualValues(50, rlTest1.Limit.RequestsPerUnit, "test1 should have isolated limit of 50") + // ShareThresholdKeyPattern should be nil for explicit matches (lazy initialization) + asrt.Nil(rlTest1.ShareThresholdKeyPattern, "ShareThresholdKeyPattern should be nil for explicit matches (no share_threshold)") + // Verify Stats: test1 should have its own stats key + asrt.Equal("test-domain.file_test1", rlTest1.Stats.Key, "test1 should have its own stats key") + + // Test wildcard matches: file_test2, file_test23, file_test123 should share threshold (100/min) + testWildcardValues := []string{"test2", "test23", "test123"} + var rateLimits []*config.RateLimit + + for _, value := range testWildcardValues { + rl := rlConfig.GetLimit( + context.TODO(), "test-domain", + &pb_struct.RateLimitDescriptor{ + Entries: []*pb_struct.RateLimitDescriptor_Entry{ + {Key: "file", Value: value}, + }, + }) + asrt.NotNil(rl, "Should find rate limit for file=%s", value) + + // Verify RateLimitConfig: Should have share_threshold pattern + asrt.NotNil(rl.ShareThresholdKeyPattern, "ShareThresholdKeyPattern should be set for %s", value) + asrt.Equal(1, len(rl.ShareThresholdKeyPattern), "Should have one pattern") + asrt.Equal("test*", rl.ShareThresholdKeyPattern[0], "Pattern should be test*") + asrt.EqualValues(100, rl.Limit.RequestsPerUnit, "Should have shared limit of 100") + + // Verify Stats: All wildcard matches should have the same stats key + asrt.Equal("test-domain.file", rl.Stats.Key, "Stats key should be same for all wildcard values") + + rateLimits = append(rateLimits, rl) + } + + // Verify all wildcard matches have same stats key + for i := 1; i < len(rateLimits); i++ { + asrt.Equal(rateLimits[0].Stats.Key, rateLimits[i].Stats.Key, "All wildcard values should have same stats key") + } + + // Verify stats counters are shared for wildcard matches + for _, rl := range rateLimits { + rl.Stats.TotalHits.Inc() + } + counterValue := stats.NewCounter("test-domain.file.total_hits").Value() + asrt.GreaterOrEqual(counterValue, uint64(len(testWildcardValues)), "All wildcard values should increment the same counter") + + // test1 should have its own counter + rlTest1.Stats.TotalHits.Inc() + counterTest1 := stats.NewCounter("test-domain.file_test1.total_hits").Value() + asrt.GreaterOrEqual(counterTest1, uint64(1), "test1 should increment its own counter") + }) +} diff --git a/test/config/share_threshold.yaml b/test/config/share_threshold.yaml new file mode 100644 index 000000000..c601576aa --- /dev/null +++ b/test/config/share_threshold.yaml @@ -0,0 +1,87 @@ +# Configuration for testing share_threshold feature +domain: test-domain +descriptors: + - key: files + value: files/* + share_threshold: true + rate_limit: + unit: minute + requests_per_unit: 100 + - key: nested + value: parent + descriptors: + - key: files + value: nested/* + share_threshold: true + rate_limit: + unit: minute + requests_per_unit: 200 + # Test case for nested descriptors with same key name at different levels + - key: files + value: top/* + share_threshold: true + descriptors: + - key: files + value: nested/* + share_threshold: true + rate_limit: + unit: minute + requests_per_unit: 300 + # Test case for share_threshold with value_to_metric + - key: route + value: api/* + share_threshold: true + value_to_metric: true + descriptors: + - key: method + rate_limit: + unit: minute + requests_per_unit: 60 + # Test case for share_threshold with detailed_metric + - key: service + value: svc/* + share_threshold: true + descriptors: + - key: endpoint + detailed_metric: true + rate_limit: + unit: minute + requests_per_unit: 80 + # Test case for nested descriptors with multiple wildcards - mixed share_threshold + - key: path + value: path/* + share_threshold: true + descriptors: + - key: file + value: file/* + share_threshold: false + descriptors: + - key: type + rate_limit: + unit: minute + requests_per_unit: 120 + # Test case for nested descriptors with multiple wildcards - both with share_threshold + - key: user + value: user/* + share_threshold: true + descriptors: + - key: resource + value: res/* + share_threshold: true + descriptors: + - key: action + rate_limit: + unit: minute + requests_per_unit: 150 + # Test case: explicit value takes precedence over wildcard with share_threshold + - key: file + value: test1 + rate_limit: + unit: minute + requests_per_unit: 50 + - key: file + value: test* + share_threshold: true + rate_limit: + unit: minute + requests_per_unit: 100 diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index 5a8379a44..abc3d726d 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -908,3 +908,82 @@ func waitForConfigReload(runner *runner.Runner, loadCountBefore uint64) (uint64, } return loadCountAfter, reloaded } + +func TestShareThreshold(t *testing.T) { + common.WithMultiRedis(t, []common.RedisConfig{ + {Port: 6383}, + {Port: 6380}, + }, func() { + t.Run("WithoutPerSecondRedis", testShareThreshold(makeSimpleRedisSettings(6383, 6380, false, 0))) + }) +} + +func testShareThreshold(s settings.Settings) func(*testing.T) { + return func(t *testing.T) { + 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) + + // Use the domain from the config file + domain := "share-threshold-test" + + // Test Case 1: share_threshold: true - different values matching files/* should share the same threshold + // Make 10 requests with files/a.pdf - can be OK or OVER_LIMIT + for i := 0; i < 10; i++ { + response, err := c.ShouldRateLimit( + context.Background(), + common.NewRateLimitRequest(domain, [][][2]string{{{"files", "files/a.pdf"}}}, 1)) + assert.NoError(err) + // Each request can be OK or OVER_LIMIT (depending on when limit is reached) + assert.True(response.OverallCode == pb.RateLimitResponse_OK || response.OverallCode == pb.RateLimitResponse_OVER_LIMIT, + "Request %d should be OK or OVER_LIMIT, got: %v", i+1, response.OverallCode) + } + + // Now make a request with files/b.csv - must be OVER_LIMIT because it shares the threshold + response, err := c.ShouldRateLimit( + context.Background(), + common.NewRateLimitRequest(domain, [][][2]string{{{"files", "files/b.csv"}}}, 1)) + assert.NoError(err) + durRemaining := response.GetStatuses()[0].DurationUntilReset + common.AssertProtoEqual( + assert, + &pb.RateLimitResponse{ + OverallCode: pb.RateLimitResponse_OVER_LIMIT, + Statuses: []*pb.RateLimitResponse_DescriptorStatus{ + newDescriptorStatus(pb.RateLimitResponse_OVER_LIMIT, 10, pb.RateLimitResponse_RateLimit_HOUR, 0, durRemaining), + }, + }, + response) + + // Test Case 2: share_threshold: false - different values should have isolated thresholds + // Use random values with prefix files_no_share to ensure uniqueness (based on timestamp) + // Each value should have its own isolated threshold, so all 10 requests should be OK + baseTimestamp := time.Now().UnixNano() + r := rand.New(rand.NewSource(baseTimestamp)) + for i := 0; i < 10; i++ { + // Generate unique value using timestamp and random number to avoid collisions + uniqueValue := fmt.Sprintf("files_no_share/%d-%d", baseTimestamp, r.Int63()) + response, err := c.ShouldRateLimit( + context.Background(), + common.NewRateLimitRequest(domain, [][][2]string{{{"files_no_share", uniqueValue}}}, 1)) + assert.NoError(err) + // Each value has its own isolated threshold, so each request should have remaining = 9 (10 - 1) + expectedRemaining := uint32(9) + durRemaining := response.GetStatuses()[0].DurationUntilReset + common.AssertProtoEqual( + assert, + &pb.RateLimitResponse{ + OverallCode: pb.RateLimitResponse_OK, + Statuses: []*pb.RateLimitResponse_DescriptorStatus{ + newDescriptorStatus(pb.RateLimitResponse_OK, 10, pb.RateLimitResponse_RateLimit_HOUR, expectedRemaining, durRemaining), + }, + }, + response) + } + } +} diff --git a/test/integration/runtime/current/ratelimit/config/share_threshold.yaml b/test/integration/runtime/current/ratelimit/config/share_threshold.yaml new file mode 100644 index 000000000..33b55dc4f --- /dev/null +++ b/test/integration/runtime/current/ratelimit/config/share_threshold.yaml @@ -0,0 +1,15 @@ +domain: share-threshold-test +descriptors: + # Test case 1: share_threshold: true - files/* should share threshold + - key: files + value: files/* + share_threshold: true + rate_limit: + unit: hour + requests_per_unit: 10 + + - key: files_no_share + value: files_no_share/* + rate_limit: + unit: hour + requests_per_unit: 10 diff --git a/test/limiter/base_limiter_test.go b/test/limiter/base_limiter_test.go index 8fe154369..ea1e77cc5 100644 --- a/test/limiter/base_limiter_test.go +++ b/test/limiter/base_limiter_test.go @@ -56,6 +56,87 @@ func TestGenerateCacheKeysPrefix(t *testing.T) { assert.Equal(uint64(1), limits[0].Stats.TotalHits.Value()) } +func TestGenerateCacheKeysWithShareThreshold(t *testing.T) { + assert := assert.New(t) + controller := gomock.NewController(t) + defer controller.Finish() + timeSource := mock_utils.NewMockTimeSource(controller) + jitterSource := mock_utils.NewMockJitterRandSource(controller) + statsStore := stats.NewStore(stats.NewNullSink(), false) + sm := mockstats.NewMockStatManager(statsStore) + timeSource.EXPECT().UnixNow().Return(int64(1234)).AnyTimes() + baseRateLimit := limiter.NewBaseRateLimit(timeSource, rand.New(jitterSource), 3600, nil, 0.8, "", sm) + + // Test 1: Simple case - different values with same wildcard prefix generate same cache key + limit := config.NewRateLimit(10, pb.RateLimitResponse_RateLimit_SECOND, sm.NewStats("files_files/*"), false, false, "", nil, false) + limit.ShareThresholdKeyPattern = []string{"files/*"} // Entry at index 0 + + request1 := common.NewRateLimitRequest("domain", [][][2]string{{{"files", "files/a.pdf"}}}, 1) + limits1 := []*config.RateLimit{limit} + cacheKeys1 := baseRateLimit.GenerateCacheKeys(request1, limits1, []uint64{1}) + assert.Equal(1, len(cacheKeys1)) + assert.Equal("domain_files_files/*_1234", cacheKeys1[0].Key) + + request2 := common.NewRateLimitRequest("domain", [][][2]string{{{"files", "files/b.csv"}}}, 1) + limits2 := []*config.RateLimit{limit} + cacheKeys2 := baseRateLimit.GenerateCacheKeys(request2, limits2, []uint64{1}) + assert.Equal(1, len(cacheKeys2)) + // Should generate the same cache key as the first request + assert.Equal("domain_files_files/*_1234", cacheKeys2[0].Key) + assert.Equal(cacheKeys1[0].Key, cacheKeys2[0].Key) + + // Test 2: Multiple different values all generate the same cache key + testValues := []string{"files/c.txt", "files/d.json", "files/e.xml", "files/subdir/f.txt"} + for _, value := range testValues { + request := common.NewRateLimitRequest("domain", [][][2]string{{{"files", value}}}, 1) + cacheKeys := baseRateLimit.GenerateCacheKeys(request, limits1, []uint64{1}) + assert.Equal(1, len(cacheKeys)) + assert.Equal("domain_files_files/*_1234", cacheKeys[0].Key, "Value %s should generate same cache key", value) + } + + // Test 3: Nested descriptors with share_threshold at second level + limitNested := config.NewRateLimit(10, pb.RateLimitResponse_RateLimit_SECOND, sm.NewStats("parent_files_nested/*"), false, false, "", nil, false) + limitNested.ShareThresholdKeyPattern = []string{"", "nested/*"} // First entry no share_threshold, second entry has it + + request3a := common.NewRateLimitRequest("domain", [][][2]string{ + {{"parent", "value1"}, {"files", "nested/file1.txt"}}, + }, 1) + limits3a := []*config.RateLimit{limitNested} + cacheKeys3a := baseRateLimit.GenerateCacheKeys(request3a, limits3a, []uint64{1}) + assert.Equal(1, len(cacheKeys3a)) + assert.Equal("domain_parent_value1_files_nested/*_1234", cacheKeys3a[0].Key) + + request3b := common.NewRateLimitRequest("domain", [][][2]string{ + {{"parent", "value1"}, {"files", "nested/file2.csv"}}, + }, 1) + cacheKeys3b := baseRateLimit.GenerateCacheKeys(request3b, limits3a, []uint64{1}) + assert.Equal(1, len(cacheKeys3b)) + // Should generate the same cache key despite different file values + assert.Equal("domain_parent_value1_files_nested/*_1234", cacheKeys3b[0].Key) + assert.Equal(cacheKeys3a[0].Key, cacheKeys3b[0].Key) + + // Test 4: Multiple entries with share_threshold at different positions + limitMulti := config.NewRateLimit(10, pb.RateLimitResponse_RateLimit_SECOND, sm.NewStats("files_top/*_files_nested/*"), false, false, "", nil, false) + limitMulti.ShareThresholdKeyPattern = []string{"top/*", "nested/*"} // Both entries have share_threshold + + request4a := common.NewRateLimitRequest("domain", [][][2]string{ + {{"files", "top/file1.txt"}, {"files", "nested/sub1.txt"}}, + }, 1) + limits4a := []*config.RateLimit{limitMulti} + cacheKeys4a := baseRateLimit.GenerateCacheKeys(request4a, limits4a, []uint64{1}) + assert.Equal(1, len(cacheKeys4a)) + assert.Equal("domain_files_top/*_files_nested/*_1234", cacheKeys4a[0].Key) + + request4b := common.NewRateLimitRequest("domain", [][][2]string{ + {{"files", "top/file2.pdf"}, {"files", "nested/sub2.csv"}}, + }, 1) + cacheKeys4b := baseRateLimit.GenerateCacheKeys(request4b, limits4a, []uint64{1}) + assert.Equal(1, len(cacheKeys4b)) + // Should generate the same cache key despite different values + assert.Equal("domain_files_top/*_files_nested/*_1234", cacheKeys4b[0].Key) + assert.Equal(cacheKeys4a[0].Key, cacheKeys4b[0].Key) +} + func TestOverLimitWithLocalCache(t *testing.T) { assert := assert.New(t) controller := gomock.NewController(t)