From ed943fb8e504b1b707cab9d540cf845d9ed1a8e3 Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Mon, 24 Aug 2020 18:48:23 +0000 Subject: [PATCH 1/5] Read endpoint from env variable --- exporters/otlp/src/otlp_exporter.cc | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/exporters/otlp/src/otlp_exporter.cc b/exporters/otlp/src/otlp_exporter.cc index 0de17e0114..77cd8f0dcc 100644 --- a/exporters/otlp/src/otlp_exporter.cc +++ b/exporters/otlp/src/otlp_exporter.cc @@ -10,7 +10,10 @@ namespace exporter namespace otlp { -const std::string kCollectorAddress = "localhost:55678"; +// Environment variable to configure endpoint for OTLP exporter +constexpr char kConfiguredEndpoint[] = "OTEL_EXPORTER_OTLP_ENDPOINT"; +// Default endpoint is the OpenTelemetry Collector default address +std::string kDefaultEndpoint = "localhost:55678"; // ----------------------------- Helper functions ------------------------------ @@ -37,7 +40,15 @@ void PopulateRequest(const nostd::span> */ std::unique_ptr MakeServiceStub() { - auto channel = grpc::CreateChannel(kCollectorAddress, grpc::InsecureChannelCredentials()); + std::string endpoint = kDefaultEndpoint; + char *configured_endpoint = getenv(kConfiguredEndpoint); + + if (configured_endpoint != NULL) + { + endpoint = configured_endpoint; + } + + auto channel = grpc::CreateChannel(endpoint, grpc::InsecureChannelCredentials()); return proto::collector::trace::v1::TraceService::NewStub(channel); } From b8ba15f0b13afc6ffd5083221dd0ab2be5676542 Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Tue, 25 Aug 2020 00:15:35 +0000 Subject: [PATCH 2/5] Add options struct --- .../exporters/otlp/otlp_exporter.h | 20 +++++++++++++-- exporters/otlp/src/otlp_exporter.cc | 25 +++++++------------ exporters/otlp/test/otlp_exporter_test.cc | 15 +++++++++++ 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_exporter.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_exporter.h index 16a5252f0b..960cc7d0db 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_exporter.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_exporter.h @@ -8,6 +8,15 @@ namespace exporter { namespace otlp { +/** + * Struct to hold OTLP exporter options. + */ +struct OtlpExporterOptions +{ + // The endpoint to export to. By default the OpenTelemetry Collector's default endpoint. + std::string endpoint = "localhost:55680"; +}; + /** * The OTLP exporter exports span data in OpenTelemetry Protocol (OTLP) format. */ @@ -15,11 +24,15 @@ class OtlpExporter final : public opentelemetry::sdk::trace::SpanExporter { public: /** - * Create an OtlpExporter. This constructor initializes a service stub to be - * used for exporting. + * Create an OtlpExporter using all default options. */ OtlpExporter(); + /** + * Create an OtlpExporter using the given options. + */ + OtlpExporter(OtlpExporterOptions options); + /** * Create a span recordable. * @return a newly initialized Recordable object @@ -42,6 +55,9 @@ class OtlpExporter final : public opentelemetry::sdk::trace::SpanExporter std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override{}; private: + // The configuration options associated with this exporter. + const OtlpExporterOptions options_; + // For testing friend class OtlpExporterTestPeer; diff --git a/exporters/otlp/src/otlp_exporter.cc b/exporters/otlp/src/otlp_exporter.cc index 77cd8f0dcc..48c79c159d 100644 --- a/exporters/otlp/src/otlp_exporter.cc +++ b/exporters/otlp/src/otlp_exporter.cc @@ -10,11 +10,6 @@ namespace exporter namespace otlp { -// Environment variable to configure endpoint for OTLP exporter -constexpr char kConfiguredEndpoint[] = "OTEL_EXPORTER_OTLP_ENDPOINT"; -// Default endpoint is the OpenTelemetry Collector default address -std::string kDefaultEndpoint = "localhost:55678"; - // ----------------------------- Helper functions ------------------------------ /** @@ -38,27 +33,25 @@ void PopulateRequest(const nostd::span> /** * Create service stub to communicate with the OpenTelemetry Collector. */ -std::unique_ptr MakeServiceStub() +std::unique_ptr MakeServiceStub( + std::string endpoint) { - std::string endpoint = kDefaultEndpoint; - char *configured_endpoint = getenv(kConfiguredEndpoint); - - if (configured_endpoint != NULL) - { - endpoint = configured_endpoint; - } - auto channel = grpc::CreateChannel(endpoint, grpc::InsecureChannelCredentials()); return proto::collector::trace::v1::TraceService::NewStub(channel); } // -------------------------------- Contructors -------------------------------- -OtlpExporter::OtlpExporter() : OtlpExporter(MakeServiceStub()) {} +OtlpExporter::OtlpExporter() : OtlpExporter(OtlpExporterOptions()) {} + +OtlpExporter::OtlpExporter(OtlpExporterOptions options) : options_(std::move(options)) +{ + trace_service_stub_ = MakeServiceStub(options_.endpoint); +} OtlpExporter::OtlpExporter( std::unique_ptr stub) - : trace_service_stub_(std::move(stub)) + : options_(OtlpExporterOptions()), trace_service_stub_(std::move(stub)) {} // ----------------------------- Exporter methods ------------------------------ diff --git a/exporters/otlp/test/otlp_exporter_test.cc b/exporters/otlp/test/otlp_exporter_test.cc index 581eb8f125..18e50c9360 100644 --- a/exporters/otlp/test/otlp_exporter_test.cc +++ b/exporters/otlp/test/otlp_exporter_test.cc @@ -23,6 +23,12 @@ class OtlpExporterTestPeer : public ::testing::Test { return std::unique_ptr(new OtlpExporter(std::move(stub_interface))); } + + // Get the options associated with the given exporter. + OtlpExporterOptions GetOptions(std::unique_ptr &exporter) + { + return exporter->options_; + } }; // Call Export() directly @@ -77,6 +83,15 @@ TEST_F(OtlpExporterTestPeer, ExportIntegrationTest) child_span->End(); parent_span->End(); } + +// Test exporter configuration options +TEST_F(OtlpExporterTestPeer, ConfigTest) +{ + OtlpExporterOptions opts; + opts.endpoint = "localhost:45454"; + std::unique_ptr exporter(new OtlpExporter(opts)); + EXPECT_EQ(GetOptions(exporter).endpoint, "localhost:45454"); +} } // namespace otlp } // namespace exporter OPENTELEMETRY_END_NAMESPACE From 08fbc15a9628aede5402e5739f54012d50d43bd1 Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Tue, 25 Aug 2020 00:33:46 +0000 Subject: [PATCH 3/5] Simplify code --- exporters/otlp/src/otlp_exporter.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/exporters/otlp/src/otlp_exporter.cc b/exporters/otlp/src/otlp_exporter.cc index 48c79c159d..269156ff43 100644 --- a/exporters/otlp/src/otlp_exporter.cc +++ b/exporters/otlp/src/otlp_exporter.cc @@ -44,10 +44,9 @@ std::unique_ptr MakeServiceStub OtlpExporter::OtlpExporter() : OtlpExporter(OtlpExporterOptions()) {} -OtlpExporter::OtlpExporter(OtlpExporterOptions options) : options_(std::move(options)) -{ - trace_service_stub_ = MakeServiceStub(options_.endpoint); -} +OtlpExporter::OtlpExporter(OtlpExporterOptions options) + : options_(std::move(options)), trace_service_stub_(MakeServiceStub(options.endpoint)) +{} OtlpExporter::OtlpExporter( std::unique_ptr stub) From 1a5b0a0c4a4398958fab04f3a4de3ef39a87539e Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Tue, 25 Aug 2020 01:17:43 +0000 Subject: [PATCH 4/5] Fix memory issue --- exporters/otlp/src/otlp_exporter.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/otlp/src/otlp_exporter.cc b/exporters/otlp/src/otlp_exporter.cc index 269156ff43..6d9ca325f0 100644 --- a/exporters/otlp/src/otlp_exporter.cc +++ b/exporters/otlp/src/otlp_exporter.cc @@ -45,7 +45,7 @@ std::unique_ptr MakeServiceStub OtlpExporter::OtlpExporter() : OtlpExporter(OtlpExporterOptions()) {} OtlpExporter::OtlpExporter(OtlpExporterOptions options) - : options_(std::move(options)), trace_service_stub_(MakeServiceStub(options.endpoint)) + : options_(options), trace_service_stub_(MakeServiceStub(options.endpoint)) {} OtlpExporter::OtlpExporter( From f2c67248b68e7c6cb5afe649264baad4360bd069 Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Tue, 25 Aug 2020 16:52:05 +0000 Subject: [PATCH 5/5] Address review comments --- .../otlp/include/opentelemetry/exporters/otlp/otlp_exporter.h | 2 +- exporters/otlp/src/otlp_exporter.cc | 2 +- exporters/otlp/test/otlp_exporter_test.cc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_exporter.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_exporter.h index 960cc7d0db..bf2ad4a5d4 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_exporter.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_exporter.h @@ -31,7 +31,7 @@ class OtlpExporter final : public opentelemetry::sdk::trace::SpanExporter /** * Create an OtlpExporter using the given options. */ - OtlpExporter(OtlpExporterOptions options); + OtlpExporter(const OtlpExporterOptions &options); /** * Create a span recordable. diff --git a/exporters/otlp/src/otlp_exporter.cc b/exporters/otlp/src/otlp_exporter.cc index 6d9ca325f0..74851976ca 100644 --- a/exporters/otlp/src/otlp_exporter.cc +++ b/exporters/otlp/src/otlp_exporter.cc @@ -44,7 +44,7 @@ std::unique_ptr MakeServiceStub OtlpExporter::OtlpExporter() : OtlpExporter(OtlpExporterOptions()) {} -OtlpExporter::OtlpExporter(OtlpExporterOptions options) +OtlpExporter::OtlpExporter(const OtlpExporterOptions &options) : options_(options), trace_service_stub_(MakeServiceStub(options.endpoint)) {} diff --git a/exporters/otlp/test/otlp_exporter_test.cc b/exporters/otlp/test/otlp_exporter_test.cc index 18e50c9360..a79046f1d3 100644 --- a/exporters/otlp/test/otlp_exporter_test.cc +++ b/exporters/otlp/test/otlp_exporter_test.cc @@ -25,7 +25,7 @@ class OtlpExporterTestPeer : public ::testing::Test } // Get the options associated with the given exporter. - OtlpExporterOptions GetOptions(std::unique_ptr &exporter) + const OtlpExporterOptions &GetOptions(std::unique_ptr &exporter) { return exporter->options_; }