Simplify identical key-value pairs in descriptors

[rofafor]

Currently Envoy Gateway passes descriptors to ratelimit with identical key and value. This makes Prometheus metrics rather unusable:, e.g.:

# HELP ratelimit_service_rate_limit_envoy_gateway_system_internal_https_httproute_envoydemo_envoydemo_rule_0_match_0_envoydemo_domain_com_httproute_envoydemo_envoydemo_rule_0_match_0_envoydemo_domain_com_masked_remote_address_0_0_0_0_0_remote_address_total_hits Metric autogenerated by statsd_exporter.
# TYPE ratelimit_service_rate_limit_envoy_gateway_system_internal_https_httproute_envoydemo_envoydemo_rule_0_match_0_envoydemo_domain_com_httproute_envoydemo_envoydemo_rule_0_match_0_envoydemo_domain_com_masked_remote_address_0_0_0_0_0_remote_address_total_hits counter
ratelimit_service_rate_limit_envoy_gateway_system_internal_https_httproute_envoydemo_envoydemo_rule_0_match_0_envoydemo_domain_com_httproute_envoydemo_envoydemo_rule_0_match_0_envoydemo_domain_com_masked_remote_address_0_0_0_0_0_remote_address_total_hits 1

We can use the following patch to avoid repeating (optional) value if it's identical to key. This will simplify Prometheus labels by removing unnecessary duplication.

diff --git a/src/config/config_impl.go b/src/config/config_impl.go
index 45c276b..3bee97b 100644
--- a/src/config/config_impl.go
+++ b/src/config/config_impl.go
@@ -127,9 +127,9 @@ func (this *rateLimitDescriptor) loadDescriptors(config RateLimitConfigToLoad, p
                        panic(newRateLimitConfigError(config.Name, "descriptor has empty key"))
                }

-               // Value is optional, so the final key for the map is either the key only or key_value.
+               // Value is optional, so the final key for the map is either the key only or key_value if value differs from key.
                finalKey := descriptorConfig.Key
-               if descriptorConfig.Value != "" {
+               if descriptorConfig.Value != "" && descriptorConfig.Value != descriptorConfig.Key {
                        finalKey += "_" + descriptorConfig.Value
                }

The patch originates from #773

[wtrocki]

Metrics are meant to represent actual rate limiting bucket key. Same value or no value might be causing confusion.
If simplification is the key, I would suggest having a new config property to skip using values as keys.

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.