Skip to content

Commit e2cb747

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 8569caf commit e2cb747

11 files changed

Lines changed: 208 additions & 4 deletions

File tree

changelogs/current.yaml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,17 @@ minor_behavior_changes:
3434
Optimized prometheus stats endpoint. Users should see a roughly 30-40% latency improvement in calls to the endpoint
3535
for cases where the scrape results in lots of cluster stats.
3636
There should be no visible changes to users, or incompatibilities.
37+
- area: stats
38+
change: |
39+
The prometheus formatter now strips a registered custom stat namespace (e.g. ``wasmcustom``) when
40+
it appears as a non-leading, non-trailing segment of a tag-extracted stat name, so that custom
41+
metrics created on a non-root scope render with the standard ``envoy_`` prefix and without the
42+
internal namespace. For example an upstream Wasm counter that previously rendered as
43+
``envoy_cluster_wasmcustom_upstream_rq_2xx`` now renders as ``envoy_cluster_upstream_rq_2xx``,
44+
matching listener Wasm and native cluster metrics. The cluster_name (and other extracted) labels
45+
are preserved. This primarily fixes upstream HTTP filter Wasm metrics emitted under cluster
46+
scope. This behavioral change can be temporarily reverted by setting runtime guard
47+
``envoy.reloadable_features.strip_scoped_custom_stat_namespace`` to ``false``.
3748
- area: tls
3849
change: |
3950
``SslSocket`` now reports ``ECONNRESET`` as ``ConnectionReset`` by reading the system error

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
@@ -102,6 +102,7 @@ RUNTIME_GUARD(envoy_reloadable_features_skip_dns_lookup_for_proxied_requests);
102102
RUNTIME_GUARD(envoy_reloadable_features_skip_pending_overflow_count_on_active_rq);
103103
RUNTIME_GUARD(envoy_reloadable_features_ssl_socket_report_connection_reset);
104104
RUNTIME_GUARD(envoy_reloadable_features_strict_stats_matcher_unpacked);
105+
RUNTIME_GUARD(envoy_reloadable_features_strip_scoped_custom_stat_namespace);
105106
RUNTIME_GUARD(envoy_reloadable_features_tcp_proxy_odcds_over_ads_fix);
106107
RUNTIME_GUARD(envoy_reloadable_features_test_feature_true);
107108
RUNTIME_GUARD(envoy_reloadable_features_tls_certificate_compression_brotli);

source/common/stats/custom_stat_namespaces_impl.cc

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
#include "source/common/common/assert.h"
44
#include "source/common/common/thread.h"
55

6+
#include "absl/strings/str_cat.h"
7+
68
namespace Envoy {
79
namespace Stats {
810

@@ -29,5 +31,32 @@ CustomStatNamespacesImpl::stripRegisteredPrefix(const absl::string_view stat_nam
2931
return absl::nullopt;
3032
};
3133

34+
absl::optional<std::string>
35+
CustomStatNamespacesImpl::stripRegisteredInnerNamespace(absl::string_view stat_name) const {
36+
ASSERT_IS_MAIN_OR_TEST_THREAD();
37+
if (namespaces_.empty()) {
38+
return absl::nullopt;
39+
}
40+
41+
// Skip the leading scope segment, scan inner segments left-to-right, stop before the leaf.
42+
size_t segment_start = stat_name.find('.');
43+
if (segment_start == absl::string_view::npos) {
44+
return absl::nullopt;
45+
}
46+
++segment_start;
47+
48+
while (segment_start < stat_name.size()) {
49+
const size_t segment_end = stat_name.find('.', segment_start);
50+
if (segment_end == absl::string_view::npos) {
51+
return absl::nullopt;
52+
}
53+
if (registered(stat_name.substr(segment_start, segment_end - segment_start))) {
54+
return absl::StrCat(stat_name.substr(0, segment_start), stat_name.substr(segment_end + 1));
55+
}
56+
segment_start = segment_end + 1;
57+
}
58+
return absl::nullopt;
59+
}
60+
3261
} // namespace Stats
3362
} // 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
@@ -207,6 +207,7 @@ envoy_cc_test(
207207
"//test/mocks/thread_local:thread_local_mocks",
208208
"//test/mocks/upstream:cluster_manager_mocks",
209209
"//test/test_common:stats_utility_lib",
210+
"//test/test_common:test_runtime_lib",
210211
"//test/test_common:utility_lib",
211212
],
212213
)

0 commit comments

Comments
 (0)