From 6c427c67b0404aac1c46fdfc47c1bb718de6e6a4 Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Fri, 18 Aug 2017 18:08:53 -0400 Subject: [PATCH 1/6] Made Stats Sinks a statically registered component --- DEPRECATED.md | 3 + docs/configuration/overview/overview.rst | 12 +- include/envoy/server/configuration.h | 16 +-- source/common/json/config_schemas.cc | 4 + source/exe/BUILD | 2 + source/server/BUILD | 1 - source/server/config/stats/BUILD | 30 +++++ source/server/config/stats/tcp_statsd.cc | 36 ++++++ source/server/config/stats/tcp_statsd.h | 27 ++++ source/server/config/stats/udp_statsd.cc | 53 ++++++++ source/server/config/stats/udp_statsd.h | 27 ++++ source/server/configuration_impl.cc | 90 +++++++++++--- source/server/configuration_impl.h | 38 ++++-- source/server/server.cc | 34 +---- source/server/server.h | 2 - test/config/integration/server.json | 17 ++- .../server_grpc_json_transcoder.json | 10 +- test/config/integration/server_http2.json | 17 ++- .../integration/server_proxy_proto.json | 10 +- test/config/integration/server_ratelimit.json | 10 +- test/config/integration/server_ssl.json | 10 +- test/config/integration/server_uds.json | 10 +- test/config/integration/server_xfcc.json | 10 +- test/config/integration/tcp_proxy.json | 10 +- test/config_test/BUILD | 2 + test/integration/BUILD | 5 +- test/mocks/server/mocks.h | 4 +- test/server/BUILD | 4 + test/server/config/stats/BUILD | 20 +++ test/server/config/stats/config_test.cc | 116 ++++++++++++++++++ test/server/configuration_impl_test.cc | 35 ++++++ 31 files changed, 576 insertions(+), 89 deletions(-) create mode 100644 source/server/config/stats/BUILD create mode 100644 source/server/config/stats/tcp_statsd.cc create mode 100644 source/server/config/stats/tcp_statsd.h create mode 100644 source/server/config/stats/udp_statsd.cc create mode 100644 source/server/config/stats/udp_statsd.h create mode 100644 test/server/config/stats/BUILD create mode 100644 test/server/config/stats/config_test.cc diff --git a/DEPRECATED.md b/DEPRECATED.md index ec7eacc4537a..3f0c9c095a49 100644 --- a/DEPRECATED.md +++ b/DEPRECATED.md @@ -24,3 +24,6 @@ The following features have been DEPRECATED and will be removed in the specified * The direction of network and HTTP filters in the configuration will be ignored from 1.4.0 and later removed from the configuration in the v2 APIs. Filter direction is now implied at the C++ type level. + +## Version 1.5.0 +* Config options `statsd_udp_ip_address` and `statsd_tcp_cluster_name` have been deprecated and replaced with a `stats_sinks` array of sink configurations. diff --git a/docs/configuration/overview/overview.rst b/docs/configuration/overview/overview.rst index aaa8794405d5..02dddecbf27e 100644 --- a/docs/configuration/overview/overview.rst +++ b/docs/configuration/overview/overview.rst @@ -19,6 +19,7 @@ specify miscellaneous configuration. "statsd_local_udp_port": "...", "statsd_udp_ip_address": "...", "statsd_tcp_cluster_name": "...", + "stats_sinks": [], "stats_flush_interval_ms": "...", "watchdog_miss_timeout_ms": "...", "watchdog_megamiss_timeout_ms": "...", @@ -57,17 +58,24 @@ statsd_local_udp_port (Warning: DEPRECATED and will be removed in 1.4.0) *(optional, integer)* The UDP port of a locally running statsd compliant listener. If specified, :ref:`statistics ` will be flushed to this port. -statsd_udp_ip_address +statsd_udp_ip_address (Warning: DEPRECATED and will be removed in 1.5.0 in favor of an entry in +stats_sinks) *(optional, string)* The UDP address of a running statsd compliant listener. If specified, :ref:`statistics ` will be flushed to this address. IPv4 addresses should have format host:port (ex: 127.0.0.1:855). IPv6 addresses should have URL format [host]:port (ex: [::1]:855). -statsd_tcp_cluster_name +statsd_tcp_cluster_name (Warning: DEPRECATED and will be removed in 1.5.0 in favor of an entry in +stats_sinks) *(optional, string)* The name of a cluster manager cluster that is running a TCP statsd compliant listener. If specified, Envoy will connect to this cluster to flush :ref:`statistics `. +stats_sinks + *(optional, array)* An array of stats sink configuration objects. Each object is required to + have a "name" string field (to look up the statically registered implementation of that sink) and + a "config" object field that contains the custom configuration for the sink. + .. _config_overview_stats_flush_interval_ms: stats_flush_interval_ms diff --git a/include/envoy/server/configuration.h b/include/envoy/server/configuration.h index 2d86379e7232..9d62cfdba206 100644 --- a/include/envoy/server/configuration.h +++ b/include/envoy/server/configuration.h @@ -63,21 +63,9 @@ class Main { virtual RateLimit::ClientFactory& rateLimitClientFactory() PURE; /** - * @return Optional the optional local/remote TCP statsd cluster to write to. - * This cluster must be defined via the cluster manager configuration. + * @return std::list& the list of stats sinks initialized from the configuration. */ - virtual Optional statsdTcpClusterName() PURE; - - // TODO(hennna): DEPRECATED - will be removed in 1.4.0. - /** - * @return Optional the optional local UDP statsd port to write to. - */ - virtual Optional statsdUdpPort() PURE; - - /** - * @return Optional the optional UDP statsd address to write to. - */ - virtual Optional statsdUdpIpAddress() PURE; + virtual std::list& statsSinks() PURE; /** * @return std::chrono::milliseconds the time interval between flushing to configured stat sinks. diff --git a/source/common/json/config_schemas.cc b/source/common/json/config_schemas.cc index efe614b73e87..d0816ec56974 100644 --- a/source/common/json/config_schemas.cc +++ b/source/common/json/config_schemas.cc @@ -1187,6 +1187,10 @@ const std::string Json::Schema::TOP_LEVEL_CONFIG_SCHEMA(R"EOF( "statsd_udp_ip_address" : {"type" : "string"}, "statsd_tcp_cluster_name" : {"type" : "string"}, "stats_flush_interval_ms" : {"type" : "integer"}, + "stats_sinks" : { + "type" : "array", + "items" : {"type": "object"} + }, "watchdog_miss_timeout_ms" : {"type" : "integer"}, "watchdog_megamiss_timeout_ms" : {"type" : "integer"}, "watchdog_kill_timeout_ms" : {"type" : "integer"}, diff --git a/source/exe/BUILD b/source/exe/BUILD index f8e1912379bd..49948ab9dd4a 100644 --- a/source/exe/BUILD +++ b/source/exe/BUILD @@ -47,6 +47,8 @@ envoy_cc_library( "//source/server/config/network:ratelimit_lib", "//source/server/config/network:redis_proxy_lib", "//source/server/config/network:tcp_proxy_lib", + "//source/server/config/stats:tcp_statsd_lib", + "//source/server/config/stats:udp_statsd_lib", "//source/server/http:health_check_lib", ], ) diff --git a/source/server/BUILD b/source/server/BUILD index ee6f7deaf972..9ad7f8b5506a 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -235,7 +235,6 @@ envoy_cc_library( "//source/common/router:rds_lib", "//source/common/runtime:runtime_lib", "//source/common/singleton:manager_impl_lib", - "//source/common/stats:statsd_lib", "//source/common/upstream:cluster_manager_lib", "//source/server/http:admin_lib", ], diff --git a/source/server/config/stats/BUILD b/source/server/config/stats/BUILD new file mode 100644 index 000000000000..d881e0cc386f --- /dev/null +++ b/source/server/config/stats/BUILD @@ -0,0 +1,30 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_package", +) + +envoy_package() + +envoy_cc_library( + name = "tcp_statsd_lib", + srcs = ["tcp_statsd.cc"], + hdrs = ["tcp_statsd.h"], + deps = [ + "//source/common/stats:statsd_lib", + "//source/server:configuration_lib", + ], +) + +envoy_cc_library( + name = "udp_statsd_lib", + srcs = ["udp_statsd.cc"], + hdrs = ["udp_statsd.h"], + deps = [ + "//source/common/network:address_lib", + "//source/common/stats:statsd_lib", + "//source/server:configuration_lib", + ], +) diff --git a/source/server/config/stats/tcp_statsd.cc b/source/server/config/stats/tcp_statsd.cc new file mode 100644 index 000000000000..4af10f6d26d1 --- /dev/null +++ b/source/server/config/stats/tcp_statsd.cc @@ -0,0 +1,36 @@ +#include "server/config/stats/tcp_statsd.h" + +#include + +#include "envoy/registry/registry.h" + +#include "common/stats/statsd.h" + +namespace Envoy { +namespace Server { +namespace Configuration { + +Stats::SinkPtr TcpStatsdSinkFactory::createStatsSink(const Json::Object& json_config, + Server::Instance& server, + Upstream::ClusterManager& cluster_manager) { + if (json_config.hasObject("cluster_name")) { + const std::string cluster_name = json_config.getString("cluster_name"); + ENVOY_LOG(info, "statsd TCP cluster: {}", cluster_name); + return Stats::SinkPtr(new Stats::Statsd::TcpStatsdSink( + server.localInfo(), cluster_name, server.threadLocal(), cluster_manager, server.stats())); + } + + throw EnvoyException( + fmt::format("Didn't find cluster_name in the {} Stats::Sink config", name())); +} + +std::string TcpStatsdSinkFactory::name() { return "statsd_tcp"; } + +/** + * Static registration for the tcp statsd sink. @see RegisterFactory. + */ +static Registry::RegisterFactory register_; + +} // namespace Configuration +} // namespace Server +} // namespace Envoy diff --git a/source/server/config/stats/tcp_statsd.h b/source/server/config/stats/tcp_statsd.h new file mode 100644 index 000000000000..a9ad72ebf5a9 --- /dev/null +++ b/source/server/config/stats/tcp_statsd.h @@ -0,0 +1,27 @@ +#pragma once + +#include + +#include "envoy/server/instance.h" + +#include "server/configuration_impl.h" + +namespace Envoy { +namespace Server { +namespace Configuration { + +/** + * Config registration for the tcp statsd sink. @see StatsSinkFactory. + */ +class TcpStatsdSinkFactory : Logger::Loggable, public StatsSinkFactory { +public: + // StatsSinkFactory + Stats::SinkPtr createStatsSink(const Json::Object& json_config, Instance& server, + Upstream::ClusterManager& cluster_manager) override; + + std::string name() override; +}; + +} // namespace Configuration +} // namespace Server +} // namespace Envoy diff --git a/source/server/config/stats/udp_statsd.cc b/source/server/config/stats/udp_statsd.cc new file mode 100644 index 000000000000..85432fe63461 --- /dev/null +++ b/source/server/config/stats/udp_statsd.cc @@ -0,0 +1,53 @@ +#include "server/config/stats/udp_statsd.h" + +#include + +#include "envoy/registry/registry.h" + +#include "common/network/address_impl.h" +#include "common/stats/statsd.h" + +namespace Envoy { +namespace Server { +namespace Configuration { + +Stats::SinkPtr UdpStatsdSinkFactory::createStatsSink(const Json::Object& json_config, + Server::Instance& server, + Upstream::ClusterManager& cluster_manager) { + UNREFERENCED_PARAMETER(cluster_manager); + if (json_config.hasObject("local_port") && json_config.hasObject("ip_address")) { + throw EnvoyException(fmt::format( + "local_port and ip_address are mutually exclusive in the {} Stats::Sink config", name())); + } + + if (json_config.hasObject("ip_address")) { + const std::string udp_ip_address = json_config.getString("ip_address"); + ENVOY_LOG(info, "statsd UDP ip address: {}", udp_ip_address); + return Stats::SinkPtr(new Stats::Statsd::UdpStatsdSink( + server.threadLocal(), Network::Utility::parseInternetAddressAndPort(udp_ip_address))); + } else if (json_config.hasObject("local_port")) { + const int64_t local_upd_port = json_config.getInteger("local_port"); + // TODO(hennna): DEPRECATED - statsdUdpPort will be removed in 1.4.0. + ENVOY_LOG(warn, "local_port has been DEPRECATED and will be removed in 1.4.0. " + "Consider setting ip_address instead."); + ENVOY_LOG(info, "statsd UDP port: {}", local_upd_port); + Network::Address::InstanceConstSharedPtr address( + new Network::Address::Ipv4Instance(local_upd_port)); + return Stats::SinkPtr( + new Stats::Statsd::UdpStatsdSink(server.threadLocal(), std::move(address))); + } + + throw EnvoyException( + fmt::format("Didn't find local_port or ip_address in the {} Stats::Sink config", name())); +} + +std::string UdpStatsdSinkFactory::name() { return "statsd_udp"; } + +/** + * Static registration for the udp statsd sink. @see RegisterFactory. + */ +static Registry::RegisterFactory register_; + +} // namespace Configuration +} // namespace Server +} // namespace Envoy diff --git a/source/server/config/stats/udp_statsd.h b/source/server/config/stats/udp_statsd.h new file mode 100644 index 000000000000..8828ad3337d2 --- /dev/null +++ b/source/server/config/stats/udp_statsd.h @@ -0,0 +1,27 @@ +#pragma once + +#include + +#include "envoy/server/instance.h" + +#include "server/configuration_impl.h" + +namespace Envoy { +namespace Server { +namespace Configuration { + +/** + * Config registration for the udp statsd sink. @see StatsSinkFactory. + */ +class UdpStatsdSinkFactory : Logger::Loggable, public StatsSinkFactory { +public: + // StatsSinkFactory + Stats::SinkPtr createStatsSink(const Json::Object& json_config, Instance& server, + Upstream::ClusterManager& cluster_manager) override; + + std::string name() override; +}; + +} // namespace Configuration +} // namespace Server +} // namespace Envoy diff --git a/source/server/configuration_impl.cc b/source/server/configuration_impl.cc index 9ea7f17571b4..728b3c92d4c5 100644 --- a/source/server/configuration_impl.cc +++ b/source/server/configuration_impl.cc @@ -60,24 +60,6 @@ void MainImpl::initialize(const Json::Object& json, const envoy::api::v2::Bootst server.listenerManager())); } - if (json.hasObject("statsd_local_udp_port") && json.hasObject("statsd_udp_ip_address")) { - throw EnvoyException("statsd_local_udp_port and statsd_udp_ip_address " - "are mutually exclusive."); - } - - // TODO(hennna): DEPRECATED - statsd_local_udp_port will be removed in 1.4.0. - if (json.hasObject("statsd_local_udp_port")) { - statsd_udp_port_.value(json.getInteger("statsd_local_udp_port")); - } - - if (json.hasObject("statsd_udp_ip_address")) { - statsd_udp_ip_address_.value(json.getString("statsd_udp_ip_address")); - } - - if (json.hasObject("statsd_tcp_cluster_name")) { - statsd_tcp_cluster_name_.value(json.getString("statsd_tcp_cluster_name")); - } - stats_flush_interval_ = std::chrono::milliseconds(json.getInteger("stats_flush_interval_ms", 5000)); @@ -102,6 +84,8 @@ void MainImpl::initialize(const Json::Object& json, const envoy::api::v2::Bootst } else { ratelimit_client_factory_.reset(new RateLimit::NullFactoryImpl()); } + + initializeStatsSinks(json, server); } void MainImpl::initializeTracers(const Json::Object& configuration, Instance& server) { @@ -141,6 +125,76 @@ void MainImpl::initializeTracers(const Json::Object& configuration, Instance& se } } +void MainImpl::initializeStatsSinks(const Json::Object& configuration, Instance& server) { + ENVOY_LOG(info, "loading stats sink configuration"); + + // TODO(mrice32): DEPRECATED - statsd_local_udp_port, statsd_udp_ip_address, + // statsd_tcp_cluster_name fields in the base json configuration will be moved to subobjects + // inside the stats_sinks array for 1.5.0. + if (configuration.hasObject("statsd_local_udp_port") && + configuration.hasObject("statsd_udp_ip_address")) { + throw EnvoyException("statsd_local_udp_port and statsd_udp_ip_address " + "are mutually exclusive."); + } + + std::vector sinks = configuration.getObjectArray("stats_sinks", true); + + // Used to convert the deprecated statsd configuration into a json snippet for consumption by the + // new sink config parsing logic. + const std::string statsd_json = R"EOF( + {{ + "name": "{}", + "config": {{ + "{}": {} + }} + }} + )EOF"; + + // TODO(hennna): DEPRECATED - statsd_local_udp_port will be removed in 1.4.0. + if (configuration.hasObject("statsd_local_udp_port")) { + Json::ObjectSharedPtr json_obj = Json::Factory::loadFromString( + fmt::format(statsd_json, "statsd_udp", "local_port", + configuration.getInteger("statsd_local_udp_port"))); + sinks.emplace_back(std::move(json_obj)); + } + + if (configuration.hasObject("statsd_udp_ip_address")) { + Json::ObjectSharedPtr json_obj = Json::Factory::loadFromString( + fmt::format(statsd_json, "statsd_udp", "ip_address", + "\"" + configuration.getString("statsd_udp_ip_address") + "\"")); + sinks.emplace_back(std::move(json_obj)); + } + + if (configuration.hasObject("statsd_tcp_cluster_name")) { + Json::ObjectSharedPtr json_obj = Json::Factory::loadFromString( + fmt::format(statsd_json, "statsd_tcp", "cluster_name", + "\"" + configuration.getString("statsd_tcp_cluster_name") + "\"")); + sinks.emplace_back(std::move(json_obj)); + } + + for (const auto& sink_object : sinks) { + if (!sink_object->hasObject("name")) { + throw EnvoyException( + "sink object does not have 'name' attribute to look up the implementation"); + } + + if (!sink_object->hasObject("config")) { + throw EnvoyException( + "sink object does not contain the 'config' object to configure the implementation"); + } + + std::string name = sink_object->getString("name"); + Json::ObjectSharedPtr sink_config = sink_object->getObject("config"); + + StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); + if (factory != nullptr) { + stats_sinks_.emplace_back(factory->createStatsSink(*sink_config, server, *cluster_manager_)); + } else { + throw EnvoyException(fmt::format("No Stats::Sink found for name: {}", name)); + } + } +} + InitialImpl::InitialImpl(const Json::Object& json) { json.validateSchema(Json::Schema::TOP_LEVEL_CONFIG_SCHEMA); Json::ObjectSharedPtr admin = json.getObject("admin"); diff --git a/source/server/configuration_impl.h b/source/server/configuration_impl.h index 2fb18f6ba461..b5f8a5536325 100644 --- a/source/server/configuration_impl.h +++ b/source/server/configuration_impl.h @@ -69,6 +69,33 @@ class HttpTracerFactory { virtual std::string name() PURE; }; +/** + * Implemented for each Stats::Sink and registered via Registry::registerFactory() or + * the convenience class RegisterFactory. + */ +class StatsSinkFactory { +public: + virtual ~StatsSinkFactory() {} + + /** + * Create a particular Stats::Sink implementation. If the implementation is unable to produce a + * Stats::Sink with the provided parameters, it should throw an EnvoyException in the case of + * general error or a Json::Exception if the json configuration is erroneous. The returned + * pointer should always be valid. + * @param json_config supplies the general json configuration for the Stats::Sink + * @param server supplies the server instance + * @param cluster_manager supplies the cluster_manager instance + */ + virtual Stats::SinkPtr createStatsSink(const Json::Object& json_config, Instance& server, + Upstream::ClusterManager& cluster_manager) PURE; + + /** + * Returns the identifying name for a particular implementation of Stats::Sink produced by the + * factory. + */ + virtual std::string name() PURE; +}; + /** * Utilities for creating a filter chain for a network connection. */ @@ -119,10 +146,7 @@ class MainImpl : Logger::Loggable, public Main { Upstream::ClusterManager& clusterManager() override { return *cluster_manager_; } Tracing::HttpTracer& httpTracer() override { return *http_tracer_; } RateLimit::ClientFactory& rateLimitClientFactory() override { return *ratelimit_client_factory_; } - Optional statsdTcpClusterName() override { return statsd_tcp_cluster_name_; } - // TODO(hennna): DEPRECATED - statsdUdpPort() will be removed in 1.4.0 - Optional statsdUdpPort() override { return statsd_udp_port_; } - Optional statsdUdpIpAddress() override { return statsd_udp_ip_address_; } + std::list& statsSinks() override { return stats_sinks_; } std::chrono::milliseconds statsFlushInterval() override { return stats_flush_interval_; } std::chrono::milliseconds wdMissTimeout() const override { return watchdog_miss_timeout_; } std::chrono::milliseconds wdMegaMissTimeout() const override { @@ -139,6 +163,8 @@ class MainImpl : Logger::Loggable, public Main { */ void initializeTracers(const Json::Object& tracing_configuration, Instance& server); + void initializeStatsSinks(const Json::Object& configuration, Instance& server); + /** * DEPRECATED - Returns a list of the currently registered NetworkConfigFactories. */ @@ -151,9 +177,7 @@ class MainImpl : Logger::Loggable, public Main { std::unique_ptr cluster_manager_; std::unique_ptr lds_api_; Tracing::HttpTracerPtr http_tracer_; - Optional statsd_tcp_cluster_name_; - Optional statsd_udp_port_; - Optional statsd_udp_ip_address_; + std::list stats_sinks_; RateLimit::ClientFactoryPtr ratelimit_client_factory_; std::chrono::milliseconds stats_flush_interval_; std::chrono::milliseconds watchdog_miss_timeout_; diff --git a/source/server/server.cc b/source/server/server.cc index 3fde57ea4046..8852c7c9953c 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -23,7 +23,6 @@ #include "common/router/rds_impl.h" #include "common/runtime/runtime_impl.h" #include "common/singleton/manager_impl.h" -#include "common/stats/statsd.h" #include "common/upstream/cluster_manager_impl.h" #include "server/configuration_impl.h" @@ -133,7 +132,7 @@ void InstanceImpl::flushStats() { server_stats_.days_until_first_cert_expiring_.set( sslContextManager().daysUntilFirstCertExpires()); - InstanceUtil::flushCountersAndGaugesToSinks(stat_sinks_, stats_store_); + InstanceUtil::flushCountersAndGaugesToSinks(config_->statsSinks(), stats_store_); stat_flush_timer_->enableTimer(config_->statsFlushInterval()); } @@ -223,7 +222,9 @@ void InstanceImpl::initialize(Options& options, ENVOY_LOG(warn, "caught and eating SIGHUP. See documentation for how to hot restart."); }); - initializeStatSinks(); + for (Stats::SinkPtr& sink : main_config->statsSinks()) { + stats_store_.addSink(*sink); + } // Some of the stat sinks may need dispatcher support so don't flush until the main loop starts. // Just setup the timer. @@ -263,33 +264,6 @@ Runtime::LoaderPtr InstanceUtil::createRuntime(Instance& server, } } -void InstanceImpl::initializeStatSinks() { - if (config_->statsdUdpIpAddress().valid()) { - ENVOY_LOG(info, "statsd UDP ip address: {}", config_->statsdUdpIpAddress().value()); - stat_sinks_.emplace_back(new Stats::Statsd::UdpStatsdSink( - thread_local_, - Network::Utility::parseInternetAddressAndPort(config_->statsdUdpIpAddress().value()))); - stats_store_.addSink(*stat_sinks_.back()); - } else if (config_->statsdUdpPort().valid()) { - // TODO(hennna): DEPRECATED - statsdUdpPort will be removed in 1.4.0. - ENVOY_LOG(warn, "statsd_local_udp_port has been DEPRECATED and will be removed in 1.4.0. " - "Consider setting statsd_udp_ip_address instead."); - ENVOY_LOG(info, "statsd UDP port: {}", config_->statsdUdpPort().value()); - Network::Address::InstanceConstSharedPtr address( - new Network::Address::Ipv4Instance(config_->statsdUdpPort().value())); - stat_sinks_.emplace_back(new Stats::Statsd::UdpStatsdSink(thread_local_, address)); - stats_store_.addSink(*stat_sinks_.back()); - } - - if (config_->statsdTcpClusterName().valid()) { - ENVOY_LOG(info, "statsd TCP cluster: {}", config_->statsdTcpClusterName().value()); - stat_sinks_.emplace_back( - new Stats::Statsd::TcpStatsdSink(*local_info_, config_->statsdTcpClusterName().value(), - thread_local_, config_->clusterManager(), stats_store_)); - stats_store_.addSink(*stat_sinks_.back()); - } -} - void InstanceImpl::loadServerFlags(const Optional& flags_path) { if (!flags_path.valid()) { return; diff --git a/source/server/server.h b/source/server/server.h index f15e3536765e..0f90b33b2dfb 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -138,7 +138,6 @@ class InstanceImpl : Logger::Loggable, public Instance { void flushStats(); void initialize(Options& options, Network::Address::InstanceConstSharedPtr local_address, ComponentFactory& component_factory); - void initializeStatSinks(); void loadServerFlags(const Optional& flags_path); uint64_t numConnections(); void startWorkers(); @@ -148,7 +147,6 @@ class InstanceImpl : Logger::Loggable, public Instance { const time_t start_time_; time_t original_start_time_; Stats::StoreRoot& stats_store_; - std::list stat_sinks_; ServerStats server_stats_; ThreadLocal::Instance& thread_local_; Api::ApiPtr api_; diff --git a/test/config/integration/server.json b/test/config/integration/server.json index 565c52826da5..ef59896b2363 100644 --- a/test/config/integration/server.json +++ b/test/config/integration/server.json @@ -363,8 +363,21 @@ "profile_path": "{{ test_tmpdir }}/envoy.prof", "address": "tcp://{{ ip_loopback_address }}:0" }, "flags_path": "/invalid_flags", - "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", - "statsd_tcp_cluster_name": "statsd", + + "stats_sinks": [ + { + "name": "statsd_udp", + "config": { + "ip_address": "{{ ip_loopback_address }}:8125" + } + }, + { + "name": "statsd_tcp", + "config": { + "cluster_name": "statsd" + } + } + ], "lds": { "cluster": "lds" diff --git a/test/config/integration/server_grpc_json_transcoder.json b/test/config/integration/server_grpc_json_transcoder.json index 02e194552b7d..33159a9d7a3e 100644 --- a/test/config/integration/server_grpc_json_transcoder.json +++ b/test/config/integration/server_grpc_json_transcoder.json @@ -60,7 +60,15 @@ }], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", + + "stats_sinks": [ + { + "name": "statsd_udp", + "config": { + "ip_address": "{{ ip_loopback_address }}:8125" + } + } + ], "cluster_manager": { "clusters": [ diff --git a/test/config/integration/server_http2.json b/test/config/integration/server_http2.json index 6d1ce16ed95d..2468f7d48f83 100644 --- a/test/config/integration/server_http2.json +++ b/test/config/integration/server_http2.json @@ -211,8 +211,21 @@ }], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", - "statsd_tcp_cluster_name": "statsd", + + "stats_sinks": [ + { + "name": "statsd_udp", + "config": { + "ip_address": "{{ ip_loopback_address }}:8125" + } + }, + { + "name": "statsd_tcp", + "config": { + "cluster_name": "statsd" + } + } + ], "runtime": { "symlink_root": "{{ test_rundir }}/test/common/runtime/test_data/current", diff --git a/test/config/integration/server_proxy_proto.json b/test/config/integration/server_proxy_proto.json index 5fbc4f5e441e..6d1d5c3a6483 100644 --- a/test/config/integration/server_proxy_proto.json +++ b/test/config/integration/server_proxy_proto.json @@ -79,7 +79,15 @@ }], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", + + "stats_sinks": [ + { + "name": "statsd_udp", + "config": { + "ip_address": "{{ ip_loopback_address }}:8125" + } + } + ], "cluster_manager": { "clusters": [ diff --git a/test/config/integration/server_ratelimit.json b/test/config/integration/server_ratelimit.json index 50c591bf3c02..1a5c1eea21d0 100644 --- a/test/config/integration/server_ratelimit.json +++ b/test/config/integration/server_ratelimit.json @@ -63,7 +63,15 @@ }], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", + + "stats_sinks": [ + { + "name": "statsd_udp", + "config": { + "ip_address": "{{ ip_loopback_address }}:8125" + } + } + ], "rate_limit_service": { "type": "grpc_service", diff --git a/test/config/integration/server_ssl.json b/test/config/integration/server_ssl.json index 5966fceec122..6b5f65ae215d 100644 --- a/test/config/integration/server_ssl.json +++ b/test/config/integration/server_ssl.json @@ -90,7 +90,15 @@ }], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", + + "stats_sinks": [ + { + "name": "statsd_udp", + "config": { + "ip_address": "{{ ip_loopback_address }}:8125" + } + } + ], "cluster_manager": { "clusters": [ diff --git a/test/config/integration/server_uds.json b/test/config/integration/server_uds.json index 1a35f43fa00a..a75097880435 100644 --- a/test/config/integration/server_uds.json +++ b/test/config/integration/server_uds.json @@ -82,7 +82,15 @@ }], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", + + "stats_sinks": [ + { + "name": "statsd_udp", + "config": { + "ip_address": "{{ ip_loopback_address }}:8125" + } + } + ], "cluster_manager": { "clusters": [ diff --git a/test/config/integration/server_xfcc.json b/test/config/integration/server_xfcc.json index e965098e75d2..e90468285c36 100644 --- a/test/config/integration/server_xfcc.json +++ b/test/config/integration/server_xfcc.json @@ -170,7 +170,15 @@ }] }], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", + + "stats_sinks": [ + { + "name": "statsd_udp", + "config": { + "ip_address": "{{ ip_loopback_address }}:8125" + } + } + ], "cluster_manager": { "clusters": [ diff --git a/test/config/integration/tcp_proxy.json b/test/config/integration/tcp_proxy.json index 555a4a191873..9286b2505c7c 100644 --- a/test/config/integration/tcp_proxy.json +++ b/test/config/integration/tcp_proxy.json @@ -69,7 +69,15 @@ ] }], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", + + "stats_sinks": [ + { + "name": "statsd_udp", + "config": { + "ip_address": "{{ ip_loopback_address }}:8125" + } + } + ], "cluster_manager": { "clusters": [ diff --git a/test/config_test/BUILD b/test/config_test/BUILD index 6497f12c1dac..1ff3180ec69d 100644 --- a/test/config_test/BUILD +++ b/test/config_test/BUILD @@ -22,6 +22,8 @@ envoy_cc_test( ], deps = [ ":config_test_lib", + "//source/server/config/stats:tcp_statsd_lib", + "//source/server/config/stats:udp_statsd_lib", "//test/test_common:environment_lib", "//test/test_common:utility_lib", ], diff --git a/test/integration/BUILD b/test/integration/BUILD index 041e60321d6c..310fb2b1595a 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -164,6 +164,8 @@ envoy_cc_test_library( "//source/server/config/network:ratelimit_lib", "//source/server/config/network:redis_proxy_lib", "//source/server/config/network:tcp_proxy_lib", + "//source/server/config/stats:tcp_statsd_lib", + "//source/server/config/stats:udp_statsd_lib", "//source/server/http:health_check_lib", "//test/mocks/buffer:buffer_mocks", "//test/mocks/upstream:upstream_mocks", @@ -187,6 +189,8 @@ envoy_cc_test( deps = [ ":integration_lib", "//source/common/http:header_map_lib", + "//source/server/config/stats:tcp_statsd_lib", + "//source/server/config/stats:udp_statsd_lib", "//test/test_common:utility_lib", ], ) @@ -216,7 +220,6 @@ envoy_cc_test( ":integration_lib", "//source/common/buffer:buffer_lib", "//source/common/http:codec_client_lib", - "//source/common/stats:stats_lib", ], ) diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index f49f37135b6b..42c2d2ab3913 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -302,9 +302,7 @@ class MockMain : public Main { MOCK_METHOD0(clusterManager, Upstream::ClusterManager&()); MOCK_METHOD0(httpTracer, Tracing::HttpTracer&()); MOCK_METHOD0(rateLimitClientFactory, RateLimit::ClientFactory&()); - MOCK_METHOD0(statsdTcpClusterName, Optional()); - MOCK_METHOD0(statsdUdpPort, Optional()); - MOCK_METHOD0(statsdUdpIpAddress, Optional()); + MOCK_METHOD0(statsSinks, std::list&()); MOCK_METHOD0(statsFlushInterval, std::chrono::milliseconds()); MOCK_CONST_METHOD0(wdMissTimeout, std::chrono::milliseconds()); MOCK_CONST_METHOD0(wdMegaMissTimeout, std::chrono::milliseconds()); diff --git a/test/server/BUILD b/test/server/BUILD index 1a51558c2e2d..33157e116e58 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -23,6 +23,8 @@ envoy_cc_test( "//source/common/event:dispatcher_lib", "//source/common/upstream:cluster_manager_lib", "//source/server:configuration_lib", + "//source/server/config/stats:tcp_statsd_lib", + "//source/server/config/stats:udp_statsd_lib", "//test/mocks:common_lib", "//test/mocks/network:network_mocks", "//test/mocks/server:server_mocks", @@ -120,6 +122,8 @@ envoy_cc_test( deps = [ "//source/common/common:version_lib", "//source/server:server_lib", + "//source/server/config/stats:tcp_statsd_lib", + "//source/server/config/stats:udp_statsd_lib", "//test/integration:integration_lib", "//test/mocks/server:server_mocks", "//test/mocks/stats:stats_mocks", diff --git a/test/server/config/stats/BUILD b/test/server/config/stats/BUILD new file mode 100644 index 000000000000..5be0ce805435 --- /dev/null +++ b/test/server/config/stats/BUILD @@ -0,0 +1,20 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_test", + "envoy_package", +) + +envoy_package() + +envoy_cc_test( + name = "config_test", + srcs = ["config_test.cc"], + deps = [ + "//source/server/config/stats:tcp_statsd_lib", + "//source/server/config/stats:udp_statsd_lib", + "//test/mocks/server:server_mocks", + "//test/test_common:utility_lib", + ], +) diff --git a/test/server/config/stats/config_test.cc b/test/server/config/stats/config_test.cc new file mode 100644 index 000000000000..1e06355fa187 --- /dev/null +++ b/test/server/config/stats/config_test.cc @@ -0,0 +1,116 @@ +#include + +#include "envoy/registry/registry.h" + +#include "server/config/stats/tcp_statsd.h" +#include "server/config/stats/udp_statsd.h" + +#include "test/mocks/server/mocks.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Envoy { +using testing::NiceMock; +using testing::Return; +using testing::ReturnRef; +using testing::_; + +namespace Server { +namespace Configuration { + +TEST(StatsConfigTest, ValidTcpStatsd) { + const std::string json_string = R"EOF( + { + "cluster_name" : "fake_cluster" + } + )EOF"; + const std::string name = "statsd_tcp"; + + Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(json_string); + NiceMock server; + StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); + ASSERT_NE(factory, nullptr); + Stats::SinkPtr sink = factory->createStatsSink(*json_config, server, server.clusterManager()); + EXPECT_NE(sink, nullptr); +} + +TEST(StatsConfigTest, InvalidTcpStatsd) { + const std::string json_string = R"EOF( + {} + )EOF"; + const std::string name = "statsd_tcp"; + + Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(json_string); + NiceMock server; + StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); + EXPECT_THROW(factory->createStatsSink(*json_config, server, server.clusterManager()), + EnvoyException); +} + +TEST(StatsConfigTest, ValidUdpPortStatsd) { + const std::string json_string = R"EOF( + { + "local_port" : 8125 + } + )EOF"; + const std::string name = "statsd_udp"; + + Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(json_string); + NiceMock server; + StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); + ASSERT_NE(factory, nullptr); + Stats::SinkPtr sink = factory->createStatsSink(*json_config, server, server.clusterManager()); + EXPECT_NE(sink, nullptr); +} + +TEST(StatsConfigTest, ValidUdpIpStatsd) { + const std::string json_string = R"EOF( + { + "ip_address" : "127.0.0.1:8125" + } + )EOF"; + const std::string name = "statsd_udp"; + + Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(json_string); + NiceMock server; + StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); + ASSERT_NE(factory, nullptr); + Stats::SinkPtr sink = factory->createStatsSink(*json_config, server, server.clusterManager()); + EXPECT_NE(sink, nullptr); +} + +TEST(StatsConfigTest, BadUdpEmptyConfig) { + const std::string json_string = R"EOF( + {} + )EOF"; + const std::string name = "statsd_udp"; + + Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(json_string); + NiceMock server; + StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); + ASSERT_NE(factory, nullptr); + EXPECT_THROW(factory->createStatsSink(*json_config, server, server.clusterManager()), + EnvoyException); +} + +TEST(StatsConfigTest, BadUdpIpAndPortConfig) { + const std::string json_string = R"EOF( + { + "ip_address" : "127.0.0.1:8125", + "local_port" : 8125 + } + )EOF"; + const std::string name = "statsd_udp"; + + Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(json_string); + NiceMock server; + StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); + ASSERT_NE(factory, nullptr); + EXPECT_THROW(factory->createStatsSink(*json_config, server, server.clusterManager()), + EnvoyException); +} + +} // namespace Configuration +} // namespace Server +} // namespace Envoy diff --git a/test/server/configuration_impl_test.cc b/test/server/configuration_impl_test.cc index e57014a16fe3..5aa4aaf2f0fc 100644 --- a/test/server/configuration_impl_test.cc +++ b/test/server/configuration_impl_test.cc @@ -260,6 +260,41 @@ TEST_F(ConfigurationImplTest, ConfigurationFailsWhenInvalidTracerSpecified) { "No HttpTracerFactory found for type: invalid"); } +TEST_F(ConfigurationImplTest, DeprecatedStatsConfiguration) { + + std::string json = R"EOF( + { + "listeners": [], + + "cluster_manager": { + "clusters": [ + { + "name": "fake_cluster", + "type": "static", + "connect_timeout_ms": 1, + "per_connection_buffer_limit_bytes": 8192, + "lb_type": "round_robin", + "hosts": [ + { "url" : "tcp://127.0.0.1:9999" } + ] + } + ] + }, + + "statsd_udp_ip_address" : "127.0.0.1:8125", + "statsd_tcp_cluster_name" : "fake_cluster" + } + )EOF"; + + Json::ObjectSharedPtr loader = Json::Factory::loadFromString(json); + envoy::api::v2::Bootstrap bootstrap; + + MainImpl config; + config.initialize(*loader, bootstrap, server_, cluster_manager_factory_); + + EXPECT_EQ(2, config.statsSinks().size()); +} + } // namespace Configuration } // namespace Server } // namespace Envoy From 42ea55acb9435c89b7d2f1371d2441043e58432c Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Mon, 21 Aug 2017 16:23:01 -0400 Subject: [PATCH 2/6] corrected doc formatting --- docs/configuration/overview/overview.rst | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/docs/configuration/overview/overview.rst b/docs/configuration/overview/overview.rst index 02dddecbf27e..3e75213fa08f 100644 --- a/docs/configuration/overview/overview.rst +++ b/docs/configuration/overview/overview.rst @@ -58,15 +58,13 @@ statsd_local_udp_port (Warning: DEPRECATED and will be removed in 1.4.0) *(optional, integer)* The UDP port of a locally running statsd compliant listener. If specified, :ref:`statistics ` will be flushed to this port. -statsd_udp_ip_address (Warning: DEPRECATED and will be removed in 1.5.0 in favor of an entry in -stats_sinks) +statsd_udp_ip_address (Warning: DEPRECATED and will be removed in 1.5.0) *(optional, string)* The UDP address of a running statsd compliant listener. If specified, :ref:`statistics ` will be flushed to this address. IPv4 addresses should have format host:port (ex: 127.0.0.1:855). IPv6 addresses should have URL format [host]:port (ex: [::1]:855). -statsd_tcp_cluster_name (Warning: DEPRECATED and will be removed in 1.5.0 in favor of an entry in -stats_sinks) +statsd_tcp_cluster_name (Warning: DEPRECATED and will be removed in 1.5.0) *(optional, string)* The name of a cluster manager cluster that is running a TCP statsd compliant listener. If specified, Envoy will connect to this cluster to flush :ref:`statistics `. From f99b5a5d05ecc13fe19d3f0eb56c53e7a3bce3d0 Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Fri, 1 Sep 2017 18:00:12 -0400 Subject: [PATCH 3/6] Fixed post-merge issues and simplified PR --- DEPRECATED.md | 8 +- docs/configuration/overview/overview.rst | 10 +- source/common/json/config_schemas.cc | 4 - source/exe/BUILD | 3 +- source/server/config/stats/BUILD | 17 +-- source/server/config/stats/statsd.cc | 54 ++++++++ .../config/stats/{tcp_statsd.h => statsd.h} | 6 +- source/server/config/stats/tcp_statsd.cc | 36 ------ source/server/config/stats/udp_statsd.cc | 53 -------- source/server/config/stats/udp_statsd.h | 27 ---- source/server/configuration_impl.cc | 81 ++---------- source/server/configuration_impl.h | 13 +- test/config/integration/server.json | 17 +-- .../server_grpc_json_transcoder.json | 10 +- test/config/integration/server_http2.json | 17 +-- .../integration/server_proxy_proto.json | 10 +- test/config/integration/server_ratelimit.json | 10 +- test/config/integration/server_ssl.json | 10 +- test/config/integration/server_uds.json | 10 +- test/config/integration/server_xfcc.json | 10 +- test/config/integration/tcp_proxy.json | 10 +- test/config_test/BUILD | 3 +- test/integration/BUILD | 6 +- test/mocks/server/mocks.h | 5 - test/server/BUILD | 6 +- test/server/config/stats/BUILD | 4 +- test/server/config/stats/config_test.cc | 121 ++++++++---------- test/server/configuration_impl_test.cc | 35 ----- 28 files changed, 155 insertions(+), 441 deletions(-) create mode 100644 source/server/config/stats/statsd.cc rename source/server/config/stats/{tcp_statsd.h => statsd.h} (69%) delete mode 100644 source/server/config/stats/tcp_statsd.cc delete mode 100644 source/server/config/stats/udp_statsd.cc delete mode 100644 source/server/config/stats/udp_statsd.h diff --git a/DEPRECATED.md b/DEPRECATED.md index 796643cc2249..4f5ea86523f2 100644 --- a/DEPRECATED.md +++ b/DEPRECATED.md @@ -22,10 +22,6 @@ The following features have been DEPRECATED and will be removed in the specified This no longer uses `protoc` generated stubs but instead utilizes C++ template generation of the RPC stubs. `Grpc::AsyncClientImpl` supports streaming, in addition to the previous unary, RPCs. * The direction of network and HTTP filters in the configuration will be ignored from 1.4.0 and - later removed from the configuration in the v2 APIs. Filter direction is now implied at the C++ - type level. The `type()` methods on the `NamedNetworkFilterConfigFactory` and + later removed from the configuration in the v2 APIs. Filter direction is now implied at the C++ type + level. The `type()` methods on the `NamedNetworkFilterConfigFactory` and `NamedHttpFilterConfigFactory` intefaces have been removed to reflect this. - -## Version 1.5.0 -* Config options `statsd_udp_ip_address` and `statsd_tcp_cluster_name` have been deprecated and - replaced with a `stats_sinks` array of sink configurations. \ No newline at end of file diff --git a/docs/configuration/overview/overview.rst b/docs/configuration/overview/overview.rst index 7b9902dedc64..05a5f9e9d9a4 100644 --- a/docs/configuration/overview/overview.rst +++ b/docs/configuration/overview/overview.rst @@ -25,7 +25,6 @@ name (which should be a string) is called *true*, it should be written in the co "flags_path": "...", "statsd_udp_ip_address": "...", "statsd_tcp_cluster_name": "...", - "stats_sinks": [], "stats_flush_interval_ms": "...", "watchdog_miss_timeout_ms": "...", "watchdog_megamiss_timeout_ms": "...", @@ -62,22 +61,17 @@ flags_path .. _config_overview_statsd_udp_ip_address: -statsd_udp_ip_address (Warning: DEPRECATED and will be removed in 1.5.0) +statsd_udp_ip_address *(optional, string)* The UDP address of a running statsd compliant listener. If specified, :ref:`statistics ` will be flushed to this address. IPv4 addresses should have format host:port (ex: 127.0.0.1:855). IPv6 addresses should have URL format [host]:port (ex: [::1]:855). -statsd_tcp_cluster_name (Warning: DEPRECATED and will be removed in 1.5.0) +statsd_tcp_cluster_name *(optional, string)* The name of a cluster manager cluster that is running a TCP statsd compliant listener. If specified, Envoy will connect to this cluster to flush :ref:`statistics `. -stats_sinks - *(optional, array)* An array of stats sink configuration objects. Each object is required to - have a "name" string field (to look up the statically registered implementation of that sink) and - a "config" object field that contains the custom configuration for the sink. - .. _config_overview_stats_flush_interval_ms: stats_flush_interval_ms diff --git a/source/common/json/config_schemas.cc b/source/common/json/config_schemas.cc index 4113ed46b414..d6c8617f0465 100644 --- a/source/common/json/config_schemas.cc +++ b/source/common/json/config_schemas.cc @@ -1206,10 +1206,6 @@ const std::string Json::Schema::TOP_LEVEL_CONFIG_SCHEMA(R"EOF( "statsd_udp_ip_address" : {"type" : "string"}, "statsd_tcp_cluster_name" : {"type" : "string"}, "stats_flush_interval_ms" : {"type" : "integer"}, - "stats_sinks" : { - "type" : "array", - "items" : {"type": "object"} - }, "watchdog_miss_timeout_ms" : {"type" : "integer"}, "watchdog_megamiss_timeout_ms" : {"type" : "integer"}, "watchdog_kill_timeout_ms" : {"type" : "integer"}, diff --git a/source/exe/BUILD b/source/exe/BUILD index 49948ab9dd4a..7fd9de2b1627 100644 --- a/source/exe/BUILD +++ b/source/exe/BUILD @@ -47,8 +47,7 @@ envoy_cc_library( "//source/server/config/network:ratelimit_lib", "//source/server/config/network:redis_proxy_lib", "//source/server/config/network:tcp_proxy_lib", - "//source/server/config/stats:tcp_statsd_lib", - "//source/server/config/stats:udp_statsd_lib", + "//source/server/config/stats:statsd_lib", "//source/server/http:health_check_lib", ], ) diff --git a/source/server/config/stats/BUILD b/source/server/config/stats/BUILD index d881e0cc386f..13b9cedd11d2 100644 --- a/source/server/config/stats/BUILD +++ b/source/server/config/stats/BUILD @@ -9,19 +9,12 @@ load( envoy_package() envoy_cc_library( - name = "tcp_statsd_lib", - srcs = ["tcp_statsd.cc"], - hdrs = ["tcp_statsd.h"], - deps = [ - "//source/common/stats:statsd_lib", - "//source/server:configuration_lib", + name = "statsd_lib", + srcs = ["statsd.cc"], + hdrs = ["statsd.h"], + external_deps = [ + "envoy_bootstrap", ], -) - -envoy_cc_library( - name = "udp_statsd_lib", - srcs = ["udp_statsd.cc"], - hdrs = ["udp_statsd.h"], deps = [ "//source/common/network:address_lib", "//source/common/stats:statsd_lib", diff --git a/source/server/config/stats/statsd.cc b/source/server/config/stats/statsd.cc new file mode 100644 index 000000000000..283c6ca919fa --- /dev/null +++ b/source/server/config/stats/statsd.cc @@ -0,0 +1,54 @@ +#include "server/config/stats/statsd.h" + +#include + +#include "envoy/registry/registry.h" + +#include "common/stats/statsd.h" + +#include "api/bootstrap.pb.h" + +namespace Envoy { +namespace Server { +namespace Configuration { + +Stats::SinkPtr StatsdSinkFactory::createStatsSink(const Protobuf::Message& config, + Server::Instance& server, + Upstream::ClusterManager& cluster_manager) { + + const auto& statsd_sink = dynamic_cast(config); + switch (statsd_sink.statsd_specifier_case()) { + case envoy::api::v2::StatsdSink::kAddress: { + Network::Address::InstanceConstSharedPtr address = + Network::Utility::fromProtoAddress(statsd_sink.address()); + ENVOY_LOG(info, "statsd UDP ip address: {}", address->asString()); + return Stats::SinkPtr( + new Stats::Statsd::UdpStatsdSink(server.threadLocal(), std::move(address))); + break; + } + case envoy::api::v2::StatsdSink::kTcpClusterName: + ENVOY_LOG(info, "statsd TCP cluster: {}", statsd_sink.tcp_cluster_name()); + return Stats::SinkPtr( + new Stats::Statsd::TcpStatsdSink(server.localInfo(), statsd_sink.tcp_cluster_name(), + server.threadLocal(), cluster_manager, server.stats())); + break; + default: + throw EnvoyException( + fmt::format("No tcp_cluster_name or address provided for {} Stats::Sink config", name())); + } +} + +ProtobufTypes::MessagePtr StatsdSinkFactory::createEmptyConfigProto() { + return std::unique_ptr(new envoy::api::v2::StatsdSink()); +} + +std::string StatsdSinkFactory::name() { return "envoy.statsd"; } + +/** + * Static registration for the statsd sink factory. @see RegisterFactory. + */ +static Registry::RegisterFactory register_; + +} // namespace Configuration +} // namespace Server +} // namespace Envoy diff --git a/source/server/config/stats/tcp_statsd.h b/source/server/config/stats/statsd.h similarity index 69% rename from source/server/config/stats/tcp_statsd.h rename to source/server/config/stats/statsd.h index a9ad72ebf5a9..943344769184 100644 --- a/source/server/config/stats/tcp_statsd.h +++ b/source/server/config/stats/statsd.h @@ -13,12 +13,14 @@ namespace Configuration { /** * Config registration for the tcp statsd sink. @see StatsSinkFactory. */ -class TcpStatsdSinkFactory : Logger::Loggable, public StatsSinkFactory { +class StatsdSinkFactory : Logger::Loggable, public StatsSinkFactory { public: // StatsSinkFactory - Stats::SinkPtr createStatsSink(const Json::Object& json_config, Instance& server, + Stats::SinkPtr createStatsSink(const Protobuf::Message& config, Instance& server, Upstream::ClusterManager& cluster_manager) override; + ProtobufTypes::MessagePtr createEmptyConfigProto() override; + std::string name() override; }; diff --git a/source/server/config/stats/tcp_statsd.cc b/source/server/config/stats/tcp_statsd.cc deleted file mode 100644 index 4af10f6d26d1..000000000000 --- a/source/server/config/stats/tcp_statsd.cc +++ /dev/null @@ -1,36 +0,0 @@ -#include "server/config/stats/tcp_statsd.h" - -#include - -#include "envoy/registry/registry.h" - -#include "common/stats/statsd.h" - -namespace Envoy { -namespace Server { -namespace Configuration { - -Stats::SinkPtr TcpStatsdSinkFactory::createStatsSink(const Json::Object& json_config, - Server::Instance& server, - Upstream::ClusterManager& cluster_manager) { - if (json_config.hasObject("cluster_name")) { - const std::string cluster_name = json_config.getString("cluster_name"); - ENVOY_LOG(info, "statsd TCP cluster: {}", cluster_name); - return Stats::SinkPtr(new Stats::Statsd::TcpStatsdSink( - server.localInfo(), cluster_name, server.threadLocal(), cluster_manager, server.stats())); - } - - throw EnvoyException( - fmt::format("Didn't find cluster_name in the {} Stats::Sink config", name())); -} - -std::string TcpStatsdSinkFactory::name() { return "statsd_tcp"; } - -/** - * Static registration for the tcp statsd sink. @see RegisterFactory. - */ -static Registry::RegisterFactory register_; - -} // namespace Configuration -} // namespace Server -} // namespace Envoy diff --git a/source/server/config/stats/udp_statsd.cc b/source/server/config/stats/udp_statsd.cc deleted file mode 100644 index 85432fe63461..000000000000 --- a/source/server/config/stats/udp_statsd.cc +++ /dev/null @@ -1,53 +0,0 @@ -#include "server/config/stats/udp_statsd.h" - -#include - -#include "envoy/registry/registry.h" - -#include "common/network/address_impl.h" -#include "common/stats/statsd.h" - -namespace Envoy { -namespace Server { -namespace Configuration { - -Stats::SinkPtr UdpStatsdSinkFactory::createStatsSink(const Json::Object& json_config, - Server::Instance& server, - Upstream::ClusterManager& cluster_manager) { - UNREFERENCED_PARAMETER(cluster_manager); - if (json_config.hasObject("local_port") && json_config.hasObject("ip_address")) { - throw EnvoyException(fmt::format( - "local_port and ip_address are mutually exclusive in the {} Stats::Sink config", name())); - } - - if (json_config.hasObject("ip_address")) { - const std::string udp_ip_address = json_config.getString("ip_address"); - ENVOY_LOG(info, "statsd UDP ip address: {}", udp_ip_address); - return Stats::SinkPtr(new Stats::Statsd::UdpStatsdSink( - server.threadLocal(), Network::Utility::parseInternetAddressAndPort(udp_ip_address))); - } else if (json_config.hasObject("local_port")) { - const int64_t local_upd_port = json_config.getInteger("local_port"); - // TODO(hennna): DEPRECATED - statsdUdpPort will be removed in 1.4.0. - ENVOY_LOG(warn, "local_port has been DEPRECATED and will be removed in 1.4.0. " - "Consider setting ip_address instead."); - ENVOY_LOG(info, "statsd UDP port: {}", local_upd_port); - Network::Address::InstanceConstSharedPtr address( - new Network::Address::Ipv4Instance(local_upd_port)); - return Stats::SinkPtr( - new Stats::Statsd::UdpStatsdSink(server.threadLocal(), std::move(address))); - } - - throw EnvoyException( - fmt::format("Didn't find local_port or ip_address in the {} Stats::Sink config", name())); -} - -std::string UdpStatsdSinkFactory::name() { return "statsd_udp"; } - -/** - * Static registration for the udp statsd sink. @see RegisterFactory. - */ -static Registry::RegisterFactory register_; - -} // namespace Configuration -} // namespace Server -} // namespace Envoy diff --git a/source/server/config/stats/udp_statsd.h b/source/server/config/stats/udp_statsd.h deleted file mode 100644 index 8828ad3337d2..000000000000 --- a/source/server/config/stats/udp_statsd.h +++ /dev/null @@ -1,27 +0,0 @@ -#pragma once - -#include - -#include "envoy/server/instance.h" - -#include "server/configuration_impl.h" - -namespace Envoy { -namespace Server { -namespace Configuration { - -/** - * Config registration for the udp statsd sink. @see StatsSinkFactory. - */ -class UdpStatsdSinkFactory : Logger::Loggable, public StatsSinkFactory { -public: - // StatsSinkFactory - Stats::SinkPtr createStatsSink(const Json::Object& json_config, Instance& server, - Upstream::ClusterManager& cluster_manager) override; - - std::string name() override; -}; - -} // namespace Configuration -} // namespace Server -} // namespace Envoy diff --git a/source/server/configuration_impl.cc b/source/server/configuration_impl.cc index 668f44511d03..24d4788ed62d 100644 --- a/source/server/configuration_impl.cc +++ b/source/server/configuration_impl.cc @@ -55,25 +55,6 @@ void MainImpl::initialize(const envoy::api::v2::Bootstrap& bootstrap, Instance& server.localInfo(), server.stats(), server.listenerManager())); } - for (const auto& stats_sink : bootstrap.stats_sinks()) { - // TODO(mrice32): Add support for pluggable stats sinks. - ASSERT(stats_sink.name() == "envoy.statsd"); - envoy::api::v2::StatsdSink statsd_sink; - MessageUtil::jsonConvert(stats_sink.config(), statsd_sink); - - switch (statsd_sink.statsd_specifier_case()) { - case envoy::api::v2::StatsdSink::kAddress: { - statsd_udp_ip_address_ = Network::Utility::fromProtoAddress(statsd_sink.address()); - break; - } - case envoy::api::v2::StatsdSink::kTcpClusterName: - statsd_tcp_cluster_name_.value(statsd_sink.tcp_cluster_name()); - break; - default: - NOT_REACHED; - } - } - stats_flush_interval_ = std::chrono::milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(bootstrap, stats_flush_interval, 5000)); @@ -96,7 +77,7 @@ void MainImpl::initialize(const envoy::api::v2::Bootstrap& bootstrap, Instance& ratelimit_client_factory_.reset(new RateLimit::NullFactoryImpl()); } - initializeStatsSinks(json, server); + initializeStatsSinks(bootstrap, server); } void MainImpl::initializeTracers(const envoy::api::v2::Tracing& configuration, Instance& server) { @@ -129,70 +110,26 @@ void MainImpl::initializeTracers(const envoy::api::v2::Tracing& configuration, I } } -void MainImpl::initializeStatsSinks(const Json::Object& configuration, Instance& server) { +void MainImpl::initializeStatsSinks(const envoy::api::v2::Bootstrap& bootstrap, Instance& server) { ENVOY_LOG(info, "loading stats sink configuration"); - // TODO(mrice32): DEPRECATED - statsd_local_udp_port, statsd_udp_ip_address, - // statsd_tcp_cluster_name fields in the base json configuration will be moved to subobjects - // inside the stats_sinks array for 1.5.0. - if (configuration.hasObject("statsd_local_udp_port") && - configuration.hasObject("statsd_udp_ip_address")) { - throw EnvoyException("statsd_local_udp_port and statsd_udp_ip_address " - "are mutually exclusive."); - } - - std::vector sinks = configuration.getObjectArray("stats_sinks", true); - - // Used to convert the deprecated statsd configuration into a json snippet for consumption by the - // new sink config parsing logic. - const std::string statsd_json = R"EOF( - {{ - "name": "{}", - "config": {{ - "{}": {} - }} - }} - )EOF"; - - // TODO(hennna): DEPRECATED - statsd_local_udp_port will be removed in 1.4.0. - if (configuration.hasObject("statsd_local_udp_port")) { - Json::ObjectSharedPtr json_obj = Json::Factory::loadFromString( - fmt::format(statsd_json, "statsd_udp", "local_port", - configuration.getInteger("statsd_local_udp_port"))); - sinks.emplace_back(std::move(json_obj)); - } - - if (configuration.hasObject("statsd_udp_ip_address")) { - Json::ObjectSharedPtr json_obj = Json::Factory::loadFromString( - fmt::format(statsd_json, "statsd_udp", "ip_address", - "\"" + configuration.getString("statsd_udp_ip_address") + "\"")); - sinks.emplace_back(std::move(json_obj)); - } - - if (configuration.hasObject("statsd_tcp_cluster_name")) { - Json::ObjectSharedPtr json_obj = Json::Factory::loadFromString( - fmt::format(statsd_json, "statsd_tcp", "cluster_name", - "\"" + configuration.getString("statsd_tcp_cluster_name") + "\"")); - sinks.emplace_back(std::move(json_obj)); - } - - for (const auto& sink_object : sinks) { - if (!sink_object->hasObject("name")) { + for (const envoy::api::v2::StatsSink& sink_object : bootstrap.stats_sinks()) { + if (sink_object.name().empty()) { throw EnvoyException( "sink object does not have 'name' attribute to look up the implementation"); } - if (!sink_object->hasObject("config")) { + if (!sink_object.has_config()) { throw EnvoyException( "sink object does not contain the 'config' object to configure the implementation"); } - std::string name = sink_object->getString("name"); - Json::ObjectSharedPtr sink_config = sink_object->getObject("config"); - + ProtobufTypes::String name = sink_object.name(); StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); if (factory != nullptr) { - stats_sinks_.emplace_back(factory->createStatsSink(*sink_config, server, *cluster_manager_)); + ProtobufTypes::MessagePtr message = factory->createEmptyConfigProto(); + MessageUtil::jsonConvert(sink_object.config(), *message); + stats_sinks_.emplace_back(factory->createStatsSink(*message, server, *cluster_manager_)); } else { throw EnvoyException(fmt::format("No Stats::Sink found for name: {}", name)); } diff --git a/source/server/configuration_impl.h b/source/server/configuration_impl.h index dabae0d87a2d..46512cc2d415 100644 --- a/source/server/configuration_impl.h +++ b/source/server/configuration_impl.h @@ -68,13 +68,20 @@ class StatsSinkFactory { * Stats::Sink with the provided parameters, it should throw an EnvoyException in the case of * general error or a Json::Exception if the json configuration is erroneous. The returned * pointer should always be valid. - * @param json_config supplies the general json configuration for the Stats::Sink + * @param config supplies the custom proto configuration for the Stats::Sink * @param server supplies the server instance * @param cluster_manager supplies the cluster_manager instance */ - virtual Stats::SinkPtr createStatsSink(const Json::Object& json_config, Instance& server, + virtual Stats::SinkPtr createStatsSink(const Protobuf::Message& config, Instance& server, Upstream::ClusterManager& cluster_manager) PURE; + /** + * @return ProtobufTypes::MessagePtr create empty config proto message for v2. The filter + * config, which arrives in an opaque google.protobuf.Struct message, will be converted to + * JSON and then parsed into this empty proto. + */ + virtual ProtobufTypes::MessagePtr createEmptyConfigProto() PURE; + /** * Returns the identifying name for a particular implementation of Stats::Sink produced by the * factory. @@ -132,7 +139,7 @@ class MainImpl : Logger::Loggable, public Main { */ void initializeTracers(const envoy::api::v2::Tracing& configuration, Instance& server); - void initializeStatsSinks(const Json::Object& configuration, Instance& server); + void initializeStatsSinks(const envoy::api::v2::Bootstrap& bootstrap, Instance& server); std::unique_ptr cluster_manager_; std::unique_ptr lds_api_; diff --git a/test/config/integration/server.json b/test/config/integration/server.json index 227119982c84..10bb0c2499b6 100644 --- a/test/config/integration/server.json +++ b/test/config/integration/server.json @@ -356,21 +356,8 @@ "profile_path": "{{ test_tmpdir }}/envoy.prof", "address": "tcp://{{ ip_loopback_address }}:0" }, "flags_path": "/invalid_flags", - - "stats_sinks": [ - { - "name": "statsd_udp", - "config": { - "ip_address": "{{ ip_loopback_address }}:8125" - } - }, - { - "name": "statsd_tcp", - "config": { - "cluster_name": "statsd" - } - } - ], + "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", + "statsd_tcp_cluster_name": "statsd", "lds": { "cluster": "lds" diff --git a/test/config/integration/server_grpc_json_transcoder.json b/test/config/integration/server_grpc_json_transcoder.json index 33159a9d7a3e..02e194552b7d 100644 --- a/test/config/integration/server_grpc_json_transcoder.json +++ b/test/config/integration/server_grpc_json_transcoder.json @@ -60,15 +60,7 @@ }], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - - "stats_sinks": [ - { - "name": "statsd_udp", - "config": { - "ip_address": "{{ ip_loopback_address }}:8125" - } - } - ], + "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", "cluster_manager": { "clusters": [ diff --git a/test/config/integration/server_http2.json b/test/config/integration/server_http2.json index 2468f7d48f83..6d1ce16ed95d 100644 --- a/test/config/integration/server_http2.json +++ b/test/config/integration/server_http2.json @@ -211,21 +211,8 @@ }], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - - "stats_sinks": [ - { - "name": "statsd_udp", - "config": { - "ip_address": "{{ ip_loopback_address }}:8125" - } - }, - { - "name": "statsd_tcp", - "config": { - "cluster_name": "statsd" - } - } - ], + "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", + "statsd_tcp_cluster_name": "statsd", "runtime": { "symlink_root": "{{ test_rundir }}/test/common/runtime/test_data/current", diff --git a/test/config/integration/server_proxy_proto.json b/test/config/integration/server_proxy_proto.json index 6d1d5c3a6483..5fbc4f5e441e 100644 --- a/test/config/integration/server_proxy_proto.json +++ b/test/config/integration/server_proxy_proto.json @@ -79,15 +79,7 @@ }], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - - "stats_sinks": [ - { - "name": "statsd_udp", - "config": { - "ip_address": "{{ ip_loopback_address }}:8125" - } - } - ], + "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", "cluster_manager": { "clusters": [ diff --git a/test/config/integration/server_ratelimit.json b/test/config/integration/server_ratelimit.json index 1a5c1eea21d0..50c591bf3c02 100644 --- a/test/config/integration/server_ratelimit.json +++ b/test/config/integration/server_ratelimit.json @@ -63,15 +63,7 @@ }], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - - "stats_sinks": [ - { - "name": "statsd_udp", - "config": { - "ip_address": "{{ ip_loopback_address }}:8125" - } - } - ], + "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", "rate_limit_service": { "type": "grpc_service", diff --git a/test/config/integration/server_ssl.json b/test/config/integration/server_ssl.json index 6b5f65ae215d..5966fceec122 100644 --- a/test/config/integration/server_ssl.json +++ b/test/config/integration/server_ssl.json @@ -90,15 +90,7 @@ }], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - - "stats_sinks": [ - { - "name": "statsd_udp", - "config": { - "ip_address": "{{ ip_loopback_address }}:8125" - } - } - ], + "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", "cluster_manager": { "clusters": [ diff --git a/test/config/integration/server_uds.json b/test/config/integration/server_uds.json index a75097880435..1a35f43fa00a 100644 --- a/test/config/integration/server_uds.json +++ b/test/config/integration/server_uds.json @@ -82,15 +82,7 @@ }], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - - "stats_sinks": [ - { - "name": "statsd_udp", - "config": { - "ip_address": "{{ ip_loopback_address }}:8125" - } - } - ], + "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", "cluster_manager": { "clusters": [ diff --git a/test/config/integration/server_xfcc.json b/test/config/integration/server_xfcc.json index e90468285c36..e965098e75d2 100644 --- a/test/config/integration/server_xfcc.json +++ b/test/config/integration/server_xfcc.json @@ -170,15 +170,7 @@ }] }], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - - "stats_sinks": [ - { - "name": "statsd_udp", - "config": { - "ip_address": "{{ ip_loopback_address }}:8125" - } - } - ], + "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", "cluster_manager": { "clusters": [ diff --git a/test/config/integration/tcp_proxy.json b/test/config/integration/tcp_proxy.json index 9286b2505c7c..555a4a191873 100644 --- a/test/config/integration/tcp_proxy.json +++ b/test/config/integration/tcp_proxy.json @@ -69,15 +69,7 @@ ] }], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - - "stats_sinks": [ - { - "name": "statsd_udp", - "config": { - "ip_address": "{{ ip_loopback_address }}:8125" - } - } - ], + "statsd_udp_ip_address": "{{ ip_loopback_address }}:8125", "cluster_manager": { "clusters": [ diff --git a/test/config_test/BUILD b/test/config_test/BUILD index e472f34905f3..af3e819682ff 100644 --- a/test/config_test/BUILD +++ b/test/config_test/BUILD @@ -22,8 +22,7 @@ envoy_cc_test( ], deps = [ ":config_test_lib", - "//source/server/config/stats:tcp_statsd_lib", - "//source/server/config/stats:udp_statsd_lib", + "//source/server/config/stats:statsd_lib", "//test/test_common:environment_lib", "//test/test_common:utility_lib", ], diff --git a/test/integration/BUILD b/test/integration/BUILD index c15f3d4d2fd0..00281f6ae4fc 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -176,8 +176,7 @@ envoy_cc_test_library( "//source/server/config/network:ratelimit_lib", "//source/server/config/network:redis_proxy_lib", "//source/server/config/network:tcp_proxy_lib", - "//source/server/config/stats:tcp_statsd_lib", - "//source/server/config/stats:udp_statsd_lib", + "//source/server/config/stats:statsd_lib", "//source/server/http:health_check_lib", "//test/mocks/buffer:buffer_mocks", "//test/mocks/upstream:upstream_mocks", @@ -201,8 +200,7 @@ envoy_cc_test( deps = [ ":integration_lib", "//source/common/http:header_map_lib", - "//source/server/config/stats:tcp_statsd_lib", - "//source/server/config/stats:udp_statsd_lib", + "//source/server/config/stats:statsd_lib", "//test/test_common:utility_lib", ], ) diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index f20c60b011bc..79a19b52aae7 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -299,12 +299,7 @@ class MockMain : public Main { MOCK_METHOD0(clusterManager, Upstream::ClusterManager&()); MOCK_METHOD0(httpTracer, Tracing::HttpTracer&()); MOCK_METHOD0(rateLimitClientFactory, RateLimit::ClientFactory&()); -<<<<<<< HEAD MOCK_METHOD0(statsSinks, std::list&()); -======= - MOCK_METHOD0(statsdTcpClusterName, Optional()); - MOCK_METHOD0(statsdUdpIpAddress, Network::Address::InstanceConstSharedPtr()); ->>>>>>> master MOCK_METHOD0(statsFlushInterval, std::chrono::milliseconds()); MOCK_CONST_METHOD0(wdMissTimeout, std::chrono::milliseconds()); MOCK_CONST_METHOD0(wdMegaMissTimeout, std::chrono::milliseconds()); diff --git a/test/server/BUILD b/test/server/BUILD index 658f6e2776a5..a2ec2da26f8c 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -23,8 +23,7 @@ envoy_cc_test( "//source/common/event:dispatcher_lib", "//source/common/upstream:cluster_manager_lib", "//source/server:configuration_lib", - "//source/server/config/stats:tcp_statsd_lib", - "//source/server/config/stats:udp_statsd_lib", + "//source/server/config/stats:statsd_lib", "//test/mocks:common_lib", "//test/mocks/network:network_mocks", "//test/mocks/server:server_mocks", @@ -121,8 +120,7 @@ envoy_cc_test( deps = [ "//source/common/common:version_lib", "//source/server:server_lib", - "//source/server/config/stats:tcp_statsd_lib", - "//source/server/config/stats:udp_statsd_lib", + "//source/server/config/stats:statsd_lib", "//test/integration:integration_lib", "//test/mocks/server:server_mocks", "//test/mocks/stats:stats_mocks", diff --git a/test/server/config/stats/BUILD b/test/server/config/stats/BUILD index 5be0ce805435..50818a5fc8a2 100644 --- a/test/server/config/stats/BUILD +++ b/test/server/config/stats/BUILD @@ -12,8 +12,8 @@ envoy_cc_test( name = "config_test", srcs = ["config_test.cc"], deps = [ - "//source/server/config/stats:tcp_statsd_lib", - "//source/server/config/stats:udp_statsd_lib", + "//source/common/stats:statsd_lib", + "//source/server/config/stats:statsd_lib", "//test/mocks/server:server_mocks", "//test/test_common:utility_lib", ], diff --git a/test/server/config/stats/config_test.cc b/test/server/config/stats/config_test.cc index 1e06355fa187..3f1895a3d224 100644 --- a/test/server/config/stats/config_test.cc +++ b/test/server/config/stats/config_test.cc @@ -2,11 +2,14 @@ #include "envoy/registry/registry.h" -#include "server/config/stats/tcp_statsd.h" -#include "server/config/stats/udp_statsd.h" +#include "common/protobuf/utility.h" +#include "common/stats/statsd.h" + +#include "server/config/stats/statsd.h" #include "test/mocks/server/mocks.h" +#include "api/bootstrap.pb.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -20,95 +23,71 @@ namespace Server { namespace Configuration { TEST(StatsConfigTest, ValidTcpStatsd) { - const std::string json_string = R"EOF( - { - "cluster_name" : "fake_cluster" - } - )EOF"; - const std::string name = "statsd_tcp"; - - Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(json_string); - NiceMock server; + const std::string name = "envoy.statsd"; + Protobuf::Struct config; + ProtobufWkt::Map& field_map = *config.mutable_fields(); + field_map["tcp_cluster_name"].set_string_value("fake_cluster"); + StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); ASSERT_NE(factory, nullptr); - Stats::SinkPtr sink = factory->createStatsSink(*json_config, server, server.clusterManager()); - EXPECT_NE(sink, nullptr); -} -TEST(StatsConfigTest, InvalidTcpStatsd) { - const std::string json_string = R"EOF( - {} - )EOF"; - const std::string name = "statsd_tcp"; + ProtobufTypes::MessagePtr message = factory->createEmptyConfigProto(); + MessageUtil::jsonConvert(config, *message); - Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(json_string); NiceMock server; - StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); - EXPECT_THROW(factory->createStatsSink(*json_config, server, server.clusterManager()), - EnvoyException); + Stats::SinkPtr sink = factory->createStatsSink(*message, server, server.clusterManager()); + EXPECT_NE(sink, nullptr); + EXPECT_NE(dynamic_cast(sink.get()), nullptr); } -TEST(StatsConfigTest, ValidUdpPortStatsd) { - const std::string json_string = R"EOF( - { - "local_port" : 8125 - } - )EOF"; - const std::string name = "statsd_udp"; +TEST(StatsConfigTest, ValidUdpIpStatsd) { + const std::string name = "envoy.statsd"; + Protobuf::Struct config; + ProtobufWkt::Map& field_map = *config.mutable_fields(); + + envoy::api::v2::Address address; + envoy::api::v2::SocketAddress& socket_address = *address.mutable_socket_address(); + socket_address.set_protocol(envoy::api::v2::SocketAddress::UDP); + socket_address.set_address("127.0.0.1"); + socket_address.set_port_value(8125); + + ProtobufWkt::Map& address_field_map = + *field_map["address"].mutable_struct_value()->mutable_fields(); + ProtobufWkt::Map& socket_address_field_map = + *address_field_map["socket_address"].mutable_struct_value()->mutable_fields(); + socket_address_field_map["protocol"].set_string_value("UDP"); + socket_address_field_map["address"].set_string_value("127.0.0.1"); + socket_address_field_map["port_value"].set_number_value(8125); - Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(json_string); - NiceMock server; StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); ASSERT_NE(factory, nullptr); - Stats::SinkPtr sink = factory->createStatsSink(*json_config, server, server.clusterManager()); - EXPECT_NE(sink, nullptr); -} -TEST(StatsConfigTest, ValidUdpIpStatsd) { - const std::string json_string = R"EOF( - { - "ip_address" : "127.0.0.1:8125" - } - )EOF"; - const std::string name = "statsd_udp"; - - Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(json_string); + ProtobufTypes::MessagePtr message = factory->createEmptyConfigProto(); + MessageUtil::jsonConvert(config, *message); + + // REMOVE THIS + auto& sink_proto = dynamic_cast(*message); + ASSERT_EQ(envoy::api::v2::SocketAddress::UDP, sink_proto.address().socket_address().protocol()); + ASSERT_EQ("127.0.0.1", sink_proto.address().socket_address().address()); + ASSERT_EQ(8125, sink_proto.address().socket_address().port_value()); + NiceMock server; - StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); - ASSERT_NE(factory, nullptr); - Stats::SinkPtr sink = factory->createStatsSink(*json_config, server, server.clusterManager()); + Stats::SinkPtr sink = factory->createStatsSink(*message, server, server.clusterManager()); EXPECT_NE(sink, nullptr); + EXPECT_NE(dynamic_cast(sink.get()), nullptr); } -TEST(StatsConfigTest, BadUdpEmptyConfig) { - const std::string json_string = R"EOF( - {} - )EOF"; - const std::string name = "statsd_udp"; +TEST(StatsConfigTest, EmptyConfig) { + const std::string name = "envoy.statsd"; + Protobuf::Struct config; - Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(json_string); - NiceMock server; StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); ASSERT_NE(factory, nullptr); - EXPECT_THROW(factory->createStatsSink(*json_config, server, server.clusterManager()), - EnvoyException); -} - -TEST(StatsConfigTest, BadUdpIpAndPortConfig) { - const std::string json_string = R"EOF( - { - "ip_address" : "127.0.0.1:8125", - "local_port" : 8125 - } - )EOF"; - const std::string name = "statsd_udp"; - Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(json_string); + ProtobufTypes::MessagePtr message = factory->createEmptyConfigProto(); + MessageUtil::jsonConvert(config, *message); NiceMock server; - StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); - ASSERT_NE(factory, nullptr); - EXPECT_THROW(factory->createStatsSink(*json_config, server, server.clusterManager()), - EnvoyException); + EXPECT_THROW(factory->createStatsSink(*message, server, server.clusterManager()), EnvoyException); } } // namespace Configuration diff --git a/test/server/configuration_impl_test.cc b/test/server/configuration_impl_test.cc index 94d0674dbca2..971a9ea9ce5f 100644 --- a/test/server/configuration_impl_test.cc +++ b/test/server/configuration_impl_test.cc @@ -252,41 +252,6 @@ TEST_F(ConfigurationImplTest, ConfigurationFailsWhenInvalidTracerSpecified) { EnvoyException, "No HttpTracerFactory found for type: invalid"); } -TEST_F(ConfigurationImplTest, DeprecatedStatsConfiguration) { - - std::string json = R"EOF( - { - "listeners": [], - - "cluster_manager": { - "clusters": [ - { - "name": "fake_cluster", - "type": "static", - "connect_timeout_ms": 1, - "per_connection_buffer_limit_bytes": 8192, - "lb_type": "round_robin", - "hosts": [ - { "url" : "tcp://127.0.0.1:9999" } - ] - } - ] - }, - - "statsd_udp_ip_address" : "127.0.0.1:8125", - "statsd_tcp_cluster_name" : "fake_cluster" - } - )EOF"; - - Json::ObjectSharedPtr loader = Json::Factory::loadFromString(json); - envoy::api::v2::Bootstrap bootstrap; - - MainImpl config; - config.initialize(*loader, bootstrap, server_, cluster_manager_factory_); - - EXPECT_EQ(2, config.statsSinks().size()); -} - } // namespace Configuration } // namespace Server } // namespace Envoy From 441b3a72d1354ea98d88c309193e1316b6fdf768 Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Fri, 1 Sep 2017 19:05:26 -0400 Subject: [PATCH 4/6] removed debug lines --- test/server/config/stats/config_test.cc | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/server/config/stats/config_test.cc b/test/server/config/stats/config_test.cc index 3f1895a3d224..490ed4f57ef9 100644 --- a/test/server/config/stats/config_test.cc +++ b/test/server/config/stats/config_test.cc @@ -65,12 +65,6 @@ TEST(StatsConfigTest, ValidUdpIpStatsd) { ProtobufTypes::MessagePtr message = factory->createEmptyConfigProto(); MessageUtil::jsonConvert(config, *message); - // REMOVE THIS - auto& sink_proto = dynamic_cast(*message); - ASSERT_EQ(envoy::api::v2::SocketAddress::UDP, sink_proto.address().socket_address().protocol()); - ASSERT_EQ("127.0.0.1", sink_proto.address().socket_address().address()); - ASSERT_EQ(8125, sink_proto.address().socket_address().port_value()); - NiceMock server; Stats::SinkPtr sink = factory->createStatsSink(*message, server, server.clusterManager()); EXPECT_NE(sink, nullptr); From d78014de7cf0ea9c08e7cf4bf3fe1fee2c41d0b4 Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Tue, 5 Sep 2017 11:48:33 -0400 Subject: [PATCH 5/6] Responded to comments and added additional tests --- source/server/config/stats/BUILD | 1 + source/server/config/stats/statsd.cc | 9 +-- source/server/config/stats/statsd.h | 3 +- source/server/configuration_impl.cc | 11 +-- source/server/configuration_impl.h | 7 +- .../access_log_manager_impl_test.cc | 2 +- test/common/common/utility_test.cc | 3 +- test/common/dynamo/dynamo_filter_test.cc | 2 +- test/common/dynamo/dynamo_utility_test.cc | 2 +- test/common/event/dispatcher_impl_test.cc | 2 +- .../common/filesystem/filesystem_impl_test.cc | 3 +- test/common/filter/auth/client_ssl_test.cc | 2 +- test/common/filter/ratelimit_test.cc | 2 +- test/common/filter/tcp_proxy_test.cc | 2 +- test/common/grpc/http1_bridge_filter_test.cc | 2 +- test/common/grpc/rpc_channel_impl_test.cc | 2 +- .../access_log/access_log_formatter_test.cc | 2 +- .../http/access_log/access_log_impl_test.cc | 2 +- test/common/http/async_client_impl_test.cc | 2 +- test/common/http/codec_client_test.cc | 2 +- test/common/http/codes_test.cc | 2 +- test/common/http/conn_manager_impl_test.cc | 2 +- test/common/http/conn_manager_utility_test.cc | 2 +- test/common/http/date_provider_impl_test.cc | 2 +- test/common/http/filter/buffer_filter_test.cc | 2 +- test/common/http/filter/fault_filter_test.cc | 2 +- test/common/http/filter/ratelimit_test.cc | 2 +- test/common/http/http1/codec_impl_test.cc | 2 +- test/common/http/http1/conn_pool_test.cc | 2 +- test/common/http/http2/conn_pool_test.cc | 2 +- test/common/json/config_schemas_test.cc | 2 +- test/common/mongo/codec_impl_test.cc | 2 +- test/common/network/connection_impl_test.cc | 2 +- .../network/filter_manager_impl_test.cc | 2 +- test/common/network/listener_impl_test.cc | 2 +- test/common/network/proxy_protocol_test.cc | 2 +- test/common/ratelimit/ratelimit_impl_test.cc | 2 +- .../redis/command_splitter_impl_test.cc | 2 +- test/common/redis/conn_pool_impl_test.cc | 2 +- test/common/router/config_impl_test.cc | 2 +- test/common/router/rds_impl_test.cc | 2 +- test/common/router/retry_state_impl_test.cc | 2 +- test/common/router/router_ratelimit_test.cc | 2 +- test/common/router/router_test.cc | 2 +- test/common/router/shadow_writer_impl_test.cc | 2 +- test/common/runtime/runtime_impl_test.cc | 2 +- test/common/ssl/connection_impl_test.cc | 2 +- test/common/stats/thread_local_store_test.cc | 2 +- test/common/stats/udp_statsd_test.cc | 2 +- .../upstream/load_balancer_impl_test.cc | 2 +- .../upstream/load_balancer_simulation_test.cc | 2 +- .../upstream/logical_dns_cluster_test.cc | 2 +- .../upstream/outlier_detection_impl_test.cc | 2 +- .../upstream/resource_manager_impl_test.cc | 2 +- test/common/upstream/ring_hash_lb_test.cc | 2 +- test/common/upstream/sds_test.cc | 2 +- test/mocks/access_log/mocks.cc | 2 +- test/mocks/api/mocks.cc | 2 +- test/mocks/event/mocks.cc | 2 +- test/mocks/http/mocks.cc | 2 +- test/mocks/init/mocks.cc | 2 +- test/mocks/local_info/mocks.cc | 2 +- test/mocks/network/mocks.cc | 2 +- test/mocks/redis/mocks.cc | 2 +- test/mocks/router/mocks.cc | 2 +- test/mocks/runtime/mocks.cc | 2 +- test/mocks/stats/mocks.cc | 2 +- test/mocks/thread_local/mocks.cc | 2 +- test/mocks/tracing/mocks.cc | 2 +- test/mocks/upstream/cluster_info.h | 2 +- test/mocks/upstream/mocks.cc | 2 +- test/mocks/upstream/mocks.h | 2 +- test/server/config/http/config_test.cc | 2 +- test/server/config/stats/BUILD | 5 ++ test/server/config/stats/config_test.cc | 23 ++---- test/server/configuration_impl_test.cc | 78 ++++++++++++++++++- test/server/guarddog_impl_test.cc | 2 +- test/server/http/admin_test.cc | 2 +- test/server/http/health_check_test.cc | 3 +- 79 files changed, 176 insertions(+), 106 deletions(-) diff --git a/source/server/config/stats/BUILD b/source/server/config/stats/BUILD index 13b9cedd11d2..a056b0b0df13 100644 --- a/source/server/config/stats/BUILD +++ b/source/server/config/stats/BUILD @@ -16,6 +16,7 @@ envoy_cc_library( "envoy_bootstrap", ], deps = [ + "//include/envoy/registry", "//source/common/network:address_lib", "//source/common/stats:statsd_lib", "//source/server:configuration_lib", diff --git a/source/server/config/stats/statsd.cc b/source/server/config/stats/statsd.cc index 283c6ca919fa..712c8e70ef17 100644 --- a/source/server/config/stats/statsd.cc +++ b/source/server/config/stats/statsd.cc @@ -13,8 +13,7 @@ namespace Server { namespace Configuration { Stats::SinkPtr StatsdSinkFactory::createStatsSink(const Protobuf::Message& config, - Server::Instance& server, - Upstream::ClusterManager& cluster_manager) { + Server::Instance& server) { const auto& statsd_sink = dynamic_cast(config); switch (statsd_sink.statsd_specifier_case()) { @@ -28,9 +27,9 @@ Stats::SinkPtr StatsdSinkFactory::createStatsSink(const Protobuf::Message& confi } case envoy::api::v2::StatsdSink::kTcpClusterName: ENVOY_LOG(info, "statsd TCP cluster: {}", statsd_sink.tcp_cluster_name()); - return Stats::SinkPtr( - new Stats::Statsd::TcpStatsdSink(server.localInfo(), statsd_sink.tcp_cluster_name(), - server.threadLocal(), cluster_manager, server.stats())); + return Stats::SinkPtr(new Stats::Statsd::TcpStatsdSink( + server.localInfo(), statsd_sink.tcp_cluster_name(), server.threadLocal(), + server.clusterManager(), server.stats())); break; default: throw EnvoyException( diff --git a/source/server/config/stats/statsd.h b/source/server/config/stats/statsd.h index 943344769184..8569b283dd67 100644 --- a/source/server/config/stats/statsd.h +++ b/source/server/config/stats/statsd.h @@ -16,8 +16,7 @@ namespace Configuration { class StatsdSinkFactory : Logger::Loggable, public StatsSinkFactory { public: // StatsSinkFactory - Stats::SinkPtr createStatsSink(const Protobuf::Message& config, Instance& server, - Upstream::ClusterManager& cluster_manager) override; + Stats::SinkPtr createStatsSink(const Protobuf::Message& config, Instance& server) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; diff --git a/source/server/configuration_impl.cc b/source/server/configuration_impl.cc index 24d4788ed62d..81870bfeb1ff 100644 --- a/source/server/configuration_impl.cc +++ b/source/server/configuration_impl.cc @@ -119,17 +119,14 @@ void MainImpl::initializeStatsSinks(const envoy::api::v2::Bootstrap& bootstrap, "sink object does not have 'name' attribute to look up the implementation"); } - if (!sink_object.has_config()) { - throw EnvoyException( - "sink object does not contain the 'config' object to configure the implementation"); - } - ProtobufTypes::String name = sink_object.name(); StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); if (factory != nullptr) { ProtobufTypes::MessagePtr message = factory->createEmptyConfigProto(); - MessageUtil::jsonConvert(sink_object.config(), *message); - stats_sinks_.emplace_back(factory->createStatsSink(*message, server, *cluster_manager_)); + if (sink_object.has_config()) { + MessageUtil::jsonConvert(sink_object.config(), *message); + } + stats_sinks_.emplace_back(factory->createStatsSink(*message, server)); } else { throw EnvoyException(fmt::format("No Stats::Sink found for name: {}", name)); } diff --git a/source/server/configuration_impl.h b/source/server/configuration_impl.h index 46512cc2d415..ee06faf824d5 100644 --- a/source/server/configuration_impl.h +++ b/source/server/configuration_impl.h @@ -65,15 +65,12 @@ class StatsSinkFactory { /** * Create a particular Stats::Sink implementation. If the implementation is unable to produce a - * Stats::Sink with the provided parameters, it should throw an EnvoyException in the case of - * general error or a Json::Exception if the json configuration is erroneous. The returned + * Stats::Sink with the provided parameters, it should throw an EnvoyException. The returned * pointer should always be valid. * @param config supplies the custom proto configuration for the Stats::Sink * @param server supplies the server instance - * @param cluster_manager supplies the cluster_manager instance */ - virtual Stats::SinkPtr createStatsSink(const Protobuf::Message& config, Instance& server, - Upstream::ClusterManager& cluster_manager) PURE; + virtual Stats::SinkPtr createStatsSink(const Protobuf::Message& config, Instance& server) PURE; /** * @return ProtobufTypes::MessagePtr create empty config proto message for v2. The filter diff --git a/test/common/access_log/access_log_manager_impl_test.cc b/test/common/access_log/access_log_manager_impl_test.cc index a296247d75b2..063703707431 100644 --- a/test/common/access_log/access_log_manager_impl_test.cc +++ b/test/common/access_log/access_log_manager_impl_test.cc @@ -11,10 +11,10 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Return; using testing::_; +namespace Envoy { namespace AccessLog { TEST(AccessLogManagerImpl, reopenAllFiles) { diff --git a/test/common/common/utility_test.cc b/test/common/common/utility_test.cc index 5b253af7c020..07c41a3f5d0f 100644 --- a/test/common/common/utility_test.cc +++ b/test/common/common/utility_test.cc @@ -8,9 +8,10 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::ContainerEq; +namespace Envoy { + TEST(StringUtil, atoul) { uint64_t out; EXPECT_FALSE(StringUtil::atoul("123b", out)); diff --git a/test/common/dynamo/dynamo_filter_test.cc b/test/common/dynamo/dynamo_filter_test.cc index 3ff5289b5094..5e949fc87d48 100644 --- a/test/common/dynamo/dynamo_filter_test.cc +++ b/test/common/dynamo/dynamo_filter_test.cc @@ -14,12 +14,12 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; using testing::Return; using testing::ReturnRef; using testing::_; +namespace Envoy { namespace Dynamo { class DynamoFilterTest : public testing::Test { diff --git a/test/common/dynamo/dynamo_utility_test.cc b/test/common/dynamo/dynamo_utility_test.cc index 089e35423ff0..f5c87b83a204 100644 --- a/test/common/dynamo/dynamo_utility_test.cc +++ b/test/common/dynamo/dynamo_utility_test.cc @@ -6,9 +6,9 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::_; +namespace Envoy { namespace Dynamo { TEST(DynamoUtility, PartitionIdStatString) { diff --git a/test/common/event/dispatcher_impl_test.cc b/test/common/event/dispatcher_impl_test.cc index 2c9ee6db8685..531909a4eee1 100644 --- a/test/common/event/dispatcher_impl_test.cc +++ b/test/common/event/dispatcher_impl_test.cc @@ -7,9 +7,9 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::InSequence; +namespace Envoy { namespace Event { class TestDeferredDeletable : public DeferredDeletable { diff --git a/test/common/filesystem/filesystem_impl_test.cc b/test/common/filesystem/filesystem_impl_test.cc index de25c6b8a21b..f8aa8b0bbda3 100644 --- a/test/common/filesystem/filesystem_impl_test.cc +++ b/test/common/filesystem/filesystem_impl_test.cc @@ -13,7 +13,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::InSequence; using testing::Invoke; using testing::NiceMock; @@ -23,6 +22,8 @@ using testing::Sequence; using testing::Throw; using testing::_; +namespace Envoy { + TEST(FileSystemImpl, BadFile) { Event::MockDispatcher dispatcher; Thread::MutexBasicLockable lock; diff --git a/test/common/filter/auth/client_ssl_test.cc b/test/common/filter/auth/client_ssl_test.cc index b673986a94b1..72728add83e7 100644 --- a/test/common/filter/auth/client_ssl_test.cc +++ b/test/common/filter/auth/client_ssl_test.cc @@ -19,7 +19,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::InSequence; using testing::Invoke; using testing::Return; @@ -28,6 +27,7 @@ using testing::ReturnRef; using testing::WithArg; using testing::_; +namespace Envoy { namespace Filter { namespace Auth { namespace ClientSsl { diff --git a/test/common/filter/ratelimit_test.cc b/test/common/filter/ratelimit_test.cc index 1e7c3e102160..5c305845f0ae 100644 --- a/test/common/filter/ratelimit_test.cc +++ b/test/common/filter/ratelimit_test.cc @@ -15,7 +15,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::InSequence; using testing::Invoke; using testing::NiceMock; @@ -23,6 +22,7 @@ using testing::Return; using testing::WithArgs; using testing::_; +namespace Envoy { namespace RateLimit { namespace TcpFilter { diff --git a/test/common/filter/tcp_proxy_test.cc b/test/common/filter/tcp_proxy_test.cc index 6b9427e04e6c..490dd81791c8 100644 --- a/test/common/filter/tcp_proxy_test.cc +++ b/test/common/filter/tcp_proxy_test.cc @@ -18,13 +18,13 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; using testing::Return; using testing::ReturnRef; using testing::SaveArg; using testing::_; +namespace Envoy { namespace Filter { TEST(TcpProxyConfigTest, NoRouteConfig) { diff --git a/test/common/grpc/http1_bridge_filter_test.cc b/test/common/grpc/http1_bridge_filter_test.cc index ac8c1aa01902..4e589cca0147 100644 --- a/test/common/grpc/http1_bridge_filter_test.cc +++ b/test/common/grpc/http1_bridge_filter_test.cc @@ -10,13 +10,13 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; using testing::Return; using testing::ReturnPointee; using testing::ReturnRef; using testing::_; +namespace Envoy { namespace Grpc { class GrpcHttp1BridgeFilterTest : public testing::Test { diff --git a/test/common/grpc/rpc_channel_impl_test.cc b/test/common/grpc/rpc_channel_impl_test.cc index 00f7c08ef6b0..496a6a58a76e 100644 --- a/test/common/grpc/rpc_channel_impl_test.cc +++ b/test/common/grpc/rpc_channel_impl_test.cc @@ -15,11 +15,11 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Invoke; using testing::Return; using testing::_; +namespace Envoy { namespace Grpc { class GrpcRequestImplTest : public testing::Test { diff --git a/test/common/http/access_log/access_log_formatter_test.cc b/test/common/http/access_log/access_log_formatter_test.cc index 2598896fc672..fee052926a30 100644 --- a/test/common/http/access_log/access_log_formatter_test.cc +++ b/test/common/http/access_log/access_log_formatter_test.cc @@ -15,12 +15,12 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; using testing::Return; using testing::ReturnRef; using testing::_; +namespace Envoy { namespace Http { namespace AccessLog { diff --git a/test/common/http/access_log/access_log_impl_test.cc b/test/common/http/access_log/access_log_impl_test.cc index 6a6332cdd55c..dff821985bb0 100644 --- a/test/common/http/access_log/access_log_impl_test.cc +++ b/test/common/http/access_log/access_log_impl_test.cc @@ -28,12 +28,12 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; using testing::Return; using testing::SaveArg; using testing::_; +namespace Envoy { namespace Http { namespace AccessLog { namespace { diff --git a/test/common/http/async_client_impl_test.cc b/test/common/http/async_client_impl_test.cc index e9af44130db1..1643c7ee7074 100644 --- a/test/common/http/async_client_impl_test.cc +++ b/test/common/http/async_client_impl_test.cc @@ -21,7 +21,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Invoke; using testing::NiceMock; using testing::Ref; @@ -30,6 +29,7 @@ using testing::ReturnRef; using testing::ReturnRefOfCopy; using testing::_; +namespace Envoy { namespace Http { class AsyncClientImplTest : public testing::Test { diff --git a/test/common/http/codec_client_test.cc b/test/common/http/codec_client_test.cc index 0d0121c05dff..65203e6a125a 100644 --- a/test/common/http/codec_client_test.cc +++ b/test/common/http/codec_client_test.cc @@ -18,7 +18,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Invoke; using testing::NiceMock; using testing::Pointee; @@ -28,6 +27,7 @@ using testing::SaveArg; using testing::Throw; using testing::_; +namespace Envoy { namespace Http { class CodecClientTest : public testing::Test { diff --git a/test/common/http/codes_test.cc b/test/common/http/codes_test.cc index bb04e7d06376..d22bf2e505b5 100644 --- a/test/common/http/codes_test.cc +++ b/test/common/http/codes_test.cc @@ -15,9 +15,9 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::_; +namespace Envoy { namespace Http { class CodeUtilityTest : public testing::Test { diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index aeeda6303f22..68a06792c5cc 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -36,7 +36,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::AnyNumber; using testing::AtLeast; using testing::InSequence; @@ -50,6 +49,7 @@ using testing::Sequence; using testing::Test; using testing::_; +namespace Envoy { namespace Http { class HttpConnectionManagerImplTest : public Test, public ConnectionManagerConfig { diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index b609b6c64066..868516a2f764 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -18,13 +18,13 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::InSequence; using testing::NiceMock; using testing::Return; using testing::ReturnRef; using testing::_; +namespace Envoy { namespace Http { class ConnectionManagerUtilityTest : public testing::Test { diff --git a/test/common/http/date_provider_impl_test.cc b/test/common/http/date_provider_impl_test.cc index 29df9ff7e614..ea037ee0f8d9 100644 --- a/test/common/http/date_provider_impl_test.cc +++ b/test/common/http/date_provider_impl_test.cc @@ -10,9 +10,9 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; +namespace Envoy { namespace Http { TEST(DateProviderImplTest, All) { diff --git a/test/common/http/filter/buffer_filter_test.cc b/test/common/http/filter/buffer_filter_test.cc index 405e3d05d289..135f427c4eaf 100644 --- a/test/common/http/filter/buffer_filter_test.cc +++ b/test/common/http/filter/buffer_filter_test.cc @@ -14,7 +14,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::DoAll; using testing::InSequence; using testing::NiceMock; @@ -22,6 +21,7 @@ using testing::Return; using testing::SaveArg; using testing::_; +namespace Envoy { namespace Http { class BufferFilterTest : public testing::Test { diff --git a/test/common/http/filter/fault_filter_test.cc b/test/common/http/filter/fault_filter_test.cc index 7b2dc745cd9e..7198605eafda 100644 --- a/test/common/http/filter/fault_filter_test.cc +++ b/test/common/http/filter/fault_filter_test.cc @@ -21,7 +21,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::DoAll; using testing::Invoke; using testing::NiceMock; @@ -30,6 +29,7 @@ using testing::ReturnRef; using testing::WithArgs; using testing::_; +namespace Envoy { namespace Http { class FaultFilterTest : public testing::Test { diff --git a/test/common/http/filter/ratelimit_test.cc b/test/common/http/filter/ratelimit_test.cc index b9e76dae3a09..f5227649eb97 100644 --- a/test/common/http/filter/ratelimit_test.cc +++ b/test/common/http/filter/ratelimit_test.cc @@ -19,7 +19,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::InSequence; using testing::Invoke; using testing::NiceMock; @@ -29,6 +28,7 @@ using testing::SetArgReferee; using testing::WithArgs; using testing::_; +namespace Envoy { namespace Http { namespace RateLimit { diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc index 5d074564b6bd..69f71cfda1c1 100644 --- a/test/common/http/http1/codec_impl_test.cc +++ b/test/common/http/http1/codec_impl_test.cc @@ -16,7 +16,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::InSequence; using testing::Invoke; using testing::NiceMock; @@ -24,6 +23,7 @@ using testing::Return; using testing::ReturnRef; using testing::_; +namespace Envoy { namespace Http { namespace Http1 { diff --git a/test/common/http/http1/conn_pool_test.cc b/test/common/http/http1/conn_pool_test.cc index 9044d6b90488..b1ac8fc24455 100644 --- a/test/common/http/http1/conn_pool_test.cc +++ b/test/common/http/http1/conn_pool_test.cc @@ -20,7 +20,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::DoAll; using testing::InSequence; using testing::Invoke; @@ -30,6 +29,7 @@ using testing::ReturnRef; using testing::SaveArg; using testing::_; +namespace Envoy { namespace Http { namespace Http1 { diff --git a/test/common/http/http2/conn_pool_test.cc b/test/common/http/http2/conn_pool_test.cc index 1b304bb670e2..8f19bcad297b 100644 --- a/test/common/http/http2/conn_pool_test.cc +++ b/test/common/http/http2/conn_pool_test.cc @@ -17,7 +17,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::DoAll; using testing::InSequence; using testing::Invoke; @@ -27,6 +26,7 @@ using testing::ReturnRef; using testing::SaveArg; using testing::_; +namespace Envoy { namespace Http { namespace Http2 { diff --git a/test/common/json/config_schemas_test.cc b/test/common/json/config_schemas_test.cc index 367ff0a3f223..d01c63778f08 100644 --- a/test/common/json/config_schemas_test.cc +++ b/test/common/json/config_schemas_test.cc @@ -11,9 +11,9 @@ #include "gtest/gtest.h" #include "spdlog/spdlog.h" -namespace Envoy { using testing::_; +namespace Envoy { namespace Json { std::vector generateTestInputs() { diff --git a/test/common/mongo/codec_impl_test.cc b/test/common/mongo/codec_impl_test.cc index 0d6e4a94b2ab..fd38c085ed35 100644 --- a/test/common/mongo/codec_impl_test.cc +++ b/test/common/mongo/codec_impl_test.cc @@ -10,11 +10,11 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Eq; using testing::NiceMock; using testing::Pointee; +namespace Envoy { namespace Mongo { class TestDecoderCallbacks : public DecoderCallbacks { diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc index 7b9d652a3e19..7186df375696 100644 --- a/test/common/network/connection_impl_test.cc +++ b/test/common/network/connection_impl_test.cc @@ -24,7 +24,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::AnyNumber; using testing::DoAll; using testing::InSequence; @@ -35,6 +34,7 @@ using testing::StrictMock; using testing::Test; using testing::_; +namespace Envoy { namespace Network { TEST(ConnectionImplUtility, updateBufferStats) { diff --git a/test/common/network/filter_manager_impl_test.cc b/test/common/network/filter_manager_impl_test.cc index bf3670e03666..7d2bd8504d19 100644 --- a/test/common/network/filter_manager_impl_test.cc +++ b/test/common/network/filter_manager_impl_test.cc @@ -20,7 +20,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::InSequence; using testing::Invoke; using testing::NiceMock; @@ -28,6 +27,7 @@ using testing::Return; using testing::WithArgs; using testing::_; +namespace Envoy { namespace Network { class NetworkFilterManagerTest : public testing::Test, public BufferSource { diff --git a/test/common/network/listener_impl_test.cc b/test/common/network/listener_impl_test.cc index b96f579f0a80..a6512e9a84a0 100644 --- a/test/common/network/listener_impl_test.cc +++ b/test/common/network/listener_impl_test.cc @@ -11,13 +11,13 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::ByRef; using testing::Eq; using testing::Invoke; using testing::Return; using testing::_; +namespace Envoy { namespace Network { static void errorCallbackTest(Address::IpVersion version) { diff --git a/test/common/network/proxy_protocol_test.cc b/test/common/network/proxy_protocol_test.cc index ec0aa3cc5c87..1047cb8ff4ca 100644 --- a/test/common/network/proxy_protocol_test.cc +++ b/test/common/network/proxy_protocol_test.cc @@ -19,11 +19,11 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Invoke; using testing::NiceMock; using testing::_; +namespace Envoy { namespace Network { class ProxyProtocolTest : public testing::TestWithParam { diff --git a/test/common/ratelimit/ratelimit_impl_test.cc b/test/common/ratelimit/ratelimit_impl_test.cc index 285146fd0fd5..ee3218d90b85 100644 --- a/test/common/ratelimit/ratelimit_impl_test.cc +++ b/test/common/ratelimit/ratelimit_impl_test.cc @@ -14,7 +14,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::AtLeast; using testing::Invoke; using testing::Ref; @@ -22,6 +21,7 @@ using testing::Return; using testing::WithArg; using testing::_; +namespace Envoy { namespace RateLimit { class MockRequestCallbacks : public RequestCallbacks { diff --git a/test/common/redis/command_splitter_impl_test.cc b/test/common/redis/command_splitter_impl_test.cc index 2c20a6967b9c..c38f03278be2 100644 --- a/test/common/redis/command_splitter_impl_test.cc +++ b/test/common/redis/command_splitter_impl_test.cc @@ -14,7 +14,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::ByRef; using testing::DoAll; using testing::Eq; @@ -24,6 +23,7 @@ using testing::Return; using testing::WithArg; using testing::_; +namespace Envoy { namespace Redis { namespace CommandSplitter { diff --git a/test/common/redis/conn_pool_impl_test.cc b/test/common/redis/conn_pool_impl_test.cc index ec946759bc62..bc899a102355 100644 --- a/test/common/redis/conn_pool_impl_test.cc +++ b/test/common/redis/conn_pool_impl_test.cc @@ -14,7 +14,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Eq; using testing::InSequence; using testing::Invoke; @@ -24,6 +23,7 @@ using testing::ReturnRef; using testing::SaveArg; using testing::_; +namespace Envoy { namespace Redis { namespace ConnPool { diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 119c1ae9b5dd..a1c417fce2bf 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -18,7 +18,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::ContainerEq; using testing::NiceMock; using testing::Return; @@ -26,6 +25,7 @@ using testing::ReturnRef; using testing::StrNe; using testing::_; +namespace Envoy { namespace Router { namespace { diff --git a/test/common/router/rds_impl_test.cc b/test/common/router/rds_impl_test.cc index 66153c9074f8..889ea0f07774 100644 --- a/test/common/router/rds_impl_test.cc +++ b/test/common/router/rds_impl_test.cc @@ -20,7 +20,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::InSequence; using testing::Invoke; using testing::Return; @@ -28,6 +27,7 @@ using testing::ReturnRef; using testing::SaveArg; using testing::_; +namespace Envoy { namespace Router { namespace { diff --git a/test/common/router/retry_state_impl_test.cc b/test/common/router/retry_state_impl_test.cc index 5e21da3af5e1..bb35555d6ef9 100644 --- a/test/common/router/retry_state_impl_test.cc +++ b/test/common/router/retry_state_impl_test.cc @@ -13,11 +13,11 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; using testing::Return; using testing::_; +namespace Envoy { namespace Router { class RouterRetryStateImplTest : public testing::Test { diff --git a/test/common/router/router_ratelimit_test.cc b/test/common/router/router_ratelimit_test.cc index 5f3d5f5da507..454f795244bd 100644 --- a/test/common/router/router_ratelimit_test.cc +++ b/test/common/router/router_ratelimit_test.cc @@ -18,11 +18,11 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; using testing::ReturnRef; using testing::_; +namespace Envoy { namespace Router { namespace { diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index ef180489ecfd..50517563664a 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -20,7 +20,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::AtLeast; using testing::Invoke; using testing::NiceMock; @@ -29,6 +28,7 @@ using testing::ReturnRef; using testing::SaveArg; using testing::_; +namespace Envoy { namespace Router { class TestFilter : public Filter { diff --git a/test/common/router/shadow_writer_impl_test.cc b/test/common/router/shadow_writer_impl_test.cc index 942eac5bac25..e5b1f3bd827b 100644 --- a/test/common/router/shadow_writer_impl_test.cc +++ b/test/common/router/shadow_writer_impl_test.cc @@ -10,10 +10,10 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Invoke; using testing::_; +namespace Envoy { namespace Router { TEST(ShadowWriterImplTest, All) { diff --git a/test/common/runtime/runtime_impl_test.cc b/test/common/runtime/runtime_impl_test.cc index 6802d3ab6d81..f7315a34c67e 100644 --- a/test/common/runtime/runtime_impl_test.cc +++ b/test/common/runtime/runtime_impl_test.cc @@ -13,11 +13,11 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; using testing::Return; using testing::ReturnNew; +namespace Envoy { namespace Runtime { TEST(UUID, checkLengthOfUUID) { diff --git a/test/common/ssl/connection_impl_test.cc b/test/common/ssl/connection_impl_test.cc index 09098ecaab9a..310f89c920ca 100644 --- a/test/common/ssl/connection_impl_test.cc +++ b/test/common/ssl/connection_impl_test.cc @@ -27,11 +27,11 @@ #include "gtest/gtest.h" #include "openssl/ssl.h" -namespace Envoy { using testing::Invoke; using testing::StrictMock; using testing::_; +namespace Envoy { namespace Ssl { namespace { diff --git a/test/common/stats/thread_local_store_test.cc b/test/common/stats/thread_local_store_test.cc index 6e43fc6e8847..28a473b2cd72 100644 --- a/test/common/stats/thread_local_store_test.cc +++ b/test/common/stats/thread_local_store_test.cc @@ -13,13 +13,13 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::InSequence; using testing::Invoke; using testing::NiceMock; using testing::Return; using testing::_; +namespace Envoy { namespace Stats { /** diff --git a/test/common/stats/udp_statsd_test.cc b/test/common/stats/udp_statsd_test.cc index 2a8a88bac299..db13ee24cd0b 100644 --- a/test/common/stats/udp_statsd_test.cc +++ b/test/common/stats/udp_statsd_test.cc @@ -12,9 +12,9 @@ #include "gtest/gtest.h" #include "spdlog/spdlog.h" -namespace Envoy { using testing::NiceMock; +namespace Envoy { namespace Stats { namespace Statsd { diff --git a/test/common/upstream/load_balancer_impl_test.cc b/test/common/upstream/load_balancer_impl_test.cc index 0fa4db70c675..19c41f05b596 100644 --- a/test/common/upstream/load_balancer_impl_test.cc +++ b/test/common/upstream/load_balancer_impl_test.cc @@ -13,10 +13,10 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; using testing::Return; +namespace Envoy { namespace Upstream { static HostSharedPtr newTestHost(Upstream::ClusterInfoConstSharedPtr cluster, diff --git a/test/common/upstream/load_balancer_simulation_test.cc b/test/common/upstream/load_balancer_simulation_test.cc index 4391ad700330..3fcee22e6102 100644 --- a/test/common/upstream/load_balancer_simulation_test.cc +++ b/test/common/upstream/load_balancer_simulation_test.cc @@ -14,10 +14,10 @@ #include "gtest/gtest.h" #include "spdlog/spdlog.h" -namespace Envoy { using testing::NiceMock; using testing::Return; +namespace Envoy { namespace Upstream { static HostSharedPtr newTestHost(Upstream::ClusterInfoConstSharedPtr cluster, diff --git a/test/common/upstream/logical_dns_cluster_test.cc b/test/common/upstream/logical_dns_cluster_test.cc index 00b9c2121088..2f5024daf5f6 100644 --- a/test/common/upstream/logical_dns_cluster_test.cc +++ b/test/common/upstream/logical_dns_cluster_test.cc @@ -19,11 +19,11 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Invoke; using testing::NiceMock; using testing::_; +namespace Envoy { namespace Upstream { class LogicalDnsClusterTest : public testing::Test { diff --git a/test/common/upstream/outlier_detection_impl_test.cc b/test/common/upstream/outlier_detection_impl_test.cc index e837e4482487..7e1a7dc2621e 100644 --- a/test/common/upstream/outlier_detection_impl_test.cc +++ b/test/common/upstream/outlier_detection_impl_test.cc @@ -20,13 +20,13 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; using testing::Return; using testing::ReturnRef; using testing::SaveArg; using testing::_; +namespace Envoy { namespace Upstream { namespace Outlier { diff --git a/test/common/upstream/resource_manager_impl_test.cc b/test/common/upstream/resource_manager_impl_test.cc index a4fc743690d3..792f442a4bb3 100644 --- a/test/common/upstream/resource_manager_impl_test.cc +++ b/test/common/upstream/resource_manager_impl_test.cc @@ -5,10 +5,10 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; using testing::Return; +namespace Envoy { namespace Upstream { TEST(ResourceManagerImplTest, RuntimeResourceManager) { diff --git a/test/common/upstream/ring_hash_lb_test.cc b/test/common/upstream/ring_hash_lb_test.cc index 1b03a20e0e48..77af034eb822 100644 --- a/test/common/upstream/ring_hash_lb_test.cc +++ b/test/common/upstream/ring_hash_lb_test.cc @@ -11,11 +11,11 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; using testing::Return; using testing::_; +namespace Envoy { namespace Upstream { static HostSharedPtr newTestHost(Upstream::ClusterInfoConstSharedPtr cluster, diff --git a/test/common/upstream/sds_test.cc b/test/common/upstream/sds_test.cc index 00ac0faddf3e..110fc0223e27 100644 --- a/test/common/upstream/sds_test.cc +++ b/test/common/upstream/sds_test.cc @@ -24,7 +24,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::DoAll; using testing::InSequence; using testing::Invoke; @@ -34,6 +33,7 @@ using testing::SaveArg; using testing::WithArg; using testing::_; +namespace Envoy { namespace Upstream { class SdsTest : public testing::Test { diff --git a/test/mocks/access_log/mocks.cc b/test/mocks/access_log/mocks.cc index 0901d1eb726c..2d8d7aa429b6 100644 --- a/test/mocks/access_log/mocks.cc +++ b/test/mocks/access_log/mocks.cc @@ -3,10 +3,10 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Return; using testing::_; +namespace Envoy { namespace AccessLog { MockAccessLogManager::MockAccessLogManager() { diff --git a/test/mocks/api/mocks.cc b/test/mocks/api/mocks.cc index 6f1eeb99925d..6e28b2b708f8 100644 --- a/test/mocks/api/mocks.cc +++ b/test/mocks/api/mocks.cc @@ -3,10 +3,10 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Return; using testing::_; +namespace Envoy { namespace Api { MockApi::MockApi() { ON_CALL(*this, createFile(_, _, _, _)).WillByDefault(Return(file_)); } diff --git a/test/mocks/event/mocks.cc b/test/mocks/event/mocks.cc index 72642f456a18..aa4d78472a1a 100644 --- a/test/mocks/event/mocks.cc +++ b/test/mocks/event/mocks.cc @@ -3,7 +3,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Invoke; using testing::NiceMock; using testing::Return; @@ -11,6 +10,7 @@ using testing::ReturnNew; using testing::SaveArg; using testing::_; +namespace Envoy { namespace Event { MockDispatcher::MockDispatcher() { diff --git a/test/mocks/http/mocks.cc b/test/mocks/http/mocks.cc index a1ec0215c340..6a571cfbb52b 100644 --- a/test/mocks/http/mocks.cc +++ b/test/mocks/http/mocks.cc @@ -6,13 +6,13 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Invoke; using testing::Return; using testing::ReturnRef; using testing::SaveArg; using testing::_; +namespace Envoy { namespace Http { MockConnectionManagerConfig::MockConnectionManagerConfig() { diff --git a/test/mocks/init/mocks.cc b/test/mocks/init/mocks.cc index a62ab110442e..c8ddb6b96ca9 100644 --- a/test/mocks/init/mocks.cc +++ b/test/mocks/init/mocks.cc @@ -5,10 +5,10 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Invoke; using testing::_; +namespace Envoy { namespace Init { MockTarget::MockTarget() { diff --git a/test/mocks/local_info/mocks.cc b/test/mocks/local_info/mocks.cc index e0782542578e..b98d86e06da3 100644 --- a/test/mocks/local_info/mocks.cc +++ b/test/mocks/local_info/mocks.cc @@ -5,11 +5,11 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Return; using testing::ReturnPointee; using testing::ReturnRef; +namespace Envoy { namespace LocalInfo { MockLocalInfo::MockLocalInfo() : address_(new Network::Address::Ipv4Instance("127.0.0.1")) { diff --git a/test/mocks/network/mocks.cc b/test/mocks/network/mocks.cc index 4bfad2ecb387..9fab8d27cde3 100644 --- a/test/mocks/network/mocks.cc +++ b/test/mocks/network/mocks.cc @@ -12,7 +12,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Invoke; using testing::Return; using testing::ReturnPointee; @@ -20,6 +19,7 @@ using testing::ReturnRef; using testing::SaveArg; using testing::_; +namespace Envoy { namespace Network { MockConnectionCallbacks::MockConnectionCallbacks() {} diff --git a/test/mocks/redis/mocks.cc b/test/mocks/redis/mocks.cc index 806629c8b508..cc5537a7b037 100644 --- a/test/mocks/redis/mocks.cc +++ b/test/mocks/redis/mocks.cc @@ -9,10 +9,10 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Invoke; using testing::_; +namespace Envoy { namespace Redis { bool operator==(const RespValue& lhs, const RespValue& rhs) { diff --git a/test/mocks/router/mocks.cc b/test/mocks/router/mocks.cc index 6e0929f0bf67..0dc7ff25215c 100644 --- a/test/mocks/router/mocks.cc +++ b/test/mocks/router/mocks.cc @@ -5,13 +5,13 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; using testing::Return; using testing::ReturnRef; using testing::SaveArg; using testing::_; +namespace Envoy { namespace Router { MockRedirectEntry::MockRedirectEntry() {} diff --git a/test/mocks/runtime/mocks.cc b/test/mocks/runtime/mocks.cc index be05a1e19871..956b5474e597 100644 --- a/test/mocks/runtime/mocks.cc +++ b/test/mocks/runtime/mocks.cc @@ -3,11 +3,11 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Return; using testing::ReturnArg; using testing::_; +namespace Envoy { namespace Runtime { MockRandomGenerator::MockRandomGenerator() { ON_CALL(*this, uuid()).WillByDefault(Return(uuid_)); } diff --git a/test/mocks/stats/mocks.cc b/test/mocks/stats/mocks.cc index a3dfe2372fb3..9dd2d44aac64 100644 --- a/test/mocks/stats/mocks.cc +++ b/test/mocks/stats/mocks.cc @@ -3,9 +3,9 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::_; +namespace Envoy { namespace Stats { MockCounter::MockCounter() {} diff --git a/test/mocks/thread_local/mocks.cc b/test/mocks/thread_local/mocks.cc index 170d51289e1c..6c40f98875f0 100644 --- a/test/mocks/thread_local/mocks.cc +++ b/test/mocks/thread_local/mocks.cc @@ -3,10 +3,10 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Invoke; using testing::_; +namespace Envoy { namespace ThreadLocal { MockInstance::MockInstance() { diff --git a/test/mocks/tracing/mocks.cc b/test/mocks/tracing/mocks.cc index a88d22366047..1e2ec1884ba0 100644 --- a/test/mocks/tracing/mocks.cc +++ b/test/mocks/tracing/mocks.cc @@ -3,10 +3,10 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Return; using testing::ReturnRef; +namespace Envoy { namespace Tracing { MockSpan::MockSpan() {} diff --git a/test/mocks/upstream/cluster_info.h b/test/mocks/upstream/cluster_info.h index 3b4666a8b73b..116cf894c7e8 100644 --- a/test/mocks/upstream/cluster_info.h +++ b/test/mocks/upstream/cluster_info.h @@ -14,9 +14,9 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; +namespace Envoy { namespace Upstream { class MockClusterInfo : public ClusterInfo { diff --git a/test/mocks/upstream/mocks.cc b/test/mocks/upstream/mocks.cc index 0866a85d2043..3cba6823cbf0 100644 --- a/test/mocks/upstream/mocks.cc +++ b/test/mocks/upstream/mocks.cc @@ -11,7 +11,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::Invoke; using testing::Return; using testing::ReturnPointee; @@ -19,6 +18,7 @@ using testing::ReturnRef; using testing::SaveArg; using testing::_; +namespace Envoy { namespace Upstream { namespace Outlier { diff --git a/test/mocks/upstream/mocks.h b/test/mocks/upstream/mocks.h index be05490359e7..d8584f0c6e01 100644 --- a/test/mocks/upstream/mocks.h +++ b/test/mocks/upstream/mocks.h @@ -21,9 +21,9 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; +namespace Envoy { namespace Upstream { class MockCluster : public Cluster { diff --git a/test/server/config/http/config_test.cc b/test/server/config/http/config_test.cc index c91f404e7efa..e515ce5cc661 100644 --- a/test/server/config/http/config_test.cc +++ b/test/server/config/http/config_test.cc @@ -20,11 +20,11 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; using testing::Return; using testing::_; +namespace Envoy { namespace Server { namespace Configuration { diff --git a/test/server/config/stats/BUILD b/test/server/config/stats/BUILD index 50818a5fc8a2..60973c409ab8 100644 --- a/test/server/config/stats/BUILD +++ b/test/server/config/stats/BUILD @@ -11,7 +11,12 @@ envoy_package() envoy_cc_test( name = "config_test", srcs = ["config_test.cc"], + external_deps = [ + "envoy_bootstrap", + ], deps = [ + "//include/envoy/registry", + "//source/common/protobuf:utility_lib", "//source/common/stats:statsd_lib", "//source/server/config/stats:statsd_lib", "//test/mocks/server:server_mocks", diff --git a/test/server/config/stats/config_test.cc b/test/server/config/stats/config_test.cc index 490ed4f57ef9..a3f2726ea6e0 100644 --- a/test/server/config/stats/config_test.cc +++ b/test/server/config/stats/config_test.cc @@ -13,19 +13,19 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; using testing::Return; using testing::ReturnRef; using testing::_; +namespace Envoy { namespace Server { namespace Configuration { TEST(StatsConfigTest, ValidTcpStatsd) { const std::string name = "envoy.statsd"; Protobuf::Struct config; - ProtobufWkt::Map& field_map = *config.mutable_fields(); + auto& field_map = *config.mutable_fields(); field_map["tcp_cluster_name"].set_string_value("fake_cluster"); StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); @@ -35,7 +35,7 @@ TEST(StatsConfigTest, ValidTcpStatsd) { MessageUtil::jsonConvert(config, *message); NiceMock server; - Stats::SinkPtr sink = factory->createStatsSink(*message, server, server.clusterManager()); + Stats::SinkPtr sink = factory->createStatsSink(*message, server); EXPECT_NE(sink, nullptr); EXPECT_NE(dynamic_cast(sink.get()), nullptr); } @@ -43,17 +43,10 @@ TEST(StatsConfigTest, ValidTcpStatsd) { TEST(StatsConfigTest, ValidUdpIpStatsd) { const std::string name = "envoy.statsd"; Protobuf::Struct config; - ProtobufWkt::Map& field_map = *config.mutable_fields(); - - envoy::api::v2::Address address; - envoy::api::v2::SocketAddress& socket_address = *address.mutable_socket_address(); - socket_address.set_protocol(envoy::api::v2::SocketAddress::UDP); - socket_address.set_address("127.0.0.1"); - socket_address.set_port_value(8125); + auto& field_map = *config.mutable_fields(); - ProtobufWkt::Map& address_field_map = - *field_map["address"].mutable_struct_value()->mutable_fields(); - ProtobufWkt::Map& socket_address_field_map = + auto& address_field_map = *field_map["address"].mutable_struct_value()->mutable_fields(); + auto& socket_address_field_map = *address_field_map["socket_address"].mutable_struct_value()->mutable_fields(); socket_address_field_map["protocol"].set_string_value("UDP"); socket_address_field_map["address"].set_string_value("127.0.0.1"); @@ -66,7 +59,7 @@ TEST(StatsConfigTest, ValidUdpIpStatsd) { MessageUtil::jsonConvert(config, *message); NiceMock server; - Stats::SinkPtr sink = factory->createStatsSink(*message, server, server.clusterManager()); + Stats::SinkPtr sink = factory->createStatsSink(*message, server); EXPECT_NE(sink, nullptr); EXPECT_NE(dynamic_cast(sink.get()), nullptr); } @@ -81,7 +74,7 @@ TEST(StatsConfigTest, EmptyConfig) { ProtobufTypes::MessagePtr message = factory->createEmptyConfigProto(); MessageUtil::jsonConvert(config, *message); NiceMock server; - EXPECT_THROW(factory->createStatsSink(*message, server, server.clusterManager()), EnvoyException); + EXPECT_THROW(factory->createStatsSink(*message, server), EnvoyException); } } // namespace Configuration diff --git a/test/server/configuration_impl_test.cc b/test/server/configuration_impl_test.cc index 971a9ea9ce5f..a32793fe1a2e 100644 --- a/test/server/configuration_impl_test.cc +++ b/test/server/configuration_impl_test.cc @@ -14,11 +14,11 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::InSequence; using testing::Return; using testing::ReturnRef; +namespace Envoy { namespace Server { namespace Configuration { @@ -252,6 +252,82 @@ TEST_F(ConfigurationImplTest, ConfigurationFailsWhenInvalidTracerSpecified) { EnvoyException, "No HttpTracerFactory found for type: invalid"); } +TEST_F(ConfigurationImplTest, ProtoSpecifiedStatsSink) { + std::string json = R"EOF( + { + "listeners": [], + + "cluster_manager": { + "clusters": [] + }, + + "admin": {"access_log_path": "/dev/null", "address": "tcp://1.2.3.4:5678"} + } + )EOF"; + + envoy::api::v2::Bootstrap bootstrap = TestUtility::parseBootstrapFromJson(json); + + auto& sink = *bootstrap.mutable_stats_sinks()->Add(); + sink.set_name("envoy.statsd"); + auto& field_map = *sink.mutable_config()->mutable_fields(); + field_map["tcp_cluster_name"].set_string_value("fake_cluster"); + + MainImpl config; + config.initialize(bootstrap, server_, cluster_manager_factory_); + + EXPECT_EQ(1, config.statsSinks().size()); +} + +TEST_F(ConfigurationImplTest, StatsSinkWithInvalidName) { + std::string json = R"EOF( + { + "listeners": [], + + "cluster_manager": { + "clusters": [] + }, + + "admin": {"access_log_path": "/dev/null", "address": "tcp://1.2.3.4:5678"} + } + )EOF"; + + envoy::api::v2::Bootstrap bootstrap = TestUtility::parseBootstrapFromJson(json); + + envoy::api::v2::StatsSink& sink = *bootstrap.mutable_stats_sinks()->Add(); + sink.set_name("envoy.invalid"); + auto& field_map = *sink.mutable_config()->mutable_fields(); + field_map["tcp_cluster_name"].set_string_value("fake_cluster"); + + MainImpl config; + EXPECT_THROW_WITH_MESSAGE(config.initialize(bootstrap, server_, cluster_manager_factory_), + EnvoyException, "No Stats::Sink found for name: envoy.invalid"); +} + +TEST_F(ConfigurationImplTest, StatsSinkWithNoName) { + std::string json = R"EOF( + { + "listeners": [], + + "cluster_manager": { + "clusters": [] + }, + + "admin": {"access_log_path": "/dev/null", "address": "tcp://1.2.3.4:5678"} + } + )EOF"; + + envoy::api::v2::Bootstrap bootstrap = TestUtility::parseBootstrapFromJson(json); + + auto& sink = *bootstrap.mutable_stats_sinks()->Add(); + auto& field_map = *sink.mutable_config()->mutable_fields(); + field_map["tcp_cluster_name"].set_string_value("fake_cluster"); + + MainImpl config; + EXPECT_THROW_WITH_MESSAGE( + config.initialize(bootstrap, server_, cluster_manager_factory_), EnvoyException, + "sink object does not have 'name' attribute to look up the implementation"); +} + } // namespace Configuration } // namespace Server } // namespace Envoy diff --git a/test/server/guarddog_impl_test.cc b/test/server/guarddog_impl_test.cc index baef92f35f3c..2259a4b8db1c 100644 --- a/test/server/guarddog_impl_test.cc +++ b/test/server/guarddog_impl_test.cc @@ -16,10 +16,10 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::InSequence; using testing::NiceMock; +namespace Envoy { namespace Server { /** diff --git a/test/server/http/admin_test.cc b/test/server/http/admin_test.cc index ed2ddebebc6f..873667000a9a 100644 --- a/test/server/http/admin_test.cc +++ b/test/server/http/admin_test.cc @@ -14,10 +14,10 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::NiceMock; using testing::_; +namespace Envoy { namespace Server { class AdminFilterTest : public testing::TestWithParam { diff --git a/test/server/http/health_check_test.cc b/test/server/http/health_check_test.cc index f9e35b0855c6..8eecc601351f 100644 --- a/test/server/http/health_check_test.cc +++ b/test/server/http/health_check_test.cc @@ -12,7 +12,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -namespace Envoy { using testing::DoAll; using testing::Invoke; using testing::NiceMock; @@ -21,6 +20,8 @@ using testing::ReturnRef; using testing::SaveArg; using testing::_; +namespace Envoy { + class HealthCheckFilterTest : public testing::Test { public: HealthCheckFilterTest(bool pass_through, bool caching) From 9dcc3a51351ce0e3bd64a3e75e0ee2a29b672661 Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Tue, 5 Sep 2017 13:38:39 -0400 Subject: [PATCH 6/6] Modified the construction of proto in new tests --- test/server/config/stats/config_test.cc | 33 +++++++++++++------------ 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/test/server/config/stats/config_test.cc b/test/server/config/stats/config_test.cc index a3f2726ea6e0..513abedd8476 100644 --- a/test/server/config/stats/config_test.cc +++ b/test/server/config/stats/config_test.cc @@ -8,6 +8,7 @@ #include "server/config/stats/statsd.h" #include "test/mocks/server/mocks.h" +#include "test/test_common/utility.h" #include "api/bootstrap.pb.h" #include "gmock/gmock.h" @@ -24,15 +25,15 @@ namespace Configuration { TEST(StatsConfigTest, ValidTcpStatsd) { const std::string name = "envoy.statsd"; - Protobuf::Struct config; - auto& field_map = *config.mutable_fields(); - field_map["tcp_cluster_name"].set_string_value("fake_cluster"); + + envoy::api::v2::StatsdSink sink_config; + sink_config.set_tcp_cluster_name("fake_cluster"); StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); ASSERT_NE(factory, nullptr); ProtobufTypes::MessagePtr message = factory->createEmptyConfigProto(); - MessageUtil::jsonConvert(config, *message); + MessageUtil::jsonConvert(sink_config, *message); NiceMock server; Stats::SinkPtr sink = factory->createStatsSink(*message, server); @@ -42,21 +43,19 @@ TEST(StatsConfigTest, ValidTcpStatsd) { TEST(StatsConfigTest, ValidUdpIpStatsd) { const std::string name = "envoy.statsd"; - Protobuf::Struct config; - auto& field_map = *config.mutable_fields(); - auto& address_field_map = *field_map["address"].mutable_struct_value()->mutable_fields(); - auto& socket_address_field_map = - *address_field_map["socket_address"].mutable_struct_value()->mutable_fields(); - socket_address_field_map["protocol"].set_string_value("UDP"); - socket_address_field_map["address"].set_string_value("127.0.0.1"); - socket_address_field_map["port_value"].set_number_value(8125); + envoy::api::v2::StatsdSink sink_config; + envoy::api::v2::Address& address = *sink_config.mutable_address(); + envoy::api::v2::SocketAddress& socket_address = *address.mutable_socket_address(); + socket_address.set_protocol(envoy::api::v2::SocketAddress::UDP); + socket_address.set_address("127.0.0.1"); + socket_address.set_port_value(8125); StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); ASSERT_NE(factory, nullptr); ProtobufTypes::MessagePtr message = factory->createEmptyConfigProto(); - MessageUtil::jsonConvert(config, *message); + MessageUtil::jsonConvert(sink_config, *message); NiceMock server; Stats::SinkPtr sink = factory->createStatsSink(*message, server); @@ -66,15 +65,17 @@ TEST(StatsConfigTest, ValidUdpIpStatsd) { TEST(StatsConfigTest, EmptyConfig) { const std::string name = "envoy.statsd"; - Protobuf::Struct config; + envoy::api::v2::StatsdSink sink_config; StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); ASSERT_NE(factory, nullptr); ProtobufTypes::MessagePtr message = factory->createEmptyConfigProto(); - MessageUtil::jsonConvert(config, *message); + MessageUtil::jsonConvert(sink_config, *message); NiceMock server; - EXPECT_THROW(factory->createStatsSink(*message, server), EnvoyException); + EXPECT_THROW_WITH_MESSAGE( + factory->createStatsSink(*message, server), EnvoyException, + "No tcp_cluster_name or address provided for envoy.statsd Stats::Sink config"); } } // namespace Configuration