proxy_protocol_filter: Add field stat_prefix to the filter configur…

…ation (envoyproxy#34414)

Commit Message:
proxy_protocol_filter: Add field stat_prefix to the filter configuration

Additional Description:
This field allows for differentiating statistics when multiple proxy protocol listener filters are configured.

This PR is a follow-up from previous conversation: envoyproxy#32861 (comment)

Risk Level: Low

All client-facing behavior changes are guarded by new filter config field.
Testing:

Stats unit tests
Proxy protocol listener filter integration tests
Docs Changes:
Done

Release Notes:
Done

Platform Specific Features:
None

Signed-off-by: Teju Nareddy <tnareddy@confluent.io>

api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto

@@ -18,6 +18,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // PROXY protocol listener filter.
 // [#extension: envoy.filters.listener.proxy_protocol]
 
+// [#next-free-field: 6]
 message ProxyProtocol {
  option (udpa.annotations.versioning).previous_message_type =
  "envoy.config.filter.listener.proxy_protocol.v2.ProxyProtocol";
@@ -85,4 +86,10 @@ message ProxyProtocol {
  // and an incoming request matches the V2 signature, the filter will allow the request through without any modification.
  // The filter treats this request as if it did not have any PROXY protocol information.
  repeated config.core.v3.ProxyProtocolConfig.Version disallowed_versions = 4;
+
+ // The human readable prefix to use when emitting statistics for the filter.
+ // If not configured, statistics will be emitted without the prefix segment.
+ // See the :ref:`filter's statistics documentation <config_listener_filters_proxy_protocol>` for
+ // more information.
+ string stat_prefix = 5;
 }

docs/root/configuration/listeners/listener_filters/proxy_protocol.rst

@@ -33,7 +33,7 @@ If there is a protocol error or an unsupported address family
 Statistics
 ----------
 
-This filter emits the following general statistics, rooted at *downstream_proxy_proto*
+This filter emits the following general statistics, rooted at *proxy_proto.[<stat_prefix>.]*
 
 .. csv-table::
  :header: Name, Type, Description
@@ -42,7 +42,7 @@ This filter emits the following general statistics, rooted at *downstream_proxy_
  not_found_disallowed, Counter, "Total number of connections that don't contain the PROXY protocol header and are rejected."
  not_found_allowed, Counter, "Total number of connections that don't contain the PROXY protocol header, but are allowed due to :ref:`allow_requests_without_proxy_protocol <envoy_v3_api_field_extensions.filters.listener.proxy_protocol.v3.ProxyProtocol.allow_requests_without_proxy_protocol>`."
 
-The filter also emits the statistics rooted at *downstream_proxy_proto.versions.<version>*
+The filter also emits the statistics rooted at *proxy_proto.[<stat_prefix>.]versions.<version>*
 for each matched PROXY protocol version. Proxy protocol versions include ``v1`` and ``v2``.
 
 .. csv-table::
@@ -53,7 +53,7 @@ for each matched PROXY protocol version. Proxy protocol versions include ``v1``
  disallowed, Counter, "Total number of ``found`` connections that are rejected due to :ref:`disallowed_versions <envoy_v3_api_field_extensions.filters.listener.proxy_protocol.v3.ProxyProtocol.disallowed_versions>`."
  error, Counter, "Total number of connections where the PROXY protocol header was malformed (and the connection was rejected)."
 
-The filter also emits the following legacy statistics, rooted at its own scope:
+The filter also emits the following legacy statistics, rooted at its own scope and **not** including the *stat_prefix*:
 
 .. csv-table::
  :header: Name, Type, Description

source/common/config/well_known_names.cc

@@ -27,7 +27,8 @@ std::string expandRegex(const std::string& regex) {
  // dashes with underscores.
  {"<ROUTE_CONFIG_NAME>", R"([\w-\.]+)"},
  // Match a prefix that is either a listener plus name or cluster plus name
- {"<LISTENER_OR_CLUSTER_WITH_NAME>", R"((?:listener|cluster)\..*?)"}});
+ {"<LISTENER_OR_CLUSTER_WITH_NAME>", R"((?:listener|cluster)\..*?)"},
+ {"<PROXY_PROTOCOL_VERSION>", R"(\d)"}});
 }
 
 const Regex::CompiledGoogleReMatcher& validTagValueRegex() {
@@ -199,8 +200,15 @@ TagNameValues::TagNameValues() {
  // listener_local_rate_limit.(<stat_prefix>.)
  addTokenized(LOCAL_LISTENER_RATELIMIT_PREFIX, "listener_local_ratelimit.$.**");
 
- // proxy_proto.(versions.v<version_number>.)**
- addRe2(PROXY_PROTOCOL_VERSION, R"(^proxy_proto\.(versions\.v(\d)\.)\w+)", "proxy_proto.versions");
+ // proxy_proto.(<stat_prefix>.)**
+ addRe2(PROXY_PROTOCOL_PREFIX, R"(^proxy_proto\.((<TAG_VALUE>)\.).+$)", "", "versions");
+
+ // proxy_proto.([<optional stat_prefix>.]versions.v(<version_number>).)(found|disallowed|error)
+ //
+ // Strips out: [<optional stat_prefix>.]versions.v(<version_number>).
+ // Leaving: proxy_proto.(found|disallowed|error)
+ addRe2(PROXY_PROTOCOL_VERSION,
+ R"(^proxy_proto\.((?:<TAG_VALUE>\.)?versions\.v(<PROXY_PROTOCOL_VERSION>)\.)\w+$)");
 }
 
 void TagNameValues::addRe2(const std::string& name, const std::string& regex,

source/common/config/well_known_names.h

@@ -155,6 +155,8 @@ class TagNameValues {
  const std::string REDIS_PREFIX = "envoy.redis_prefix";
  // Proxy Protocol version for a connection (Proxy Protocol listener filter).
  const std::string PROXY_PROTOCOL_VERSION = "envoy.proxy_protocol_version";
+ // Stats prefix for the proxy protocol listener filter.
+ const std::string PROXY_PROTOCOL_PREFIX = "envoy.proxy_protocol_prefix";
 
  // Mapping from the names above to their respective regex strings.
  const std::vector<std::pair<std::string, std::string>> name_regex_pairs_;

source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc

@@ -53,17 +53,22 @@ namespace ProxyProtocol {
 constexpr absl::string_view kProxyProtoStatsPrefix = "proxy_proto.";
 constexpr absl::string_view kVersionStatsPrefix = "versions.";
 
-ProxyProtocolStats ProxyProtocolStats::create(Stats::Scope& scope) {
+ProxyProtocolStats ProxyProtocolStats::create(Stats::Scope& scope, absl::string_view stat_prefix) {
+ std::string filter_stat_prefix = std::string(kProxyProtoStatsPrefix);
+ if (!stat_prefix.empty()) {
+ filter_stat_prefix = absl::StrCat(kProxyProtoStatsPrefix, stat_prefix, ".");
+ }
+
  return {
  /*legacy_=*/{LEGACY_PROXY_PROTOCOL_STATS(POOL_COUNTER(scope))},
  /*general_=*/
- {GENERAL_PROXY_PROTOCOL_STATS(POOL_COUNTER_PREFIX(scope, kProxyProtoStatsPrefix))},
+ {GENERAL_PROXY_PROTOCOL_STATS(POOL_COUNTER_PREFIX(scope, filter_stat_prefix))},
  /*v1_=*/
  {VERSIONED_PROXY_PROTOCOL_STATS(POOL_COUNTER_PREFIX(
- scope, absl::StrCat(kProxyProtoStatsPrefix, kVersionStatsPrefix, "v1.")))},
+ scope, absl::StrCat(filter_stat_prefix, kVersionStatsPrefix, "v1.")))},
  /*v2_=*/
  {VERSIONED_PROXY_PROTOCOL_STATS(POOL_COUNTER_PREFIX(
- scope, absl::StrCat(kProxyProtoStatsPrefix, kVersionStatsPrefix, "v2.")))},
+ scope, absl::StrCat(filter_stat_prefix, kVersionStatsPrefix, "v2.")))},
  };
 }
 
