mongo_proxy: support configurable command list for metrics (#13494)

Signed-off-by: David Bartley <bartle@stripe.com>

api/envoy/extensions/filters/network/mongo_proxy/v3/mongo_proxy.proto

@@ -17,6 +17,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // MongoDB :ref:`configuration overview <config_network_filters_mongo_proxy>`.
 // [#extension: envoy.filters.network.mongo_proxy]
 
+// [#next-free-field: 6]
 message MongoProxy {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.config.filter.network.mongo_proxy.v2.MongoProxy";
@@ -39,4 +40,9 @@ message MongoProxy {
   // Flag to specify whether :ref:`dynamic metadata
   // <config_network_filters_mongo_proxy_dynamic_metadata>` should be emitted. Defaults to false.
   bool emit_dynamic_metadata = 4;
+
+  // List of commands to emit metrics for. Defaults to "delete", "insert", and "update".
+  // Note that metrics will not be emitted for "find" commands, since those are considered
+  // queries, and metrics for those are emitted under a dedicated "query" namespace.
+  repeated string commands = 5;
 }

docs/root/configuration/listeners/network_filters/mongo_proxy_filter.rst

@@ -95,6 +95,10 @@ namespace.
   reply_size, Histogram, Size of the reply in bytes
   reply_time_ms, Histogram, Command time in milliseconds
 
+The list of commands that these metrics are emitted for can be configured via the
+:ref:`configuration <envoy_v3_api_field_extensions.filters.network.mongo_proxy.v3.MongoProxy.commands>`;
+by default, metrics are emitted for *delete*, *insert*, and *update*.
+
 .. _config_network_filters_mongo_proxy_collection_stats:
 
 Per collection query statistics

docs/root/version_history/current.rst

@@ -26,6 +26,7 @@ New Features
 ------------
 * grpc: implemented header value syntax support when defining :ref:`initial metadata <envoy_v3_api_field_config.core.v3.GrpcService.initial_metadata>` for gRPC-based `ext_authz` :ref:`HTTP <envoy_v3_api_field_extensions.filters.http.ext_authz.v3.ExtAuthz.grpc_service>` and :ref:`network <envoy_v3_api_field_extensions.filters.network.ext_authz.v3.ExtAuthz.grpc_service>` filters, and :ref:`ratelimit <envoy_v3_api_field_config.ratelimit.v3.RateLimitServiceConfig.grpc_service>` filters.
 * health_check: added option to use :ref:`no_traffic_healthy_interval <envoy_v3_api_field_config.core.v3.HealthCheck.no_traffic_healthy_interval>` which allows a different no traffic interval when the host is healthy.
+* mongo_proxy: the list of commands to produce metrics for is now :ref:`configurable <envoy_v3_api_field_extensions.filters.network.mongo_proxy.v3.MongoProxy.commands>`.
 
 Deprecated
 ----------

source/extensions/filters/network/mongo_proxy/config.cc

@@ -34,7 +34,13 @@ Network::FilterFactoryCb MongoProxyFilterConfigFactory::createFilterFactoryFromP
     fault_config = std::make_shared<Filters::Common::Fault::FaultDelayConfig>(proto_config.delay());
   }
 
-  auto stats = std::make_shared<MongoStats>(context.scope(), stat_prefix);
+  auto commands = std::vector<std::string>{"delete", "insert", "update"};
+  if (proto_config.commands_size() > 0) {
+    commands =
+        std::vector<std::string>(proto_config.commands().begin(), proto_config.commands().end());
+  }
+
+  auto stats = std::make_shared<MongoStats>(context.scope(), stat_prefix, commands);
   const bool emit_dynamic_metadata = proto_config.emit_dynamic_metadata();
   return [stat_prefix, &context, access_log, fault_config, emit_dynamic_metadata,
           stats](Network::FilterManager& filter_manager) -> void {

source/extensions/filters/network/mongo_proxy/mongo_stats.cc

@@ -13,7 +13,8 @@ namespace Extensions {
 namespace NetworkFilters {
 namespace MongoProxy {
 
-MongoStats::MongoStats(Stats::Scope& scope, absl::string_view prefix)
+MongoStats::MongoStats(Stats::Scope& scope, absl::string_view prefix,
+                       const std::vector<std::string>& commands)
     : scope_(scope), stat_name_set_(scope.symbolTable().makeSet("Mongo")),
       prefix_(stat_name_set_->add(prefix)), callsite_(stat_name_set_->add("callsite")),
       cmd_(stat_name_set_->add("cmd")), collection_(stat_name_set_->add("collection")),
@@ -25,10 +26,9 @@ MongoStats::MongoStats(Stats::Scope& scope, absl::string_view prefix)
       scatter_get_(stat_name_set_->add("scatter_get")), total_(stat_name_set_->add("total")),
       unknown_command_(stat_name_set_->add("unknown_command")) {
 
-  // TODO(jmarantz): is this the right set of mongo commands to use as builtins?
-  // Should we also have builtins for callsites or collections, or do those need
-  // to be dynamic?
-  stat_name_set_->rememberBuiltins({"insert", "query", "update", "delete"});
+  for (const auto& cmd : commands) {
+    stat_name_set_->rememberBuiltin(cmd);
+  }
 }
 
 Stats::ElementVec MongoStats::addPrefix(const Stats::ElementVec& names) {

source/extensions/filters/network/mongo_proxy/mongo_stats.h

@@ -16,7 +16,8 @@ namespace MongoProxy {
 
 class MongoStats {
 public:
-  MongoStats(Stats::Scope& scope, absl::string_view prefix);
+  MongoStats(Stats::Scope& scope, absl::string_view prefix,
+             const std::vector<std::string>& commands);
 
   void incCounter(const Stats::ElementVec& names);
   void recordHistogram(const Stats::ElementVec& names, Stats::Histogram::Unit unit,

source/extensions/filters/network/mongo_proxy/utility.cc

@@ -22,6 +22,17 @@ QueryMessageInfo::QueryMessageInfo(const QueryMessage& query)
     if (command_ == "find") {
       command_ = "";
       parseFindCommand(*command);
+      // command aliases
+    } else if (command_ == "collstats") {
+      command_ = "collStats";
+    } else if (command_ == "dbstats") {
+      command_ = "dbStats";
+    } else if (command_ == "findandmodify") {
+      command_ = "findAndModify";
+    } else if (command_ == "getlasterror") {
+      command_ = "getLastError";
+    } else if (command_ == "ismaster") {
+      command_ = "isMaster";
     }
 
     return;

test/extensions/filters/network/mongo_proxy/config_test.cc

@@ -30,6 +30,9 @@ TEST(MongoFilterConfigTest, CorrectConfigurationNoFaults) {
   const std::string yaml_string = R"EOF(
   stat_prefix: my_stat_prefix
   access_log: path/to/access/log
+  commands:
+  - foo
+  - bar
   )EOF";
 
   envoy::extensions::filters::network::mongo_proxy::v3::MongoProxy proto_config;
@@ -47,6 +50,8 @@ TEST(MongoFilterConfigTest, ValidProtoConfigurationNoFaults) {
 
   config.set_access_log("path/to/access/log");
   config.set_stat_prefix("my_stat_prefix");
+  config.add_commands("foo");
+  config.add_commands("bar");
 
   NiceMock<Server::Configuration::MockFactoryContext> context;
   MongoProxyFilterConfigFactory factory;
@@ -64,6 +69,8 @@ TEST(MongoFilterConfigTest, MongoFilterWithEmptyProto) {
           factory.createEmptyConfigProto().get());
   config.set_access_log("path/to/access/log");
   config.set_stat_prefix("my_stat_prefix");
+  config.add_commands("foo");
+  config.add_commands("bar");
 
   Network::FilterFactoryCb cb = factory.createFilterFactoryFromProto(config, context);
   Network::MockConnection connection;

test/extensions/filters/network/mongo_proxy/proxy_test.cc

@@ -59,7 +59,11 @@ class TestProxyFilter : public ProxyFilter {
 
 class MongoProxyFilterTest : public testing::Test {
 public:
-  MongoProxyFilterTest() : mongo_stats_(std::make_shared<MongoStats>(store_, "test")) { setup(); }
+  MongoProxyFilterTest()
+      : mongo_stats_(std::make_shared<MongoStats>(store_, "test",
+                                                  std::vector<std::string>{"insert", "count"})) {
+    setup();
+  }
 
   void setup() {
     ON_CALL(runtime_.snapshot_, featureEnabled("mongo.proxy_enabled", 100))

test/extensions/filters/network/mongo_proxy/utility_test.cc

@@ -189,13 +189,18 @@ TEST(QueryMessageInfoTest, Command) {
     EXPECT_THROW((QueryMessageInfo(q)), EnvoyException);
   }
 
-  {
+  std::vector<std::pair<std::string, std::string>> test_cases = {
+      {"collstats", "collStats"},         {"dbstats", "dbStats"},
+      {"findandmodify", "findAndModify"}, {"getlasterror", "getLastError"},
+      {"ismaster", "isMaster"},
+  };
+  for (const auto& test : test_cases) {
     QueryMessageImpl q(0, 0);
     q.fullCollectionName("db.$cmd");
     q.query(Bson::DocumentImpl::create()->addDocument(
-        "$query", Bson::DocumentImpl::create()->addInt32("ismaster", 1)));
+        "$query", Bson::DocumentImpl::create()->addInt32(test.first, 1)));
     QueryMessageInfo info(q);
-    EXPECT_EQ("ismaster", info.command());
+    EXPECT_EQ(test.second, info.command());
   }
 }