Skip to content

Commit 582c822

Browse files
committed
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 <zuoyueshang.zys@alibaba-inc.com>
1 parent 39cf01e commit 582c822

11 files changed

Lines changed: 205 additions & 4 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
The prometheus formatter now strips a registered custom stat namespace (e.g. ``wasmcustom``) when
2+
it appears as a non-leading, non-trailing segment of a tag-extracted stat name, so that custom
3+
metrics created on a non-root scope render with the standard ``envoy_`` prefix and without the
4+
internal namespace. For example an upstream Wasm counter that previously rendered as
5+
``envoy_cluster_wasmcustom_upstream_rq_2xx`` now renders as ``envoy_cluster_upstream_rq_2xx``,
6+
matching listener Wasm and native cluster metrics. The ``cluster_name`` and other extracted labels
7+
are preserved. This primarily fixes upstream HTTP filter Wasm metrics emitted under cluster scope.
8+
This behavioral change can be temporarily reverted by setting runtime guard
9+
``envoy.reloadable_features.strip_scoped_custom_stat_namespace`` to ``false``.

envoy/stats/custom_stat_namespaces.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#pragma once
22

3+
#include <string>
4+
35
#include "envoy/common/pure.h"
46

57
#include "absl/strings/string_view.h"
@@ -46,6 +48,15 @@ class CustomStatNamespaces {
4648
*/
4749
virtual absl::optional<absl::string_view>
4850
stripRegisteredPrefix(const absl::string_view stat_name) const PURE;
51+
52+
/**
53+
* Like stripRegisteredPrefix, but for a registered namespace appearing as a non-leading,
54+
* non-trailing segment of the dotted stat name (e.g. "cluster.wasmcustom.<m>"). Returns
55+
* std::string rather than string_view because the result spans both sides of the removed
56+
* segment.
57+
*/
58+
virtual absl::optional<std::string>
59+
stripRegisteredInnerNamespace(absl::string_view stat_name) const PURE;
4960
};
5061

5162
} // namespace Stats

source/common/runtime/runtime_features.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ RUNTIME_GUARD(envoy_reloadable_features_skip_dns_lookup_for_proxied_requests);
106106
RUNTIME_GUARD(envoy_reloadable_features_skip_pending_overflow_count_on_active_rq);
107107
RUNTIME_GUARD(envoy_reloadable_features_ssl_socket_report_connection_reset);
108108
RUNTIME_GUARD(envoy_reloadable_features_strict_stats_matcher_unpacked);
109+
RUNTIME_GUARD(envoy_reloadable_features_strip_scoped_custom_stat_namespace);
109110
RUNTIME_GUARD(envoy_reloadable_features_tcp_proxy_odcds_over_ads_fix);
110111
RUNTIME_GUARD(envoy_reloadable_features_test_feature_true);
111112
RUNTIME_GUARD(envoy_reloadable_features_tls_certificate_compression_brotli);

source/common/stats/custom_stat_namespaces_impl.cc

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
#include "source/common/stats/custom_stat_namespaces_impl.h"
22

3+
#include <vector>
4+
35
#include "source/common/common/assert.h"
46
#include "source/common/common/thread.h"
57

8+
#include "absl/strings/str_join.h"
9+
#include "absl/strings/str_split.h"
10+
611
namespace Envoy {
712
namespace Stats {
813

@@ -29,5 +34,28 @@ CustomStatNamespacesImpl::stripRegisteredPrefix(const absl::string_view stat_nam
2934
return absl::nullopt;
3035
};
3136

37+
absl::optional<std::string>
38+
CustomStatNamespacesImpl::stripRegisteredInnerNamespace(absl::string_view stat_name) const {
39+
ASSERT_IS_MAIN_OR_TEST_THREAD();
40+
if (namespaces_.empty()) {
41+
return absl::nullopt;
42+
}
43+
44+
std::vector<absl::string_view> segments = absl::StrSplit(stat_name, '.');
45+
// Need at least scope.namespace.leaf for an inner segment to exist.
46+
if (segments.size() < 3) {
47+
return absl::nullopt;
48+
}
49+
50+
// Skip the leading scope segment and the trailing leaf.
51+
for (size_t i = 1; i + 1 < segments.size(); ++i) {
52+
if (registered(segments[i])) {
53+
segments.erase(segments.begin() + i);
54+
return absl::StrJoin(segments, ".");
55+
}
56+
}
57+
return absl::nullopt;
58+
}
59+
3260
} // namespace Stats
3361
} // namespace Envoy

