accesslog: support stat-level AccessLogFilter in stats AccessLog

api/envoy/extensions/access_loggers/stats/v3/BUILD

@@ -6,6 +6,7 @@ licenses(["notice"])  # Apache 2
 
 api_proto_package(
     deps = [
+        "//envoy/config/accesslog/v3:pkg",
         "@com_github_cncf_xds//udpa/annotations:pkg",
         "@com_github_cncf_xds//xds/annotations/v3:pkg",
     ],

api/envoy/extensions/access_loggers/stats/v3/stats.proto

@@ -2,6 +2,8 @@ syntax = "proto3";
 
 package envoy.extensions.access_loggers.stats.v3;
 
+import "envoy/config/accesslog/v3/accesslog.proto";
+
 import "google/protobuf/wrappers.proto";
 
 import "xds/annotations/v3/status.proto";
@@ -39,13 +41,17 @@ message Config {
     string value_format = 2 [(validate.rules).string = {min_len: 1}];
   }
 
-  // Defines the name and tags of a stat.
+  // Defines the common settings of a stat.
   message Stat {
     // The name of the stat.
     string name = 1 [(validate.rules).string = {min_len: 1}];
 
     // The tags for the stat.
     repeated Tag tags = 2;
+
+    // Filter which is used to determine if stats should be emitted for
+    // particular log calls.
+    config.accesslog.v3.AccessLogFilter filter = 3;
   }
 
   // Configuration for a histogram stat.
@@ -60,8 +66,9 @@ message Config {
 
       Milliseconds = 3;
 
-      // Values are scaled to range 0-1.0, indicating 0%-100%. Values can be outside this range,
-      // but must be positive. Values extremely far out of this range may overflow.
+      // Values are scaled to range 0-1.0, indicating 0%-100%. Values can be
+      // outside this range, but must be positive. Values extremely far out of
+      // this range may overflow.
       Percent = 4;
     }
 
@@ -71,8 +78,9 @@ message Config {
     // The units for this histogram.
     Unit unit = 2 [(validate.rules).enum = {defined_only: true}];
 
-    // The format string for the value of this histogram, using :ref:`command operators <config_access_log_command_operators>`.
-    // This must evaluate to a positive number.
+    // The format string for the value of this histogram, using :ref:`command
+    // operators <config_access_log_command_operators>`. This must evaluate to a
+    // positive number.
     string value_format = 3 [(validate.rules).string = {min_len: 1 prefix: "%" suffix: "%"}];
   }
 
@@ -81,8 +89,9 @@ message Config {
     // The name and tags of this counter.
     Stat stat = 1 [(validate.rules).message = {required: true}];
 
-    // The format string for the value to add to this counter, using :ref:`command operators <config_access_log_command_operators>`.
-    // One of ``value_format`` or ``value_fixed`` must be configured.
+    // The format string for the value to add to this counter, using
+    // :ref:`command operators <config_access_log_command_operators>`. One of
+    // ``value_format`` or ``value_fixed`` must be configured.
     string value_format = 2
         [(validate.rules).string = {prefix: "%" suffix: "%" ignore_empty: true}];
 

source/extensions/access_loggers/stats/BUILD

@@ -19,6 +19,7 @@ envoy_cc_library(
         "//source/common/access_log:access_log_lib",
         "//source/common/formatter:substitution_format_string_lib",
         "//source/extensions/access_loggers/common:access_log_base",
+        "@envoy_api//envoy/data/accesslog/v3:pkg_cc_proto",
         "@envoy_api//envoy/extensions/access_loggers/stats/v3:pkg_cc_proto",
     ],
 )

source/extensions/access_loggers/stats/stats.cc

@@ -1,5 +1,7 @@
 #include "source/extensions/access_loggers/stats/stats.h"
 
+#include "envoy/data/accesslog/v3/accesslog.pb.h"
+
 #include "source/common/formatter/substitution_formatter.h"
 
 namespace Envoy {
@@ -45,12 +47,12 @@ StatsAccessLog::StatsAccessLog(const envoy::extensions::access_loggers::stats::v
                                Server::Configuration::GenericFactoryContext& context,
                                AccessLog::FilterPtr&& filter,
                                const std::vector<Formatter::CommandParserPtr>& commands)
-    : Common::ImplBase(std::move(filter)),
+    : AccessLoggers::Common::ImplBase(std::move(filter)),
       scope_(context.statsScope().createScope(config.stat_prefix(), true /* evictable */)),
       stat_name_pool_(scope_->symbolTable()), histograms_([&]() {
         std::vector<Histogram> histograms;
         for (const auto& hist_cfg : config.histograms()) {
-          histograms.emplace_back(NameAndTags(hist_cfg.stat(), stat_name_pool_, commands),
+          histograms.emplace_back(Common(hist_cfg.stat(), stat_name_pool_, commands, context),
                                   convertUnitEnum(hist_cfg.unit()),
                                   parseValueFormat(hist_cfg.value_format(), commands));
         }
@@ -60,7 +62,7 @@ StatsAccessLog::StatsAccessLog(const envoy::extensions::access_loggers::stats::v
         std::vector<Counter> counters;
         for (const auto& counter_cfg : config.counters()) {
           Counter& inserted = counters.emplace_back(
-              NameAndTags(counter_cfg.stat(), stat_name_pool_, commands), nullptr, 0);
+              Common(counter_cfg.stat(), stat_name_pool_, commands, context), nullptr, 0);
           if (!counter_cfg.value_format().empty() && counter_cfg.has_value_fixed()) {
             throw EnvoyException(
                 "Stats logger cannot have both `value_format` and `value_fixed` configured.");
@@ -78,13 +80,17 @@ StatsAccessLog::StatsAccessLog(const envoy::extensions::access_loggers::stats::v
         return counters;
       }()) {}
 
-StatsAccessLog::NameAndTags::NameAndTags(
+StatsAccessLog::Common::Common(
     const envoy::extensions::access_loggers::stats::v3::Config::Stat& cfg,
-    Stats::StatNamePool& pool, const std::vector<Formatter::CommandParserPtr>& commands) {
+    Stats::StatNamePool& pool, const std::vector<Formatter::CommandParserPtr>& commands,
+    Server::Configuration::GenericFactoryContext& context) {
   name_ = pool.add(cfg.name());
   for (const auto& tag_cfg : cfg.tags()) {
     dynamic_tags_.emplace_back(tag_cfg, pool, commands);
   }
+  if (cfg.has_filter()) {
+    filter_ = AccessLog::FilterFactory::fromProto(cfg.filter(), context);
+  }
 }
 
 StatsAccessLog::DynamicTag::DynamicTag(
@@ -96,9 +102,8 @@ StatsAccessLog::DynamicTag::DynamicTag(
           Formatter::FormatterPtr)) {}
 
 std::pair<Stats::StatNameTagVector, std::vector<Stats::StatNameDynamicStorage>>
-StatsAccessLog::NameAndTags::tags(const Formatter::Context& context,
-                                  const StreamInfo::StreamInfo& stream_info,
-                                  Stats::Scope& scope) const {
+StatsAccessLog::Common::tags(const Formatter::Context& context,
+                             const StreamInfo::StreamInfo& stream_info, Stats::Scope& scope) const {
   Stats::StatNameTagVector tags;
 
   std::vector<Stats::StatNameDynamicStorage> dynamic_storage;
@@ -149,6 +154,9 @@ void StatsAccessLog::emitLog(const Formatter::Context& context,
 void StatsAccessLog::emitLogConst(const Formatter::Context& context,
                                   const StreamInfo::StreamInfo& stream_info) const {
   for (const auto& histogram : histograms_) {
+    if (histogram.stat_.filter_ && !histogram.stat_.filter_->evaluate(context, stream_info)) {
+      continue;
+    }
     absl::optional<uint64_t> computed_value_opt =
         getFormatValue(*histogram.value_formatter_, context, stream_info,
                        histogram.unit_ == Stats::Histogram::Unit::Percent);
@@ -165,6 +173,9 @@ void StatsAccessLog::emitLogConst(const Formatter::Context& context,
   }
 
   for (const auto& counter : counters_) {
+    if (counter.stat_.filter_ && !counter.stat_.filter_->evaluate(context, stream_info)) {
+      continue;
+    }
     uint64_t value;
     if (counter.value_formatter_ != nullptr) {
       absl::optional<uint64_t> computed_value_opt =

source/extensions/access_loggers/stats/stats.h

@@ -39,28 +39,29 @@ class StatsAccessLog : public Common::ImplBase {
     Formatter::FormatterPtr value_formatter_;
   };
 
-  class NameAndTags {
+  class Common {
   public:
-    NameAndTags(const envoy::extensions::access_loggers::stats::v3::Config::Stat& cfg,
-                Stats::StatNamePool& pool,
-                const std::vector<Formatter::CommandParserPtr>& commands);
+    Common(const envoy::extensions::access_loggers::stats::v3::Config::Stat& cfg,
+           Stats::StatNamePool& pool, const std::vector<Formatter::CommandParserPtr>& commands,
+           Server::Configuration::GenericFactoryContext& context);
 
     std::pair<Stats::StatNameTagVector, std::vector<Stats::StatNameDynamicStorage>>
     tags(const Formatter::Context& context, const StreamInfo::StreamInfo& stream_info,
          Stats::Scope& scope) const;
 
     Stats::StatName name_;
     std::vector<DynamicTag> dynamic_tags_;
+    AccessLog::FilterPtr filter_;
   };
 
   struct Histogram {
-    NameAndTags stat_;
+    Common stat_;
     Stats::Histogram::Unit unit_;
     Formatter::FormatterProviderPtr value_formatter_;
   };
 
   struct Counter {
-    NameAndTags stat_;
+    Common stat_;
     Formatter::FormatterProviderPtr value_formatter_;
     uint64_t value_fixed_;
   };

test/extensions/access_loggers/stats/stats_test.cc

@@ -297,6 +297,40 @@ TEST_F(StatsAccessLoggerTest, EmptyTagFormatter) {
   logger_->log(formatter_context_, stream_info_);
 }
 
+TEST_F(StatsAccessLoggerTest, StatFilter) {
+  const std::string yaml = R"EOF(
+    stat_prefix: test_stat_prefix
+    counters:
+      - stat:
+          name: counter
+          filter:
+            status_code_filter:
+              comparison:
+                op: EQ
+                value:
+                  default_value: 200
+        value_fixed: 1
+)EOF";
+
+  initialize(yaml);
+
+  // Case 1: Filter matches (200)
+  EXPECT_CALL(stream_info_, responseCode())
+      .WillRepeatedly(testing::Return(absl::optional<uint32_t>{200}));
+  EXPECT_CALL(store_, counter(_));
+  EXPECT_CALL(store_.counter_, add(1));
+  logger_->log(formatter_context_, stream_info_);
+
+  // Case 2: Filter does not match (404)
+  testing::Mock::VerifyAndClearExpectations(&store_);
+  testing::Mock::VerifyAndClearExpectations(&store_.counter_);
+
+  EXPECT_CALL(stream_info_, responseCode())
+      .WillRepeatedly(testing::Return(absl::optional<uint32_t>{404}));
+  EXPECT_CALL(store_, counter(_)).Times(0);
+  logger_->log(formatter_context_, stream_info_);
+}
+
 } // namespace StatsAccessLog
 } // namespace AccessLoggers
 } // namespace Extensions