From 78578736a7d6a17cd1da3312a38e6c229e7f6523 Mon Sep 17 00:00:00 2001 From: David Bartley Date: Mon, 12 Oct 2020 18:22:50 -0700 Subject: [PATCH] mongo_proxy: support configurable command list for metrics (#13494) Signed-off-by: David Bartley --- .../filters/network/mongo_proxy/v3/mongo_proxy.proto | 6 ++++++ .../listeners/network_filters/mongo_proxy_filter.rst | 4 ++++ docs/root/version_history/current.rst | 1 + .../filters/network/mongo_proxy/v3/mongo_proxy.proto | 6 ++++++ .../extensions/filters/network/mongo_proxy/config.cc | 8 +++++++- .../filters/network/mongo_proxy/mongo_stats.cc | 10 +++++----- .../filters/network/mongo_proxy/mongo_stats.h | 3 ++- .../extensions/filters/network/mongo_proxy/utility.cc | 11 +++++++++++ .../filters/network/mongo_proxy/config_test.cc | 7 +++++++ .../filters/network/mongo_proxy/proxy_test.cc | 6 +++++- .../filters/network/mongo_proxy/utility_test.cc | 11 ++++++++--- 11 files changed, 62 insertions(+), 11 deletions(-) diff --git a/api/envoy/extensions/filters/network/mongo_proxy/v3/mongo_proxy.proto b/api/envoy/extensions/filters/network/mongo_proxy/v3/mongo_proxy.proto index 93abc90bdc24..ebdfb6f2fcc0 100644 --- a/api/envoy/extensions/filters/network/mongo_proxy/v3/mongo_proxy.proto +++ b/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 `. // [#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 // ` 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; } diff --git a/docs/root/configuration/listeners/network_filters/mongo_proxy_filter.rst b/docs/root/configuration/listeners/network_filters/mongo_proxy_filter.rst index 8c5a451ba4ce..ee0896985338 100644 --- a/docs/root/configuration/listeners/network_filters/mongo_proxy_filter.rst +++ b/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 `; +by default, metrics are emitted for *delete*, *insert*, and *update*. + .. _config_network_filters_mongo_proxy_collection_stats: Per collection query statistics diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 46645594f49c..0c24a46dd201 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -26,6 +26,7 @@ New Features ------------ * grpc: implemented header value syntax support when defining :ref:`initial metadata ` for gRPC-based `ext_authz` :ref:`HTTP ` and :ref:`network ` filters, and :ref:`ratelimit ` filters. * health_check: added option to use :ref:`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 `. Deprecated ---------- diff --git a/generated_api_shadow/envoy/extensions/filters/network/mongo_proxy/v3/mongo_proxy.proto b/generated_api_shadow/envoy/extensions/filters/network/mongo_proxy/v3/mongo_proxy.proto index 93abc90bdc24..ebdfb6f2fcc0 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/mongo_proxy/v3/mongo_proxy.proto +++ b/generated_api_shadow/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 `. // [#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 // ` 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; } diff --git a/source/extensions/filters/network/mongo_proxy/config.cc b/source/extensions/filters/network/mongo_proxy/config.cc index 13ed4139bbb4..ecdc60644888 100644 --- a/source/extensions/filters/network/mongo_proxy/config.cc +++ b/source/extensions/filters/network/mongo_proxy/config.cc @@ -34,7 +34,13 @@ Network::FilterFactoryCb MongoProxyFilterConfigFactory::createFilterFactoryFromP fault_config = std::make_shared(proto_config.delay()); } - auto stats = std::make_shared(context.scope(), stat_prefix); + auto commands = std::vector{"delete", "insert", "update"}; + if (proto_config.commands_size() > 0) { + commands = + std::vector(proto_config.commands().begin(), proto_config.commands().end()); + } + + auto stats = std::make_shared(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 { diff --git a/source/extensions/filters/network/mongo_proxy/mongo_stats.cc b/source/extensions/filters/network/mongo_proxy/mongo_stats.cc index 6059b461f94c..aca9ffaf35fd 100644 --- a/source/extensions/filters/network/mongo_proxy/mongo_stats.cc +++ b/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& 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) { diff --git a/source/extensions/filters/network/mongo_proxy/mongo_stats.h b/source/extensions/filters/network/mongo_proxy/mongo_stats.h index b19561df6788..3df51affd0a4 100644 --- a/source/extensions/filters/network/mongo_proxy/mongo_stats.h +++ b/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& commands); void incCounter(const Stats::ElementVec& names); void recordHistogram(const Stats::ElementVec& names, Stats::Histogram::Unit unit, diff --git a/source/extensions/filters/network/mongo_proxy/utility.cc b/source/extensions/filters/network/mongo_proxy/utility.cc index c51468f01e72..bb4537744288 100644 --- a/source/extensions/filters/network/mongo_proxy/utility.cc +++ b/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; diff --git a/test/extensions/filters/network/mongo_proxy/config_test.cc b/test/extensions/filters/network/mongo_proxy/config_test.cc index 97779c90e6b8..9df9c5530cf6 100644 --- a/test/extensions/filters/network/mongo_proxy/config_test.cc +++ b/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 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; diff --git a/test/extensions/filters/network/mongo_proxy/proxy_test.cc b/test/extensions/filters/network/mongo_proxy/proxy_test.cc index 71df3fb3d7d7..5b20f6cd5106 100644 --- a/test/extensions/filters/network/mongo_proxy/proxy_test.cc +++ b/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(store_, "test")) { setup(); } + MongoProxyFilterTest() + : mongo_stats_(std::make_shared(store_, "test", + std::vector{"insert", "count"})) { + setup(); + } void setup() { ON_CALL(runtime_.snapshot_, featureEnabled("mongo.proxy_enabled", 100)) diff --git a/test/extensions/filters/network/mongo_proxy/utility_test.cc b/test/extensions/filters/network/mongo_proxy/utility_test.cc index ad28e35e9cc1..db1ad741325c 100644 --- a/test/extensions/filters/network/mongo_proxy/utility_test.cc +++ b/test/extensions/filters/network/mongo_proxy/utility_test.cc @@ -189,13 +189,18 @@ TEST(QueryMessageInfoTest, Command) { EXPECT_THROW((QueryMessageInfo(q)), EnvoyException); } - { + std::vector> 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()); } }