From 582c8222ef15ce8a0efdd545f50097ae69a206a7 Mon Sep 17 00:00:00 2001 From: Yueshang zuo Date: Thu, 14 May 2026 20:45:52 +0800 Subject: [PATCH] stats: strip inner custom namespaces in prometheus names The Prometheus formatter only stripped a registered custom stat namespace when it was the leading segment of the tag-extracted name. This worked for listener/root-scoped Wasm custom stats but not for upstream Wasm stats scoped under a cluster, where after tag extraction the namespace ends up in the middle (e.g. `cluster.wasmcustom.foo`), producing names like `envoy_cluster_wasmcustom_foo`. Add `CustomStatNamespaces::stripRegisteredInnerNamespace()` and call it from the Prometheus formatter to strip a registered namespace appearing as a non-leading, non-trailing segment, without hard-coding `wasmcustom` or a specific scope depth. Gated by `envoy.reloadable_features.strip_scoped_custom_stat_namespace` (default true). Signed-off-by: Yueshang zuo --- ...er-strips-inner-custom-stat-namespaces.rst | 9 ++ envoy/stats/custom_stat_namespaces.h | 11 +++ source/common/runtime/runtime_features.cc | 1 + .../stats/custom_stat_namespaces_impl.cc | 28 ++++++ .../stats/custom_stat_namespaces_impl.h | 2 + source/server/admin/BUILD | 1 + source/server/admin/prometheus_stats.cc | 12 +++ source/server/admin/prometheus_stats.h | 10 ++- .../stats/custom_stat_namespaces_impl_test.cc | 48 +++++++++++ test/server/admin/BUILD | 1 + test/server/admin/prometheus_stats_test.cc | 86 +++++++++++++++++++ 11 files changed, 205 insertions(+), 4 deletions(-) create mode 100644 changelogs/current/minor_behavior_changes/stats__prometheus-formatter-strips-inner-custom-stat-namespaces.rst diff --git a/changelogs/current/minor_behavior_changes/stats__prometheus-formatter-strips-inner-custom-stat-namespaces.rst b/changelogs/current/minor_behavior_changes/stats__prometheus-formatter-strips-inner-custom-stat-namespaces.rst new file mode 100644 index 0000000000000..6c06433f4c879 --- /dev/null +++ b/changelogs/current/minor_behavior_changes/stats__prometheus-formatter-strips-inner-custom-stat-namespaces.rst @@ -0,0 +1,9 @@ +The prometheus formatter now strips a registered custom stat namespace (e.g. ``wasmcustom``) when +it appears as a non-leading, non-trailing segment of a tag-extracted stat name, so that custom +metrics created on a non-root scope render with the standard ``envoy_`` prefix and without the +internal namespace. For example an upstream Wasm counter that previously rendered as +``envoy_cluster_wasmcustom_upstream_rq_2xx`` now renders as ``envoy_cluster_upstream_rq_2xx``, +matching listener Wasm and native cluster metrics. The ``cluster_name`` and other extracted labels +are preserved. This primarily fixes upstream HTTP filter Wasm metrics emitted under cluster scope. +This behavioral change can be temporarily reverted by setting runtime guard +``envoy.reloadable_features.strip_scoped_custom_stat_namespace`` to ``false``. diff --git a/envoy/stats/custom_stat_namespaces.h b/envoy/stats/custom_stat_namespaces.h index 175a80efe6b7a..880574fc44c2c 100644 --- a/envoy/stats/custom_stat_namespaces.h +++ b/envoy/stats/custom_stat_namespaces.h @@ -1,5 +1,7 @@ #pragma once +#include + #include "envoy/common/pure.h" #include "absl/strings/string_view.h" @@ -46,6 +48,15 @@ class CustomStatNamespaces { */ virtual absl::optional stripRegisteredPrefix(const absl::string_view stat_name) const PURE; + + /** + * Like stripRegisteredPrefix, but for a registered namespace appearing as a non-leading, + * non-trailing segment of the dotted stat name (e.g. "cluster.wasmcustom."). Returns + * std::string rather than string_view because the result spans both sides of the removed + * segment. + */ + virtual absl::optional + stripRegisteredInnerNamespace(absl::string_view stat_name) const PURE; }; } // namespace Stats diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index e86d19ee11798..1809f1679a56b 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -106,6 +106,7 @@ RUNTIME_GUARD(envoy_reloadable_features_skip_dns_lookup_for_proxied_requests); RUNTIME_GUARD(envoy_reloadable_features_skip_pending_overflow_count_on_active_rq); RUNTIME_GUARD(envoy_reloadable_features_ssl_socket_report_connection_reset); RUNTIME_GUARD(envoy_reloadable_features_strict_stats_matcher_unpacked); +RUNTIME_GUARD(envoy_reloadable_features_strip_scoped_custom_stat_namespace); RUNTIME_GUARD(envoy_reloadable_features_tcp_proxy_odcds_over_ads_fix); RUNTIME_GUARD(envoy_reloadable_features_test_feature_true); RUNTIME_GUARD(envoy_reloadable_features_tls_certificate_compression_brotli); diff --git a/source/common/stats/custom_stat_namespaces_impl.cc b/source/common/stats/custom_stat_namespaces_impl.cc index f5acb34ebcfbe..7dd2a9af70ba9 100644 --- a/source/common/stats/custom_stat_namespaces_impl.cc +++ b/source/common/stats/custom_stat_namespaces_impl.cc @@ -1,8 +1,13 @@ #include "source/common/stats/custom_stat_namespaces_impl.h" +#include + #include "source/common/common/assert.h" #include "source/common/common/thread.h" +#include "absl/strings/str_join.h" +#include "absl/strings/str_split.h" + namespace Envoy { namespace Stats { @@ -29,5 +34,28 @@ CustomStatNamespacesImpl::stripRegisteredPrefix(const absl::string_view stat_nam return absl::nullopt; }; +absl::optional +CustomStatNamespacesImpl::stripRegisteredInnerNamespace(absl::string_view stat_name) const { + ASSERT_IS_MAIN_OR_TEST_THREAD(); + if (namespaces_.empty()) { + return absl::nullopt; + } + + std::vector segments = absl::StrSplit(stat_name, '.'); + // Need at least scope.namespace.leaf for an inner segment to exist. + if (segments.size() < 3) { + return absl::nullopt; + } + + // Skip the leading scope segment and the trailing leaf. + for (size_t i = 1; i + 1 < segments.size(); ++i) { + if (registered(segments[i])) { + segments.erase(segments.begin() + i); + return absl::StrJoin(segments, "."); + } + } + return absl::nullopt; +} + } // namespace Stats } // namespace Envoy diff --git a/source/common/stats/custom_stat_namespaces_impl.h b/source/common/stats/custom_stat_namespaces_impl.h index 0f90b28d5ba43..0a204e963d4ae 100644 --- a/source/common/stats/custom_stat_namespaces_impl.h +++ b/source/common/stats/custom_stat_namespaces_impl.h @@ -16,6 +16,8 @@ class CustomStatNamespacesImpl : public CustomStatNamespaces { void registerStatNamespace(const absl::string_view name) override; absl::optional stripRegisteredPrefix(const absl::string_view stat_name) const override; + absl::optional + stripRegisteredInnerNamespace(absl::string_view stat_name) const override; private: absl::flat_hash_set namespaces_; diff --git a/source/server/admin/BUILD b/source/server/admin/BUILD index 2e6603cf6cee3..f7ba5d594ca48 100644 --- a/source/server/admin/BUILD +++ b/source/server/admin/BUILD @@ -224,6 +224,7 @@ envoy_cc_library( ":utils_lib", "//envoy/stats:custom_stat_namespaces_interface", "//source/common/buffer:buffer_lib", + "//source/common/runtime:runtime_lib", "//source/common/stats:histogram_lib", "//source/common/upstream:host_utility_lib", "@prometheus_metrics_model//:client_model_cc_proto", diff --git a/source/server/admin/prometheus_stats.cc b/source/server/admin/prometheus_stats.cc index 297a11f49d522..dd5f6d4ee9f87 100644 --- a/source/server/admin/prometheus_stats.cc +++ b/source/server/admin/prometheus_stats.cc @@ -8,6 +8,7 @@ #include "source/common/common/macros.h" #include "source/common/common/regex.h" #include "source/common/protobuf/protobuf.h" +#include "source/common/runtime/runtime_features.h" #include "source/common/stats/histogram_impl.h" #include "source/common/upstream/host_utility.h" @@ -956,6 +957,17 @@ PrometheusStatsFormatter::metricName(std::string&& extracted_name, return sanitized_name; } + // Inner-segment custom namespace (e.g. upstream Wasm under a cluster scope). + if (Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.strip_scoped_custom_stat_namespace")) { + absl::optional inner_namespace_stripped = + custom_namespaces.stripRegisteredInnerNamespace(extracted_name); + if (inner_namespace_stripped.has_value()) { + sanitizeNameInPlace(inner_namespace_stripped.value()); + return absl::StrCat("envoy_", inner_namespace_stripped.value()); + } + } + // If it does not have a custom namespace, add namespacing prefix to avoid conflicts, as per best // practice: https://prometheus.io/docs/practices/naming/#metric-names Also, naming conventions on // https://prometheus.io/docs/concepts/data_model/ diff --git a/source/server/admin/prometheus_stats.h b/source/server/admin/prometheus_stats.h index 1377511963fea..ec8482cc74e4f 100644 --- a/source/server/admin/prometheus_stats.h +++ b/source/server/admin/prometheus_stats.h @@ -126,10 +126,12 @@ class PrometheusStatsFormatter { const Http::RequestHeaderMap& headers); /** - * Format the given metric name, and prefixed with "envoy_" if it does not have a custom - * stat namespace. If it has a custom stat namespace AND the name without the custom namespace - * has a valid prometheus namespace, the trimmed name is returned. - * Otherwise, return nullopt. + * Format a tag-extracted name for Prometheus output: + * - leading registered custom namespace -> stripped, no "envoy_" prefix. + * - inner registered custom namespace -> stripped, "envoy_" prefix kept. + * Gated by "envoy.reloadable_features.strip_scoped_custom_stat_namespace". + * - otherwise -> "envoy_" prefix added. + * Returns nullopt if the result would not be a valid Prometheus name. */ static absl::optional metricName(std::string&& extracted_name, diff --git a/test/common/stats/custom_stat_namespaces_impl_test.cc b/test/common/stats/custom_stat_namespaces_impl_test.cc index e0500fe5edcf7..643806414ded2 100644 --- a/test/common/stats/custom_stat_namespaces_impl_test.cc +++ b/test/common/stats/custom_stat_namespaces_impl_test.cc @@ -32,5 +32,53 @@ TEST(CustomStatNamespacesImpl, StripRegisteredPrefix) { EXPECT_EQ(actual.value(), "my.extension.metric"); } +TEST(CustomStatNamespacesImpl, StripRegisteredInnerNamespace) { + CustomStatNamespacesImpl namespaces; + + // Empty registry. + EXPECT_FALSE(namespaces.stripRegisteredInnerNamespace("cluster.foo.metric").has_value()); + + namespaces.registerStatNamespace("wasmcustom"); + + // Depth 1. + { + const absl::optional actual = + namespaces.stripRegisteredInnerNamespace("cluster.wasmcustom.upstream_rq_2xx"); + EXPECT_TRUE(actual.has_value()); + EXPECT_EQ(actual.value(), "cluster.upstream_rq_2xx"); + } + + // Depth 2. + { + const absl::optional actual = + namespaces.stripRegisteredInnerNamespace("scope.subscope.wasmcustom.metric"); + EXPECT_TRUE(actual.has_value()); + EXPECT_EQ(actual.value(), "scope.subscope.metric"); + } + + // Multi-token leaf. + { + const absl::optional actual = + namespaces.stripRegisteredInnerNamespace("cluster.wasmcustom.foo.bar.baz"); + EXPECT_TRUE(actual.has_value()); + EXPECT_EQ(actual.value(), "cluster.foo.bar.baz"); + } + + // Leading match: caller should use stripRegisteredPrefix instead. + EXPECT_FALSE(namespaces.stripRegisteredInnerNamespace("wasmcustom.metric").has_value()); + + // Trailing match: that's the metric leaf, not a namespace boundary. + EXPECT_FALSE(namespaces.stripRegisteredInnerNamespace("cluster.foo.wasmcustom").has_value()); + + // No registered segment in the middle. + EXPECT_FALSE(namespaces.stripRegisteredInnerNamespace("cluster.foo.bar.metric").has_value()); + + // Substring within a segment doesn't count. + EXPECT_FALSE(namespaces.stripRegisteredInnerNamespace("cluster.wasmcustomx.metric").has_value()); + + // No dot at all. + EXPECT_FALSE(namespaces.stripRegisteredInnerNamespace("wasmcustom").has_value()); +} + } // namespace Stats } // namespace Envoy diff --git a/test/server/admin/BUILD b/test/server/admin/BUILD index 1322d6dbf0d1a..6f5ee59e4d151 100644 --- a/test/server/admin/BUILD +++ b/test/server/admin/BUILD @@ -210,6 +210,7 @@ envoy_cc_test( "//test/mocks/thread_local:thread_local_mocks", "//test/mocks/upstream:cluster_manager_mocks", "//test/test_common:stats_utility_lib", + "//test/test_common:test_runtime_lib", "//test/test_common:utility_lib", ], ) diff --git a/test/server/admin/prometheus_stats_test.cc b/test/server/admin/prometheus_stats_test.cc index 440c7f2155bff..31c3cf0f65e6b 100644 --- a/test/server/admin/prometheus_stats_test.cc +++ b/test/server/admin/prometheus_stats_test.cc @@ -18,6 +18,7 @@ #include "test/mocks/thread_local/mocks.h" #include "test/mocks/upstream/cluster_manager.h" #include "test/test_common/stats_utility.h" +#include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" using testing::NiceMock; @@ -179,6 +180,67 @@ TEST_F(PrometheusStatsFormatterTest, CustomNamespace) { EXPECT_EQ(expected, actual.value()); } +TEST_F(PrometheusStatsFormatterTest, ScopedCustomNamespace) { + Stats::CustomStatNamespacesImpl custom_namespaces; + custom_namespaces.registerStatNamespace("wasmcustom"); + std::string raw = "cluster.wasmcustom.upstream_rq_2xx"; + std::string expected = "envoy_cluster_upstream_rq_2xx"; + auto actual = PrometheusStatsFormatter::metricName(std::move(raw), custom_namespaces); + EXPECT_TRUE(actual.has_value()); + EXPECT_EQ(expected, actual.value()); +} + +TEST_F(PrometheusStatsFormatterTest, ScopedCustomNamespaceDeeperNesting) { + Stats::CustomStatNamespacesImpl custom_namespaces; + custom_namespaces.registerStatNamespace("wasmcustom"); + std::string raw = "scope.subscope.wasmcustom.metric"; + std::string expected = "envoy_scope_subscope_metric"; + auto actual = PrometheusStatsFormatter::metricName(std::move(raw), custom_namespaces); + EXPECT_TRUE(actual.has_value()); + EXPECT_EQ(expected, actual.value()); +} + +TEST_F(PrometheusStatsFormatterTest, ScopedCustomNamespaceRuntimeGuardDisabled) { + // Guard off: inner namespace is preserved verbatim. + TestScopedRuntime scoped_runtime; + scoped_runtime.mergeValues( + {{"envoy.reloadable_features.strip_scoped_custom_stat_namespace", "false"}}); + + Stats::CustomStatNamespacesImpl custom_namespaces; + custom_namespaces.registerStatNamespace("wasmcustom"); + std::string raw = "cluster.wasmcustom.upstream_rq_2xx"; + std::string expected = "envoy_cluster_wasmcustom_upstream_rq_2xx"; + auto actual = PrometheusStatsFormatter::metricName(std::move(raw), custom_namespaces); + EXPECT_TRUE(actual.has_value()); + EXPECT_EQ(expected, actual.value()); +} + +TEST_F(PrometheusStatsFormatterTest, LeadingCustomNamespaceUnaffectedByGuard) { + // Prefix stripping (used by listener Wasm) must keep working when the inner-segment guard is + // off. + TestScopedRuntime scoped_runtime; + scoped_runtime.mergeValues( + {{"envoy.reloadable_features.strip_scoped_custom_stat_namespace", "false"}}); + + Stats::CustomStatNamespacesImpl custom_namespaces; + custom_namespaces.registerStatNamespace("wasmcustom"); + std::string raw = "wasmcustom.user_metric"; + std::string expected = "user_metric"; + auto actual = PrometheusStatsFormatter::metricName(std::move(raw), custom_namespaces); + EXPECT_TRUE(actual.has_value()); + EXPECT_EQ(expected, actual.value()); +} + +TEST_F(PrometheusStatsFormatterTest, ScopedUnregisteredCustomNamespace) { + Stats::CustomStatNamespacesImpl custom_namespaces; + custom_namespaces.registerStatNamespace("promstattest"); + std::string raw = "cluster.wasmcustom.upstream_rq_2xx"; + std::string expected = "envoy_cluster_wasmcustom_upstream_rq_2xx"; + auto actual = PrometheusStatsFormatter::metricName(std::move(raw), custom_namespaces); + EXPECT_TRUE(actual.has_value()); + EXPECT_EQ(expected, actual.value()); +} + TEST_F(PrometheusStatsFormatterTest, CustomNamespaceWithInvalidPromnamespace) { Stats::CustomStatNamespacesImpl custom_namespaces; custom_namespaces.registerStatNamespace("promstattest"); @@ -202,6 +264,30 @@ TEST_F(PrometheusStatsFormatterTest, FormattedTags) { EXPECT_EQ(expected, actual); } +// End-to-end: cluster-scoped custom namespace counter renders with namespace stripped and +// cluster_name preserved as a label. +TEST_F(PrometheusStatsFormatterTest, ScopedCustomNamespaceFullPipeline) { + Stats::CustomStatNamespacesImpl custom_namespaces; + custom_namespaces.registerStatNamespace("wasmcustom"); + + addCounter("cluster.wasmcustom.upstream_rq_2xx", + {{makeStat("envoy.cluster_name"), makeStat("test_cluster_1")}}); + + Buffer::OwnedImpl response; + const uint64_t size = PrometheusStatsFormatter::statsAsPrometheusText( + counters_, gauges_, histograms_, textReadouts_, endpoints_helper_->cm_, response, + StatsParams(), custom_namespaces); + EXPECT_EQ(1UL, size); + + const std::string output = response.toString(); + EXPECT_NE(output.find("# TYPE envoy_cluster_upstream_rq_2xx counter"), std::string::npos) + << output; + EXPECT_NE(output.find("envoy_cluster_upstream_rq_2xx{envoy_cluster_name=\"test_cluster_1\"}"), + std::string::npos) + << output; + EXPECT_EQ(output.find("wasmcustom"), std::string::npos) << output; +} + TEST_F(PrometheusStatsFormatterTest, MetricNameCollison) { Stats::CustomStatNamespacesImpl custom_namespaces;