From 59868bfe2851e19046333d264d21fc54da4c3892 Mon Sep 17 00:00:00 2001 From: Alex Ellis Date: Mon, 19 Jun 2023 00:28:27 -0400 Subject: [PATCH 01/16] 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 3a36a3f91fd50..a081d6077e16d 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 2d6ff1be89771..d2d054d57d525 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 a3119c187e836..b53a000a8b4a7 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 0000000000000..bd8798095bc14 --- /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 a344a253ab31e..82eb830dcba52 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 f80cb12fd3f76..67982689f2519 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 c1ac977443d69..058cae48f2c80 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/16] 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 e9c7430dcfdd7..453217051bf66 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/16] 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 a081d6077e16d..0171bb68d0d32 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 0000000000000..c99178a3dd768 --- /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 0000000000000..e6d48056b6a85 --- /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 b53a000a8b4a7..b1bf2bb74df46 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 35f734c87b824..2b8f769f1a17b 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 bd8798095bc14..0dba0ce7bc0a2 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/16] 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 0171bb68d0d32..b4bb9f4daa80c 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 d2d054d57d525..40f70a0b8b0a8 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 c99178a3dd768..bf0b991dfdd55 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 e6d48056b6a85..5dbb804689173 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 b1bf2bb74df46..bea7535504a3e 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 2b8f769f1a17b..dd9bb68453b88 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 0dba0ce7bc0a2..cd35c64040e19 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/16] 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 453217051bf66..e4fe63a21c335 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/16] 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 058cae48f2c80..bb9b1677397c0 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 0000000000000..c8fc9289cf665 --- /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/16] 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 c8fc9289cf665..f87c500cdabe3 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/16] 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 b4bb9f4daa80c..246e2b9cdd858 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 bf0b991dfdd55..45bceb55904d6 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 5dbb804689173..bea7304c56ee2 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 bea7535504a3e..153d943a3e395 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 67982689f2519..8d4573996ac26 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 0000000000000..127af00d42e44 --- /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 bb9b1677397c0..1a949ec68090f 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 f87c500cdabe3..cc98c2b0c2560 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/16] 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 127af00d42e44..bb14b901ddd57 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 1a949ec68090f..96e7e5d915375 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 cc98c2b0c2560..aa0eedd8fc3b3 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/16] 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 aa0eedd8fc3b3..ee2eb8411d28a 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/16] 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 ee2eb8411d28a..f5d3973e22966 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/16] 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 7a427a500955f..52c048705a0f8 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/16] 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 52c048705a0f8..6c176c250a4fc 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/16] 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 cd35c64040e19..f6b49d45e6fcc 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 f5d3973e22966..be1f5bd829ffc 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"; From 48e7fb6b7add1ea81f4dd3b2583c7324cc4da7d4 Mon Sep 17 00:00:00 2001 From: Joao Grassi Date: Wed, 16 Aug 2023 16:30:37 +0200 Subject: [PATCH 15/16] Refactor config and add HTTP headers Signed-off-by: Joao Grassi --- api/envoy/config/trace/v3/opentelemetry.proto | 52 +++++++++++-------- .../opentelemetry/http_trace_exporter.cc | 39 +++++++++----- .../opentelemetry/http_trace_exporter.h | 3 ++ .../opentelemetry_tracer_impl.cc | 21 +++++--- 4 files changed, 72 insertions(+), 43 deletions(-) diff --git a/api/envoy/config/trace/v3/opentelemetry.proto b/api/envoy/config/trace/v3/opentelemetry.proto index e4fe63a21c335..8d8cc936be36d 100644 --- a/api/envoy/config/trace/v3/opentelemetry.proto +++ b/api/envoy/config/trace/v3/opentelemetry.proto @@ -2,10 +2,13 @@ syntax = "proto3"; package envoy.config.trace.v3; +import "envoy/config/core/v3/base.proto"; import "envoy/config/core/v3/grpc_service.proto"; -import "envoy/config/core/v3/http_uri.proto"; + +import "google/protobuf/duration.proto"; import "udpa/annotations/status.proto"; +import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.config.trace.v3"; option java_outer_classname = "OpentelemetryProto"; @@ -21,37 +24,40 @@ message OpenTelemetryConfig { // 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 { - enum HttpFormat { - // Binary Protobuf Encoding. - BINARY_PROTOBUF = 0; - - // JSON Protobuf Encoding. - JSON_PROTOBUF = 1; - } // The upstream cluster that will receive OTLP traces over HTTP. - core.v3.HttpUri http_uri = 1; + string cluster_name = 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; + // The path to the trace ingest endpoint. The path is appended with the host name configured in the cluster. + // Optional: If omitted, "/v1/traces" will be used as default. + string traces_path = 2; - // Optional path. If omitted, the default path of "/v1/traces" will be used. - string collector_path = 3; + // Sets the maximum duration in milliseconds that a response can take to arrive upon request. + google.protobuf.Duration timeout = 3 [(validate.rules).duration = { + required: true + gte {} + }]; - // Hostname to include with the request to the collector in the Host header. - string collector_hostname = 4; + // Custom header values to be added to the OTLP HTTP request. + repeated config.core.v3.HeaderValue headers = 4; + + // The hostname to include in the Host header of the OTLP HTTP request. + string hostname = 5; } - // 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; + oneof export_protocol { + option (validate.required) = true; + + // 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 configuration to export OTLP traces via HTTP. + HttpConfig http_config = 3; + } // 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; } diff --git a/source/extensions/tracers/opentelemetry/http_trace_exporter.cc b/source/extensions/tracers/opentelemetry/http_trace_exporter.cc index 45bceb55904d6..943ff6c20d75c 100644 --- a/source/extensions/tracers/opentelemetry/http_trace_exporter.cc +++ b/source/extensions/tracers/opentelemetry/http_trace_exporter.cc @@ -1,3 +1,7 @@ +#include +#include +#include + #include "http_trace_exporter.h" #include "source/common/common/logger.h" @@ -18,33 +22,42 @@ bool OpenTelemetryHttpTraceExporter::log(const ExportTraceServiceRequest& reques 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; - } + const auto ok = request.SerializeToString(&request_body); + if (!ok) { + ENVOY_LOG(warn, "Error while serializing the binary proto ExportTraceServiceRequest."); + 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); + + // If traces_path is omitted, send to /v1/traces by default + if (http_config_.traces_path().empty()) { + message->headers().setPath(TRACES_PATH); + } else { + message->headers().setPath(http_config_.traces_path()); + } + + // TODO: Can we get the hostname that is configured in the cluster "socker_address" field? + message->headers().setHost(http_config_.hostname()); + + // add all custom headers to the request + for (const envoy::config::core::v3::HeaderValue& header : http_config_.headers()) { + message->headers().setCopy(Http::LowerCaseString(header.key()), header.value()); + } + message->body().add(request_body); const auto thread_local_cluster = - cluster_manager_.getThreadLocalCluster(http_config_.http_uri().cluster()); + cluster_manager_.getThreadLocalCluster(http_config_.cluster_name()); 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())); + std::chrono::nanoseconds(http_config_.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(); diff --git a/source/extensions/tracers/opentelemetry/http_trace_exporter.h b/source/extensions/tracers/opentelemetry/http_trace_exporter.h index bea7304c56ee2..8c5b0e7a6fcd2 100644 --- a/source/extensions/tracers/opentelemetry/http_trace_exporter.h +++ b/source/extensions/tracers/opentelemetry/http_trace_exporter.h @@ -32,6 +32,9 @@ class OpenTelemetryHttpTraceExporter : public OpenTelemetryTraceExporter, envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config, OpenTelemetryTracerStats& tracing_stats); + // The default path to use when OpenTelemetryConfig::HttpConfig::traces_path is empty + const Http::LowerCaseString TRACES_PATH{"/v1/traces"}; + bool log(const ExportTraceServiceRequest& request) override; // Http::AsyncClient::Callbacks. diff --git a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc index 153d943a3e395..6a2581b2da843 100644 --- a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc +++ b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc @@ -32,16 +32,23 @@ Driver::Driver(const envoy::config::trace::v3::OpenTelemetryConfig& opentelemetr // Create the tracer in Thread Local Storage. tls_slot_ptr_->set([opentelemetry_config, &factory_context, this](Event::Dispatcher& dispatcher) { OpenTelemetryTraceExporterPtr exporter; - if (opentelemetry_config.has_grpc_service()) { - Grpc::AsyncClientFactoryPtr&& factory = + switch (opentelemetry_config.export_protocol_case()) { + case envoy::config::trace::v3::OpenTelemetryConfig::ExportProtocolCase::kGrpcService: { + Grpc::AsyncClientFactoryPtr&& factory = factory_context.clusterManager().grpcAsyncClientManager().factoryForGrpcService( opentelemetry_config.grpc_service(), factory_context.scope(), true); - 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( + const Grpc::RawAsyncClientSharedPtr& async_client_shared_ptr = + factory->createUncachedRawAsyncClient(); + exporter = std::make_unique(async_client_shared_ptr); + break; + } + case envoy::config::trace::v3::OpenTelemetryConfig::ExportProtocolCase::kHttpConfig: { + exporter = std::make_unique( factory_context.clusterManager(), opentelemetry_config.http_config(), tracing_stats_); + break; + } + default: + break; } TracerPtr tracer = std::make_unique( std::move(exporter), factory_context.timeSource(), factory_context.api().randomGenerator(), From 9f74e9557eed7303db1aa327ead0e14409820f7d Mon Sep 17 00:00:00 2001 From: Joao Grassi Date: Tue, 22 Aug 2023 11:34:47 +0200 Subject: [PATCH 16/16] Modify tests Signed-off-by: Joao Grassi --- .../tracers/opentelemetry/config_test.cc | 32 ++++- .../opentelemetry/http_trace_exporter_test.cc | 136 +++++++++++++----- .../opentelemetry_tracer_impl_test.cc | 20 +-- 3 files changed, 142 insertions(+), 46 deletions(-) diff --git a/test/extensions/tracers/opentelemetry/config_test.cc b/test/extensions/tracers/opentelemetry/config_test.cc index 72206f07910df..ae9c11970eaaf 100644 --- a/test/extensions/tracers/opentelemetry/config_test.cc +++ b/test/extensions/tracers/opentelemetry/config_test.cc @@ -16,7 +16,7 @@ namespace Extensions { namespace Tracers { namespace OpenTelemetry { -TEST(OpenTelemetryTracerConfigTest, OpenTelemetryHttpTracer) { +TEST(OpenTelemetryTracerConfigTest, OpenTelemetryTracerWithGrpcExporter) { NiceMock context; context.server_factory_context_.cluster_manager_.initializeClusters({"fake_cluster"}, {}); OpenTelemetryTracerFactory factory; @@ -41,7 +41,7 @@ TEST(OpenTelemetryTracerConfigTest, OpenTelemetryHttpTracer) { EXPECT_NE(nullptr, opentelemetry_tracer); } -TEST(OpenTelemetryTracerConfigTest, OpenTelemetryHttpTracerNoExporter) { +TEST(OpenTelemetryTracerConfigTest, OpenTelemetryTracerWithHttpExporter) { NiceMock context; context.server_factory_context_.cluster_manager_.initializeClusters({"fake_cluster"}, {}); OpenTelemetryTracerFactory factory; @@ -51,6 +51,14 @@ TEST(OpenTelemetryTracerConfigTest, OpenTelemetryHttpTracerNoExporter) { name: envoy.tracers.opentelemetry typed_config: "@type": type.googleapis.com/envoy.config.trace.v3.OpenTelemetryConfig + http_config: + cluster_name: "my_o11y_backend" + traces_path: "/otlp/v1/traces" + hostname: "some-o11y.com" + headers: + - key: "Authorization" + value: "auth-token" + timeout: 0.250s )EOF"; envoy::config::trace::v3::Tracing configuration; TestUtility::loadFromYaml(yaml_string, configuration); @@ -61,6 +69,26 @@ TEST(OpenTelemetryTracerConfigTest, OpenTelemetryHttpTracerNoExporter) { EXPECT_NE(nullptr, opentelemetry_tracer); } +TEST(OpenTelemetryTracerConfigTest, OpenTelemetryTracerNoExporter) { + NiceMock context; + context.server_factory_context_.cluster_manager_.initializeClusters({"fake_cluster"}, {}); + OpenTelemetryTracerFactory factory; + + const std::string yaml_string = R"EOF( + http: + name: envoy.tracers.opentelemetry + typed_config: + "@type": type.googleapis.com/envoy.config.trace.v3.OpenTelemetryConfig + )EOF"; + envoy::config::trace::v3::Tracing configuration; + TestUtility::loadFromYaml(yaml_string, configuration); + + auto message = Config::Utility::translateToFactoryConfig( + configuration.http(), ProtobufMessage::getStrictValidationVisitor(), factory); + + EXPECT_THROW_WITH_REGEX(factory.createTracerDriver(*message, context), EnvoyException, "field: \"export_protocol\", reason: is required"); +} + } // namespace OpenTelemetry } // namespace Tracers } // namespace Extensions diff --git a/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc b/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc index be1f5bd829ffc..e564deab21687 100644 --- a/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc +++ b/test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc @@ -28,12 +28,12 @@ class OpenTelemetryHttpTraceExporterTest : public testing::Test { OpenTelemetryHttpTraceExporterTest() = default; 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"}); + cluster_manager_.thread_local_cluster_.cluster_.info_->name_ = "my_o11y_backend"; + cluster_manager_.initializeThreadLocalClusters({"my_o11y_backend"}); ON_CALL(cluster_manager_.thread_local_cluster_, httpAsyncClient()) .WillByDefault(ReturnRef(cluster_manager_.thread_local_cluster_.async_client_)); - cluster_manager_.initializeClusters({"fake_collector"}, {}); + cluster_manager_.initializeClusters({"my_o11y_backend"}, {}); trace_exporter_ = std::make_unique(cluster_manager_, http_config, stats_); @@ -50,13 +50,15 @@ class OpenTelemetryHttpTraceExporterTest : public testing::Test { 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" + cluster_name: "my_o11y_backend" + traces_path: "/otlp/v1/traces" + hostname: "some-o11y.com" + headers: + - key: "Authorization" + value: "auth-token" + - key: "x-custom-header" + value: "custom-value" + timeout: 0.250s )EOF"); envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config; @@ -74,20 +76,21 @@ TEST_F(OpenTelemetryHttpTraceExporterTest, CreateExporterAndExportSpan) { 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().MethodValues.Post, message->headers().getMethodValue()); + EXPECT_EQ(Http::Headers::get().ContentTypeValues.Protobuf, message->headers().getContentTypeValue()); + + EXPECT_EQ("/otlp/v1/traces", message->headers().getPathValue()); + EXPECT_EQ("some-o11y.com", message->headers().getHostValue()); + + // Custom headers provided in the configuration + EXPECT_EQ("auth-token", + message->headers().get(Http::LowerCaseString("authorization"))[0]->value().getStringView()); + EXPECT_EQ("custom-value", + message->headers().get(Http::LowerCaseString("x-custom-header"))[0]->value().getStringView()); 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; @@ -113,13 +116,10 @@ TEST_F(OpenTelemetryHttpTraceExporterTest, CreateExporterAndExportSpan) { 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" + cluster_name: "my_o11y_backend" + traces_path: "/otlp/v1/traces" + hostname: "some-o11y.com" + timeout: 0.250s )EOF"); envoy::config::trace::v3::OpenTelemetryConfig::HttpConfig http_config; @@ -128,15 +128,9 @@ TEST_F(OpenTelemetryHttpTraceExporterTest, UnsuccessfulLogWithoutThreadLocalClus Http::MockAsyncClientRequest request(&cluster_manager_.thread_local_cluster_.async_client_); - ON_CALL(cluster_manager_, getThreadLocalCluster(absl::string_view("fake_collector"))) + ON_CALL(cluster_manager_, getThreadLocalCluster(absl::string_view("my_o11y_backend"))) .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; @@ -145,6 +139,80 @@ TEST_F(OpenTelemetryHttpTraceExporterTest, UnsuccessfulLogWithoutThreadLocalClus EXPECT_FALSE(trace_exporter_->log(export_trace_service_request)); } +TEST_F(OpenTelemetryHttpTraceExporterTest, CreateExporterAndExportSpanWithDefaultPath) { + std::string yaml_string = fmt::format(R"EOF( + cluster_name: "my_o11y_backend" + hostname: "some-o11y.com" + timeout: 0.250s + )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; + + // Default path is used when omitted in the config + EXPECT_EQ("/v1/traces", message->headers().getPathValue()); + + return &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; + EXPECT_TRUE(trace_exporter_->log(export_trace_service_request)); + + Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl( + 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()); + 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, ErrorSerializingOtlpRequest) { + std::string yaml_string = fmt::format(R"EOF( + cluster_name: "my_o11y_backend" + hostname: "some-o11y.com" + timeout: 0.250s + )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_); + + opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest + export_trace_service_request; + + // Fake an invalid OTLP message passed to the exporter + std::string invalid_message = "invalid"; + export_trace_service_request.ParseFromArray(invalid_message.c_str(), invalid_message.size()); + + EXPECT_FALSE(trace_exporter_->log(export_trace_service_request)); +} + } // namespace OpenTelemetry } // namespace Tracers } // namespace Extensions diff --git a/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc b/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc index 6c176c250a4fc..bf5f5b91bc2b9 100644 --- a/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc +++ b/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc @@ -63,13 +63,13 @@ class OpenTelemetryDriverTest : public testing::Test { 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" + cluster_name: "my_o11y_backend" + traces_path: "/otlp/v1/traces" + hostname: "some-o11y.com" + headers: + - key: "Authorization" + value: "auth-token" + timeout: 0.250s )EOF"; envoy::config::trace::v3::OpenTelemetryConfig opentelemetry_config; TestUtility::loadFromYaml(yaml_string, opentelemetry_config); @@ -535,14 +535,14 @@ TEST_F(OpenTelemetryDriverTest, ExportSpanWithCustomServiceName) { TEST_F(OpenTelemetryDriverTest, ExportOTLPSpanHTTP) { context_.server_factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->name_ = - "opentelemetry_collector"; + "my_o11y_backend"; context_.server_factory_context_.cluster_manager_.initializeThreadLocalClusters( - {"opentelemetry_collector"}); + {"my_o11y_backend"}); 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_.initializeClusters({"my_o11y_backend"}, {}); setupValidDriverWithHttpExporter();