From 59868bfe2851e19046333d264d21fc54da4c3892 Mon Sep 17 00:00:00 2001 From: Alex Ellis Date: Mon, 19 Jun 2023 00:28:27 -0400 Subject: [PATCH 01/14] Refactor gRPC exporter to use general base class we can later reuse Signed-off-by: Alex Ellis --- source/extensions/tracers/opentelemetry/BUILD | 9 ++++--- .../opentelemetry/grpc_trace_exporter.h | 8 +++--- .../opentelemetry_tracer_impl.cc | 4 ++- .../tracers/opentelemetry/trace_exporter.h | 25 +++++++++++++++++++ .../tracers/opentelemetry/tracer.cc | 2 +- .../extensions/tracers/opentelemetry/tracer.h | 4 +-- test/extensions/tracers/opentelemetry/BUILD | 2 +- 7 files changed, 42 insertions(+), 12 deletions(-) create mode 100644 source/extensions/tracers/opentelemetry/trace_exporter.h diff --git a/source/extensions/tracers/opentelemetry/BUILD b/source/extensions/tracers/opentelemetry/BUILD index 3a36a3f91fd5..a081d6077e16 100644 --- a/source/extensions/tracers/opentelemetry/BUILD +++ b/source/extensions/tracers/opentelemetry/BUILD @@ -36,7 +36,7 @@ envoy_cc_library( "tracer.h", ], deps = [ - ":grpc_trace_exporter", + ":trace_exporter", "//envoy/thread_local:thread_local_interface", "//source/common/config:utility_lib", "//source/common/tracing:http_tracer_lib", @@ -47,9 +47,12 @@ envoy_cc_library( ) envoy_cc_library( - name = "grpc_trace_exporter", + name = "trace_exporter", srcs = ["grpc_trace_exporter.cc"], - hdrs = ["grpc_trace_exporter.h"], + hdrs = [ + "grpc_trace_exporter.h", + "trace_exporter.h" + ], deps = [ "//envoy/grpc:async_client_manager_interface", "//source/common/grpc:typed_async_client_lib", diff --git a/source/extensions/tracers/opentelemetry/grpc_trace_exporter.h b/source/extensions/tracers/opentelemetry/grpc_trace_exporter.h index 2d6ff1be8977..d2d054d57d52 100644 --- a/source/extensions/tracers/opentelemetry/grpc_trace_exporter.h +++ b/source/extensions/tracers/opentelemetry/grpc_trace_exporter.h @@ -7,6 +7,8 @@ #include "opentelemetry/proto/collector/trace/v1/trace_service.pb.h" +#include "trace_exporter.h" + namespace Envoy { namespace Extensions { namespace Tracers { @@ -80,18 +82,16 @@ class OpenTelemetryGrpcTraceExporterClient : Logger::Loggable { +class OpenTelemetryGrpcTraceExporter : public OpenTelemetryTraceExporter { public: OpenTelemetryGrpcTraceExporter(const Grpc::RawAsyncClientSharedPtr& client); - bool log(const ExportTraceServiceRequest& request); + bool log(const ExportTraceServiceRequest& request) override; private: OpenTelemetryGrpcTraceExporterClient client_; }; -using OpenTelemetryGrpcTraceExporterPtr = std::unique_ptr; - } // namespace OpenTelemetry } // namespace Tracers } // namespace Extensions diff --git a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc index a3119c187e83..b53a000a8b4a 100644 --- a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc +++ b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc @@ -13,6 +13,7 @@ #include "opentelemetry/proto/trace/v1/trace.pb.h" #include "span_context.h" #include "span_context_extractor.h" +#include "trace_exporter.h" #include "tracer.h" namespace Envoy { @@ -28,7 +29,7 @@ Driver::Driver(const envoy::config::trace::v3::OpenTelemetryConfig& opentelemetr auto& factory_context = context.serverFactoryContext(); // Create the tracer in Thread Local Storage. tls_slot_ptr_->set([opentelemetry_config, &factory_context, this](Event::Dispatcher& dispatcher) { - OpenTelemetryGrpcTraceExporterPtr exporter; + OpenTelemetryTraceExporterPtr exporter; if (opentelemetry_config.has_grpc_service()) { Grpc::AsyncClientFactoryPtr&& factory = factory_context.clusterManager().grpcAsyncClientManager().factoryForGrpcService( @@ -37,6 +38,7 @@ Driver::Driver(const envoy::config::trace::v3::OpenTelemetryConfig& opentelemetr factory->createUncachedRawAsyncClient(); exporter = std::make_unique(async_client_shared_ptr); } + // TODO: check for HTTP service in config TracerPtr tracer = std::make_unique( std::move(exporter), factory_context.timeSource(), factory_context.api().randomGenerator(), factory_context.runtime(), dispatcher, tracing_stats_, opentelemetry_config.service_name()); diff --git a/source/extensions/tracers/opentelemetry/trace_exporter.h b/source/extensions/tracers/opentelemetry/trace_exporter.h new file mode 100644 index 000000000000..bd8798095bc1 --- /dev/null +++ b/source/extensions/tracers/opentelemetry/trace_exporter.h @@ -0,0 +1,25 @@ +#pragma once + +#include "opentelemetry/proto/collector/trace/v1/trace_service.pb.h" + +using opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest; + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +class OpenTelemetryTraceExporter : Logger::Loggable { +public: + virtual ~OpenTelemetryTraceExporter() = default; + + virtual bool log(const ExportTraceServiceRequest& request) = 0; +}; + + +using OpenTelemetryTraceExporterPtr = std::unique_ptr; + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy \ No newline at end of file diff --git a/source/extensions/tracers/opentelemetry/tracer.cc b/source/extensions/tracers/opentelemetry/tracer.cc index a344a253ab31..82eb830dcba5 100644 --- a/source/extensions/tracers/opentelemetry/tracer.cc +++ b/source/extensions/tracers/opentelemetry/tracer.cc @@ -91,7 +91,7 @@ void Span::setTag(absl::string_view name, absl::string_view value) { *span_.add_attributes() = key_value; } -Tracer::Tracer(OpenTelemetryGrpcTraceExporterPtr exporter, Envoy::TimeSource& time_source, +Tracer::Tracer(OpenTelemetryTraceExporterPtr exporter, Envoy::TimeSource& time_source, Random::RandomGenerator& random, Runtime::Loader& runtime, Event::Dispatcher& dispatcher, OpenTelemetryTracerStats tracing_stats, const std::string& service_name) diff --git a/source/extensions/tracers/opentelemetry/tracer.h b/source/extensions/tracers/opentelemetry/tracer.h index f80cb12fd3f7..67982689f251 100644 --- a/source/extensions/tracers/opentelemetry/tracer.h +++ b/source/extensions/tracers/opentelemetry/tracer.h @@ -33,7 +33,7 @@ struct OpenTelemetryTracerStats { */ class Tracer : Logger::Loggable { public: - Tracer(OpenTelemetryGrpcTraceExporterPtr exporter, Envoy::TimeSource& time_source, + Tracer(OpenTelemetryTraceExporterPtr exporter, Envoy::TimeSource& time_source, Random::RandomGenerator& random, Runtime::Loader& runtime, Event::Dispatcher& dispatcher, OpenTelemetryTracerStats tracing_stats, const std::string& service_name); @@ -55,7 +55,7 @@ class Tracer : Logger::Loggable { */ void flushSpans(); - OpenTelemetryGrpcTraceExporterPtr exporter_; + OpenTelemetryTraceExporterPtr exporter_; Envoy::TimeSource& time_source_; Random::RandomGenerator& random_; std::vector<::opentelemetry::proto::trace::v1::Span> span_buffer_; diff --git a/test/extensions/tracers/opentelemetry/BUILD b/test/extensions/tracers/opentelemetry/BUILD index c1ac977443d6..058cae48f2c8 100644 --- a/test/extensions/tracers/opentelemetry/BUILD +++ b/test/extensions/tracers/opentelemetry/BUILD @@ -73,7 +73,7 @@ envoy_extension_cc_test( srcs = ["grpc_trace_exporter_test.cc"], extension_names = ["envoy.tracers.opentelemetry"], deps = [ - "//source/extensions/tracers/opentelemetry:grpc_trace_exporter", + "//source/extensions/tracers/opentelemetry:trace_exporter", "//test/mocks/grpc:grpc_mocks", "//test/mocks/http:http_mocks", "//test/test_common:utility_lib", From 2ee33cafe82cae2882efc1620cea06f2d420aec7 Mon Sep 17 00:00:00 2001 From: Alex Ellis Date: Tue, 20 Jun 2023 00:32:40 -0400 Subject: [PATCH 02/14] Update OpenTelemetryConfig with HTTP-related config Signed-off-by: Alex Ellis --- api/envoy/config/trace/v3/opentelemetry.proto | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/api/envoy/config/trace/v3/opentelemetry.proto b/api/envoy/config/trace/v3/opentelemetry.proto index e9c7430dcfdd..453217051bf6 100644 --- a/api/envoy/config/trace/v3/opentelemetry.proto +++ b/api/envoy/config/trace/v3/opentelemetry.proto @@ -3,6 +3,7 @@ syntax = "proto3"; package envoy.config.trace.v3; import "envoy/config/core/v3/grpc_service.proto"; +import "envoy/config/core/v3/http_uri.proto"; import "udpa/annotations/status.proto"; @@ -25,4 +26,32 @@ message OpenTelemetryConfig { // The name for the service. This will be populated in the ResourceSpan Resource attributes. // If it is not provided, it will default to "unknown_service:envoy". string service_name = 2; + + // Configuration for sending traces to the collector over HTTP. Note that + // this will only be used if the grpc_service is not set above. + message HttpConfig { + // The upstream cluster that will receive OTLP traces over HTTP. + config.core.v3.HttpUri http_uri = 1; + + enum HttpFormat { + // Binary Protobuf Encoding. + BINARY_PROTOBUF = 0; + + // JSON Protobuf Encoding. + JSON_PROTOBUF = 1; + } + + // OTLP/HTTP uses Protobuf payloads encoded either in binary format or in JSON format. + // See https://opentelemetry.io/docs/specs/otlp/#otlphttp. + HttpFormat http_format = 2; + + // Optional path. If omitted, the default path of "/v1/traces" will be used. + string collector_path = 3; + + // Hostname to include with the request to the collector in the Host header. + string collector_hostname = 4; + } + + // This field can be also left empty to disable reporting traces to the collector. + HttpConfig http_config = 3; } From a0a21296e69a27af07424c8ecb32300f65c7bb63 Mon Sep 17 00:00:00 2001 From: Alex Ellis Date: Tue, 20 Jun 2023 00:33:36 -0400 Subject: [PATCH 03/14] Write and wire up HTTP trace exporter Signed-off-by: Alex Ellis --- source/extensions/tracers/opentelemetry/BUILD | 12 +++- .../opentelemetry/http_trace_exporter.cc | 70 +++++++++++++++++++ .../opentelemetry/http_trace_exporter.h | 48 +++++++++++++ .../opentelemetry_tracer_impl.cc | 7 +- .../opentelemetry/opentelemetry_tracer_impl.h | 3 +- .../tracers/opentelemetry/trace_exporter.h | 2 +- 6 files changed, 137 insertions(+), 5 deletions(-) create mode 100644 source/extensions/tracers/opentelemetry/http_trace_exporter.cc create mode 100644 source/extensions/tracers/opentelemetry/http_trace_exporter.h diff --git a/source/extensions/tracers/opentelemetry/BUILD b/source/extensions/tracers/opentelemetry/BUILD index a081d6077e16..0171bb68d0d3 100644 --- a/source/extensions/tracers/opentelemetry/BUILD +++ b/source/extensions/tracers/opentelemetry/BUILD @@ -48,15 +48,25 @@ envoy_cc_library( envoy_cc_library( name = "trace_exporter", - srcs = ["grpc_trace_exporter.cc"], + srcs = [ + "grpc_trace_exporter.cc", + "http_trace_exporter.cc", + ], hdrs = [ "grpc_trace_exporter.h", + "http_trace_exporter.h", "trace_exporter.h" ], deps = [ "//envoy/grpc:async_client_manager_interface", "//source/common/grpc:typed_async_client_lib", + "//envoy/upstream:cluster_manager_interface", + "//source/common/http:async_client_utility_lib", + "//source/common/http:header_map_lib", + "//source/common/http:message_lib", + "//source/common/http:utility_lib", "//source/common/protobuf", + "@envoy_api//envoy/config/core/v3:pkg_cc_proto", "@opentelemetry_proto//:trace_cc_proto", ], ) diff --git a/source/extensions/tracers/opentelemetry/http_trace_exporter.cc b/source/extensions/tracers/opentelemetry/http_trace_exporter.cc new file mode 100644 index 000000000000..c99178a3dd76 --- /dev/null +++ b/source/extensions/tracers/opentelemetry/http_trace_exporter.cc @@ -0,0 +1,70 @@ + +#include "http_trace_exporter.h" + +#include "source/common/common/logger.h" +#include "source/common/protobuf/protobuf.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +OpenTelemetryHttpTraceExporter::OpenTelemetryHttpTraceExporter( + Upstream::ClusterManager& cluster_manager, + envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config) + : cluster_manager_(cluster_manager), http_config_(http_config) {} + +bool OpenTelemetryHttpTraceExporter::log(const ExportTraceServiceRequest& request) { + + std::string request_body; + + // TODO: add JSON option as well (though it may need custom serializing, see https://opentelemetry.io/docs/specs/otlp/#json-protobuf-encoding) + if (http_config_.http_format() == envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig::BINARY_PROTOBUF) { + const auto ok = request.SerializeToString(&request_body); + if (!ok) { + ENVOY_LOG(warn, "Error during Span proto serialization."); + return false; + } + } + + Http::RequestMessagePtr message = std::make_unique(); + message->headers().setReferenceMethod(Http::Headers::get().MethodValues.Post); + message->headers().setPath(http_config_.collector_path()); + message->headers().setHost(http_config_.collector_hostname()); + message->headers().setReferenceContentType(Http::Headers::get().ContentTypeValues.Protobuf); + message->body().add(request_body); + + const auto thread_local_cluster = cluster_manager_.getThreadLocalCluster(http_config_.http_uri().cluster()); + if (thread_local_cluster == nullptr) { + ENVOY_LOG(warn, "Thread local cluster not found for collector."); + return false; + } + + std::chrono::milliseconds timeout = + std::chrono::duration_cast( + std::chrono::nanoseconds(http_config_.http_uri().timeout().nanos())); + Http::AsyncClient::Request* http_request = + thread_local_cluster->httpAsyncClient().send( + std::move(message), *this, + Http::AsyncClient::RequestOptions().setTimeout(timeout)); + + return http_request; +} + +void OpenTelemetryHttpTraceExporter::onSuccess(const Http::AsyncClient::Request&, + Http::ResponseMessagePtr&& message) { + const auto response_code = message->headers().Status()->value().getStringView(); + if (response_code != "200") { + ENVOY_LOG(warn, "response code: {}", response_code); + } +} + +void OpenTelemetryHttpTraceExporter::onFailure(const Http::AsyncClient::Request&, + Http::AsyncClient::FailureReason) { + ENVOY_LOG(debug, "Request failed."); +} + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/tracers/opentelemetry/http_trace_exporter.h b/source/extensions/tracers/opentelemetry/http_trace_exporter.h new file mode 100644 index 000000000000..e6d48056b6a8 --- /dev/null +++ b/source/extensions/tracers/opentelemetry/http_trace_exporter.h @@ -0,0 +1,48 @@ + +#pragma once + +#include "envoy/config/core/v3/http_uri.pb.h" +#include "envoy/config/trace/v3/opentelemetry.pb.h" +#include "envoy/upstream/cluster_manager.h" + +#include "source/common/common/logger.h" +#include "source/common/http/async_client_impl.h" +#include "source/common/http/async_client_utility.h" +#include "source/common/http/headers.h" +#include "source/common/http/message_impl.h" +#include "source/common/http/utility.h" + +#include "trace_exporter.h" + +#include "opentelemetry/proto/collector/trace/v1/trace_service.pb.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +/** + * Exporter for OTLP traces over HTTP. + */ +class OpenTelemetryHttpTraceExporter : public OpenTelemetryTraceExporter, + public Http::AsyncClient::Callbacks { +public: + OpenTelemetryHttpTraceExporter(Upstream::ClusterManager& cluster_manager, + envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config); + + bool log(const ExportTraceServiceRequest& request) override; + + // Http::AsyncClient::Callbacks. + void onSuccess(const Http::AsyncClient::Request&, Http::ResponseMessagePtr&&) override; + void onFailure(const Http::AsyncClient::Request&, Http::AsyncClient::FailureReason) override; + void onBeforeFinalizeUpstreamSpan(Tracing::Span&, const Http::ResponseHeaderMap*) override {} + +private: + Upstream::ClusterManager& cluster_manager_; + envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config_; +}; + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc index b53a000a8b4a..b1bf2bb74df4 100644 --- a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc +++ b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc @@ -11,6 +11,9 @@ #include "opentelemetry/proto/collector/trace/v1/trace_service.pb.h" #include "opentelemetry/proto/trace/v1/trace.pb.h" + +#include "grpc_trace_exporter.h" +#include "http_trace_exporter.h" #include "span_context.h" #include "span_context_extractor.h" #include "trace_exporter.h" @@ -37,8 +40,10 @@ Driver::Driver(const envoy::config::trace::v3::OpenTelemetryConfig& opentelemetr const Grpc::RawAsyncClientSharedPtr& async_client_shared_ptr = factory->createUncachedRawAsyncClient(); exporter = std::make_unique(async_client_shared_ptr); + } else if (opentelemetry_config.has_http_config()) { + exporter = std::make_unique( + factory_context.clusterManager(), opentelemetry_config.http_config()); } - // TODO: check for HTTP service in config TracerPtr tracer = std::make_unique( std::move(exporter), factory_context.timeSource(), factory_context.api().randomGenerator(), factory_context.runtime(), dispatcher, tracing_stats_, opentelemetry_config.service_name()); diff --git a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.h b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.h index 35f734c87b82..2b8f769f1a17 100644 --- a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.h +++ b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.h @@ -8,8 +8,7 @@ #include "source/common/common/logger.h" #include "source/common/singleton/const_singleton.h" #include "source/extensions/tracers/common/factory_base.h" -#include "source/extensions/tracers/opentelemetry/grpc_trace_exporter.h" -#include "source/extensions/tracers/opentelemetry/tracer.h" +#include "tracer.h" namespace Envoy { namespace Extensions { diff --git a/source/extensions/tracers/opentelemetry/trace_exporter.h b/source/extensions/tracers/opentelemetry/trace_exporter.h index bd8798095bc1..0dba0ce7bc0a 100644 --- a/source/extensions/tracers/opentelemetry/trace_exporter.h +++ b/source/extensions/tracers/opentelemetry/trace_exporter.h @@ -9,7 +9,7 @@ namespace Extensions { namespace Tracers { namespace OpenTelemetry { -class OpenTelemetryTraceExporter : Logger::Loggable { +class OpenTelemetryTraceExporter : public Logger::Loggable { public: virtual ~OpenTelemetryTraceExporter() = default; From 1455975c766ae5018cc7b4c722b890cee82a1c0a Mon Sep 17 00:00:00 2001 From: Alex Ellis Date: Tue, 20 Jun 2023 00:35:06 -0400 Subject: [PATCH 04/14] Code formatting Signed-off-by: Alex Ellis --- source/extensions/tracers/opentelemetry/BUILD | 17 ++++++------ .../opentelemetry/grpc_trace_exporter.h | 1 - .../opentelemetry/http_trace_exporter.cc | 27 +++++++++---------- .../opentelemetry/http_trace_exporter.h | 8 +++--- .../opentelemetry_tracer_impl.cc | 7 +++-- .../opentelemetry/opentelemetry_tracer_impl.h | 1 + .../tracers/opentelemetry/trace_exporter.h | 3 +-- 7 files changed, 31 insertions(+), 33 deletions(-) diff --git a/source/extensions/tracers/opentelemetry/BUILD b/source/extensions/tracers/opentelemetry/BUILD index 0171bb68d0d3..b4bb9f4daa80 100644 --- a/source/extensions/tracers/opentelemetry/BUILD +++ b/source/extensions/tracers/opentelemetry/BUILD @@ -49,24 +49,25 @@ envoy_cc_library( envoy_cc_library( name = "trace_exporter", srcs = [ - "grpc_trace_exporter.cc", - "http_trace_exporter.cc", - ], + "grpc_trace_exporter.cc", + "http_trace_exporter.cc", + ], hdrs = [ - "grpc_trace_exporter.h", - "http_trace_exporter.h", - "trace_exporter.h" - ], + "grpc_trace_exporter.h", + "http_trace_exporter.h", + "trace_exporter.h", + ], deps = [ "//envoy/grpc:async_client_manager_interface", - "//source/common/grpc:typed_async_client_lib", "//envoy/upstream:cluster_manager_interface", + "//source/common/grpc:typed_async_client_lib", "//source/common/http:async_client_utility_lib", "//source/common/http:header_map_lib", "//source/common/http:message_lib", "//source/common/http:utility_lib", "//source/common/protobuf", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", + "@envoy_api//envoy/config/trace/v3:pkg_cc_proto", "@opentelemetry_proto//:trace_cc_proto", ], ) diff --git a/source/extensions/tracers/opentelemetry/grpc_trace_exporter.h b/source/extensions/tracers/opentelemetry/grpc_trace_exporter.h index d2d054d57d52..40f70a0b8b0a 100644 --- a/source/extensions/tracers/opentelemetry/grpc_trace_exporter.h +++ b/source/extensions/tracers/opentelemetry/grpc_trace_exporter.h @@ -6,7 +6,6 @@ #include "source/common/grpc/typed_async_client.h" #include "opentelemetry/proto/collector/trace/v1/trace_service.pb.h" - #include "trace_exporter.h" namespace Envoy { diff --git a/source/extensions/tracers/opentelemetry/http_trace_exporter.cc b/source/extensions/tracers/opentelemetry/http_trace_exporter.cc index c99178a3dd76..bf0b991dfdd5 100644 --- a/source/extensions/tracers/opentelemetry/http_trace_exporter.cc +++ b/source/extensions/tracers/opentelemetry/http_trace_exporter.cc @@ -1,4 +1,3 @@ - #include "http_trace_exporter.h" #include "source/common/common/logger.h" @@ -10,16 +9,18 @@ namespace Tracers { namespace OpenTelemetry { OpenTelemetryHttpTraceExporter::OpenTelemetryHttpTraceExporter( - Upstream::ClusterManager& cluster_manager, - envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config) + Upstream::ClusterManager& cluster_manager, + envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config) : cluster_manager_(cluster_manager), http_config_(http_config) {} bool OpenTelemetryHttpTraceExporter::log(const ExportTraceServiceRequest& request) { std::string request_body; - // TODO: add JSON option as well (though it may need custom serializing, see https://opentelemetry.io/docs/specs/otlp/#json-protobuf-encoding) - if (http_config_.http_format() == envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig::BINARY_PROTOBUF) { + // TODO: add JSON option as well (though it may need custom serializing, see + // https://opentelemetry.io/docs/specs/otlp/#json-protobuf-encoding) + if (http_config_.http_format() == + envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig::BINARY_PROTOBUF) { const auto ok = request.SerializeToString(&request_body); if (!ok) { ENVOY_LOG(warn, "Error during Span proto serialization."); @@ -34,25 +35,23 @@ bool OpenTelemetryHttpTraceExporter::log(const ExportTraceServiceRequest& reques message->headers().setReferenceContentType(Http::Headers::get().ContentTypeValues.Protobuf); message->body().add(request_body); - const auto thread_local_cluster = cluster_manager_.getThreadLocalCluster(http_config_.http_uri().cluster()); + const auto thread_local_cluster = + cluster_manager_.getThreadLocalCluster(http_config_.http_uri().cluster()); if (thread_local_cluster == nullptr) { ENVOY_LOG(warn, "Thread local cluster not found for collector."); return false; } - std::chrono::milliseconds timeout = - std::chrono::duration_cast( + std::chrono::milliseconds timeout = std::chrono::duration_cast( std::chrono::nanoseconds(http_config_.http_uri().timeout().nanos())); - Http::AsyncClient::Request* http_request = - thread_local_cluster->httpAsyncClient().send( - std::move(message), *this, - Http::AsyncClient::RequestOptions().setTimeout(timeout)); + Http::AsyncClient::Request* http_request = thread_local_cluster->httpAsyncClient().send( + std::move(message), *this, Http::AsyncClient::RequestOptions().setTimeout(timeout)); return http_request; } void OpenTelemetryHttpTraceExporter::onSuccess(const Http::AsyncClient::Request&, - Http::ResponseMessagePtr&& message) { + Http::ResponseMessagePtr&& message) { const auto response_code = message->headers().Status()->value().getStringView(); if (response_code != "200") { ENVOY_LOG(warn, "response code: {}", response_code); @@ -60,7 +59,7 @@ void OpenTelemetryHttpTraceExporter::onSuccess(const Http::AsyncClient::Request& } void OpenTelemetryHttpTraceExporter::onFailure(const Http::AsyncClient::Request&, - Http::AsyncClient::FailureReason) { + Http::AsyncClient::FailureReason) { ENVOY_LOG(debug, "Request failed."); } diff --git a/source/extensions/tracers/opentelemetry/http_trace_exporter.h b/source/extensions/tracers/opentelemetry/http_trace_exporter.h index e6d48056b6a8..5dbb80468917 100644 --- a/source/extensions/tracers/opentelemetry/http_trace_exporter.h +++ b/source/extensions/tracers/opentelemetry/http_trace_exporter.h @@ -12,9 +12,8 @@ #include "source/common/http/message_impl.h" #include "source/common/http/utility.h" -#include "trace_exporter.h" - #include "opentelemetry/proto/collector/trace/v1/trace_service.pb.h" +#include "trace_exporter.h" namespace Envoy { namespace Extensions { @@ -27,8 +26,9 @@ namespace OpenTelemetry { class OpenTelemetryHttpTraceExporter : public OpenTelemetryTraceExporter, public Http::AsyncClient::Callbacks { public: - OpenTelemetryHttpTraceExporter(Upstream::ClusterManager& cluster_manager, - envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config); + OpenTelemetryHttpTraceExporter( + Upstream::ClusterManager& cluster_manager, + envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config); bool log(const ExportTraceServiceRequest& request) override; diff --git a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc index b1bf2bb74df4..bea7535504a3 100644 --- a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc +++ b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc @@ -9,11 +9,10 @@ #include "source/common/config/utility.h" #include "source/common/tracing/http_tracer_impl.h" -#include "opentelemetry/proto/collector/trace/v1/trace_service.pb.h" -#include "opentelemetry/proto/trace/v1/trace.pb.h" - #include "grpc_trace_exporter.h" #include "http_trace_exporter.h" +#include "opentelemetry/proto/collector/trace/v1/trace_service.pb.h" +#include "opentelemetry/proto/trace/v1/trace.pb.h" #include "span_context.h" #include "span_context_extractor.h" #include "trace_exporter.h" @@ -42,7 +41,7 @@ Driver::Driver(const envoy::config::trace::v3::OpenTelemetryConfig& opentelemetr exporter = std::make_unique(async_client_shared_ptr); } else if (opentelemetry_config.has_http_config()) { exporter = std::make_unique( - factory_context.clusterManager(), opentelemetry_config.http_config()); + factory_context.clusterManager(), opentelemetry_config.http_config()); } TracerPtr tracer = std::make_unique( std::move(exporter), factory_context.timeSource(), factory_context.api().randomGenerator(), diff --git a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.h b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.h index 2b8f769f1a17..dd9bb68453b8 100644 --- a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.h +++ b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.h @@ -8,6 +8,7 @@ #include "source/common/common/logger.h" #include "source/common/singleton/const_singleton.h" #include "source/extensions/tracers/common/factory_base.h" + #include "tracer.h" namespace Envoy { diff --git a/source/extensions/tracers/opentelemetry/trace_exporter.h b/source/extensions/tracers/opentelemetry/trace_exporter.h index 0dba0ce7bc0a..cd35c64040e1 100644 --- a/source/extensions/tracers/opentelemetry/trace_exporter.h +++ b/source/extensions/tracers/opentelemetry/trace_exporter.h @@ -16,10 +16,9 @@ class OpenTelemetryTraceExporter : public Logger::Loggable virtual bool log(const ExportTraceServiceRequest& request) = 0; }; - using OpenTelemetryTraceExporterPtr = std::unique_ptr; } // namespace OpenTelemetry } // namespace Tracers } // namespace Extensions -} // namespace Envoy \ No newline at end of file +} // namespace Envoy From a7fa79bba36a6ea4921c3eab8bc4f7d4c7ab587c Mon Sep 17 00:00:00 2001 From: Alex Ellis Date: Tue, 20 Jun 2023 00:44:31 -0400 Subject: [PATCH 05/14] Proto format Signed-off-by: Alex Ellis --- api/envoy/config/trace/v3/opentelemetry.proto | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/api/envoy/config/trace/v3/opentelemetry.proto b/api/envoy/config/trace/v3/opentelemetry.proto index 453217051bf6..e4fe63a21c33 100644 --- a/api/envoy/config/trace/v3/opentelemetry.proto +++ b/api/envoy/config/trace/v3/opentelemetry.proto @@ -18,21 +18,9 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // Configuration for the OpenTelemetry tracer. // [#extension: envoy.tracers.opentelemetry] message OpenTelemetryConfig { - // The upstream gRPC cluster that will receive OTLP traces. - // Note that the tracer drops traces if the server does not read data fast enough. - // This field can be left empty to disable reporting traces to the collector. - core.v3.GrpcService grpc_service = 1; - - // The name for the service. This will be populated in the ResourceSpan Resource attributes. - // If it is not provided, it will default to "unknown_service:envoy". - string service_name = 2; - // Configuration for sending traces to the collector over HTTP. Note that // this will only be used if the grpc_service is not set above. message HttpConfig { - // The upstream cluster that will receive OTLP traces over HTTP. - config.core.v3.HttpUri http_uri = 1; - enum HttpFormat { // Binary Protobuf Encoding. BINARY_PROTOBUF = 0; @@ -41,6 +29,9 @@ message OpenTelemetryConfig { JSON_PROTOBUF = 1; } + // The upstream cluster that will receive OTLP traces over HTTP. + core.v3.HttpUri http_uri = 1; + // OTLP/HTTP uses Protobuf payloads encoded either in binary format or in JSON format. // See https://opentelemetry.io/docs/specs/otlp/#otlphttp. HttpFormat http_format = 2; @@ -52,6 +43,15 @@ message OpenTelemetryConfig { string collector_hostname = 4; } + // The upstream gRPC cluster that will receive OTLP traces. + // Note that the tracer drops traces if the server does not read data fast enough. + // This field can be left empty to disable reporting traces to the collector. + core.v3.GrpcService grpc_service = 1; + + // The name for the service. This will be populated in the ResourceSpan Resource attributes. + // If it is not provided, it will default to "unknown_service:envoy". + string service_name = 2; + // This field can be also left empty to disable reporting traces to the collector. HttpConfig http_config = 3; } From b707eae9e59e9b829d51647c6f3c65ae8f5420e4 Mon Sep 17 00:00:00 2001 From: Alex Ellis Date: Thu, 22 Jun 2023 22:43:26 -0400 Subject: [PATCH 06/14] Add basic tests Signed-off-by: Alex Ellis --- test/extensions/tracers/opentelemetry/BUILD | 12 ++ .../opentelemetry/http_trace_exporter_test.cc | 130 ++++++++++++++++++ 2 files changed, 142 insertions(+) create mode 100644 test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc diff --git a/test/extensions/tracers/opentelemetry/BUILD b/test/extensions/tracers/opentelemetry/BUILD index 058cae48f2c8..bb9b1677397c 100644 --- a/test/extensions/tracers/opentelemetry/BUILD +++ b/test/extensions/tracers/opentelemetry/BUILD @@ -79,3 +79,15 @@ envoy_extension_cc_test( "//test/test_common:utility_lib", ], ) + +envoy_extension_cc_test( + name = "http_trace_exporter_test", + srcs = ["http_trace_exporter_test.cc"], + extension_names = ["envoy.tracers.opentelemetry"], + deps = [ + "//source/extensions/tracers/opentelemetry:trace_exporter", + "//test/mocks/http:http_mocks", + "//test/mocks/upstream:cluster_manager_mocks", + "//test/test_common:utility_lib", + ], +) diff --git a/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc b/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc new file mode 100644 index 000000000000..c8fc9289cf66 --- /dev/null +++ b/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc @@ -0,0 +1,130 @@ + +#include + +#include "source/common/buffer/zero_copy_input_stream_impl.h" +#include "source/extensions/tracers/opentelemetry/http_trace_exporter.h" + +#include "test/mocks/common.h" +#include "test/mocks/grpc/mocks.h" +#include "test/mocks/upstream/cluster_manager.h" +#include "test/test_common/utility.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +using testing::_; +using testing::Invoke; +using testing::Return; +using testing::ReturnRef; + +class OpenTelemetryHttpTraceExporterTest : public testing::Test { +public: + + OpenTelemetryHttpTraceExporterTest() {} + + void setup(envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config) { + cluster_manager_.thread_local_cluster_.cluster_.info_->name_ = "fake_collector"; + cluster_manager_.initializeThreadLocalClusters({"fake_collector"}); + ON_CALL(cluster_manager_.thread_local_cluster_, httpAsyncClient()) + .WillByDefault(ReturnRef(cluster_manager_.thread_local_cluster_.async_client_)); + + cluster_manager_.initializeClusters({"fake_collector"}, {}); + + trace_exporter_ = std::make_unique(cluster_manager_, http_config); + } + +protected: + NiceMock cluster_manager_; + std::unique_ptr trace_exporter_; +}; + +TEST_F(OpenTelemetryHttpTraceExporterTest, CreateExporterAndExportSpan) { + std::string yaml_string = fmt::format(R"EOF( + http_uri: + uri: "https://www.example.com/v1/traces" + cluster: fake_collector + timeout: 0.250s + http_format: BINARY_PROTOBUF + collector_path: "/v1/traces" + collector_hostname: "example.com" + )EOF"); + + envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config; + TestUtility::loadFromYaml(yaml_string, http_config); + setup(http_config); + + Http::MockAsyncClientRequest request(&cluster_manager_.thread_local_cluster_.async_client_); + Http::AsyncClient::Callbacks* callback; + + EXPECT_CALL(cluster_manager_.thread_local_cluster_.async_client_, + send_(_, _, Http::AsyncClient::RequestOptions().setTimeout( + std::chrono::milliseconds(250) + ))) + .WillOnce( + Invoke([&](Http::RequestMessagePtr& message, Http::AsyncClient::Callbacks& callbacks, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + callback = &callbacks; + + EXPECT_EQ("/v1/traces", message->headers().getPathValue()); + EXPECT_EQ("example.com", message->headers().getHostValue()); + EXPECT_EQ(Http::Headers::get().ContentTypeValues.Protobuf, message->headers().getContentTypeValue()); + + return &request; + })); + + std::string request_yaml = R"EOF( + resource_spans: + scope_spans: + - spans: + - name: "test" + )EOF"; + opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest export_trace_service_request; + opentelemetry::proto::trace::v1::Span span; + span.set_name("test"); + *export_trace_service_request.add_resource_spans()->add_scope_spans()->add_spans() = span; + EXPECT_TRUE(trace_exporter_->log(export_trace_service_request)); +} + +TEST_F(OpenTelemetryHttpTraceExporterTest, UnsuccessfulLogWithoutThreadLocalCluster) { + std::string yaml_string = fmt::format(R"EOF( + http_uri: + uri: "https://www.example.com/v1/traces" + cluster: fake_collector + timeout: 0.250s + http_format: BINARY_PROTOBUF + collector_path: "/v1/traces" + collector_hostname: "example.com" + )EOF"); + + envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config; + TestUtility::loadFromYaml(yaml_string, http_config); + setup(http_config); + + Http::MockAsyncClientRequest request(&cluster_manager_.thread_local_cluster_.async_client_); + + ON_CALL(cluster_manager_, getThreadLocalCluster(absl::string_view("fake_collector"))) + .WillByDefault(Return(nullptr)); + + + std::string request_yaml = R"EOF( + resource_spans: + scope_spans: + - spans: + - name: "test" + )EOF"; + opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest export_trace_service_request; + opentelemetry::proto::trace::v1::Span span; + span.set_name("test"); + *export_trace_service_request.add_resource_spans()->add_scope_spans()->add_spans() = span; + EXPECT_FALSE(trace_exporter_->log(export_trace_service_request)); +} + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy From a530005c62e7fc029e5d3b7d1c814a2ec56e6fbe Mon Sep 17 00:00:00 2001 From: Alex Ellis Date: Thu, 22 Jun 2023 22:51:40 -0400 Subject: [PATCH 07/14] code format Signed-off-by: Alex Ellis --- .../opentelemetry/http_trace_exporter_test.cc | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc b/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc index c8fc9289cf66..f87c500cdabe 100644 --- a/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc +++ b/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc @@ -1,4 +1,3 @@ - #include #include "source/common/buffer/zero_copy_input_stream_impl.h" @@ -24,7 +23,6 @@ using testing::ReturnRef; class OpenTelemetryHttpTraceExporterTest : public testing::Test { public: - OpenTelemetryHttpTraceExporterTest() {} void setup(envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config) { @@ -35,7 +33,8 @@ class OpenTelemetryHttpTraceExporterTest : public testing::Test { cluster_manager_.initializeClusters({"fake_collector"}, {}); - trace_exporter_ = std::make_unique(cluster_manager_, http_config); + trace_exporter_ = + std::make_unique(cluster_manager_, http_config); } protected: @@ -61,18 +60,18 @@ TEST_F(OpenTelemetryHttpTraceExporterTest, CreateExporterAndExportSpan) { Http::MockAsyncClientRequest request(&cluster_manager_.thread_local_cluster_.async_client_); Http::AsyncClient::Callbacks* callback; - EXPECT_CALL(cluster_manager_.thread_local_cluster_.async_client_, - send_(_, _, Http::AsyncClient::RequestOptions().setTimeout( - std::chrono::milliseconds(250) - ))) + EXPECT_CALL( + cluster_manager_.thread_local_cluster_.async_client_, + send_(_, _, Http::AsyncClient::RequestOptions().setTimeout(std::chrono::milliseconds(250)))) .WillOnce( Invoke([&](Http::RequestMessagePtr& message, Http::AsyncClient::Callbacks& callbacks, - const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { callback = &callbacks; EXPECT_EQ("/v1/traces", message->headers().getPathValue()); EXPECT_EQ("example.com", message->headers().getHostValue()); - EXPECT_EQ(Http::Headers::get().ContentTypeValues.Protobuf, message->headers().getContentTypeValue()); + EXPECT_EQ(Http::Headers::get().ContentTypeValues.Protobuf, + message->headers().getContentTypeValue()); return &request; })); @@ -83,7 +82,8 @@ TEST_F(OpenTelemetryHttpTraceExporterTest, CreateExporterAndExportSpan) { - spans: - name: "test" )EOF"; - opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest export_trace_service_request; + opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest + export_trace_service_request; opentelemetry::proto::trace::v1::Span span; span.set_name("test"); *export_trace_service_request.add_resource_spans()->add_scope_spans()->add_spans() = span; @@ -110,14 +110,14 @@ TEST_F(OpenTelemetryHttpTraceExporterTest, UnsuccessfulLogWithoutThreadLocalClus ON_CALL(cluster_manager_, getThreadLocalCluster(absl::string_view("fake_collector"))) .WillByDefault(Return(nullptr)); - std::string request_yaml = R"EOF( resource_spans: scope_spans: - spans: - name: "test" )EOF"; - opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest export_trace_service_request; + opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest + export_trace_service_request; opentelemetry::proto::trace::v1::Span span; span.set_name("test"); *export_trace_service_request.add_resource_spans()->add_scope_spans()->add_spans() = span; From 85b0607f139e38d8b417a6827794716ee1b11670 Mon Sep 17 00:00:00 2001 From: Alex Ellis Date: Mon, 17 Jul 2023 23:08:15 -0400 Subject: [PATCH 08/14] Add additional counters for http exporter and test Signed-off-by: Alex Ellis --- source/extensions/tracers/opentelemetry/BUILD | 2 ++ .../opentelemetry/http_trace_exporter.cc | 8 ++++-- .../opentelemetry/http_trace_exporter.h | 5 +++- .../opentelemetry_tracer_impl.cc | 2 +- .../extensions/tracers/opentelemetry/tracer.h | 9 +------ .../tracers/opentelemetry/tracer_stats.h | 25 +++++++++++++++++++ test/extensions/tracers/opentelemetry/BUILD | 2 ++ .../opentelemetry/http_trace_exporter_test.cc | 19 +++++++++++++- 8 files changed, 59 insertions(+), 13 deletions(-) create mode 100644 source/extensions/tracers/opentelemetry/tracer_stats.h diff --git a/source/extensions/tracers/opentelemetry/BUILD b/source/extensions/tracers/opentelemetry/BUILD index b4bb9f4daa80..246e2b9cdd85 100644 --- a/source/extensions/tracers/opentelemetry/BUILD +++ b/source/extensions/tracers/opentelemetry/BUILD @@ -34,6 +34,7 @@ envoy_cc_library( "span_context.h", "span_context_extractor.h", "tracer.h", + "tracer_stats.h", ], deps = [ ":trace_exporter", @@ -56,6 +57,7 @@ envoy_cc_library( "grpc_trace_exporter.h", "http_trace_exporter.h", "trace_exporter.h", + "tracer_stats.h", ], deps = [ "//envoy/grpc:async_client_manager_interface", diff --git a/source/extensions/tracers/opentelemetry/http_trace_exporter.cc b/source/extensions/tracers/opentelemetry/http_trace_exporter.cc index bf0b991dfdd5..45bceb55904d 100644 --- a/source/extensions/tracers/opentelemetry/http_trace_exporter.cc +++ b/source/extensions/tracers/opentelemetry/http_trace_exporter.cc @@ -10,8 +10,9 @@ namespace OpenTelemetry { OpenTelemetryHttpTraceExporter::OpenTelemetryHttpTraceExporter( Upstream::ClusterManager& cluster_manager, - envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config) - : cluster_manager_(cluster_manager), http_config_(http_config) {} + envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config, + OpenTelemetryTracerStats& tracing_stats) + : cluster_manager_(cluster_manager), http_config_(http_config), tracing_stats_(tracing_stats) {} bool OpenTelemetryHttpTraceExporter::log(const ExportTraceServiceRequest& request) { @@ -46,12 +47,14 @@ bool OpenTelemetryHttpTraceExporter::log(const ExportTraceServiceRequest& reques std::chrono::nanoseconds(http_config_.http_uri().timeout().nanos())); Http::AsyncClient::Request* http_request = thread_local_cluster->httpAsyncClient().send( std::move(message), *this, Http::AsyncClient::RequestOptions().setTimeout(timeout)); + tracing_stats_.http_reports_sent_.inc(); return http_request; } void OpenTelemetryHttpTraceExporter::onSuccess(const Http::AsyncClient::Request&, Http::ResponseMessagePtr&& message) { + tracing_stats_.http_reports_success_.inc(); const auto response_code = message->headers().Status()->value().getStringView(); if (response_code != "200") { ENVOY_LOG(warn, "response code: {}", response_code); @@ -61,6 +64,7 @@ void OpenTelemetryHttpTraceExporter::onSuccess(const Http::AsyncClient::Request& void OpenTelemetryHttpTraceExporter::onFailure(const Http::AsyncClient::Request&, Http::AsyncClient::FailureReason) { ENVOY_LOG(debug, "Request failed."); + tracing_stats_.http_reports_failed_.inc(); } } // namespace OpenTelemetry diff --git a/source/extensions/tracers/opentelemetry/http_trace_exporter.h b/source/extensions/tracers/opentelemetry/http_trace_exporter.h index 5dbb80468917..bea7304c56ee 100644 --- a/source/extensions/tracers/opentelemetry/http_trace_exporter.h +++ b/source/extensions/tracers/opentelemetry/http_trace_exporter.h @@ -14,6 +14,7 @@ #include "opentelemetry/proto/collector/trace/v1/trace_service.pb.h" #include "trace_exporter.h" +#include "tracer_stats.h" namespace Envoy { namespace Extensions { @@ -28,7 +29,8 @@ class OpenTelemetryHttpTraceExporter : public OpenTelemetryTraceExporter, public: OpenTelemetryHttpTraceExporter( Upstream::ClusterManager& cluster_manager, - envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config); + envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config, + OpenTelemetryTracerStats& tracing_stats); bool log(const ExportTraceServiceRequest& request) override; @@ -40,6 +42,7 @@ class OpenTelemetryHttpTraceExporter : public OpenTelemetryTraceExporter, private: Upstream::ClusterManager& cluster_manager_; envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config_; + OpenTelemetryTracerStats& tracing_stats_; }; } // namespace OpenTelemetry diff --git a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc index bea7535504a3..153d943a3e39 100644 --- a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc +++ b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc @@ -41,7 +41,7 @@ Driver::Driver(const envoy::config::trace::v3::OpenTelemetryConfig& opentelemetr exporter = std::make_unique(async_client_shared_ptr); } else if (opentelemetry_config.has_http_config()) { exporter = std::make_unique( - factory_context.clusterManager(), opentelemetry_config.http_config()); + factory_context.clusterManager(), opentelemetry_config.http_config(), tracing_stats_); } TracerPtr tracer = std::make_unique( std::move(exporter), factory_context.timeSource(), factory_context.api().randomGenerator(), diff --git a/source/extensions/tracers/opentelemetry/tracer.h b/source/extensions/tracers/opentelemetry/tracer.h index 67982689f251..8d4573996ac2 100644 --- a/source/extensions/tracers/opentelemetry/tracer.h +++ b/source/extensions/tracers/opentelemetry/tracer.h @@ -14,20 +14,13 @@ #include "absl/strings/escaping.h" #include "span_context.h" +#include "tracer_stats.h" namespace Envoy { namespace Extensions { namespace Tracers { namespace OpenTelemetry { -#define OPENTELEMETRY_TRACER_STATS(COUNTER) \ - COUNTER(spans_sent) \ - COUNTER(timer_flushed) - -struct OpenTelemetryTracerStats { - OPENTELEMETRY_TRACER_STATS(GENERATE_COUNTER_STRUCT) -}; - /** * OpenTelemetry Tracer. It is stored in TLS and contains the exporter. */ diff --git a/source/extensions/tracers/opentelemetry/tracer_stats.h b/source/extensions/tracers/opentelemetry/tracer_stats.h new file mode 100644 index 000000000000..127af00d42e4 --- /dev/null +++ b/source/extensions/tracers/opentelemetry/tracer_stats.h @@ -0,0 +1,25 @@ +#pragma once + +#include "envoy/stats/stats_macros.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +#define OPENTELEMETRY_TRACER_STATS(COUNTER) \ + COUNTER(http_reports_failed) \ + COUNTER(http_reports_sent) \ + COUNTER(http_reports_success) \ + COUNTER(spans_sent) \ + COUNTER(timer_flushed) + +struct OpenTelemetryTracerStats { + OPENTELEMETRY_TRACER_STATS(GENERATE_COUNTER_STRUCT) +}; + + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/tracers/opentelemetry/BUILD b/test/extensions/tracers/opentelemetry/BUILD index bb9b1677397c..1a949ec68090 100644 --- a/test/extensions/tracers/opentelemetry/BUILD +++ b/test/extensions/tracers/opentelemetry/BUILD @@ -89,5 +89,7 @@ envoy_extension_cc_test( "//test/mocks/http:http_mocks", "//test/mocks/upstream:cluster_manager_mocks", "//test/test_common:utility_lib", + "//test/mocks/server:tracer_factory_context_mocks", + "//test/mocks/stats:stats_mocks", ], ) diff --git a/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc b/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc index f87c500cdabe..cc98c2b0c256 100644 --- a/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc +++ b/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc @@ -5,6 +5,8 @@ #include "test/mocks/common.h" #include "test/mocks/grpc/mocks.h" +#include "test/mocks/server/tracer_factory_context.h" +#include "test/mocks/stats/mocks.h" #include "test/mocks/upstream/cluster_manager.h" #include "test/test_common/utility.h" @@ -34,12 +36,16 @@ class OpenTelemetryHttpTraceExporterTest : public testing::Test { cluster_manager_.initializeClusters({"fake_collector"}, {}); trace_exporter_ = - std::make_unique(cluster_manager_, http_config); + std::make_unique(cluster_manager_, http_config, stats_); } protected: NiceMock cluster_manager_; std::unique_ptr trace_exporter_; + NiceMock context_; + NiceMock& mock_scope_ = context_.server_factory_context_.store_; + OpenTelemetryTracerStats stats_{OpenTelemetryTracerStats{ + OPENTELEMETRY_TRACER_STATS(POOL_COUNTER_PREFIX(mock_scope_, "tracing.opentelemetry."))}}; }; TEST_F(OpenTelemetryHttpTraceExporterTest, CreateExporterAndExportSpan) { @@ -88,6 +94,17 @@ TEST_F(OpenTelemetryHttpTraceExporterTest, CreateExporterAndExportSpan) { span.set_name("test"); *export_trace_service_request.add_resource_spans()->add_scope_spans()->add_spans() = span; EXPECT_TRUE(trace_exporter_->log(export_trace_service_request)); + + Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl( + Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "200"}}})); + callback->onSuccess(request, std::move(msg)); + EXPECT_EQ(1U, mock_scope_.counter("tracing.opentelemetry.http_reports_sent").value()); + EXPECT_EQ(1U, mock_scope_.counter("tracing.opentelemetry.http_reports_success").value()); + EXPECT_EQ(0U, mock_scope_.counter("tracing.opentelemetry.http_reports_failed").value()); + + callback->onFailure(request, Http::AsyncClient::FailureReason::Reset); + + EXPECT_EQ(1U, mock_scope_.counter("tracing.opentelemetry.http_reports_failed").value()); } TEST_F(OpenTelemetryHttpTraceExporterTest, UnsuccessfulLogWithoutThreadLocalCluster) { From 0952ef950ec2948c4a908645853821db265274bf Mon Sep 17 00:00:00 2001 From: Alex Ellis Date: Mon, 17 Jul 2023 23:10:35 -0400 Subject: [PATCH 09/14] Formatting Signed-off-by: Alex Ellis --- .../extensions/tracers/opentelemetry/tracer_stats.h | 11 +++++------ test/extensions/tracers/opentelemetry/BUILD | 4 ++-- .../tracers/opentelemetry/http_trace_exporter_test.cc | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/source/extensions/tracers/opentelemetry/tracer_stats.h b/source/extensions/tracers/opentelemetry/tracer_stats.h index 127af00d42e4..bb14b901ddd5 100644 --- a/source/extensions/tracers/opentelemetry/tracer_stats.h +++ b/source/extensions/tracers/opentelemetry/tracer_stats.h @@ -7,18 +7,17 @@ namespace Extensions { namespace Tracers { namespace OpenTelemetry { -#define OPENTELEMETRY_TRACER_STATS(COUNTER) \ - COUNTER(http_reports_failed) \ - COUNTER(http_reports_sent) \ - COUNTER(http_reports_success) \ - COUNTER(spans_sent) \ +#define OPENTELEMETRY_TRACER_STATS(COUNTER) \ + COUNTER(http_reports_failed) \ + COUNTER(http_reports_sent) \ + COUNTER(http_reports_success) \ + COUNTER(spans_sent) \ COUNTER(timer_flushed) struct OpenTelemetryTracerStats { OPENTELEMETRY_TRACER_STATS(GENERATE_COUNTER_STRUCT) }; - } // namespace OpenTelemetry } // namespace Tracers } // namespace Extensions diff --git a/test/extensions/tracers/opentelemetry/BUILD b/test/extensions/tracers/opentelemetry/BUILD index 1a949ec68090..96e7e5d91537 100644 --- a/test/extensions/tracers/opentelemetry/BUILD +++ b/test/extensions/tracers/opentelemetry/BUILD @@ -87,9 +87,9 @@ envoy_extension_cc_test( deps = [ "//source/extensions/tracers/opentelemetry:trace_exporter", "//test/mocks/http:http_mocks", - "//test/mocks/upstream:cluster_manager_mocks", - "//test/test_common:utility_lib", "//test/mocks/server:tracer_factory_context_mocks", "//test/mocks/stats:stats_mocks", + "//test/mocks/upstream:cluster_manager_mocks", + "//test/test_common:utility_lib", ], ) diff --git a/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc b/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc index cc98c2b0c256..aa0eedd8fc3b 100644 --- a/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc +++ b/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc @@ -45,7 +45,7 @@ class OpenTelemetryHttpTraceExporterTest : public testing::Test { NiceMock context_; NiceMock& mock_scope_ = context_.server_factory_context_.store_; OpenTelemetryTracerStats stats_{OpenTelemetryTracerStats{ - OPENTELEMETRY_TRACER_STATS(POOL_COUNTER_PREFIX(mock_scope_, "tracing.opentelemetry."))}}; + OPENTELEMETRY_TRACER_STATS(POOL_COUNTER_PREFIX(mock_scope_, "tracing.opentelemetry."))}}; }; TEST_F(OpenTelemetryHttpTraceExporterTest, CreateExporterAndExportSpan) { From 82f94262db7d7e25b61973d3feb7bef1253ad17b Mon Sep 17 00:00:00 2001 From: Alex Ellis Date: Tue, 18 Jul 2023 10:07:10 -0400 Subject: [PATCH 10/14] Coverage improvements Signed-off-by: Alex Ellis --- .../tracers/opentelemetry/http_trace_exporter_test.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc b/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc index aa0eedd8fc3b..ee2eb8411d28 100644 --- a/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc +++ b/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc @@ -96,7 +96,11 @@ TEST_F(OpenTelemetryHttpTraceExporterTest, CreateExporterAndExportSpan) { EXPECT_TRUE(trace_exporter_->log(export_trace_service_request)); Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl( - Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "200"}}})); + Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "202"}}})); + // onBeforeFinalizeUpstreamSpan is a noop — included for coverage + Tracing::NullSpan null_span; + callback->onBeforeFinalizeUpstreamSpan(null_span, nullptr); + callback->onSuccess(request, std::move(msg)); EXPECT_EQ(1U, mock_scope_.counter("tracing.opentelemetry.http_reports_sent").value()); EXPECT_EQ(1U, mock_scope_.counter("tracing.opentelemetry.http_reports_success").value()); @@ -141,6 +145,8 @@ TEST_F(OpenTelemetryHttpTraceExporterTest, UnsuccessfulLogWithoutThreadLocalClus EXPECT_FALSE(trace_exporter_->log(export_trace_service_request)); } + + } // namespace OpenTelemetry } // namespace Tracers } // namespace Extensions From 66a10c34a427e6fb083db690af2f8e3cb9bf1eaa Mon Sep 17 00:00:00 2001 From: Alex Ellis Date: Tue, 18 Jul 2023 10:09:47 -0400 Subject: [PATCH 11/14] Formatting Signed-off-by: Alex Ellis --- .../tracers/opentelemetry/http_trace_exporter_test.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc b/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc index ee2eb8411d28..f5d3973e2296 100644 --- a/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc +++ b/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc @@ -145,8 +145,6 @@ TEST_F(OpenTelemetryHttpTraceExporterTest, UnsuccessfulLogWithoutThreadLocalClus EXPECT_FALSE(trace_exporter_->log(export_trace_service_request)); } - - } // namespace OpenTelemetry } // namespace Tracers } // namespace Extensions From e3be1ffe0858888dcb90b043fb1d6cc8bbb141ac Mon Sep 17 00:00:00 2001 From: Alex Ellis Date: Tue, 18 Jul 2023 20:51:24 -0400 Subject: [PATCH 12/14] Add higher level Driver test for HTTP exporting Signed-off-by: Alex Ellis --- .../opentelemetry_tracer_impl_test.cc | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc b/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc index 7a427a500955..52c048705a0f 100644 --- a/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc +++ b/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc @@ -60,6 +60,23 @@ class OpenTelemetryDriverTest : public testing::Test { setup(opentelemetry_config); } + void setupValidDriverWithHttpExporter() { + const std::string yaml_string = R"EOF( + http_config: + http_uri: + uri: "https://www.example.com/v1/traces" + cluster: opentelemetry_collector + timeout: 0.250s + http_format: BINARY_PROTOBUF + collector_path: "/v1/traces" + collector_hostname: "example.com" + )EOF"; + envoy::config::trace::v3::OpenTelemetryConfig opentelemetry_config; + TestUtility::loadFromYaml(yaml_string, opentelemetry_config); + + setup(opentelemetry_config); + } + protected: const std::string operation_name_{"test"}; NiceMock context_; @@ -80,6 +97,11 @@ TEST_F(OpenTelemetryDriverTest, InitializeDriverValidConfig) { EXPECT_NE(driver_, nullptr); } +TEST_F(OpenTelemetryDriverTest, InitializeDriverValidConfigHttpExporter) { + setupValidDriverWithHttpExporter(); + EXPECT_NE(driver_, nullptr); +} + TEST_F(OpenTelemetryDriverTest, ParseSpanContextFromHeadersTest) { // Set up driver setupValidDriver(); @@ -511,6 +533,34 @@ TEST_F(OpenTelemetryDriverTest, ExportSpanWithCustomServiceName) { EXPECT_EQ(1U, stats_.counter("tracing.opentelemetry.spans_sent").value()); } +TEST_F(OpenTelemetryDriverTest, ExportOTLPSpanHTTP) { + context_.server_factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->name_ = "opentelemetry_collector"; + context_.server_factory_context_.cluster_manager_.initializeThreadLocalClusters({"opentelemetry_collector"}); + ON_CALL(context_.server_factory_context_.cluster_manager_.thread_local_cluster_, httpAsyncClient()) + .WillByDefault(ReturnRef(context_.server_factory_context_.cluster_manager_.thread_local_cluster_.async_client_)); + context_.server_factory_context_.cluster_manager_.initializeClusters({"opentelemetry_collector"}, {}); + setupValidDriverWithHttpExporter(); + + Http::TestRequestHeaderMapImpl request_headers{ + {":authority", "test.com"}, {":path", "/"}, {":method", "GET"}}; + Tracing::SpanPtr span = driver_->startSpan(mock_tracing_config_, request_headers, stream_info_, + operation_name_, {Tracing::Reason::Sampling, true}); + EXPECT_NE(span.get(), nullptr); + + // Flush after a single span. + EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.opentelemetry.min_flush_spans", 5U)) + .Times(1) + .WillRepeatedly(Return(1)); + // We should see a call to the async client to export that single span. + EXPECT_CALL( + context_.server_factory_context_.cluster_manager_.thread_local_cluster_.async_client_, + send_(_, _, _)); + + span->finishSpan(); + + EXPECT_EQ(1U, stats_.counter("tracing.opentelemetry.spans_sent").value()); +} + } // namespace OpenTelemetry } // namespace Tracers } // namespace Extensions From 85e7428a9131749badd2c425fc26169c78e88ef6 Mon Sep 17 00:00:00 2001 From: Alex Ellis Date: Tue, 18 Jul 2023 20:52:39 -0400 Subject: [PATCH 13/14] More formatting Signed-off-by: Alex Ellis --- .../opentelemetry_tracer_impl_test.cc | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc b/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc index 52c048705a0f..6c176c250a4f 100644 --- a/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc +++ b/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc @@ -534,11 +534,16 @@ TEST_F(OpenTelemetryDriverTest, ExportSpanWithCustomServiceName) { } TEST_F(OpenTelemetryDriverTest, ExportOTLPSpanHTTP) { - context_.server_factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->name_ = "opentelemetry_collector"; - context_.server_factory_context_.cluster_manager_.initializeThreadLocalClusters({"opentelemetry_collector"}); - ON_CALL(context_.server_factory_context_.cluster_manager_.thread_local_cluster_, httpAsyncClient()) - .WillByDefault(ReturnRef(context_.server_factory_context_.cluster_manager_.thread_local_cluster_.async_client_)); - context_.server_factory_context_.cluster_manager_.initializeClusters({"opentelemetry_collector"}, {}); + context_.server_factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->name_ = + "opentelemetry_collector"; + context_.server_factory_context_.cluster_manager_.initializeThreadLocalClusters( + {"opentelemetry_collector"}); + ON_CALL(context_.server_factory_context_.cluster_manager_.thread_local_cluster_, + httpAsyncClient()) + .WillByDefault(ReturnRef( + context_.server_factory_context_.cluster_manager_.thread_local_cluster_.async_client_)); + context_.server_factory_context_.cluster_manager_.initializeClusters({"opentelemetry_collector"}, + {}); setupValidDriverWithHttpExporter(); Http::TestRequestHeaderMapImpl request_headers{ @@ -552,9 +557,8 @@ TEST_F(OpenTelemetryDriverTest, ExportOTLPSpanHTTP) { .Times(1) .WillRepeatedly(Return(1)); // We should see a call to the async client to export that single span. - EXPECT_CALL( - context_.server_factory_context_.cluster_manager_.thread_local_cluster_.async_client_, - send_(_, _, _)); + EXPECT_CALL(context_.server_factory_context_.cluster_manager_.thread_local_cluster_.async_client_, + send_(_, _, _)); span->finishSpan(); From 2553904e75943ed6c42ace231a712b5bf846a7c9 Mon Sep 17 00:00:00 2001 From: Alex Ellis Date: Tue, 18 Jul 2023 23:18:47 -0400 Subject: [PATCH 14/14] Clang tidy Signed-off-by: Alex Ellis --- source/extensions/tracers/opentelemetry/trace_exporter.h | 2 ++ .../tracers/opentelemetry/http_trace_exporter_test.cc | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/source/extensions/tracers/opentelemetry/trace_exporter.h b/source/extensions/tracers/opentelemetry/trace_exporter.h index cd35c64040e1..f6b49d45e6fc 100644 --- a/source/extensions/tracers/opentelemetry/trace_exporter.h +++ b/source/extensions/tracers/opentelemetry/trace_exporter.h @@ -1,5 +1,7 @@ #pragma once +#include "source/common/common/logger.h" + #include "opentelemetry/proto/collector/trace/v1/trace_service.pb.h" using opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest; diff --git a/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc b/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc index f5d3973e2296..be1f5bd829ff 100644 --- a/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc +++ b/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc @@ -25,7 +25,7 @@ using testing::ReturnRef; class OpenTelemetryHttpTraceExporterTest : public testing::Test { public: - OpenTelemetryHttpTraceExporterTest() {} + OpenTelemetryHttpTraceExporterTest() = default; void setup(envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config) { cluster_manager_.thread_local_cluster_.cluster_.info_->name_ = "fake_collector";