Skip to content

Refactor initializeTracers #847

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattklein123 merged 3 commits into from
Apr 28, 2017
Merged

Refactor initializeTracers #847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a53fff9
Refactor initializeTracers
goaway Apr 27, 2017
a3f1bc0
fix formatting
goaway Apr 27, 2017
6c881b6
add tests
goaway Apr 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 34 additions & 30 deletions source/server/configuration_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -92,40 +88,48 @@ 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")) {
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 Apr 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check to make sure we have test coverage on both of these branches? It might be better to combine this into the previous if check for easier coverage, but as long as we have coverage of both I'm fine with either way.

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<lightstep::TracerOptions> 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<lightstep::TracerOptions> 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<Server::Configuration::ListenerPtr>& MainImpl::listeners() { return listeners_; }
Expand Down
60 changes: 60 additions & 0 deletions test/server/configuration_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::MockInstance> server;
server.local_info_.cluster_name_ = "";
MainImpl config(server);
config.initialize(*loader);

EXPECT_NE(nullptr, dynamic_cast<Tracing::HttpNullTracer*>(&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::MockInstance> server;
server.local_info_.cluster_name_ = "";
MainImpl config(server);
config.initialize(*loader);

EXPECT_NE(nullptr, dynamic_cast<Tracing::HttpNullTracer*>(&config.httpTracer()));
}

} // Configuration
} // Server