From a53fff9cc014a668586beb70e45cae2694294df9 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Thu, 27 Apr 2017 11:28:21 -0700 Subject: [PATCH 1/3] Refactor initializeTracers --- source/server/configuration_impl.cc | 65 ++++++++++++++++------------- 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/source/server/configuration_impl.cc b/source/server/configuration_impl.cc index 1f9310a6096b8..92fceb142c86e 100644 --- a/source/server/configuration_impl.cc +++ b/source/server/configuration_impl.cc @@ -74,11 +74,7 @@ void MainImpl::initialize(const Json::Object& json) { watchdog_multikill_timeout_ = std::chrono::milliseconds(json.getInteger("watchdog_multikill_timeout_ms", 0)); - if (json.hasObject("tracing")) { - initializeTracers(*json.getObject("tracing")); - } else { - http_tracer_.reset(new Tracing::HttpNullTracer()); - } + initializeTracers(json); if (json.hasObject("rate_limit_service")) { Json::ObjectPtr rate_limit_service_config = json.getObject("rate_limit_service"); @@ -92,40 +88,49 @@ void MainImpl::initialize(const Json::Object& json) { } } -void MainImpl::initializeTracers(const Json::Object& tracing_configuration) { +void MainImpl::initializeTracers(const Json::Object& configuration) { log().info("loading tracing configuration"); - // Initialize tracing driver. - if (tracing_configuration.hasObject("http")) { - Json::ObjectPtr http_tracer_config = tracing_configuration.getObject("http"); - Json::ObjectPtr driver = http_tracer_config->getObject("driver"); + if (!configuration.hasObject("tracing")) { + http_tracer_.reset(new Tracing::HttpNullTracer()); + return; + } - std::string type = driver->getString("type"); - log().info(fmt::format(" loading tracing driver: {}", type)); + Json::ObjectPtr tracing_configuration = configuration.getObject("tracing"); + if (!tracing_configuration->hasObject("http")) { + http_tracer_.reset(new Tracing::HttpNullTracer()); + return; + } - ASSERT(type == "lightstep"); - ::Runtime::RandomGenerator& rand = server_.random(); - Json::ObjectPtr lightstep_config = driver->getObject("config"); + // Initialize tracing driver. + Json::ObjectPtr http_tracer_config = tracing_configuration->getObject("http"); + Json::ObjectPtr driver = http_tracer_config->getObject("driver"); - std::unique_ptr opts(new lightstep::TracerOptions()); - opts->access_token = - server_.api().fileReadToEnd(lightstep_config->getString("access_token_file")); - StringUtil::rtrim(opts->access_token); + std::string type = driver->getString("type"); + log().info(fmt::format(" loading tracing driver: {}", type)); - if (server_.localInfo().clusterName().empty()) { - throw EnvoyException("cluster name must be defined if LightStep tracing is enabled. See " - "--service-cluster option."); - } - opts->tracer_attributes["lightstep.component_name"] = server_.localInfo().clusterName(); - opts->guid_generator = [&rand]() { return rand.random(); }; + ASSERT(type == "lightstep"); + ::Runtime::RandomGenerator& rand = server_.random(); + Json::ObjectPtr lightstep_config = driver->getObject("config"); - Tracing::DriverPtr lightstep_driver( - new Tracing::LightStepDriver(*lightstep_config, *cluster_manager_, server_.stats(), - server_.threadLocal(), server_.runtime(), std::move(opts))); + std::unique_ptr opts(new lightstep::TracerOptions()); + opts->access_token = + server_.api().fileReadToEnd(lightstep_config->getString("access_token_file")); + StringUtil::rtrim(opts->access_token); - http_tracer_.reset( - new Tracing::HttpTracerImpl(std::move(lightstep_driver), server_.localInfo())); + if (server_.localInfo().clusterName().empty()) { + throw EnvoyException("cluster name must be defined if LightStep tracing is enabled. See " + "--service-cluster option."); } + opts->tracer_attributes["lightstep.component_name"] = server_.localInfo().clusterName(); + opts->guid_generator = [&rand]() { return rand.random(); }; + + Tracing::DriverPtr lightstep_driver( + new Tracing::LightStepDriver(*lightstep_config, *cluster_manager_, server_.stats(), + server_.threadLocal(), server_.runtime(), std::move(opts))); + + http_tracer_.reset( + new Tracing::HttpTracerImpl(std::move(lightstep_driver), server_.localInfo())); } const std::list& MainImpl::listeners() { return listeners_; } From a3f1bc093511a0a78db119e975e4ff6d54b02403 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Thu, 27 Apr 2017 15:14:58 -0700 Subject: [PATCH 2/3] fix formatting --- source/server/configuration_impl.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/source/server/configuration_impl.cc b/source/server/configuration_impl.cc index 92fceb142c86e..676a349bd453c 100644 --- a/source/server/configuration_impl.cc +++ b/source/server/configuration_impl.cc @@ -120,17 +120,16 @@ void MainImpl::initializeTracers(const Json::Object& configuration) { if (server_.localInfo().clusterName().empty()) { throw EnvoyException("cluster name must be defined if LightStep tracing is enabled. See " - "--service-cluster option."); + "--service-cluster option."); } opts->tracer_attributes["lightstep.component_name"] = server_.localInfo().clusterName(); opts->guid_generator = [&rand]() { return rand.random(); }; Tracing::DriverPtr lightstep_driver( new Tracing::LightStepDriver(*lightstep_config, *cluster_manager_, server_.stats(), - server_.threadLocal(), server_.runtime(), std::move(opts))); + server_.threadLocal(), server_.runtime(), std::move(opts))); - http_tracer_.reset( - new Tracing::HttpTracerImpl(std::move(lightstep_driver), server_.localInfo())); + http_tracer_.reset(new Tracing::HttpTracerImpl(std::move(lightstep_driver), server_.localInfo())); } const std::list& MainImpl::listeners() { return listeners_; } From 6c881b663410cad009cd1e23b4637e40a38b0196 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Fri, 28 Apr 2017 12:51:13 -0700 Subject: [PATCH 3/3] add tests --- test/server/configuration_impl_test.cc | 60 ++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/test/server/configuration_impl_test.cc b/test/server/configuration_impl_test.cc index 431b08e28180c..52ee77e3f145b 100644 --- a/test/server/configuration_impl_test.cc +++ b/test/server/configuration_impl_test.cc @@ -307,5 +307,65 @@ TEST(ConfigurationImplTest, ServiceClusterNotSetWhenLSTracing) { EXPECT_THROW(config.initialize(*loader), EnvoyException); } +TEST(ConfigurationImplTest, NullTracerSetWhenTracingConfigurationAbsent) { + std::string json = R"EOF( + { + "listeners" : [ + { + "address": "tcp://127.0.0.1:1234", + "filters": [] + } + ], + "cluster_manager": { + "clusters": [] + } + } + )EOF"; + + Json::ObjectPtr loader = Json::Factory::loadFromString(json); + + NiceMock server; + server.local_info_.cluster_name_ = ""; + MainImpl config(server); + config.initialize(*loader); + + EXPECT_NE(nullptr, dynamic_cast(&config.httpTracer())); +} + +TEST(ConfigurationImplTest, NullTracerSetWhenHttpKeyAbsentFromTracerConfiguration) { + std::string json = R"EOF( + { + "listeners" : [ + { + "address": "tcp://127.0.0.1:1234", + "filters": [] + } + ], + "cluster_manager": { + "clusters": [] + }, + "tracing": { + "not_http": { + "driver": { + "type": "lightstep", + "config": { + "access_token_file": "/etc/envoy/envoy.cfg" + } + } + } + } + } + )EOF"; + + Json::ObjectPtr loader = Json::Factory::loadFromString(json); + + NiceMock server; + server.local_info_.cluster_name_ = ""; + MainImpl config(server); + config.initialize(*loader); + + EXPECT_NE(nullptr, dynamic_cast(&config.httpTracer())); +} + } // Configuration } // Server