-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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.
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 ProtobufTypes::MessagePtr{new envoy::config::trace::v2::DynamicOtConfig()}; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: std::make_unique |
||
| } | ||
|
|
||
| std::string name() override; | ||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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{ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. std::make_unique
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry missed that. |
||
| new 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()); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 ProtobufTypes::MessagePtr{new envoy::config::trace::v2::LightstepConfig()}; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto |
||
| } | ||
|
|
||
| std::string name() override; | ||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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(); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use MessageUtil::getJsonStringFromMessage