From 2332a1bc89a5a29e57b07033f942eeae33e2d005 Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Thu, 2 Jul 2020 23:32:10 +0000 Subject: [PATCH 01/12] Add exporter --- WORKSPACE | 25 ++++++++++ bazel/opentelemetry_proto.BUILD | 24 +++++++++ exporters/otlp/BUILD | 29 +++++++++++ exporters/otlp/otlp_exporter.cc | 71 ++++++++++++++++++++++++++ exporters/otlp/otlp_exporter.h | 41 +++++++++++++++ exporters/otlp/otlp_exporter_test.cc | 74 ++++++++++++++++++++++++++++ 6 files changed, 264 insertions(+) create mode 100644 exporters/otlp/otlp_exporter.cc create mode 100644 exporters/otlp/otlp_exporter.h create mode 100644 exporters/otlp/otlp_exporter_test.cc diff --git a/WORKSPACE b/WORKSPACE index 1dd2ea37fc..b8d0fb0752 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -16,6 +16,31 @@ workspace(name = "io_opentelemetry_cpp") load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") +# Load gRPC dependency +# Note that this dependency needs to be loaded first due to +# https://github.com/bazelbuild/bazel/issues/6664 +http_archive( + name = "com_github_grpc_grpc", + sha256 = "d6af0859d3ae4693b1955e972aa2e590d6f4d44baaa82651467c6beea453e30e", + strip_prefix = "grpc-1.26.0-pre1", + urls = [ + "https://github.com/grpc/grpc/archive/v1.26.0-pre1.tar.gz", + ], +) + +load("@com_github_grpc_grpc//bazel:grpc_deps.bzl", "grpc_deps") + +grpc_deps() + +# Load extra gRPC dependencies due to https://github.com/grpc/grpc/issues/20511 +load("@com_github_grpc_grpc//bazel:grpc_extra_deps.bzl", "grpc_extra_deps") + +grpc_extra_deps() + +load("@upb//bazel:repository_defs.bzl", "bazel_version_repository") + +bazel_version_repository(name = "upb_bazel_version") + # Uses older protobuf version because of # https://github.com/protocolbuffers/protobuf/issues/7179 http_archive( diff --git a/bazel/opentelemetry_proto.BUILD b/bazel/opentelemetry_proto.BUILD index ddb9915bf2..9c91edc2d0 100644 --- a/bazel/opentelemetry_proto.BUILD +++ b/bazel/opentelemetry_proto.BUILD @@ -15,6 +15,7 @@ package(default_visibility = ["//visibility:public"]) load("@rules_proto//proto:defs.bzl", "proto_library") +load("@com_github_grpc_grpc//bazel:cc_grpc_library.bzl", "cc_grpc_library") proto_library( name = "common_proto", @@ -58,3 +59,26 @@ cc_proto_library( name = "trace_proto_cc", deps = [":trace_proto"], ) + +proto_library( + name = "trace_service_proto", + srcs = [ + "opentelemetry/proto/collector/trace/v1/trace_service.proto", + ], + deps = [ + ":trace_proto", + ], +) + +cc_proto_library( + name = "trace_service_proto_cc", + deps = [":trace_service_proto"], +) + +cc_grpc_library( + name = "trace_service_grpc_cc", + srcs = [":trace_service_proto"], + grpc_only = True, + deps = [":trace_service_proto_cc"], + generate_mocks = True, +) diff --git a/exporters/otlp/BUILD b/exporters/otlp/BUILD index fd1f048889..3b80a00380 100644 --- a/exporters/otlp/BUILD +++ b/exporters/otlp/BUILD @@ -25,10 +25,30 @@ cc_library( include_prefix = "exporters/otlp", deps = [ "//sdk/src/trace", + "//sdk/src/common:random", "@com_github_opentelemetry_proto//:trace_proto_cc", ], ) +cc_library( + name = "otlp_exporter", + srcs = [ + 'otlp_exporter.h', + 'otlp_exporter.cc', + ], + deps = [ + ":recordable", + "//api", + "//sdk/src/trace", + "@com_github_opentelemetry_proto//:trace_proto_cc", + "@com_github_opentelemetry_proto//:trace_service_proto_cc", + + # For gRPC + "@com_github_opentelemetry_proto//:trace_service_grpc_cc", + "@com_github_grpc_grpc//:grpc++", + ], +) + cc_test( name = "recordable_test", srcs = ["recordable_test.cc"], @@ -37,3 +57,12 @@ cc_test( "@com_google_googletest//:gtest_main", ], ) + +cc_test( + name = "otlp_exporter_test", + srcs = ["otlp_exporter_test.cc"], + deps = [ + ":otlp_exporter", + "@com_google_googletest//:gtest_main", + ], +) diff --git a/exporters/otlp/otlp_exporter.cc b/exporters/otlp/otlp_exporter.cc new file mode 100644 index 0000000000..d3c274cb65 --- /dev/null +++ b/exporters/otlp/otlp_exporter.cc @@ -0,0 +1,71 @@ +#include "otlp_exporter.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace exporter +{ +namespace otlp +{ + +const std::string KCollectorAddress = "localhost:55678"; + +// ----------------------------- Helper functions ------------------------------ + +// Add span protobufs contained in recordables to request +void PopulateRequest(const nostd::span> &spans, + proto::collector::trace::v1::ExportTraceServiceRequest *request) +{ + auto resource_span = request->add_resource_spans(); + auto instrumentation_lib = resource_span->add_instrumentation_library_spans(); + + for (auto &recordable : spans) { + auto rec = std::unique_ptr( + static_cast(recordable.release())); + + proto::trace::v1::Span* span = instrumentation_lib->add_spans(); + span->CopyFrom(rec->span()); + } +} + +// Establish connection to OpenTelemetry Collector +std::unique_ptr MakeServiceStub() +{ + auto channel = grpc::CreateChannel(KCollectorAddress, grpc::InsecureChannelCredentials()); + return proto::collector::trace::v1::TraceService::NewStub(channel); +} + +// -------------------------------- Contructors -------------------------------- + +OtlpExporter::OtlpExporter(): OtlpExporter(MakeServiceStub()) {} + +OtlpExporter::OtlpExporter( + std::unique_ptr stub): + trace_service_stub_(std::move(stub)) {} + +// ----------------------------- Exporter methods ------------------------------ + +std::unique_ptr OtlpExporter::MakeRecordable() noexcept +{ + return std::unique_ptr(new Recordable); +} + +sdk::trace::ExportResult OtlpExporter::Export( + const nostd::span> &spans) noexcept +{ + proto::collector::trace::v1::ExportTraceServiceRequest request; + + PopulateRequest(spans, &request); + + grpc::ClientContext context; + proto::collector::trace::v1::ExportTraceServiceResponse response; + + grpc::Status status = trace_service_stub_->Export(&context, request, &response); + + if(!status.ok()){ + std::cerr << "OTLP trace exporter: Export() failed\n"; + return sdk::trace::ExportResult::kFailure; + } + return sdk::trace::ExportResult::kSuccess; +} +} // namespace otlp +} // namespace exporter +OPENTELEMETRY_END_NAMESPACE diff --git a/exporters/otlp/otlp_exporter.h b/exporters/otlp/otlp_exporter.h new file mode 100644 index 0000000000..3565f96b1e --- /dev/null +++ b/exporters/otlp/otlp_exporter.h @@ -0,0 +1,41 @@ +#pragma once + +#include "opentelemetry/sdk/trace/exporter.h" +#include "opentelemetry/sdk/trace/span_data.h" +#include "opentelemetry/proto/collector/trace/v1/trace_service.pb.h" +#include "opentelemetry/proto/collector/trace/v1/trace_service.grpc.pb.h" +#include "opentelemetry/proto/trace/v1/trace.pb.h" +#include "src/common/random.h" +#include "recordable.h" + +#include +#include + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace exporter +{ +namespace otlp +{ +class OtlpExporter final : public opentelemetry::sdk::trace::SpanExporter +{ +public: + OtlpExporter(); + + std::unique_ptr MakeRecordable() noexcept override; + + sdk::trace::ExportResult Export( + const nostd::span> &spans) noexcept override; + + void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override {}; + +private: + // For testing + friend class OtlpExporterTestPeer; + + std::unique_ptr trace_service_stub_; + + OtlpExporter(std::unique_ptr stub); +}; +} // namespace otlp +} // namespace exporter +OPENTELEMETRY_END_NAMESPACE diff --git a/exporters/otlp/otlp_exporter_test.cc b/exporters/otlp/otlp_exporter_test.cc new file mode 100644 index 0000000000..17e8cb9eb0 --- /dev/null +++ b/exporters/otlp/otlp_exporter_test.cc @@ -0,0 +1,74 @@ +#include "otlp_exporter.h" +#include "opentelemetry/sdk/trace/simple_processor.h" +#include "opentelemetry/sdk/trace/tracer_provider.h" +#include "opentelemetry/trace/provider.h" +#include "opentelemetry/proto/collector/trace/v1/trace_service_mock.grpc.pb.h" + +#include + +using namespace testing; + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace exporter +{ +namespace otlp +{ + +class OtlpExporterTestPeer : public ::testing::Test +{ +public: + std::unique_ptr GetExporter( + proto::collector::trace::v1::TraceService::StubInterface* mock_stub) + { + return std::unique_ptr( + new OtlpExporter(std::unique_ptr(mock_stub))); + } +}; + +// Call Export() directly +TEST_F(OtlpExporterTestPeer, ExportUnitTest) +{ + auto mock_stub = new proto::collector::trace::v1::MockTraceServiceStub(); + auto exporter = std::shared_ptr(GetExporter(mock_stub)); + + auto recordable_1 = exporter->MakeRecordable(); + recordable_1->SetName("Test span 1"); + auto recordable_2 = exporter->MakeRecordable(); + recordable_2->SetName("Test span 2"); + + // Test successful RPC + nostd::span> batch_1(&recordable_1, 1); + EXPECT_CALL(*mock_stub, Export(_,_,_)).Times(Exactly(1)).WillOnce(Return(grpc::Status::OK)); + auto result = exporter->Export(batch_1); + EXPECT_EQ(sdk::trace::ExportResult::kSuccess, result); + + // Test failed RPC + nostd::span> batch_2(&recordable_2, 1); + EXPECT_CALL(*mock_stub, Export(_,_,_)).Times(Exactly(1)).WillOnce(Return(grpc::Status::CANCELLED)); + result = exporter->Export(batch_2); + EXPECT_EQ(sdk::trace::ExportResult::kFailure, result); +} + +// Create spans, let processor call Export() +TEST_F(OtlpExporterTestPeer, ExportIntegrationTest) +{ + auto mock_stub = new proto::collector::trace::v1::MockTraceServiceStub(); + auto exporter = std::unique_ptr(GetExporter(mock_stub)); + + auto processor = std::shared_ptr( + new sdk::trace::SimpleSpanProcessor(std::move(exporter))); + auto provider = nostd::shared_ptr(new sdk::trace::TracerProvider(processor)); + trace::Provider::SetTracerProvider(provider); + + nostd::shared_ptr tracer = provider->GetTracer("test"); + + EXPECT_CALL(*mock_stub, Export(_,_,_)).Times(AtLeast(1)).WillRepeatedly(Return(grpc::Status::OK)); + + auto parent_span = tracer->StartSpan("Test parent span"); + auto child_span = tracer->StartSpan("Test child span"); + child_span->End(); + parent_span->End(); +} +} // namespace otlp +} // namespace exporter +OPENTELEMETRY_END_NAMESPACE From e3a748bec2d49c705748998f3550a414ef7ee06f Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Fri, 3 Jul 2020 16:23:23 +0000 Subject: [PATCH 02/12] Address review comment --- exporters/otlp/otlp_exporter_test.cc | 39 ++++++++++++++++------------ 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/exporters/otlp/otlp_exporter_test.cc b/exporters/otlp/otlp_exporter_test.cc index 17e8cb9eb0..ea98357054 100644 --- a/exporters/otlp/otlp_exporter_test.cc +++ b/exporters/otlp/otlp_exporter_test.cc @@ -1,8 +1,8 @@ #include "otlp_exporter.h" +#include "opentelemetry/proto/collector/trace/v1/trace_service_mock.grpc.pb.h" #include "opentelemetry/sdk/trace/simple_processor.h" #include "opentelemetry/sdk/trace/tracer_provider.h" #include "opentelemetry/trace/provider.h" -#include "opentelemetry/proto/collector/trace/v1/trace_service_mock.grpc.pb.h" #include @@ -17,11 +17,10 @@ namespace otlp class OtlpExporterTestPeer : public ::testing::Test { public: - std::unique_ptr GetExporter( - proto::collector::trace::v1::TraceService::StubInterface* mock_stub) + std::unique_ptr GetExporter( + std::unique_ptr &stub_interface) { - return std::unique_ptr( - new OtlpExporter(std::unique_ptr(mock_stub))); + return std::unique_ptr(new OtlpExporter(std::move(stub_interface))); } }; @@ -29,7 +28,9 @@ class OtlpExporterTestPeer : public ::testing::Test TEST_F(OtlpExporterTestPeer, ExportUnitTest) { auto mock_stub = new proto::collector::trace::v1::MockTraceServiceStub(); - auto exporter = std::shared_ptr(GetExporter(mock_stub)); + std::unique_ptr stub_interface( + mock_stub); + auto exporter = GetExporter(stub_interface); auto recordable_1 = exporter->MakeRecordable(); recordable_1->SetName("Test span 1"); @@ -38,13 +39,15 @@ TEST_F(OtlpExporterTestPeer, ExportUnitTest) // Test successful RPC nostd::span> batch_1(&recordable_1, 1); - EXPECT_CALL(*mock_stub, Export(_,_,_)).Times(Exactly(1)).WillOnce(Return(grpc::Status::OK)); + EXPECT_CALL(*mock_stub, Export(_, _, _)).Times(Exactly(1)).WillOnce(Return(grpc::Status::OK)); auto result = exporter->Export(batch_1); EXPECT_EQ(sdk::trace::ExportResult::kSuccess, result); // Test failed RPC nostd::span> batch_2(&recordable_2, 1); - EXPECT_CALL(*mock_stub, Export(_,_,_)).Times(Exactly(1)).WillOnce(Return(grpc::Status::CANCELLED)); + EXPECT_CALL(*mock_stub, Export(_, _, _)) + .Times(Exactly(1)) + .WillOnce(Return(grpc::Status::CANCELLED)); result = exporter->Export(batch_2); EXPECT_EQ(sdk::trace::ExportResult::kFailure, result); } @@ -53,19 +56,23 @@ TEST_F(OtlpExporterTestPeer, ExportUnitTest) TEST_F(OtlpExporterTestPeer, ExportIntegrationTest) { auto mock_stub = new proto::collector::trace::v1::MockTraceServiceStub(); - auto exporter = std::unique_ptr(GetExporter(mock_stub)); + std::unique_ptr stub_interface( + mock_stub); - auto processor = std::shared_ptr( - new sdk::trace::SimpleSpanProcessor(std::move(exporter))); - auto provider = nostd::shared_ptr(new sdk::trace::TracerProvider(processor)); - trace::Provider::SetTracerProvider(provider); + auto exporter = GetExporter(stub_interface); - nostd::shared_ptr tracer = provider->GetTracer("test"); + auto processor = std::shared_ptr( + new sdk::trace::SimpleSpanProcessor(std::move(exporter))); + auto provider = + nostd::shared_ptr(new sdk::trace::TracerProvider(processor)); + auto tracer = provider->GetTracer("test"); - EXPECT_CALL(*mock_stub, Export(_,_,_)).Times(AtLeast(1)).WillRepeatedly(Return(grpc::Status::OK)); + EXPECT_CALL(*mock_stub, Export(_, _, _)) + .Times(AtLeast(1)) + .WillRepeatedly(Return(grpc::Status::OK)); auto parent_span = tracer->StartSpan("Test parent span"); - auto child_span = tracer->StartSpan("Test child span"); + auto child_span = tracer->StartSpan("Test child span"); child_span->End(); parent_span->End(); } From e3dc3301093a7fcca0ecb9323f349d8001fb5239 Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Fri, 3 Jul 2020 17:19:18 +0000 Subject: [PATCH 03/12] Fix build by updating gRPC version --- WORKSPACE | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index b8d0fb0752..3d3f824fe8 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -21,10 +21,9 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") # https://github.com/bazelbuild/bazel/issues/6664 http_archive( name = "com_github_grpc_grpc", - sha256 = "d6af0859d3ae4693b1955e972aa2e590d6f4d44baaa82651467c6beea453e30e", - strip_prefix = "grpc-1.26.0-pre1", + strip_prefix = "grpc-1.28.0", urls = [ - "https://github.com/grpc/grpc/archive/v1.26.0-pre1.tar.gz", + "https://github.com/grpc/grpc/archive/v1.28.0.tar.gz", ], ) From a92bcd0a75e6e7ff4493899525eeebfc5a20cb09 Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Fri, 3 Jul 2020 17:56:15 +0000 Subject: [PATCH 04/12] Attempt to fix build on MacOS by updating gRPC version --- WORKSPACE | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 3d3f824fe8..2758715211 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -21,9 +21,9 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") # https://github.com/bazelbuild/bazel/issues/6664 http_archive( name = "com_github_grpc_grpc", - strip_prefix = "grpc-1.28.0", + strip_prefix = "grpc-1.29.0", urls = [ - "https://github.com/grpc/grpc/archive/v1.28.0.tar.gz", + "https://github.com/grpc/grpc/archive/v1.29.0.tar.gz", ], ) From fcda784ea06e02c75818fb4f4f9d9e3daea3b0da Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Fri, 3 Jul 2020 18:37:33 +0000 Subject: [PATCH 05/12] Change gRPC version for MacOS --- WORKSPACE | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 2758715211..1b90a03ed7 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -21,9 +21,9 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") # https://github.com/bazelbuild/bazel/issues/6664 http_archive( name = "com_github_grpc_grpc", - strip_prefix = "grpc-1.29.0", + strip_prefix = "grpc-1.27.0", urls = [ - "https://github.com/grpc/grpc/archive/v1.29.0.tar.gz", + "https://github.com/grpc/grpc/archive/v1.27.0.tar.gz", ], ) From 26c0ab8706ca87558883b8ba06ba68747b2f56c7 Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Fri, 3 Jul 2020 21:01:15 +0000 Subject: [PATCH 06/12] Address review comments --- exporters/otlp/BUILD | 3 +-- exporters/otlp/otlp_exporter.cc | 33 ++++++++++++++++++++------------- exporters/otlp/otlp_exporter.h | 28 ++++++++++++++++++++++++++-- 3 files changed, 47 insertions(+), 17 deletions(-) diff --git a/exporters/otlp/BUILD b/exporters/otlp/BUILD index 3b80a00380..302c20890e 100644 --- a/exporters/otlp/BUILD +++ b/exporters/otlp/BUILD @@ -25,7 +25,6 @@ cc_library( include_prefix = "exporters/otlp", deps = [ "//sdk/src/trace", - "//sdk/src/common:random", "@com_github_opentelemetry_proto//:trace_proto_cc", ], ) @@ -38,7 +37,6 @@ cc_library( ], deps = [ ":recordable", - "//api", "//sdk/src/trace", "@com_github_opentelemetry_proto//:trace_proto_cc", "@com_github_opentelemetry_proto//:trace_service_proto_cc", @@ -63,6 +61,7 @@ cc_test( srcs = ["otlp_exporter_test.cc"], deps = [ ":otlp_exporter", + "//api", "@com_google_googletest//:gtest_main", ], ) diff --git a/exporters/otlp/otlp_exporter.cc b/exporters/otlp/otlp_exporter.cc index d3c274cb65..5ed6c45c85 100644 --- a/exporters/otlp/otlp_exporter.cc +++ b/exporters/otlp/otlp_exporter.cc @@ -10,23 +10,28 @@ const std::string KCollectorAddress = "localhost:55678"; // ----------------------------- Helper functions ------------------------------ -// Add span protobufs contained in recordables to request +/** + * Add span protobufs contained in recordables to request. + * @param spans the spans to export + * @param request the current request + */ void PopulateRequest(const nostd::span> &spans, proto::collector::trace::v1::ExportTraceServiceRequest *request) { - auto resource_span = request->add_resource_spans(); + auto resource_span = request->add_resource_spans(); auto instrumentation_lib = resource_span->add_instrumentation_library_spans(); - for (auto &recordable : spans) { - auto rec = std::unique_ptr( - static_cast(recordable.release())); + for (auto &recordable : spans) + { + auto rec = std::unique_ptr(static_cast(recordable.release())); - proto::trace::v1::Span* span = instrumentation_lib->add_spans(); - span->CopyFrom(rec->span()); + *instrumentation_lib->add_spans() = std::move(rec->span()); } } -// Establish connection to OpenTelemetry Collector +/** + * Create service stub to communicate with the OpenTelemetry Collector. + */ std::unique_ptr MakeServiceStub() { auto channel = grpc::CreateChannel(KCollectorAddress, grpc::InsecureChannelCredentials()); @@ -35,11 +40,12 @@ std::unique_ptr MakeServiceStub // -------------------------------- Contructors -------------------------------- -OtlpExporter::OtlpExporter(): OtlpExporter(MakeServiceStub()) {} +OtlpExporter::OtlpExporter() : OtlpExporter(MakeServiceStub()) {} OtlpExporter::OtlpExporter( - std::unique_ptr stub): - trace_service_stub_(std::move(stub)) {} + std::unique_ptr stub) + : trace_service_stub_(std::move(stub)) +{} // ----------------------------- Exporter methods ------------------------------ @@ -49,7 +55,7 @@ std::unique_ptr OtlpExporter::MakeRecordable() noexcept } sdk::trace::ExportResult OtlpExporter::Export( - const nostd::span> &spans) noexcept + const nostd::span> &spans) noexcept { proto::collector::trace::v1::ExportTraceServiceRequest request; @@ -60,7 +66,8 @@ sdk::trace::ExportResult OtlpExporter::Export( grpc::Status status = trace_service_stub_->Export(&context, request, &response); - if(!status.ok()){ + if (!status.ok()) + { std::cerr << "OTLP trace exporter: Export() failed\n"; return sdk::trace::ExportResult::kFailure; } diff --git a/exporters/otlp/otlp_exporter.h b/exporters/otlp/otlp_exporter.h index 3565f96b1e..5e7d272932 100644 --- a/exporters/otlp/otlp_exporter.h +++ b/exporters/otlp/otlp_exporter.h @@ -1,11 +1,9 @@ #pragma once #include "opentelemetry/sdk/trace/exporter.h" -#include "opentelemetry/sdk/trace/span_data.h" #include "opentelemetry/proto/collector/trace/v1/trace_service.pb.h" #include "opentelemetry/proto/collector/trace/v1/trace_service.grpc.pb.h" #include "opentelemetry/proto/trace/v1/trace.pb.h" -#include "src/common/random.h" #include "recordable.h" #include @@ -16,24 +14,50 @@ namespace exporter { namespace otlp { +/** + * The OTLP exporter exports span data in OpenTelemetry Protocol (OTLP) format. + */ class OtlpExporter final : public opentelemetry::sdk::trace::SpanExporter { public: + /** + * Create an OtlpExporter. This constructor initializes a service stub to be + * used for exporting. + */ OtlpExporter(); + /** + * Create a span recordable. + * @return a newly initialized Recordable object + */ std::unique_ptr MakeRecordable() noexcept override; + /** + * Export a batch of span recordables in OTLP format. + * @param spans a span of unique pointers to span recordables + */ sdk::trace::ExportResult Export( const nostd::span> &spans) noexcept override; + /** + * Shut down the exporter. + * @param timeout an optional timeout, the default timeout of 0 means that no + * timeout is applied. + */ void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override {}; private: // For testing friend class OtlpExporterTestPeer; + // Store service stub internally. Useful for testing. std::unique_ptr trace_service_stub_; + /** + * Create an OtlpExporter using the specified service stub. + * Only tests can call this constructor directly. + * @param stub the service stub to be used for exporting + */ OtlpExporter(std::unique_ptr stub); }; } // namespace otlp From 06cba203210a6151d9a28789a97d6f3312b3502a Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Fri, 3 Jul 2020 21:39:30 +0000 Subject: [PATCH 07/12] MacOS build fix --- WORKSPACE | 24 +++++++++++++++--------- exporters/otlp/otlp_exporter.cc | 1 - 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 1b90a03ed7..78e8df29cc 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -19,26 +19,32 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") # Load gRPC dependency # Note that this dependency needs to be loaded first due to # https://github.com/bazelbuild/bazel/issues/6664 +#gRPC http_archive( name = "com_github_grpc_grpc", - strip_prefix = "grpc-1.27.0", - urls = [ - "https://github.com/grpc/grpc/archive/v1.27.0.tar.gz", - ], + strip_prefix = "grpc-master", + urls = ["https://github.com/grpc/grpc/archive/master.tar.gz"], ) load("@com_github_grpc_grpc//bazel:grpc_deps.bzl", "grpc_deps") grpc_deps() -# Load extra gRPC dependencies due to https://github.com/grpc/grpc/issues/20511 -load("@com_github_grpc_grpc//bazel:grpc_extra_deps.bzl", "grpc_extra_deps") +# grpc_deps() cannot load() its deps, this WORKSPACE has to do it. +# See also: https://github.com/bazelbuild/bazel/issues/1943 +load( + "@build_bazel_rules_apple//apple:repositories.bzl", + "apple_rules_dependencies", +) -grpc_extra_deps() +apple_rules_dependencies() -load("@upb//bazel:repository_defs.bzl", "bazel_version_repository") +load( + "@build_bazel_apple_support//lib:repositories.bzl", + "apple_support_dependencies", +) -bazel_version_repository(name = "upb_bazel_version") +apple_support_dependencies() # Uses older protobuf version because of # https://github.com/protocolbuffers/protobuf/issues/7179 diff --git a/exporters/otlp/otlp_exporter.cc b/exporters/otlp/otlp_exporter.cc index 5ed6c45c85..c5fa79aafc 100644 --- a/exporters/otlp/otlp_exporter.cc +++ b/exporters/otlp/otlp_exporter.cc @@ -24,7 +24,6 @@ void PopulateRequest(const nostd::span> for (auto &recordable : spans) { auto rec = std::unique_ptr(static_cast(recordable.release())); - *instrumentation_lib->add_spans() = std::move(rec->span()); } } From a2e952ebf24c961a3405d4fdf45bed02cd1aef71 Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Fri, 3 Jul 2020 22:17:43 +0000 Subject: [PATCH 08/12] Add option to bazelrc for MacOs --- .bazelrc | 3 +++ WORKSPACE | 24 +++++++++--------------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/.bazelrc b/.bazelrc index 58140be3c9..e893a2e2fa 100644 --- a/.bazelrc +++ b/.bazelrc @@ -1,6 +1,9 @@ # bazel configurations for running tests under sanitizers. # Based on https://github.com/bazelment/trunk/blob/master/tools/bazel.rc +# Needed by gRPC to build on some platforms. +build --copt -DGRPC_BAZEL_BUILD + # --config=asan : Address Sanitizer. common:asan --copt -fsanitize=address common:asan --copt -DADDRESS_SANITIZER diff --git a/WORKSPACE b/WORKSPACE index 78e8df29cc..84ab876bc7 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -19,32 +19,26 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") # Load gRPC dependency # Note that this dependency needs to be loaded first due to # https://github.com/bazelbuild/bazel/issues/6664 -#gRPC http_archive( name = "com_github_grpc_grpc", - strip_prefix = "grpc-master", - urls = ["https://github.com/grpc/grpc/archive/master.tar.gz"], + strip_prefix = "grpc-1.28.0", + urls = [ + "https://github.com/grpc/grpc/archive/v1.28.0.tar.gz", + ], ) load("@com_github_grpc_grpc//bazel:grpc_deps.bzl", "grpc_deps") grpc_deps() -# grpc_deps() cannot load() its deps, this WORKSPACE has to do it. -# See also: https://github.com/bazelbuild/bazel/issues/1943 -load( - "@build_bazel_rules_apple//apple:repositories.bzl", - "apple_rules_dependencies", -) +# Load extra gRPC dependencies due to https://github.com/grpc/grpc/issues/20511 +load("@com_github_grpc_grpc//bazel:grpc_extra_deps.bzl", "grpc_extra_deps") -apple_rules_dependencies() +grpc_extra_deps() -load( - "@build_bazel_apple_support//lib:repositories.bzl", - "apple_support_dependencies", -) +load("@upb//bazel:repository_defs.bzl", "bazel_version_repository") -apple_support_dependencies() +bazel_version_repository(name = "upb_bazel_version") # Uses older protobuf version because of # https://github.com/protocolbuffers/protobuf/issues/7179 From 4aff53e0a9516e809b57b3376fe1ad498b1f5104 Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Mon, 6 Jul 2020 15:55:59 +0000 Subject: [PATCH 09/12] Address more review comments --- exporters/otlp/BUILD | 4 +++- exporters/otlp/otlp_exporter.cc | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/exporters/otlp/BUILD b/exporters/otlp/BUILD index 302c20890e..3d4c53087e 100644 --- a/exporters/otlp/BUILD +++ b/exporters/otlp/BUILD @@ -32,9 +32,11 @@ cc_library( cc_library( name = "otlp_exporter", srcs = [ - 'otlp_exporter.h', 'otlp_exporter.cc', ], + hdrs = [ + 'otlp_exporter.h', + ], deps = [ ":recordable", "//sdk/src/trace", diff --git a/exporters/otlp/otlp_exporter.cc b/exporters/otlp/otlp_exporter.cc index c5fa79aafc..47fc33677d 100644 --- a/exporters/otlp/otlp_exporter.cc +++ b/exporters/otlp/otlp_exporter.cc @@ -6,7 +6,7 @@ namespace exporter namespace otlp { -const std::string KCollectorAddress = "localhost:55678"; +const std::string kCollectorAddress = "localhost:55678"; // ----------------------------- Helper functions ------------------------------ @@ -33,7 +33,7 @@ void PopulateRequest(const nostd::span> */ std::unique_ptr MakeServiceStub() { - auto channel = grpc::CreateChannel(KCollectorAddress, grpc::InsecureChannelCredentials()); + auto channel = grpc::CreateChannel(kCollectorAddress, grpc::InsecureChannelCredentials()); return proto::collector::trace::v1::TraceService::NewStub(channel); } From da2d5a5d725e3ca0c06e66633ee8f8b89a006b21 Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Tue, 7 Jul 2020 20:58:43 +0000 Subject: [PATCH 10/12] Address more comments --- exporters/otlp/BUILD | 2 -- exporters/otlp/otlp_exporter.cc | 6 +++++- exporters/otlp/otlp_exporter.h | 6 ------ 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/exporters/otlp/BUILD b/exporters/otlp/BUILD index 3d4c53087e..e8c4a8ae7f 100644 --- a/exporters/otlp/BUILD +++ b/exporters/otlp/BUILD @@ -40,8 +40,6 @@ cc_library( deps = [ ":recordable", "//sdk/src/trace", - "@com_github_opentelemetry_proto//:trace_proto_cc", - "@com_github_opentelemetry_proto//:trace_service_proto_cc", # For gRPC "@com_github_opentelemetry_proto//:trace_service_grpc_cc", diff --git a/exporters/otlp/otlp_exporter.cc b/exporters/otlp/otlp_exporter.cc index 47fc33677d..fe8d4cb727 100644 --- a/exporters/otlp/otlp_exporter.cc +++ b/exporters/otlp/otlp_exporter.cc @@ -1,4 +1,8 @@ #include "otlp_exporter.h" +#include "recordable.h" + +#include +#include OPENTELEMETRY_BEGIN_NAMESPACE namespace exporter @@ -67,7 +71,7 @@ sdk::trace::ExportResult OtlpExporter::Export( if (!status.ok()) { - std::cerr << "OTLP trace exporter: Export() failed\n"; + std::cerr << "[OTLP Exporter] Export() failed: " << status.error_message() << std::endl; return sdk::trace::ExportResult::kFailure; } return sdk::trace::ExportResult::kSuccess; diff --git a/exporters/otlp/otlp_exporter.h b/exporters/otlp/otlp_exporter.h index 5e7d272932..4ac324f3ca 100644 --- a/exporters/otlp/otlp_exporter.h +++ b/exporters/otlp/otlp_exporter.h @@ -1,13 +1,7 @@ #pragma once #include "opentelemetry/sdk/trace/exporter.h" -#include "opentelemetry/proto/collector/trace/v1/trace_service.pb.h" #include "opentelemetry/proto/collector/trace/v1/trace_service.grpc.pb.h" -#include "opentelemetry/proto/trace/v1/trace.pb.h" -#include "recordable.h" - -#include -#include OPENTELEMETRY_BEGIN_NAMESPACE namespace exporter From 7841ddde5ee893e76e4f3868dfa2cd7dac5f60e0 Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Tue, 7 Jul 2020 21:25:42 +0000 Subject: [PATCH 11/12] Fix newline --- exporters/otlp/otlp_exporter.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/otlp/otlp_exporter.cc b/exporters/otlp/otlp_exporter.cc index fe8d4cb727..951597044a 100644 --- a/exporters/otlp/otlp_exporter.cc +++ b/exporters/otlp/otlp_exporter.cc @@ -71,7 +71,7 @@ sdk::trace::ExportResult OtlpExporter::Export( if (!status.ok()) { - std::cerr << "[OTLP Exporter] Export() failed: " << status.error_message() << std::endl; + std::cerr << "[OTLP Exporter] Export() failed: " << status.error_message() << "\n"; return sdk::trace::ExportResult::kFailure; } return sdk::trace::ExportResult::kSuccess; From 337985f3f4af943baed5bf3d8d65b6d79ced4cdf Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Tue, 7 Jul 2020 22:05:58 +0000 Subject: [PATCH 12/12] Trigger GitHub Actions