-
Notifications
You must be signed in to change notification settings - Fork 511
[ISSUE] Fix: Wildcard Stats Key Behavior Changes #1017
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5a4a039
5b51726
a016fba
26eaac9
f93cebe
8b3be79
e06479e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -368,13 +368,13 @@ func (this *rateLimitConfigImpl) GetLimit( | |
|
|
||
| logger.Debugf("looking up key: %s", finalKey) | ||
| nextDescriptor := descriptorsMap[finalKey] | ||
| matchedViaWildcard := false | ||
| var matchedWildcardKey string | ||
|
|
||
| if nextDescriptor == nil && len(prevDescriptor.wildcardKeys) > 0 { | ||
| for _, wildcardKey := range prevDescriptor.wildcardKeys { | ||
| if strings.HasPrefix(finalKey, strings.TrimSuffix(wildcardKey, "*")) { | ||
| nextDescriptor = descriptorsMap[wildcardKey] | ||
| matchedViaWildcard = true | ||
| matchedWildcardKey = wildcardKey | ||
| break | ||
| } | ||
| } | ||
|
|
@@ -389,7 +389,7 @@ func (this *rateLimitConfigImpl) GetLimit( | |
| } | ||
|
|
||
| // Track share_threshold pattern when matching via wildcard, even if no rate_limit at this level | ||
| if matchedViaWildcard && nextDescriptor != nil && nextDescriptor.shareThreshold && nextDescriptor.wildcardPattern != "" { | ||
| if matchedWildcardKey != "" && 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) | ||
|
|
@@ -403,57 +403,38 @@ func (this *rateLimitConfigImpl) GetLimit( | |
| // Build value_to_metric metrics path for this level | ||
| valueToMetricFullKey.WriteString(".") | ||
| if nextDescriptor != nil { | ||
| // Check if share_threshold is enabled for this entry | ||
| // Determine the value to use for this entry | ||
| var valueToUse string | ||
| hasShareThreshold := shareThresholdPatterns[i] != "" | ||
| if matchedViaWildcard { | ||
| // 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) | ||
|
|
||
| if matchedWildcardKey != "" { | ||
| // Matched via wildcard | ||
| if hasShareThreshold { | ||
| // share_threshold: always use wildcard pattern with * | ||
| valueToUse = shareThresholdPatterns[i] | ||
| } else if nextDescriptor.valueToMetric { | ||
| valueToMetricFullKey.WriteString(entry.Key) | ||
| if entry.Value != "" { | ||
| valueToMetricFullKey.WriteString("_") | ||
| valueToMetricFullKey.WriteString(entry.Value) | ||
| } | ||
| // value_to_metric: use actual runtime value | ||
| valueToUse = entry.Value | ||
| } else { | ||
| valueToMetricFullKey.WriteString(entry.Key) | ||
| // No flags: preserve wildcard pattern | ||
| valueToUse = strings.TrimPrefix(matchedWildcardKey, entry.Key+"_") | ||
| } | ||
| } else if matchedUsingValue { | ||
| // Matched explicit key+value in config | ||
| // 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 { | ||
| valueToMetricFullKey.WriteString(entry.Key) | ||
| if entry.Value != "" { | ||
| valueToMetricFullKey.WriteString("_") | ||
| valueToMetricFullKey.WriteString(entry.Value) | ||
| } | ||
| } | ||
| // Matched explicit key+value in config (share_threshold can't apply here) | ||
| valueToUse = entry.Value | ||
| } else { | ||
| // Matched default key (no value) in config | ||
| // 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("_") | ||
| valueToMetricFullKey.WriteString(entry.Value) | ||
| } | ||
| } else { | ||
| valueToMetricFullKey.WriteString(entry.Key) | ||
| if nextDescriptor.valueToMetric { | ||
| valueToUse = entry.Value | ||
| } | ||
| } | ||
|
|
||
| // Write key and value (if any) | ||
| valueToMetricFullKey.WriteString(entry.Key) | ||
| if valueToUse != "" { | ||
| valueToMetricFullKey.WriteString("_") | ||
| valueToMetricFullKey.WriteString(valueToUse) | ||
| } | ||
| } else { | ||
| // No next descriptor found; still append something deterministic | ||
| valueToMetricFullKey.WriteString(entry.Key) | ||
|
|
@@ -510,22 +491,20 @@ 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 share_threshold is enabled, always use wildcard pattern with * | ||
| if rateLimit != nil && rateLimit.DetailedMetric { | ||
| // 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 | ||
| // Build metric key with wildcard pattern (including *) 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) | ||
| shareThresholdMetricKey.WriteString(rateLimit.ShareThresholdKeyPattern[i]) | ||
| } else { | ||
| // Include full key_value for entries without share_threshold | ||
| shareThresholdMetricKey.WriteString(entry.Key) | ||
|
|
@@ -552,10 +531,8 @@ func (this *rateLimitConfigImpl) GetLimit( | |
| 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 = NewRateLimit(rateLimit.Limit.RequestsPerUnit, rateLimit.Limit.Unit, this.statsManager.NewStats(enhancedKey), rateLimit.Unlimited, rateLimit.ShadowMode, rateLimit.Name, rateLimit.Replaces, rateLimit.DetailedMetric) | ||
| rateLimit.ShareThresholdKeyPattern = originalShareThresholdKeyPattern | ||
| rateLimit.Stats = this.statsManager.NewStats(enhancedKey) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove these codes because each time FullKey is automatically assigned to stat.string when rateLimit is initialized; as a result, remove the assignment (redundant). |
||
| rateLimit.FullKey = enhancedKey | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor the logic to include value in stats. It's not necessary to repeat:
several times in this logic