Skip to content

tracing: Configure tracer from v2 proto configuration #4518

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 4 commits into from
Oct 3, 2018
Merged

tracing: Configure tracer from v2 proto configuration #4518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4e2635e
tracing: Configure tracer from v2 proto configuration
objectiser Sep 24, 2018
2a8658e
Factor out creation of empty proto message
objectiser Sep 28, 2018
2c1f825
Fix additional comments
objectiser Sep 29, 2018
d184823
Changes based on further comments
objectiser Oct 3, 2018
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
18 changes: 13 additions & 5 deletions source/extensions/tracers/dynamic_ot/config.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,19 @@ namespace Extensions {
namespace Tracers {
namespace DynamicOt {

Tracing::HttpTracerPtr
DynamicOpenTracingTracerFactory::createHttpTracer(const Json::Object& json_config,
Server::Instance& server) {
const std::string library = json_config.getString("library");
const std::string config = json_config.getObject("config")->asJsonString();
Tracing::HttpTracerPtr DynamicOpenTracingTracerFactory::createHttpTracer(
const envoy::config::trace::v2::Tracing& configuration, Server::Instance& server) {
ProtobufTypes::MessagePtr config_ptr = createEmptyConfigProto();

if (configuration.http().has_config()) {
MessageUtil::jsonConvert(configuration.http().config(), *config_ptr);
}

const auto& dynaot_config =
dynamic_cast<const envoy::config::trace::v2::DynamicOtConfig&>(*config_ptr);

const std::string library = dynaot_config.library();
const std::string config = MessageUtil::getJsonStringFromMessage(dynaot_config.config());
Tracing::DriverPtr dynamic_driver{
std::make_unique<DynamicOpenTracingDriver>(server.stats(), library, config)};
return std::make_unique<Tracing::HttpTracerImpl>(std::move(dynamic_driver), server.localInfo());
Expand Down
7 changes: 6 additions & 1 deletion source/extensions/tracers/dynamic_ot/config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,13 @@ namespace DynamicOt {
class DynamicOpenTracingTracerFactory : public Server::Configuration::TracerFactory {
public:
// TracerFactory
Tracing::HttpTracerPtr createHttpTracer(const Json::Object& json_config,
Tracing::HttpTracerPtr createHttpTracer(const envoy::config::trace::v2::Tracing& configuration,
Server::Instance& server) override;

ProtobufTypes::MessagePtr createEmptyConfigProto() override {
return std::make_unique<envoy::config::trace::v2::DynamicOtConfig>();
}

std::string name() override;
};

Expand Down
23 changes: 16 additions & 7 deletions source/extensions/tracers/lightstep/config.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,28 @@ namespace Extensions {
namespace Tracers {
namespace Lightstep {

Tracing::HttpTracerPtr LightstepTracerFactory::createHttpTracer(const Json::Object& json_config,
Server::Instance& server) {
Tracing::HttpTracerPtr
LightstepTracerFactory::createHttpTracer(const envoy::config::trace::v2::Tracing& configuration,
Server::Instance& server) {
ProtobufTypes::MessagePtr config_ptr = createEmptyConfigProto();

if (configuration.http().has_config()) {
MessageUtil::jsonConvert(configuration.http().config(), *config_ptr);
}

const auto& lightstep_config =
dynamic_cast<const envoy::config::trace::v2::LightstepConfig&>(*config_ptr);

std::unique_ptr<lightstep::LightStepTracerOptions> opts(new lightstep::LightStepTracerOptions());
const auto access_token_file =
server.api().fileReadToEnd(json_config.getString("access_token_file"));
const auto access_token_file = server.api().fileReadToEnd(lightstep_config.access_token_file());
const auto access_token_sv = StringUtil::rtrim(access_token_file);
opts->access_token.assign(access_token_sv.data(), access_token_sv.size());
opts->component_name = server.localInfo().clusterName();

Tracing::DriverPtr lightstep_driver{new LightStepDriver{
json_config, server.clusterManager(), server.stats(), server.threadLocal(), server.runtime(),
std::move(opts), Common::Ot::OpenTracingDriver::PropagationMode::TracerNative}};
Tracing::DriverPtr lightstep_driver{std::make_unique<LightStepDriver>(
lightstep_config, server.clusterManager(), server.stats(), server.threadLocal(),
server.runtime(), std::move(opts),
Common::Ot::OpenTracingDriver::PropagationMode::TracerNative)};
return std::make_unique<Tracing::HttpTracerImpl>(std::move(lightstep_driver), server.localInfo());
}

Expand Down
6 changes: 5 additions & 1 deletion source/extensions/tracers/lightstep/config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@ namespace Lightstep {
class LightstepTracerFactory : public Server::Configuration::TracerFactory {
public:
// TracerFactory
Tracing::HttpTracerPtr createHttpTracer(const Json::Object& json_config,
Tracing::HttpTracerPtr createHttpTracer(const envoy::config::trace::v2::Tracing& configuration,
Server::Instance& server) override;

ProtobufTypes::MessagePtr createEmptyConfigProto() override {
return std::make_unique<envoy::config::trace::v2::LightstepConfig>();
}

std::string name() override;
};

Expand Down
6 changes: 3 additions & 3 deletions source/extensions/tracers/lightstep/lightstep_tracer_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ void LightStepDriver::TlsLightStepTracer::enableTimer() {
flush_timer_->enableTimer(std::chrono::milliseconds(flush_interval));
}

LightStepDriver::LightStepDriver(const Json::Object& config,
LightStepDriver::LightStepDriver(const envoy::config::trace::v2::LightstepConfig& lightstep_config,
Upstream::ClusterManager& cluster_manager, Stats::Store& stats,
ThreadLocal::SlotAllocator& tls, Runtime::Loader& runtime,
std::unique_ptr<lightstep::LightStepTracerOptions>&& options,
Expand All @@ -138,10 +138,10 @@ LightStepDriver::LightStepDriver(const Json::Object& config,
tracer_stats_{LIGHTSTEP_TRACER_STATS(POOL_COUNTER_PREFIX(stats, "tracing.lightstep."))},
tls_{tls.allocateSlot()}, runtime_{runtime}, options_{std::move(options)},
propagation_mode_{propagation_mode} {
Upstream::ThreadLocalCluster* cluster = cm_.get(config.getString("collector_cluster"));
Upstream::ThreadLocalCluster* cluster = cm_.get(lightstep_config.collector_cluster());
if (!cluster) {
throw EnvoyException(fmt::format("{} collector cluster is not defined on cluster manager level",
config.getString("collector_cluster")));
lightstep_config.collector_cluster()));
}
cluster_ = cluster->info();

Expand Down
5 changes: 3 additions & 2 deletions source/extensions/tracers/lightstep/lightstep_tracer_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ class LightStepLogger : Logger::Loggable<Logger::Id::tracing> {
*/
class LightStepDriver : public Common::Ot::OpenTracingDriver {
public:
LightStepDriver(const Json::Object& config, Upstream::ClusterManager& cluster_manager,
Stats::Store& stats, ThreadLocal::SlotAllocator& tls, Runtime::Loader& runtime,
LightStepDriver(const envoy::config::trace::v2::LightstepConfig& lightstep_config,
Upstream::ClusterManager& cluster_manager, Stats::Store& stats,
ThreadLocal::SlotAllocator& tls, Runtime::Loader& runtime,
std::unique_ptr<lightstep::LightStepTracerOptions>&& options,
PropagationMode propagation_mode);

Expand Down
19 changes: 14 additions & 5 deletions source/extensions/tracers/zipkin/config.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,23 @@ namespace Extensions {
namespace Tracers {
namespace Zipkin {

Tracing::HttpTracerPtr ZipkinTracerFactory::createHttpTracer(const Json::Object& json_config,
Server::Instance& server) {
Tracing::HttpTracerPtr
ZipkinTracerFactory::createHttpTracer(const envoy::config::trace::v2::Tracing& configuration,
Server::Instance& server) {

Envoy::Runtime::RandomGenerator& rand = server.random();
ProtobufTypes::MessagePtr config_ptr = createEmptyConfigProto();

Tracing::DriverPtr zipkin_driver(
new Zipkin::Driver(json_config, server.clusterManager(), server.stats(), server.threadLocal(),
server.runtime(), server.localInfo(), rand, server.timeSystem()));
if (configuration.http().has_config()) {
MessageUtil::jsonConvert(configuration.http().config(), *config_ptr);
}

const auto& zipkin_config =
dynamic_cast<const envoy::config::trace::v2::ZipkinConfig&>(*config_ptr);

Tracing::DriverPtr zipkin_driver{std::make_unique<Zipkin::Driver>(
zipkin_config, server.clusterManager(), server.stats(), server.threadLocal(),
server.runtime(), server.localInfo(), rand, server.timeSystem())};

return Tracing::HttpTracerPtr(
new Tracing::HttpTracerImpl(std::move(zipkin_driver), server.localInfo()));
Expand Down
7 changes: 6 additions & 1 deletion source/extensions/tracers/zipkin/config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@ namespace Zipkin {
class ZipkinTracerFactory : public Server::Configuration::TracerFactory {
public:
// TracerFactory
Tracing::HttpTracerPtr createHttpTracer(const Json::Object& json_config,
Tracing::HttpTracerPtr createHttpTracer(const envoy::config::trace::v2::Tracing& configuration,
Server::Instance& server) override;

ProtobufTypes::MessagePtr createEmptyConfigProto() override {
return std::make_unique<envoy::config::trace::v2::ZipkinConfig>();
}

std::string name() override;
};

Expand Down
1 change: 0 additions & 1 deletion source/extensions/tracers/zipkin/zipkin_core_constants.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ class ZipkinCoreConstantValues {
const std::string NOT_SAMPLED = "0";

const std::string DEFAULT_COLLECTOR_ENDPOINT = "/api/v1/spans";
const bool DEFAULT_TRACE_ID_128BIT = false;
const bool DEFAULT_SHARED_SPAN_CONTEXT = true;
};

Expand Down
22 changes: 12 additions & 10 deletions source/extensions/tracers/zipkin/zipkin_tracer_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,30 +53,32 @@ Tracing::SpanPtr ZipkinSpan::spawnChild(const Tracing::Config& config, const std
Driver::TlsTracer::TlsTracer(TracerPtr&& tracer, Driver& driver)
: tracer_(std::move(tracer)), driver_(driver) {}

Driver::Driver(const Json::Object& config, Upstream::ClusterManager& cluster_manager,
Stats::Store& stats, ThreadLocal::SlotAllocator& tls, Runtime::Loader& runtime,
Driver::Driver(const envoy::config::trace::v2::ZipkinConfig& zipkin_config,
Upstream::ClusterManager& cluster_manager, Stats::Store& stats,
ThreadLocal::SlotAllocator& tls, Runtime::Loader& runtime,
const LocalInfo::LocalInfo& local_info, Runtime::RandomGenerator& random_generator,
TimeSource& time_source)
: cm_(cluster_manager), tracer_stats_{ZIPKIN_TRACER_STATS(
POOL_COUNTER_PREFIX(stats, "tracing.zipkin."))},
tls_(tls.allocateSlot()), runtime_(runtime), local_info_(local_info),
time_source_(time_source) {

Upstream::ThreadLocalCluster* cluster = cm_.get(config.getString("collector_cluster"));
Upstream::ThreadLocalCluster* cluster = cm_.get(zipkin_config.collector_cluster());
if (!cluster) {
throw EnvoyException(fmt::format("{} collector cluster is not defined on cluster manager level",
config.getString("collector_cluster")));
zipkin_config.collector_cluster()));
}
cluster_ = cluster->info();

const std::string collector_endpoint =
config.getString("collector_endpoint", ZipkinCoreConstants::get().DEFAULT_COLLECTOR_ENDPOINT);
std::string collector_endpoint = ZipkinCoreConstants::get().DEFAULT_COLLECTOR_ENDPOINT;
if (zipkin_config.collector_endpoint().size() > 0) {
collector_endpoint = zipkin_config.collector_endpoint();
}

const bool trace_id_128bit =
config.getBoolean("trace_id_128bit", ZipkinCoreConstants::get().DEFAULT_TRACE_ID_128BIT);
const bool trace_id_128bit = zipkin_config.trace_id_128bit();
Copy link
Contributor Author

@objectiser objectiser Sep 25, 2018

Choose a reason for hiding this comment

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

Wondering whether this field should be changed to BoolValue wrapper? The default is currently false, but in the future we may want to change that to true as 128bit ids are becoming more common.

Copy link
Member

@lizan lizan Sep 28, 2018

Choose a reason for hiding this comment

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

It might not worth, changing bool to BoolValue is a backward incompatible change so you'll need to follow deprecation process, changing default to true is another breaking change I guess?

Copy link
Contributor Author

@objectiser objectiser Sep 29, 2018

Choose a reason for hiding this comment

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

Agree - so will leave for now and wait for it to be raised as an issue.


const bool shared_span_context = config.getBoolean(
"shared_span_context", ZipkinCoreConstants::get().DEFAULT_SHARED_SPAN_CONTEXT);
const bool shared_span_context = PROTOBUF_GET_WRAPPED_OR_DEFAULT(
zipkin_config, shared_span_context, ZipkinCoreConstants::get().DEFAULT_SHARED_SPAN_CONTEXT);

tls_->set([this, collector_endpoint, &random_generator, trace_id_128bit, shared_span_context](
Event::Dispatcher& dispatcher) -> ThreadLocal::ThreadLocalObjectSharedPtr {
Expand Down
3 changes: 2 additions & 1 deletion source/extensions/tracers/zipkin/zipkin_tracer_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ class Driver : public Tracing::Driver {
* Constructor. It adds itself and a newly-created Zipkin::Tracer object to a thread-local store.
* Also, it associates the given random-number generator to the Zipkin::Tracer object it creates.
*/
Driver(const Json::Object& config, Upstream::ClusterManager& cluster_manager, Stats::Store& stats,
Driver(const envoy::config::trace::v2::ZipkinConfig& zipkin_config,
Upstream::ClusterManager& cluster_manager, Stats::Store& stats,
ThreadLocal::SlotAllocator& tls, Runtime::Loader& runtime,
const LocalInfo::LocalInfo& localinfo, Runtime::RandomGenerator& random_generator,
TimeSource& time_source);
Expand Down
6 changes: 1 addition & 5 deletions source/server/configuration_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,9 @@ void MainImpl::initializeTracers(const envoy::config::trace::v2::Tracing& config
std::string type = configuration.http().name();
ENVOY_LOG(info, " loading tracing driver: {}", type);

// TODO(htuch): Make this dynamically pluggable one day.
Json::ObjectSharedPtr driver_config =
MessageUtil::getJsonObjectFromMessage(configuration.http().config());

// Now see if there is a factory that will accept the config.
auto& factory = Config::Utility::getAndCheckFactory<TracerFactory>(type);
http_tracer_ = factory.createHttpTracer(*driver_config, server);
http_tracer_ = factory.createHttpTracer(configuration, server);
}

void MainImpl::initializeStatsSinks(const envoy::config::bootstrap::v2::Bootstrap& bootstrap,
Expand Down
11 changes: 9 additions & 2 deletions source/server/configuration_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,15 @@ class TracerFactory {
* @param json_config supplies the general json configuration for the HttpTracer
* @param server supplies the server instance
*/
virtual Tracing::HttpTracerPtr createHttpTracer(const Json::Object& json_config,
Instance& server) PURE;
virtual Tracing::HttpTracerPtr
createHttpTracer(const envoy::config::trace::v2::Tracing& configuration, Instance& server) PURE;

/**
* @return ProtobufTypes::MessagePtr create empty config proto message for v2. The tracing
* 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 tracer produced by the
Expand Down
22 changes: 12 additions & 10 deletions test/extensions/tracers/dynamic_ot/config_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,21 @@ TEST(DynamicOtTracerConfigTest, DynamicOpentracingHttpTracer) {
ON_CALL(*server.cluster_manager_.thread_local_cluster_.cluster_.info_, features())
.WillByDefault(Return(Upstream::ClusterInfo::Features::HTTP2));

const std::string valid_config = fmt::sprintf(R"EOF(
{
"library": "%s/external/io_opentracing_cpp/mocktracer/libmocktracer_plugin.so",
"config": {
"output_file" : "fake_file"
}
}
const std::string yaml_string = fmt::sprintf(R"EOF(
http:
name: envoy.dynamic.ot
config:
library: %s/external/io_opentracing_cpp/mocktracer/libmocktracer_plugin.so
config:
output_file: fake_file
)EOF",
TestEnvironment::runfilesDirectory());
const Json::ObjectSharedPtr valid_json = Json::Factory::loadFromString(valid_config);
TestEnvironment::runfilesDirectory());
envoy::config::trace::v2::Tracing configuration;
MessageUtil::loadFromYaml(yaml_string, configuration);

DynamicOpenTracingTracerFactory factory;

const Tracing::HttpTracerPtr tracer = factory.createHttpTracer(*valid_json, server);
const Tracing::HttpTracerPtr tracer = factory.createHttpTracer(configuration, server);
EXPECT_NE(nullptr, tracer);
}

Expand Down
18 changes: 10 additions & 8 deletions test/extensions/tracers/lightstep/config_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,18 @@ TEST(LightstepTracerConfigTest, LightstepHttpTracer) {
ON_CALL(*server.cluster_manager_.thread_local_cluster_.cluster_.info_, features())
.WillByDefault(Return(Upstream::ClusterInfo::Features::HTTP2));

std::string valid_config = R"EOF(
{
"collector_cluster": "fake_cluster",
"access_token_file": "fake_file"
}
)EOF";
Json::ObjectSharedPtr valid_json = Json::Factory::loadFromString(valid_config);
const std::string yaml_string = R"EOF(
http:
name: envoy.lightstep
config:
collector_cluster: fake_cluster
access_token_file: fake_file
)EOF";
envoy::config::trace::v2::Tracing configuration;
MessageUtil::loadFromYaml(yaml_string, configuration);

LightstepTracerFactory factory;
Tracing::HttpTracerPtr lightstep_tracer = factory.createHttpTracer(*valid_json, server);
Tracing::HttpTracerPtr lightstep_tracer = factory.createHttpTracer(configuration, server);
EXPECT_NE(nullptr, lightstep_tracer);
}

Expand Down
Loading