Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,17 @@ minor_behavior_changes:
Optimized prometheus stats endpoint. Users should see a roughly 30-40% latency improvement in calls to the endpoint
for cases where the scrape results in lots of cluster stats.
There should be no visible changes to users, or incompatibilities.
- area: stats
change: |
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``.
- area: tls
change: |
``SslSocket`` now reports ``ECONNRESET`` as ``ConnectionReset`` by reading the system error
Expand Down
11 changes: 11 additions & 0 deletions envoy/stats/custom_stat_namespaces.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#pragma once

#include <string>

#include "envoy/common/pure.h"

#include "absl/strings/string_view.h"
Expand Down Expand Up @@ -46,6 +48,15 @@ class CustomStatNamespaces {
*/
virtual absl::optional<absl::string_view>
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.<m>"). Returns
* std::string rather than string_view because the result spans both sides of the removed
* segment.
*/
virtual absl::optional<std::string>
stripRegisteredInnerNamespace(absl::string_view stat_name) const PURE;
};

} // namespace Stats
Expand Down
1 change: 1 addition & 0 deletions source/common/runtime/runtime_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,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);
Expand Down
29 changes: 29 additions & 0 deletions source/common/stats/custom_stat_namespaces_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#include "source/common/common/assert.h"
#include "source/common/common/thread.h"

#include "absl/strings/str_cat.h"

namespace Envoy {
namespace Stats {

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

absl::optional<std::string>
CustomStatNamespacesImpl::stripRegisteredInnerNamespace(absl::string_view stat_name) const {
ASSERT_IS_MAIN_OR_TEST_THREAD();
if (namespaces_.empty()) {
return absl::nullopt;
}

// Skip the leading scope segment, scan inner segments left-to-right, stop before the leaf.
size_t segment_start = stat_name.find('.');
if (segment_start == absl::string_view::npos) {
return absl::nullopt;
}
++segment_start;

while (segment_start < stat_name.size()) {
const size_t segment_end = stat_name.find('.', segment_start);
if (segment_end == absl::string_view::npos) {
return absl::nullopt;
}
if (registered(stat_name.substr(segment_start, segment_end - segment_start))) {
return absl::StrCat(stat_name.substr(0, segment_start), stat_name.substr(segment_end + 1));
}
segment_start = segment_end + 1;
}
return absl::nullopt;
}

} // namespace Stats
} // namespace Envoy
2 changes: 2 additions & 0 deletions source/common/stats/custom_stat_namespaces_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ class CustomStatNamespacesImpl : public CustomStatNamespaces {
void registerStatNamespace(const absl::string_view name) override;
absl::optional<absl::string_view>
stripRegisteredPrefix(const absl::string_view stat_name) const override;
absl::optional<std::string>
stripRegisteredInnerNamespace(absl::string_view stat_name) const override;

private:
absl::flat_hash_set<std::string> namespaces_;
Expand Down
1 change: 1 addition & 0 deletions source/server/admin/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
12 changes: 12 additions & 0 deletions source/server/admin/prometheus_stats.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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<std::string> 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/
Expand Down
10 changes: 6 additions & 4 deletions source/server/admin/prometheus_stats.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string>
metricName(std::string&& extracted_name,
Expand Down
48 changes: 48 additions & 0 deletions test/common/stats/custom_stat_namespaces_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string> 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<std::string> 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<std::string> 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
1 change: 1 addition & 0 deletions test/server/admin/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,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",
],
)
Expand Down
86 changes: 86 additions & 0 deletions test/server/admin/prometheus_stats_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand All @@ -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;

Expand Down
Loading