source/common/stats/custom_stat_namespaces_impl.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ class CustomStatNamespacesImpl : public CustomStatNamespaces {
1616
void registerStatNamespace(const absl::string_view name) override;
1717
absl::optional<absl::string_view>
1818
stripRegisteredPrefix(const absl::string_view stat_name) const override;
19+
absl::optional<std::string>
20+
stripRegisteredInnerNamespace(absl::string_view stat_name) const override;
1921

2022
private:
2123
absl::flat_hash_set<std::string> namespaces_;

source/server/admin/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ envoy_cc_library(
224224
":utils_lib",
225225
"//envoy/stats:custom_stat_namespaces_interface",
226226
"//source/common/buffer:buffer_lib",
227+
"//source/common/runtime:runtime_lib",
227228
"//source/common/stats:histogram_lib",
228229
"//source/common/upstream:host_utility_lib",
229230
"@prometheus_metrics_model//:client_model_cc_proto",

source/server/admin/prometheus_stats.cc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include "source/common/common/macros.h"
99
#include "source/common/common/regex.h"
1010
#include "source/common/protobuf/protobuf.h"
11+
#include "source/common/runtime/runtime_features.h"
1112
#include "source/common/stats/histogram_impl.h"
1213
#include "source/common/upstream/host_utility.h"
1314

@@ -956,6 +957,17 @@ PrometheusStatsFormatter::metricName(std::string&& extracted_name,
956957
return sanitized_name;
957958
}
958959

960+
// Inner-segment custom namespace (e.g. upstream Wasm under a cluster scope).
961+
if (Runtime::runtimeFeatureEnabled(
962+
"envoy.reloadable_features.strip_scoped_custom_stat_namespace")) {
963+
absl::optional<std::string> inner_namespace_stripped =
964+
custom_namespaces.stripRegisteredInnerNamespace(extracted_name);
965+
if (inner_namespace_stripped.has_value()) {
966+
sanitizeNameInPlace(inner_namespace_stripped.value());
967+
return absl::StrCat("envoy_", inner_namespace_stripped.value());
968+
}
969+
}
970+
959971
// If it does not have a custom namespace, add namespacing prefix to avoid conflicts, as per best
960972
// practice: https://prometheus.io/docs/practices/naming/#metric-names Also, naming conventions on
961973
// https://prometheus.io/docs/concepts/data_model/

source/server/admin/prometheus_stats.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,12 @@ class PrometheusStatsFormatter {
126126
const Http::RequestHeaderMap& headers);
127127

128128
/**
129-
* Format the given metric name, and prefixed with "envoy_" if it does not have a custom
130-
* stat namespace. If it has a custom stat namespace AND the name without the custom namespace
131-
* has a valid prometheus namespace, the trimmed name is returned.
132-
* Otherwise, return nullopt.
129+
* Format a tag-extracted name for Prometheus output:
130+
* - leading registered custom namespace -> stripped, no "envoy_" prefix.
131+
* - inner registered custom namespace -> stripped, "envoy_" prefix kept.
132+
* Gated by "envoy.reloadable_features.strip_scoped_custom_stat_namespace".
133+
* - otherwise -> "envoy_" prefix added.
134+
* Returns nullopt if the result would not be a valid Prometheus name.
133135
*/
134136
static absl::optional<std::string>
135137
metricName(std::string&& extracted_name,

test/common/stats/custom_stat_namespaces_impl_test.cc

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,5 +32,53 @@ TEST(CustomStatNamespacesImpl, StripRegisteredPrefix) {
3232
EXPECT_EQ(actual.value(), "my.extension.metric");
3333
}
3434

35+
TEST(CustomStatNamespacesImpl, StripRegisteredInnerNamespace) {
36+
CustomStatNamespacesImpl namespaces;
37+
38+
// Empty registry.
39+
EXPECT_FALSE(namespaces.stripRegisteredInnerNamespace("cluster.foo.metric").has_value());
40+
41+
namespaces.registerStatNamespace("wasmcustom");
42+
43+
// Depth 1.
44+
{
45+
const absl::optional<std::string> actual =
46+
namespaces.stripRegisteredInnerNamespace("cluster.wasmcustom.upstream_rq_2xx");
47+
EXPECT_TRUE(actual.has_value());
48+
EXPECT_EQ(actual.value(), "cluster.upstream_rq_2xx");
49+
}
50+
51+
// Depth 2.
52+
{
53+
const absl::optional<std::string> actual =
54+
namespaces.stripRegisteredInnerNamespace("scope.subscope.wasmcustom.metric");
55+
EXPECT_TRUE(actual.has_value());
56+
EXPECT_EQ(actual.value(), "scope.subscope.metric");
57+
}
58+
59+
// Multi-token leaf.
60+
{
61+
const absl::optional<std::string> actual =
62+
namespaces.stripRegisteredInnerNamespace("cluster.wasmcustom.foo.bar.baz");
63+
EXPECT_TRUE(actual.has_value());
64+
EXPECT_EQ(actual.value(), "cluster.foo.bar.baz");
65+
}
66+
67+
// Leading match: caller should use stripRegisteredPrefix instead.
68+
EXPECT_FALSE(namespaces.stripRegisteredInnerNamespace("wasmcustom.metric").has_value());
69+
70+
// Trailing match: that's the metric leaf, not a namespace boundary.
71+
EXPECT_FALSE(namespaces.stripRegisteredInnerNamespace("cluster.foo.wasmcustom").has_value());
72+
73+
// No registered segment in the middle.
74+
EXPECT_FALSE(namespaces.stripRegisteredInnerNamespace("cluster.foo.bar.metric").has_value());
75+
76+
// Substring within a segment doesn't count.
77+
EXPECT_FALSE(namespaces.stripRegisteredInnerNamespace("cluster.wasmcustomx.metric").has_value());
78+
79+
// No dot at all.
80+
EXPECT_FALSE(namespaces.stripRegisteredInnerNamespace("wasmcustom").has_value());
81+
}
82+
3583
} // namespace Stats
3684
} // namespace Envoy

test/server/admin/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ envoy_cc_test(
210210
"//test/mocks/thread_local:thread_local_mocks",
211211
"//test/mocks/upstream:cluster_manager_mocks",
212212
"//test/test_common:stats_utility_lib",
213+
"//test/test_common:test_runtime_lib",
213214
"//test/test_common:utility_lib",
214215
],
215216
)

0 commit comments

Comments
 (0)