envoyproxy / envoyproxy/envoy

stats: Tag extraction doesn't work if resource name contains dots

Offen
#4,357 7 Kommentare 1 Reaktion 0 zugewiesene Personen Auf GitHub ansehen
bug help wanted
Vorherrschende Sprache
C++
Sterne
28.9k
Forks
5.6k
Ø Merge
1 T. 22 Std.
Gemergte PRs (30 T.)
430

Beschreibung

*Title*: *Tag extraction doesn't work if resource name contains dots*

*Description*:
_I've tested this for [CLUSTER_NAME](https://github.com/envoyproxy/envoy/blob/master/source/common/config/well_known_names.h#L103), 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
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)
```

Beitragsleitfaden

Beitragsleitfaden öffnen

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.