stats: Eliminate duplicate tag-name check to allow multiple regexes to populate the same tag name. (#23024)
Commit Message: Currently, there can be multiple built-in regexes targeting the same tag name, and in fact there's at least one case where this occurs:pull/626/head466e78586a/source/common/config/well_known_names.cc (L133)
466e78586a/source/common/config/well_known_names.cc (L136)
This change prevents a second tag value for a given name being from being extracted, to meet Prometheus' requirements. Having two alternate ways of generating the same tag value allows them to be expressed using two distinct regexes, which are easier to understand, and possible for the infrastructure to optimize with the prefix-map. This situation also occurs with Istio/Wasm, which for reasons that elude me, generate stats with two very different syntaxes both meaning HTTP Response Code, and adds those extractors using configuration. An alternate approach is to add complexity to the regex processing to allow matches in an ORed regex, which is a bit confusing, and results in regexes that cannot be optimized well by our current system. There is no one prefix that can be used to reduce the set of regexes that need to be evaluated against every stat, and the long regexes with captures are hard for humans to read. See https://github.com/envoyproxy/envoy/pull/22791 The disadvantage of allowing multiple regexes to generate the same tag, is that it may create more scenarios where a stats sink like Prometheus may be given multiple tags with the same name, and it would be good to get some notion that this is OK. Currently such cases would be rejected during process startup (for CLI-based tags) or during config processing. I opened this up for review to initiate this discussion, but want to make sure various stakeholders have a chance to weigh in. Though no protobufs were changed structurally in this PR, it's kind of an API change (with .proto comments) and should probably be approved as one. Additional Description: Risk Level: medium Testing: //test/... Docs Changes: changed comments in proto file that previously indicated dups were not allowed Release Notes: Platform Specific Features: Fixes: https://github.com/envoyproxy/envoy/issues/22591 Signed-off-by: Joshua Marantz <jmarantz@google.com> Mirrored from https://github.com/envoyproxy/envoy @ 45f062466a40216d29117320ede012d087ca1318
parent
4989805211
commit
7d8efae7d3
1 changed files with 5 additions and 1 deletions
Loading…
Reference in new issue