-
Notifications
You must be signed in to change notification settings - Fork 5.3k
stats: Tag extraction doesn't work if resource name contains dots #4357
Description
Title: Tag extraction doesn't work if resource name contains dots
Description:
I've tested this for CLUSTER_NAME, but I assume it holds for others with similar regexes like listener, vhost, etc.
If you have a cluster name like foo.bar (this is every single cluster for us, since they are namespaced), then you'll have stats like cluster.foo.bar.upstream_cx_active. The tag extraction regex, however will match only foo and then use bar.upstream_cx_active for the stat name.
A reasonable workaround is to not use dots. However, if we take this approach, we should warn about this in the docs.
A fix will be a bit trickier. How do we possibly differentiate the resource name from the metric name when they both contain dots?
This issue might be considered an extension of #2687 and solving that in a general way would likely solve this. But it's a bit distinct in that there isn't a collision per se (in that two different stats don't resolve to the same thing), but it does have to do with tag extraction. Further, it isn't going to be solved with testing like in some of the other issues since it is dependent on the configuration. Either way, let me know if you think this is a dupe and I can move it to a comment on that issue.
Repro steps:
This repros on master with a trivial unit test change:
diff --git a/test/common/stats/tag_extractor_test.cc b/test/common/stats/tag_extractor_test.cc
index ac1c89c..7ca5dda 100644
--- a/test/common/stats/tag_extractor_test.cc
+++ b/test/common/stats/tag_extractor_test.cc
@@ -138,9 +138,9 @@ TEST(TagExtractorTest, DefaultTagExtractors) {
// Cluster name
Tag cluster_tag;
cluster_tag.name_ = tag_names.CLUSTER_NAME;
- cluster_tag.value_ = "ratelimit";
+ cluster_tag.value_ = "foo.bar";
- regex_tester.testRegex("cluster.ratelimit.upstream_rq_timeout", "cluster.upstream_rq_timeout",
+ regex_tester.testRegex("cluster.foo.bar.upstream_rq_timeout", "cluster.upstream_rq_timeout",
{cluster_tag});
// Listener SSL
@@ -163,7 +163,7 @@ TEST(TagExtractorTest, DefaultTagExtractors) {
cipher_suite.name_ = tag_names.SSL_CIPHER_SUITE;
cipher_suite.value_ = "ECDHE-RSA-AES128-GCM-SHA256";
- regex_tester.testRegex("cluster.ratelimit.ssl.ciphers.ECDHE-RSA-AES128-GCM-SHA256",
+ regex_tester.testRegex("cluster.foo.bar.ssl.ciphers.ECDHE-RSA-AES128-GCM-SHA256",
"cluster.ssl.ciphers", {cluster_tag, cipher_suite});
// ipv6 non-loopback (for alphabetical chars)