From 041df2470d4a606d814cfe14dab49dfb5ae092a4 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Mon, 26 Jul 2021 19:22:07 +0300 Subject: [PATCH 1/3] Fix exporting of Jaeger exporter target name (#925) --- exporters/jaeger/CMakeLists.txt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/exporters/jaeger/CMakeLists.txt b/exporters/jaeger/CMakeLists.txt index 2e99e9d438..1f000d69bc 100644 --- a/exporters/jaeger/CMakeLists.txt +++ b/exporters/jaeger/CMakeLists.txt @@ -16,9 +16,8 @@ set(JAEGER_EXPORTER_SOURCES add_library(opentelemetry_exporter_jaeger_trace ${JAEGER_EXPORTER_SOURCES} ${JAEGER_THRIFT_GENCPP_SOURCES}) -set_target_properties( - opentelemetry_exporter_jaeger_trace - PROPERTIES EXPORT_NAME opentelemetry_exporter_jaeger_trace) +set_target_properties(opentelemetry_exporter_jaeger_trace + PROPERTIES EXPORT_NAME jaeger_trace_exporter) target_include_directories( opentelemetry_exporter_jaeger_trace From a1db1ca68904725205acf03b6b81ab64c29e5dd1 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Tue, 27 Jul 2021 00:42:08 +0530 Subject: [PATCH 2/3] remove recordable header from installable (#923) --- exporters/elasticsearch/CMakeLists.txt | 3 ++- exporters/jaeger/CMakeLists.txt | 3 ++- exporters/otlp/CMakeLists.txt | 3 ++- exporters/zipkin/CMakeLists.txt | 3 ++- .../include/opentelemetry/exporters/zipkin/recordable.h | 6 ------ .../opentelemetry/exporters/zipkin/zipkin_exporter.h | 7 ++++++- 6 files changed, 14 insertions(+), 11 deletions(-) diff --git a/exporters/elasticsearch/CMakeLists.txt b/exporters/elasticsearch/CMakeLists.txt index cad3b044ee..8f675e530a 100644 --- a/exporters/elasticsearch/CMakeLists.txt +++ b/exporters/elasticsearch/CMakeLists.txt @@ -22,7 +22,8 @@ install( DIRECTORY include/opentelemetry/exporters/elasticsearch DESTINATION include/opentelemetry/exporters/ FILES_MATCHING - PATTERN "*.h") + PATTERN "*.h" + PATTERN "es_log_recordable.h" EXCLUDE) if(BUILD_TESTING) add_executable(es_log_exporter_test test/es_log_exporter_test.cc) diff --git a/exporters/jaeger/CMakeLists.txt b/exporters/jaeger/CMakeLists.txt index 1f000d69bc..aeb0305fba 100644 --- a/exporters/jaeger/CMakeLists.txt +++ b/exporters/jaeger/CMakeLists.txt @@ -49,7 +49,8 @@ install( DIRECTORY include/opentelemetry/exporters/jaeger DESTINATION include/opentelemetry/exporters/ FILES_MATCHING - PATTERN "*.h") + PATTERN "*.h" + PATTERN "recordable.h" EXCLUDE) if(BUILD_TESTING) add_executable(jaeger_recordable_test test/jaeger_recordable_test.cc) diff --git a/exporters/otlp/CMakeLists.txt b/exporters/otlp/CMakeLists.txt index 1ca788af58..27c0171774 100644 --- a/exporters/otlp/CMakeLists.txt +++ b/exporters/otlp/CMakeLists.txt @@ -52,7 +52,8 @@ install( DIRECTORY include/opentelemetry/exporters/otlp DESTINATION include/opentelemetry/exporters/ FILES_MATCHING - PATTERN "*.h") + PATTERN "*.h" + PATTERN "otlp_recordable.h" EXCLUDE) if(BUILD_TESTING) add_executable(otlp_recordable_test test/otlp_recordable_test.cc) diff --git a/exporters/zipkin/CMakeLists.txt b/exporters/zipkin/CMakeLists.txt index 5f6f9887a6..2316860960 100644 --- a/exporters/zipkin/CMakeLists.txt +++ b/exporters/zipkin/CMakeLists.txt @@ -34,7 +34,8 @@ install( DIRECTORY include/opentelemetry/exporters/zipkin DESTINATION include/opentelemetry/exporters/ FILES_MATCHING - PATTERN "*.h") + PATTERN "*.h" + PATTERN "recordable.h" EXCLUDE) if(BUILD_TESTING) add_executable(zipkin_recordable_test test/zipkin_recordable_test.cc) diff --git a/exporters/zipkin/include/opentelemetry/exporters/zipkin/recordable.h b/exporters/zipkin/include/opentelemetry/exporters/zipkin/recordable.h index 628d7330a8..51f83211f0 100644 --- a/exporters/zipkin/include/opentelemetry/exporters/zipkin/recordable.h +++ b/exporters/zipkin/include/opentelemetry/exporters/zipkin/recordable.h @@ -14,12 +14,6 @@ namespace zipkin { using ZipkinSpan = nlohmann::json; -enum class TransportFormat -{ - kJson, - kProtobuf -}; - class Recordable final : public sdk::trace::Recordable { public: diff --git a/exporters/zipkin/include/opentelemetry/exporters/zipkin/zipkin_exporter.h b/exporters/zipkin/include/opentelemetry/exporters/zipkin/zipkin_exporter.h index cb244867d7..7e9321435c 100644 --- a/exporters/zipkin/include/opentelemetry/exporters/zipkin/zipkin_exporter.h +++ b/exporters/zipkin/include/opentelemetry/exporters/zipkin/zipkin_exporter.h @@ -3,7 +3,6 @@ #pragma once -#include "opentelemetry/exporters/zipkin/recordable.h" #include "opentelemetry/ext/http/client/http_client_factory.h" #include "opentelemetry/ext/http/common/url_parser.h" #include "opentelemetry/sdk/trace/exporter.h" @@ -41,6 +40,12 @@ inline const std::string GetDefaultZipkinEndpoint() return std::string{endpoint_from_env ? endpoint_from_env : kZipkinEndpointDefault}; } +enum class TransportFormat +{ + kJson, + kProtobuf +}; + /** * Struct to hold Zipkin exporter options. */ From 914df66c021516e9526fe8aba36e7f20c4549171 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Wed, 28 Jul 2021 14:49:55 +0300 Subject: [PATCH 3/3] Add Jaeger Thrift HTTP exporter (#926) --- CHANGELOG.md | 1 + examples/jaeger/main.cc | 2 +- exporters/jaeger/CMakeLists.txt | 11 +++- .../exporters/jaeger/jaeger_exporter.h | 8 ++- exporters/jaeger/src/THttpTransport.cc | 61 +++++++++++++++++++ exporters/jaeger/src/THttpTransport.h | 40 ++++++++++++ exporters/jaeger/src/http_transport.cc | 37 +++++++++++ exporters/jaeger/src/http_transport.h | 39 ++++++++++++ exporters/jaeger/src/jaeger_exporter.cc | 16 +++-- exporters/jaeger/src/thrift_sender.cc | 4 +- exporters/jaeger/src/transport.h | 4 +- exporters/jaeger/src/udp_transport.cc | 4 +- exporters/jaeger/src/udp_transport.h | 3 +- 13 files changed, 212 insertions(+), 18 deletions(-) create mode 100644 exporters/jaeger/src/THttpTransport.cc create mode 100644 exporters/jaeger/src/THttpTransport.h create mode 100644 exporters/jaeger/src/http_transport.cc create mode 100644 exporters/jaeger/src/http_transport.h diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e1b02456f..65c8b03289 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Increment the: ## [Unreleased] * [BUILD] Allow to use local GSL +* [EXPORTER] Jaeger Exporter - Add Thrift HTTP exporter ([#926](https://github.com/open-telemetry/opentelemetry-cpp/pull/926)) ## [1.0.0-rc3] 2021-07-12 diff --git a/examples/jaeger/main.cc b/examples/jaeger/main.cc index f3f94884ca..63ed476aad 100644 --- a/examples/jaeger/main.cc +++ b/examples/jaeger/main.cc @@ -33,7 +33,7 @@ int main(int argc, char *argv[]) { if (argc == 2) { - opts.server_addr = argv[1]; + opts.endpoint = argv[1]; } // Removing this line will leave the default noop TracerProvider in place. InitTracer(); diff --git a/exporters/jaeger/CMakeLists.txt b/exporters/jaeger/CMakeLists.txt index aeb0305fba..0ae9162c61 100644 --- a/exporters/jaeger/CMakeLists.txt +++ b/exporters/jaeger/CMakeLists.txt @@ -10,8 +10,13 @@ set(JAEGER_THRIFT_GENCPP_SOURCES thrift-gen/zipkincore_types.cpp) set(JAEGER_EXPORTER_SOURCES - src/jaeger_exporter.cc src/thrift_sender.cc src/udp_transport.cc - src/recordable.cc src/TUDPTransport.cc) + src/jaeger_exporter.cc + src/thrift_sender.cc + src/udp_transport.cc + src/recordable.cc + src/TUDPTransport.cc + src/http_transport.cc + src/THttpTransport.cc) add_library(opentelemetry_exporter_jaeger_trace ${JAEGER_EXPORTER_SOURCES} ${JAEGER_THRIFT_GENCPP_SOURCES}) @@ -26,7 +31,7 @@ target_include_directories( target_link_libraries( opentelemetry_exporter_jaeger_trace - PUBLIC opentelemetry_resources + PUBLIC opentelemetry_resources http_client_curl PRIVATE thrift::thrift) if(MSVC) diff --git a/exporters/jaeger/include/opentelemetry/exporters/jaeger/jaeger_exporter.h b/exporters/jaeger/include/opentelemetry/exporters/jaeger/jaeger_exporter.h index 4e046ad7d1..31f6a8bf35 100644 --- a/exporters/jaeger/include/opentelemetry/exporters/jaeger/jaeger_exporter.h +++ b/exporters/jaeger/include/opentelemetry/exporters/jaeger/jaeger_exporter.h @@ -3,6 +3,7 @@ #pragma once +#include #include OPENTELEMETRY_BEGIN_NAMESPACE @@ -25,10 +26,11 @@ class ThriftSender; */ struct JaegerExporterOptions { - // The endpoint to export to. - std::string server_addr = "localhost"; - uint16_t server_port = 6831; TransportFormat transport_format = TransportFormat::kThriftUdpCompact; + std::string endpoint = "localhost"; + uint16_t server_port = 6831; + // Only applicable when using kThriftHttp transport. + ext::http::client::Headers headers; }; namespace trace_sdk = opentelemetry::sdk::trace; diff --git a/exporters/jaeger/src/THttpTransport.cc b/exporters/jaeger/src/THttpTransport.cc new file mode 100644 index 0000000000..cbb1b65cb2 --- /dev/null +++ b/exporters/jaeger/src/THttpTransport.cc @@ -0,0 +1,61 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include "THttpTransport.h" +#include "opentelemetry/ext/http/client/http_client_factory.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace exporter +{ +namespace jaeger +{ + +THttpTransport::THttpTransport(std::string endpoint, ext::http::client::Headers extra_headers) + : endpoint(std::move(endpoint)), + headers(std::move(extra_headers)), + client(ext::http::client::HttpClientFactory::CreateSync()) +{ + headers.insert({{"Content-Type", "application/vnd.apache.thrift.binary"}}); +} + +THttpTransport::~THttpTransport() {} + +bool THttpTransport::isOpen() const +{ + return true; +} + +uint32_t THttpTransport::read(uint8_t *buf, uint32_t len) +{ + (void)buf; + (void)len; + return 0; +} + +void THttpTransport::write(const uint8_t *buf, uint32_t len) +{ + request_buffer.insert(request_buffer.end(), buf, buf + len); +} + +bool THttpTransport::sendSpans() +{ + auto result = client->Post(endpoint, request_buffer, headers); + request_buffer.clear(); + + // TODO: Add logging once global log handling is available. + if (!result) + { + return false; + } + + if (result.GetResponse().GetStatusCode() >= 400) + { + return false; + } + + return true; +} + +} // namespace jaeger +} // namespace exporter +OPENTELEMETRY_END_NAMESPACE diff --git a/exporters/jaeger/src/THttpTransport.h b/exporters/jaeger/src/THttpTransport.h new file mode 100644 index 0000000000..9c796e67a0 --- /dev/null +++ b/exporters/jaeger/src/THttpTransport.h @@ -0,0 +1,40 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include +#include + +#include + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace exporter +{ +namespace jaeger +{ + +class THttpTransport : public apache::thrift::transport::TVirtualTransport +{ +public: + THttpTransport(std::string endpoint, ext::http::client::Headers extra_headers); + ~THttpTransport() override; + + bool isOpen() const override; + + uint32_t read(uint8_t *buf, uint32_t len); + + void write(const uint8_t *buf, uint32_t len); + + bool sendSpans(); + +private: + std::string endpoint; + ext::http::client::Headers headers; + std::shared_ptr client; + std::vector request_buffer; +}; + +} // namespace jaeger +} // namespace exporter +OPENTELEMETRY_END_NAMESPACE diff --git a/exporters/jaeger/src/http_transport.cc b/exporters/jaeger/src/http_transport.cc new file mode 100644 index 0000000000..f804ccc843 --- /dev/null +++ b/exporters/jaeger/src/http_transport.cc @@ -0,0 +1,37 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include "http_transport.h" + +#include + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace exporter +{ +namespace jaeger +{ + +using TBinaryProtocol = apache::thrift::protocol::TBinaryProtocol; +using TTransport = apache::thrift::transport::TTransport; + +HttpTransport::HttpTransport(std::string endpoint, ext::http::client::Headers headers) +{ + endpoint_transport_ = std::make_shared(std::move(endpoint), std::move(headers)); + protocol_ = std::shared_ptr(new TBinaryProtocol(endpoint_transport_)); +} + +int HttpTransport::EmitBatch(const thrift::Batch &batch) +{ + batch.write(protocol_.get()); + + if (!endpoint_transport_->sendSpans()) + { + return 0; + } + + return static_cast(batch.spans.size()); +} + +} // namespace jaeger +} // namespace exporter +OPENTELEMETRY_END_NAMESPACE diff --git a/exporters/jaeger/src/http_transport.h b/exporters/jaeger/src/http_transport.h new file mode 100644 index 0000000000..8f5c9c0571 --- /dev/null +++ b/exporters/jaeger/src/http_transport.h @@ -0,0 +1,39 @@ +#pragma once + +#include "THttpTransport.h" +#include "transport.h" + +#include +#include +#include +#include + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace exporter +{ +namespace jaeger +{ + +using TProtocol = apache::thrift::protocol::TProtocol; + +class HttpTransport : public Transport +{ +public: + HttpTransport(std::string endpoint, ext::http::client::Headers headers); + + int EmitBatch(const thrift::Batch &batch) override; + + uint32_t MaxPacketSize() const override + { + // Default to 4 MiB POST body size. + return 1 << 22; + } + +private: + std::shared_ptr endpoint_transport_; + std::shared_ptr protocol_; +}; + +} // namespace jaeger +} // namespace exporter +OPENTELEMETRY_END_NAMESPACE diff --git a/exporters/jaeger/src/jaeger_exporter.cc b/exporters/jaeger/src/jaeger_exporter.cc index 5546248c82..2bf181c54a 100644 --- a/exporters/jaeger/src/jaeger_exporter.cc +++ b/exporters/jaeger/src/jaeger_exporter.cc @@ -5,6 +5,7 @@ #include #include +#include "http_transport.h" #include "thrift_sender.h" #include "udp_transport.h" @@ -64,14 +65,21 @@ void JaegerExporter::InitializeEndpoint() { // TODO: do we need support any authentication mechanism? auto transport = std::unique_ptr( - static_cast(new UDPTransport(options_.server_addr, options_.server_port))); + static_cast(new UDPTransport(options_.endpoint, options_.server_port))); sender_ = std::unique_ptr(new ThriftSender(std::move(transport))); + return; } - else + + if (options_.transport_format == TransportFormat::kThriftHttp) { - // The transport format is not implemented. - assert(false); + auto transport = + std::unique_ptr(new HttpTransport(options_.endpoint, options_.headers)); + sender_ = std::unique_ptr(new ThriftSender(std::move(transport))); + return; } + + // The transport format is not implemented. + assert(false); } } // namespace jaeger diff --git a/exporters/jaeger/src/thrift_sender.cc b/exporters/jaeger/src/thrift_sender.cc index 66feb6c64c..770bb81991 100644 --- a/exporters/jaeger/src/thrift_sender.cc +++ b/exporters/jaeger/src/thrift_sender.cc @@ -79,11 +79,11 @@ int ThriftSender::Flush() batch.__set_process(process_); batch.__set_spans(span_buffer_); - transport_->EmitBatch(batch); + int spans_flushed = transport_->EmitBatch(batch); ResetBuffers(); - return static_cast(batch.spans.size()); + return spans_flushed; } void ThriftSender::Close() diff --git a/exporters/jaeger/src/transport.h b/exporters/jaeger/src/transport.h index a507d5b4a3..8121e30076 100644 --- a/exporters/jaeger/src/transport.h +++ b/exporters/jaeger/src/transport.h @@ -21,8 +21,8 @@ class Transport Transport() = default; virtual ~Transport() = default; - virtual void EmitBatch(const thrift::Batch &batch) = 0; - virtual uint32_t MaxPacketSize() const = 0; + virtual int EmitBatch(const thrift::Batch &batch) = 0; + virtual uint32_t MaxPacketSize() const = 0; }; } // namespace jaeger diff --git a/exporters/jaeger/src/udp_transport.cc b/exporters/jaeger/src/udp_transport.cc index 82624ae313..1470a4260f 100644 --- a/exporters/jaeger/src/udp_transport.cc +++ b/exporters/jaeger/src/udp_transport.cc @@ -63,7 +63,7 @@ void UDPTransport::CleanSocket() #endif } -void UDPTransport::EmitBatch(const thrift::Batch &batch) +int UDPTransport::EmitBatch(const thrift::Batch &batch) { try { @@ -71,6 +71,8 @@ void UDPTransport::EmitBatch(const thrift::Batch &batch) } catch (...) {} + + return static_cast(batch.spans.size()); } } // namespace jaeger diff --git a/exporters/jaeger/src/udp_transport.h b/exporters/jaeger/src/udp_transport.h index b75987e4a5..0997a27c63 100644 --- a/exporters/jaeger/src/udp_transport.h +++ b/exporters/jaeger/src/udp_transport.h @@ -27,7 +27,6 @@ using TBinaryProtocol = apache::thrift::protocol::TBinaryProtocol; using TCompactProtocol = apache::thrift::protocol::TCompactProtocol; using TBufferedTransport = apache::thrift::transport::TBufferedTransport; using TProtocol = apache::thrift::protocol::TProtocol; -using TSocket = apache::thrift::transport::TSocket; using TTransport = apache::thrift::transport::TTransport; class UDPTransport : public Transport @@ -38,7 +37,7 @@ class UDPTransport : public Transport UDPTransport(const std::string &addr, uint16_t port); virtual ~UDPTransport(); - void EmitBatch(const thrift::Batch &batch) override; + int EmitBatch(const thrift::Batch &batch) override; uint32_t MaxPacketSize() const override { return max_packet_size_; }