@@ -103,7 +108,7 @@ void VersionedProxyProtocolStats::increment(ReadOrParseState decision) {
 Config::Config(
  Stats::Scope& scope,
  const envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol& proto_config)
- : stats_(ProxyProtocolStats::create(scope)),
+ : stats_(ProxyProtocolStats::create(scope, proto_config.stat_prefix())),
  allow_requests_without_proxy_protocol_(proto_config.allow_requests_without_proxy_protocol()),
  pass_all_tlvs_(proto_config.has_pass_through_tlvs()
  ? proto_config.pass_through_tlvs().match_type() ==

source/extensions/filters/listener/proxy_protocol/proxy_protocol.h

@@ -96,7 +96,7 @@ struct ProxyProtocolStats {
  * For backwards compatibility, the legacy stats are rooted under their own scope.
  * The general and versioned stats are correctly rooted at this filter's own scope.
  */
- static ProxyProtocolStats create(Stats::Scope& scope);
+ static ProxyProtocolStats create(Stats::Scope& scope, absl::string_view stat_prefix);
 };
 
 /**

test/common/stats/tag_extractor_impl_test.cc

@@ -431,12 +431,23 @@ TEST(TagExtractorTest, DefaultTagExtractors) {
  redis_prefix.value_ = "my_redis_prefix";
  regex_tester.testRegex("redis.my_redis_prefix.response", "redis.response", {redis_prefix});
 
+ // Proxy Protocol stat prefix
+ Tag proxy_protocol_prefix;
+ proxy_protocol_prefix.name_ = tag_names.PROXY_PROTOCOL_PREFIX;
+ proxy_protocol_prefix.value_ = "test_stat_prefix";
+ regex_tester.testRegex("proxy_proto.not_found_disallowed", "proxy_proto.not_found_disallowed",
+ {});
+ regex_tester.testRegex("proxy_proto.test_stat_prefix.not_found_disallowed",
+ "proxy_proto.not_found_disallowed", {proxy_protocol_prefix});
+
  // Proxy Protocol version prefix
  Tag proxy_protocol_version;
  proxy_protocol_version.name_ = tag_names.PROXY_PROTOCOL_VERSION;
  proxy_protocol_version.value_ = "2";
  regex_tester.testRegex("proxy_proto.versions.v2.error", "proxy_proto.error",
  {proxy_protocol_version});
+ regex_tester.testRegex("proxy_proto.test_stat_prefix.versions.v2.error", "proxy_proto.error",
+ {proxy_protocol_prefix, proxy_protocol_version});
 }
 
 TEST(TagExtractorTest, ExtAuthzTagExtractors) {

test/extensions/filters/listener/proxy_protocol/proxy_proto_integration_test.cc

@@ -115,8 +115,9 @@ TEST_P(ProxyProtoIntegrationTest, V2RouterRequestAndResponseWithBodyNoBufferV6)
 
  testRouterRequestAndResponseWithBody(1024, 512, false, false, &creator);
 
- // Verify stats (with tags for proxy protocol version).
+ // Verify stats (with tags for proxy protocol version, but no stat prefix).
  const auto found_counter = test_server_->counter("proxy_proto.versions.v2.found");
+ ASSERT_NE(found_counter.get(), nullptr);
  EXPECT_EQ(found_counter->value(), 1UL);
  EXPECT_EQ(found_counter->tagExtractedName(), "proxy_proto.found");
  EXPECT_THAT(found_counter->tags(), IsSupersetOf(Stats::TagVector{
@@ -377,6 +378,7 @@ ProxyProtoDisallowedVersionsIntegrationTest::ProxyProtoDisallowedVersionsIntegra
  // V1 is disallowed.
  ::envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proxy_protocol;
  proxy_protocol.add_disallowed_versions(::envoy::config::core::v3::ProxyProtocolConfig::V1);
+ proxy_protocol.set_stat_prefix("test_stat_prefix");
 
  auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0);
  auto* ppv_filter = listener->mutable_listener_filters(0);
@@ -399,19 +401,25 @@ TEST_P(ProxyProtoDisallowedVersionsIntegrationTest, V1Disallowed) {
  /*end_stream=*/false, /*verify=*/false));
  tcp_client->waitForDisconnect();
 
- // Verify stats (with tags for proxy protocol version).
- const auto found_counter = test_server_->counter("proxy_proto.versions.v1.found");
+ // Verify stats (with tags for proxy protocol version and stat prefix).
+ const auto found_counter =
+ test_server_->counter("proxy_proto.test_stat_prefix.versions.v1.found");
+ ASSERT_NE(found_counter.get(), nullptr);
  EXPECT_EQ(found_counter->value(), 1UL);
  EXPECT_EQ(found_counter->tagExtractedName(), "proxy_proto.found");
  EXPECT_THAT(found_counter->tags(), IsSupersetOf(Stats::TagVector{
  {"envoy.proxy_protocol_version", "1"},
+ {"envoy.proxy_protocol_prefix", "test_stat_prefix"},
  }));
 
- const auto disallowed_counter = test_server_->counter("proxy_proto.versions.v1.disallowed");
+ const auto disallowed_counter =
+ test_server_->counter("proxy_proto.test_stat_prefix.versions.v1.disallowed");
+ ASSERT_NE(disallowed_counter.get(), nullptr);
  EXPECT_EQ(disallowed_counter->value(), 1UL);
  EXPECT_EQ(disallowed_counter->tagExtractedName(), "proxy_proto.disallowed");
  EXPECT_THAT(disallowed_counter->tags(), IsSupersetOf(Stats::TagVector{
  {"envoy.proxy_protocol_version", "1"},
+ {"envoy.proxy_protocol_prefix", "test_stat_prefix"},
  }));
 }
 
@@ -429,12 +437,15 @@ TEST_P(ProxyProtoDisallowedVersionsIntegrationTest, V2Error) {
  ASSERT_TRUE(tcp_client->write(buf.toString(), /*end_stream=*/false, /*verify=*/false));
  tcp_client->waitForDisconnect();
 
- // Verify stats (with tags for proxy protocol version).
- const auto found_counter = test_server_->counter("proxy_proto.versions.v2.error");
+ // Verify stats (with tags for proxy protocol version and stat prefix).
+ const auto found_counter =
+ test_server_->counter("proxy_proto.test_stat_prefix.versions.v2.error");
+ ASSERT_NE(found_counter.get(), nullptr);
  EXPECT_EQ(found_counter->value(), 1UL);
  EXPECT_EQ(found_counter->tagExtractedName(), "proxy_proto.error");
  EXPECT_THAT(found_counter->tags(), IsSupersetOf(Stats::TagVector{
  {"envoy.proxy_protocol_version", "2"},
+ {"envoy.proxy_protocol_prefix", "test_stat_prefix"},
  }));
 }