From 6292ec248c3076bc7d82acd5217f31ac85f68fd6 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Fri, 20 Mar 2020 12:14:09 -0700 Subject: [PATCH 01/19] Add interfaces for exporter, processor and span data --- .../opentelemetry/sdk/trace/exporter.h | 43 ++++++++++++++++++ .../opentelemetry/sdk/trace/processor.h | 38 ++++++++++++++++ .../opentelemetry/sdk/trace/span_data.h | 45 +++++++++++++++++++ 3 files changed, 126 insertions(+) create mode 100644 sdk/include/opentelemetry/sdk/trace/exporter.h create mode 100644 sdk/include/opentelemetry/sdk/trace/processor.h create mode 100644 sdk/include/opentelemetry/sdk/trace/span_data.h diff --git a/sdk/include/opentelemetry/sdk/trace/exporter.h b/sdk/include/opentelemetry/sdk/trace/exporter.h new file mode 100644 index 0000000000..a159c73fb0 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/trace/exporter.h @@ -0,0 +1,43 @@ +#pragma once + +#include "opentelemetry/nostd/shared_ptr.h" +#include "opentelemetry/nostd/span.h" +#include "opentelemetry/sdk/trace/span_data.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace trace +{ +/** + * ExportResult is returned as result of exporting a span batch. + */ +enum class ExportResult +{ + // Batch was successfully exported. + rSuccess = 0, + // Exporting failed. The caller must not retry exporting the same batch; the + // batch must be dropped. + rFailedNotRetryable +}; +/** + * SpanExporter defines the interface that protocol-specific span exporters must + * implement. + */ +class SpanExporter +{ +public: + virtual ~SpanExporter() = default; + + // Exports a batch of spans. + // + // This method must not be called concurrently for the same exporter + // instance. + virtual ExportResult Export( + nostd::span> &spans) noexcept = 0; + + virtual void Shutdown() noexcept = 0; +}; +} // namespace trace +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/trace/processor.h b/sdk/include/opentelemetry/sdk/trace/processor.h new file mode 100644 index 0000000000..7426e4a685 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/trace/processor.h @@ -0,0 +1,38 @@ +#pragma once + +#include "opentelemetry/nostd/shared_ptr.h" +#include "opentelemetry/nostd/span.h" +#include "opentelemetry/sdk/trace/span_data.h" +#include "opentelemetry/trace/span.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace trace +{ +/** + * Span processor allow hooks for span start and end method invocations. + * + * Built-in span processors are responsible for batching and conversion of + * spans to exportable representation and passing batches to exporters. + */ +class SpanProcessor +{ +public: + virtual ~SpanProcessor() = default; + + // OnStart is called when a span is started. + virtual void OnStart(nostd::shared_ptr &span) noexcept = 0; + + // OnEnd is called when a span is ended. + virtual void OnEnd(nostd::shared_ptr &span) noexcept = 0; + + // Export all ended spans that have not yet been exported. + virtual void ForceFlush() noexcept = 0; + + // Shut down the processor and do any cleanup required. + virtual void Shutdown() noexcept = 0; +}; +} // namespace trace +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/trace/span_data.h b/sdk/include/opentelemetry/sdk/trace/span_data.h new file mode 100644 index 0000000000..464dab0288 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/trace/span_data.h @@ -0,0 +1,45 @@ +#pragma once + +#include "opentelemetry/core/timestamp.h" +#include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/trace/canonical_code.h" +#include "opentelemetry/trace/span_id.h" +#include "opentelemetry/trace/trace_id.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace trace +{ +/** + * SpanData is an immutable representation of all data collected by a span. + */ +class SpanData +{ +public: + virtual ~SpanData() = default; + + // The trace id for this span. + virtual opentelemetry::trace::TraceId GetTraceId() const noexcept = 0; + + // The span id for this span. + virtual opentelemetry::trace::SpanId GetSpanId() const noexcept = 0; + + // The span id for this span's parent. + virtual opentelemetry::trace::SpanId GetParentSpanId() const noexcept = 0; + + // The name of this span. + virtual opentelemetry::nostd::string_view GetName() const noexcept = 0; + + // The status of this span. + virtual opentelemetry::trace::CanonicalCode GetStatus() const noexcept = 0; + + // The start epoch timestamp in nanos of this span. + virtual opentelemetry::core::SystemTimestamp GetStartTime() const noexcept = 0; + + // The end epoch timestamp in nanos of this span. + virtual opentelemetry::core::SystemTimestamp GetEndTime() const noexcept = 0; +}; +} // namespace trace +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE From 54a913079756bd719d526664bea625d684e1fb3d Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Sat, 21 Mar 2020 16:40:55 -0700 Subject: [PATCH 02/19] PR comments --- sdk/include/opentelemetry/sdk/trace/exporter.h | 13 +++++++------ sdk/include/opentelemetry/sdk/trace/processor.h | 7 ++++--- .../opentelemetry/sdk}/trace/recorder.h | 0 sdk/include/opentelemetry/sdk/trace/span_data.h | 5 ++--- sdk/src/trace/tracer.h | 2 +- 5 files changed, 14 insertions(+), 13 deletions(-) rename sdk/{src => include/opentelemetry/sdk}/trace/recorder.h (100%) diff --git a/sdk/include/opentelemetry/sdk/trace/exporter.h b/sdk/include/opentelemetry/sdk/trace/exporter.h index a159c73fb0..969f8efcf5 100644 --- a/sdk/include/opentelemetry/sdk/trace/exporter.h +++ b/sdk/include/opentelemetry/sdk/trace/exporter.h @@ -2,7 +2,7 @@ #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/span.h" -#include "opentelemetry/sdk/trace/span_data.h" +#include "opentelemetry/sdk/trace/recordable.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk @@ -29,12 +29,13 @@ class SpanExporter public: virtual ~SpanExporter() = default; - // Exports a batch of spans. - // - // This method must not be called concurrently for the same exporter - // instance. + /** + * Exports a batch of spans. + * + * This method must not be called concurrently for the same exporter instance. + */ virtual ExportResult Export( - nostd::span> &spans) noexcept = 0; + nostd::span> &spans) noexcept = 0; virtual void Shutdown() noexcept = 0; }; diff --git a/sdk/include/opentelemetry/sdk/trace/processor.h b/sdk/include/opentelemetry/sdk/trace/processor.h index 7426e4a685..0dff075595 100644 --- a/sdk/include/opentelemetry/sdk/trace/processor.h +++ b/sdk/include/opentelemetry/sdk/trace/processor.h @@ -22,13 +22,14 @@ class SpanProcessor virtual ~SpanProcessor() = default; // OnStart is called when a span is started. - virtual void OnStart(nostd::shared_ptr &span) noexcept = 0; + virtual void OnStart(opentelemetry::trace::Span &span) noexcept = 0; // OnEnd is called when a span is ended. - virtual void OnEnd(nostd::shared_ptr &span) noexcept = 0; + virtual void OnEnd( + nostd::shared_ptr &recordable) noexcept = 0; // Export all ended spans that have not yet been exported. - virtual void ForceFlush() noexcept = 0; + virtual void ForceFlush(uint64_t timeout_ms) noexcept = 0; // Shut down the processor and do any cleanup required. virtual void Shutdown() noexcept = 0; diff --git a/sdk/src/trace/recorder.h b/sdk/include/opentelemetry/sdk/trace/recorder.h similarity index 100% rename from sdk/src/trace/recorder.h rename to sdk/include/opentelemetry/sdk/trace/recorder.h diff --git a/sdk/include/opentelemetry/sdk/trace/span_data.h b/sdk/include/opentelemetry/sdk/trace/span_data.h index 464dab0288..973b6e7bb6 100644 --- a/sdk/include/opentelemetry/sdk/trace/span_data.h +++ b/sdk/include/opentelemetry/sdk/trace/span_data.h @@ -2,6 +2,7 @@ #include "opentelemetry/core/timestamp.h" #include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/sdk/trace/recordable.h" #include "opentelemetry/trace/canonical_code.h" #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/trace_id.h" @@ -14,11 +15,9 @@ namespace trace /** * SpanData is an immutable representation of all data collected by a span. */ -class SpanData +class SpanData final : public Recordable { public: - virtual ~SpanData() = default; - // The trace id for this span. virtual opentelemetry::trace::TraceId GetTraceId() const noexcept = 0; diff --git a/sdk/src/trace/tracer.h b/sdk/src/trace/tracer.h index 65d1318e5b..e78dccf36c 100644 --- a/sdk/src/trace/tracer.h +++ b/sdk/src/trace/tracer.h @@ -1,8 +1,8 @@ #pragma once +#include "opentelemetry/sdk/trace/recorder.h" #include "opentelemetry/trace/tracer.h" #include "opentelemetry/version.h" -#include "src/trace/recorder.h" #include From e8d867c610de72c142c6ae0a5edf67be14eeffa4 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Sat, 21 Mar 2020 16:41:50 -0700 Subject: [PATCH 03/19] Fix enum name --- sdk/include/opentelemetry/sdk/trace/exporter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/include/opentelemetry/sdk/trace/exporter.h b/sdk/include/opentelemetry/sdk/trace/exporter.h index 969f8efcf5..80135e988d 100644 --- a/sdk/include/opentelemetry/sdk/trace/exporter.h +++ b/sdk/include/opentelemetry/sdk/trace/exporter.h @@ -18,7 +18,7 @@ enum class ExportResult rSuccess = 0, // Exporting failed. The caller must not retry exporting the same batch; the // batch must be dropped. - rFailedNotRetryable + rFailure }; /** * SpanExporter defines the interface that protocol-specific span exporters must From 132f0e743330dc1bb8f275b10b14e1bcd9804777 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Sat, 21 Mar 2020 16:50:49 -0700 Subject: [PATCH 04/19] Revert some changes --- sdk/{include/opentelemetry/sdk => src}/trace/recordable.h | 0 sdk/{include/opentelemetry/sdk => src}/trace/recorder.h | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename sdk/{include/opentelemetry/sdk => src}/trace/recordable.h (100%) rename sdk/{include/opentelemetry/sdk => src}/trace/recorder.h (100%) diff --git a/sdk/include/opentelemetry/sdk/trace/recordable.h b/sdk/src/trace/recordable.h similarity index 100% rename from sdk/include/opentelemetry/sdk/trace/recordable.h rename to sdk/src/trace/recordable.h diff --git a/sdk/include/opentelemetry/sdk/trace/recorder.h b/sdk/src/trace/recorder.h similarity index 100% rename from sdk/include/opentelemetry/sdk/trace/recorder.h rename to sdk/src/trace/recorder.h From e7711b2733aa0851a322c33b9ab2fbc995d5d8fa Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Sat, 21 Mar 2020 16:51:54 -0700 Subject: [PATCH 05/19] Revert some changes --- sdk/src/trace/recorder.h | 2 +- sdk/src/trace/tracer.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/src/trace/recorder.h b/sdk/src/trace/recorder.h index 22730a74cb..b701c113d6 100644 --- a/sdk/src/trace/recorder.h +++ b/sdk/src/trace/recorder.h @@ -1,9 +1,9 @@ #pragma once -#include "opentelemetry/sdk/trace/recordable.h" #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/trace_id.h" #include "opentelemetry/version.h" +#include "src/trace/recordable.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk diff --git a/sdk/src/trace/tracer.h b/sdk/src/trace/tracer.h index e78dccf36c..65d1318e5b 100644 --- a/sdk/src/trace/tracer.h +++ b/sdk/src/trace/tracer.h @@ -1,8 +1,8 @@ #pragma once -#include "opentelemetry/sdk/trace/recorder.h" #include "opentelemetry/trace/tracer.h" #include "opentelemetry/version.h" +#include "src/trace/recorder.h" #include From c71261cc8b3ad17d942b53c64faa72d3b974312b Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 23 Mar 2020 09:43:37 -0700 Subject: [PATCH 06/19] PR comments --- sdk/include/opentelemetry/sdk/trace/processor.h | 5 ++++- sdk/include/opentelemetry/sdk/trace/span_data.h | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/processor.h b/sdk/include/opentelemetry/sdk/trace/processor.h index 0dff075595..3c0870a036 100644 --- a/sdk/include/opentelemetry/sdk/trace/processor.h +++ b/sdk/include/opentelemetry/sdk/trace/processor.h @@ -29,7 +29,10 @@ class SpanProcessor nostd::shared_ptr &recordable) noexcept = 0; // Export all ended spans that have not yet been exported. - virtual void ForceFlush(uint64_t timeout_ms) noexcept = 0; + // + // Optionally a timeout can be specified. The default timeout of 0 means that + // no timeout is applied. + virtual void ForceFlush(uint64_t timeout_ms = 0) noexcept = 0; // Shut down the processor and do any cleanup required. virtual void Shutdown() noexcept = 0; diff --git a/sdk/include/opentelemetry/sdk/trace/span_data.h b/sdk/include/opentelemetry/sdk/trace/span_data.h index 973b6e7bb6..f6293dacbf 100644 --- a/sdk/include/opentelemetry/sdk/trace/span_data.h +++ b/sdk/include/opentelemetry/sdk/trace/span_data.h @@ -33,10 +33,10 @@ class SpanData final : public Recordable // The status of this span. virtual opentelemetry::trace::CanonicalCode GetStatus() const noexcept = 0; - // The start epoch timestamp in nanos of this span. + // The start time of this span. virtual opentelemetry::core::SystemTimestamp GetStartTime() const noexcept = 0; - // The end epoch timestamp in nanos of this span. + // The end time of this span. virtual opentelemetry::core::SystemTimestamp GetEndTime() const noexcept = 0; }; } // namespace trace From 773a8b93676b1139d4ca96cc78e8ba0574c6efc3 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 23 Mar 2020 14:45:36 -0700 Subject: [PATCH 07/19] PR comments --- sdk/include/opentelemetry/sdk/trace/processor.h | 10 +++++----- sdk/include/opentelemetry/sdk/trace/span_data.h | 14 +++++++------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/processor.h b/sdk/include/opentelemetry/sdk/trace/processor.h index 3c0870a036..15b42667a4 100644 --- a/sdk/include/opentelemetry/sdk/trace/processor.h +++ b/sdk/include/opentelemetry/sdk/trace/processor.h @@ -1,8 +1,7 @@ #pragma once -#include "opentelemetry/nostd/shared_ptr.h" -#include "opentelemetry/nostd/span.h" -#include "opentelemetry/sdk/trace/span_data.h" +#include +#include "opentelemetry/sdk/trace/recordable.h" #include "opentelemetry/trace/span.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -26,13 +25,14 @@ class SpanProcessor // OnEnd is called when a span is ended. virtual void OnEnd( - nostd::shared_ptr &recordable) noexcept = 0; + std::shared_ptr &recordable) noexcept = 0; // Export all ended spans that have not yet been exported. // // Optionally a timeout can be specified. The default timeout of 0 means that // no timeout is applied. - virtual void ForceFlush(uint64_t timeout_ms = 0) noexcept = 0; + virtual void ForceFlush( + std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept = 0; // Shut down the processor and do any cleanup required. virtual void Shutdown() noexcept = 0; diff --git a/sdk/include/opentelemetry/sdk/trace/span_data.h b/sdk/include/opentelemetry/sdk/trace/span_data.h index f6293dacbf..bcf97ab230 100644 --- a/sdk/include/opentelemetry/sdk/trace/span_data.h +++ b/sdk/include/opentelemetry/sdk/trace/span_data.h @@ -19,25 +19,25 @@ class SpanData final : public Recordable { public: // The trace id for this span. - virtual opentelemetry::trace::TraceId GetTraceId() const noexcept = 0; + opentelemetry::trace::TraceId GetTraceId() const noexcept = 0; // The span id for this span. - virtual opentelemetry::trace::SpanId GetSpanId() const noexcept = 0; + opentelemetry::trace::SpanId GetSpanId() const noexcept = 0; // The span id for this span's parent. - virtual opentelemetry::trace::SpanId GetParentSpanId() const noexcept = 0; + opentelemetry::trace::SpanId GetParentSpanId() const noexcept = 0; // The name of this span. - virtual opentelemetry::nostd::string_view GetName() const noexcept = 0; + opentelemetry::nostd::string_view GetName() const noexcept = 0; // The status of this span. - virtual opentelemetry::trace::CanonicalCode GetStatus() const noexcept = 0; + opentelemetry::trace::CanonicalCode GetStatus() const noexcept = 0; // The start time of this span. - virtual opentelemetry::core::SystemTimestamp GetStartTime() const noexcept = 0; + opentelemetry::core::SystemTimestamp GetStartTime() const noexcept = 0; // The end time of this span. - virtual opentelemetry::core::SystemTimestamp GetEndTime() const noexcept = 0; + opentelemetry::core::SystemTimestamp GetEndTime() const noexcept = 0; }; } // namespace trace } // namespace sdk From 56f10e6e709ad1db319850f220d9367378006af6 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Tue, 24 Mar 2020 21:36:46 -0700 Subject: [PATCH 08/19] PR comments --- sdk/include/opentelemetry/sdk/trace/processor.h | 4 ++++ sdk/{src => include/opentelemetry/sdk}/trace/recordable.h | 0 sdk/include/opentelemetry/sdk/trace/span_data.h | 5 +++-- sdk/src/trace/recorder.h | 2 +- 4 files changed, 8 insertions(+), 3 deletions(-) rename sdk/{src => include/opentelemetry/sdk}/trace/recordable.h (100%) diff --git a/sdk/include/opentelemetry/sdk/trace/processor.h b/sdk/include/opentelemetry/sdk/trace/processor.h index 15b42667a4..499c7d14f9 100644 --- a/sdk/include/opentelemetry/sdk/trace/processor.h +++ b/sdk/include/opentelemetry/sdk/trace/processor.h @@ -35,6 +35,10 @@ class SpanProcessor std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept = 0; // Shut down the processor and do any cleanup required. + // + // Ended spans are exported before shutdown. After the call to Shutdown, + // subsequent calls to OnStart, OnEnd, ForceFlush or Shutdown will return + // immediately without doing anything. virtual void Shutdown() noexcept = 0; }; } // namespace trace diff --git a/sdk/src/trace/recordable.h b/sdk/include/opentelemetry/sdk/trace/recordable.h similarity index 100% rename from sdk/src/trace/recordable.h rename to sdk/include/opentelemetry/sdk/trace/recordable.h diff --git a/sdk/include/opentelemetry/sdk/trace/span_data.h b/sdk/include/opentelemetry/sdk/trace/span_data.h index bcf97ab230..7e5e3e26ad 100644 --- a/sdk/include/opentelemetry/sdk/trace/span_data.h +++ b/sdk/include/opentelemetry/sdk/trace/span_data.h @@ -1,5 +1,6 @@ #pragma once +#include #include "opentelemetry/core/timestamp.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/trace/recordable.h" @@ -36,8 +37,8 @@ class SpanData final : public Recordable // The start time of this span. opentelemetry::core::SystemTimestamp GetStartTime() const noexcept = 0; - // The end time of this span. - opentelemetry::core::SystemTimestamp GetEndTime() const noexcept = 0; + // The duration of this span. + std::chrono::nanoseconds GetDuration() const noexcept = 0; }; } // namespace trace } // namespace sdk diff --git a/sdk/src/trace/recorder.h b/sdk/src/trace/recorder.h index b701c113d6..22730a74cb 100644 --- a/sdk/src/trace/recorder.h +++ b/sdk/src/trace/recorder.h @@ -1,9 +1,9 @@ #pragma once +#include "opentelemetry/sdk/trace/recordable.h" #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/trace_id.h" #include "opentelemetry/version.h" -#include "src/trace/recordable.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk From fe15d7b6fd95a83b58bab049e4882e70647961a8 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Thu, 26 Mar 2020 13:30:06 -0700 Subject: [PATCH 09/19] Use unique_ptr --- sdk/include/opentelemetry/sdk/trace/processor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/include/opentelemetry/sdk/trace/processor.h b/sdk/include/opentelemetry/sdk/trace/processor.h index 499c7d14f9..7e0f80e16c 100644 --- a/sdk/include/opentelemetry/sdk/trace/processor.h +++ b/sdk/include/opentelemetry/sdk/trace/processor.h @@ -25,7 +25,7 @@ class SpanProcessor // OnEnd is called when a span is ended. virtual void OnEnd( - std::shared_ptr &recordable) noexcept = 0; + std::unique_ptr &&recordable) noexcept = 0; // Export all ended spans that have not yet been exported. // From b18a6633d1127961ac71ac83900d597b129d2901 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Wed, 1 Apr 2020 12:39:59 -0700 Subject: [PATCH 10/19] Add implementations --- .../opentelemetry/sdk/trace/exporter.h | 30 +++-- .../opentelemetry/sdk/trace/processor.h | 43 +++++--- .../opentelemetry/sdk/trace/recordable.h | 32 ++++++ .../opentelemetry/sdk/trace/span_data.h | 103 +++++++++++++++--- sdk/src/trace/simple_processor.h | 52 +++++++++ sdk/test/trace/BUILD | 22 ++++ sdk/test/trace/CMakeLists.txt | 3 +- sdk/test/trace/simple_processor_test.cc | 55 ++++++++++ sdk/test/trace/span_data_test.cc | 50 +++++++++ 9 files changed, 353 insertions(+), 37 deletions(-) create mode 100644 sdk/src/trace/simple_processor.h create mode 100644 sdk/test/trace/simple_processor_test.cc create mode 100644 sdk/test/trace/span_data_test.cc diff --git a/sdk/include/opentelemetry/sdk/trace/exporter.h b/sdk/include/opentelemetry/sdk/trace/exporter.h index 80135e988d..9883ee652a 100644 --- a/sdk/include/opentelemetry/sdk/trace/exporter.h +++ b/sdk/include/opentelemetry/sdk/trace/exporter.h @@ -14,10 +14,14 @@ namespace trace */ enum class ExportResult { - // Batch was successfully exported. + /** + * Batch was successfully exported. + */ rSuccess = 0, - // Exporting failed. The caller must not retry exporting the same batch; the - // batch must be dropped. + /** + * Exporting failed. The caller must not retry exporting the same batch; the + * batch must be dropped. + */ rFailure }; /** @@ -30,13 +34,25 @@ class SpanExporter virtual ~SpanExporter() = default; /** - * Exports a batch of spans. - * - * This method must not be called concurrently for the same exporter instance. + * Create a span recordable. This object will be used to record span data and + * will subsequently be passed to SpanExporter::Export. Vendors can implement + * custom recordables or use the default SpanData recordable provided by the + * SDK. + * @return a newly initialized Recordable object + */ + virtual std::unique_ptr MakeRecordable() noexcept = 0; + + /** + * Exports a batch of span recordables. This method must not be called + * concurrently for the same exporter instance. + * @param spans a span of shared pointers to span recordables */ virtual ExportResult Export( - nostd::span> &spans) noexcept = 0; + nostd::span> &spans) noexcept = 0; + /** + * Shut down the exporter. + */ virtual void Shutdown() noexcept = 0; }; } // namespace trace diff --git a/sdk/include/opentelemetry/sdk/trace/processor.h b/sdk/include/opentelemetry/sdk/trace/processor.h index 7e0f80e16c..979122eb19 100644 --- a/sdk/include/opentelemetry/sdk/trace/processor.h +++ b/sdk/include/opentelemetry/sdk/trace/processor.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include "opentelemetry/sdk/trace/recordable.h" #include "opentelemetry/trace/span.h" @@ -20,25 +21,39 @@ class SpanProcessor public: virtual ~SpanProcessor() = default; - // OnStart is called when a span is started. - virtual void OnStart(opentelemetry::trace::Span &span) noexcept = 0; + /** + * Create a span recordable. This requests a new span recordable from the + * associated exporter. + * @return a newly initialized recordable + */ + virtual std::unique_ptr MakeRecordable() noexcept = 0; - // OnEnd is called when a span is ended. - virtual void OnEnd( - std::unique_ptr &&recordable) noexcept = 0; + /** + * OnStart is called when a span is started. + * @param span a recordable for a span that was just started + */ + virtual void OnStart(Recordable &span) noexcept = 0; - // Export all ended spans that have not yet been exported. - // - // Optionally a timeout can be specified. The default timeout of 0 means that - // no timeout is applied. + /** + * OnEnd is called when a span is ended. + * @param span a recordable for a span that was ended + */ + virtual void OnEnd(std::unique_ptr &&span) noexcept = 0; + + /** + * Export all ended spans that have not yet been exported. + * @param timeout an optional timeout, the default timeout of 0 means that no + * timeout is applied. + */ virtual void ForceFlush( std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept = 0; - // Shut down the processor and do any cleanup required. - // - // Ended spans are exported before shutdown. After the call to Shutdown, - // subsequent calls to OnStart, OnEnd, ForceFlush or Shutdown will return - // immediately without doing anything. + /** + * Shut down the processor and do any cleanup required. Ended spans are + * exported before shutdown. After the call to Shutdown, subsequent calls to + * OnStart, OnEnd, ForceFlush or Shutdown will return immediately without + * doing anything. + */ virtual void Shutdown() noexcept = 0; }; } // namespace trace diff --git a/sdk/include/opentelemetry/sdk/trace/recordable.h b/sdk/include/opentelemetry/sdk/trace/recordable.h index 32646a3bbe..ec8de8805d 100644 --- a/sdk/include/opentelemetry/sdk/trace/recordable.h +++ b/sdk/include/opentelemetry/sdk/trace/recordable.h @@ -3,6 +3,8 @@ #include "opentelemetry/core/timestamp.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/trace/canonical_code.h" +#include "opentelemetry/trace/span_id.h" +#include "opentelemetry/trace/trace_id.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -21,6 +23,24 @@ class Recordable public: virtual ~Recordable() = default; + /** + * Set a trace id for this span. + * @param trace_id the trace id to set + */ + virtual void SetTraceId(opentelemetry::trace::TraceId trace_id) noexcept = 0; + + /** + * Set a span id for this span. + * @param span_id the span id to set + */ + virtual void SetSpanId(opentelemetry::trace::SpanId span_id) noexcept = 0; + + /** + * Set a parent span id for this span. + * @param parent_span_id the parent span id to set + */ + virtual void SetParentSpanId(opentelemetry::trace::SpanId parent_span_id) noexcept = 0; + /** * Add an event to a span. * @param name the name of the event @@ -41,6 +61,18 @@ class Recordable * @param name the name to set */ virtual void SetName(nostd::string_view name) noexcept = 0; + + /** + * Set the start time of the span. + * @param start_time the start time to set + */ + virtual void SetStartTime(opentelemetry::core::SystemTimestamp start_time) noexcept = 0; + + /** + * Set the duration of the span. + * @param duration the duration to set + */ + virtual void SetDuration(std::chrono::nanoseconds duration) noexcept = 0; }; } // namespace trace } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/trace/span_data.h b/sdk/include/opentelemetry/sdk/trace/span_data.h index 7e5e3e26ad..53ae0e7922 100644 --- a/sdk/include/opentelemetry/sdk/trace/span_data.h +++ b/sdk/include/opentelemetry/sdk/trace/span_data.h @@ -14,31 +14,104 @@ namespace sdk namespace trace { /** - * SpanData is an immutable representation of all data collected by a span. + * SpanData is a representation of all data collected by a span. */ class SpanData final : public Recordable { public: - // The trace id for this span. - opentelemetry::trace::TraceId GetTraceId() const noexcept = 0; + /** + * Initialized all span fields with default values. Time stamps and duration + * are initialized with 0, the status of the span is initialized with OK. + */ + SpanData() noexcept : duration_(0), status_code_(opentelemetry::trace::CanonicalCode::OK) {} - // The span id for this span. - opentelemetry::trace::SpanId GetSpanId() const noexcept = 0; + /** + * Get the trace id for this span + * @return the trace id for this span + */ + opentelemetry::trace::TraceId GetTraceId() const noexcept { return trace_id_; } - // The span id for this span's parent. - opentelemetry::trace::SpanId GetParentSpanId() const noexcept = 0; + /** + * Get the span id for this span + * @return the span id for this span + */ + opentelemetry::trace::SpanId GetSpanId() const noexcept { return span_id_; } - // The name of this span. - opentelemetry::nostd::string_view GetName() const noexcept = 0; + /** + * Get the parent span id for this span + * @return the span id for this span's parent + */ + opentelemetry::trace::SpanId GetParentSpanId() const noexcept { return parent_span_id_; } - // The status of this span. - opentelemetry::trace::CanonicalCode GetStatus() const noexcept = 0; + /** + * Get the name for this span + * @return the name for this span + */ + opentelemetry::nostd::string_view GetName() const noexcept { return name_; } - // The start time of this span. - opentelemetry::core::SystemTimestamp GetStartTime() const noexcept = 0; + /** + * Get the status for this span + * @return the status for this span + */ + opentelemetry::trace::CanonicalCode GetStatus() const noexcept { return status_code_; } - // The duration of this span. - std::chrono::nanoseconds GetDuration() const noexcept = 0; + /** + * Get the status description for this span + * @return the description of the the status of this span + */ + opentelemetry::nostd::string_view GetDescription() const noexcept { return status_desc_; } + + /** + * Get the start time for this span + * @return the start time for this span + */ + opentelemetry::core::SystemTimestamp GetStartTime() const noexcept { return start_time_; } + + /** + * Get the duration for this span + * @return the duration for this span + */ + std::chrono::nanoseconds GetDuration() const noexcept { return duration_; } + + void SetTraceId(opentelemetry::trace::TraceId trace_id) noexcept { trace_id_ = trace_id; } + + void SetSpanId(opentelemetry::trace::SpanId span_id) noexcept { span_id_ = span_id; } + + void SetParentSpanId(opentelemetry::trace::SpanId parent_span_id) noexcept + { + parent_span_id_ = parent_span_id; + } + + void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept + { + (void)name; + (void)timestamp; + } + + void SetStatus(trace_api::CanonicalCode code, nostd::string_view description) noexcept + { + status_code_ = code; + status_desc_ = std::string(description); + } + + void SetName(nostd::string_view name) noexcept { name_ = std::string(name); } + + void SetStartTime(opentelemetry::core::SystemTimestamp start_time) noexcept + { + start_time_ = start_time; + } + + void SetDuration(std::chrono::nanoseconds duration) noexcept { duration_ = duration; } + +private: + opentelemetry::trace::TraceId trace_id_; + opentelemetry::trace::SpanId span_id_; + opentelemetry::trace::SpanId parent_span_id_; + core::SystemTimestamp start_time_; + std::chrono::nanoseconds duration_; + std::string name_; + opentelemetry::trace::CanonicalCode status_code_; + std::string status_desc_; }; } // namespace trace } // namespace sdk diff --git a/sdk/src/trace/simple_processor.h b/sdk/src/trace/simple_processor.h new file mode 100644 index 0000000000..c62948aa8c --- /dev/null +++ b/sdk/src/trace/simple_processor.h @@ -0,0 +1,52 @@ +#pragma once + +#include "opentelemetry/sdk/trace/exporter.h" +#include "opentelemetry/sdk/trace/processor.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace trace +{ +/** + * The simple span processor passes finished recordables to the configured + * SpanExporter, as soon as they are finished. + * + * OnEnd, ForceFlush and Shutdown are no-ops. + */ +class SimpleSpanProcessor +{ +public: + /** + * Initialize a simple span processor. + * @param exporter the exporter used by the span processor + */ + SimpleSpanProcessor(std::unique_ptr &&exporter) noexcept + : exporter_(std::move(exporter)) + {} + + std::unique_ptr MakeRecordable() noexcept { return exporter_->MakeRecordable(); } + + void OnStart(Recordable &span) noexcept {} + + void OnEnd(std::unique_ptr &&span) noexcept + { + std::shared_ptr recordable{std::move(span)}; + nostd::span> s(&recordable, 1); + if (exporter_->Export(s) == ExportResult::rFailure) + { + /* Once it is defined how the SDK does logging, an error should be + * logged in this case. */ + } + } + + void ForceFlush(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept {} + + void Shutdown() noexcept {} + +private: + std::unique_ptr exporter_; +}; +} // namespace trace +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/test/trace/BUILD b/sdk/test/trace/BUILD index 858ff28601..8a27e5f717 100644 --- a/sdk/test/trace/BUILD +++ b/sdk/test/trace/BUILD @@ -8,3 +8,25 @@ cc_test( "@com_google_googletest//:gtest_main", ], ) + +cc_test( + name = "span_data", + srcs = [ + "span_data_test.cc", + ], + deps = [ + "//sdk/src/trace", + "@com_google_googletest//:gtest_main", + ], +) + +cc_test( + name = "simple_processor", + srcs = [ + "simple_processor_test.cc", + ], + deps = [ + "//sdk/src/trace", + "@com_google_googletest//:gtest_main", + ], +) diff --git a/sdk/test/trace/CMakeLists.txt b/sdk/test/trace/CMakeLists.txt index b12d584eed..ec14273050 100644 --- a/sdk/test/trace/CMakeLists.txt +++ b/sdk/test/trace/CMakeLists.txt @@ -1,4 +1,5 @@ -foreach(testname default_tracer_provider_test) +foreach(testname default_tracer_provider_test span_data_test + simple_processor_test) add_executable(${testname} "${testname}.cc") target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} opentelemetry_trace) diff --git a/sdk/test/trace/simple_processor_test.cc b/sdk/test/trace/simple_processor_test.cc new file mode 100644 index 0000000000..4bfff91d13 --- /dev/null +++ b/sdk/test/trace/simple_processor_test.cc @@ -0,0 +1,55 @@ +#include "src/trace/simple_processor.h" +#include "opentelemetry/nostd/span.h" +#include "opentelemetry/sdk/trace/span_data.h" + +#include + +using namespace opentelemetry::sdk::trace; + +/** + * A mock exporter that switches a flag once a valid recordable was received. + */ +class MockSpanExporter final : public SpanExporter +{ +public: + MockSpanExporter(std::shared_ptr span_received) noexcept : span_received_(span_received) {} + + std::unique_ptr MakeRecordable() noexcept + { + return std::unique_ptr(new SpanData); + } + + ExportResult Export(opentelemetry::nostd::span> &spans) noexcept + { + for (auto span : spans) + { + if (std::static_pointer_cast(span)) + { + *span_received_ = true; + } + } + return ExportResult::rSuccess; + } + + void Shutdown() noexcept {} + +private: + std::shared_ptr span_received_; +}; + +TEST(SimpleSpanProcessor, ToMockSpanExporter) +{ + std::shared_ptr span_received(new bool(false)); + std::unique_ptr exporter(new MockSpanExporter(span_received)); + SimpleSpanProcessor processor(std::move(exporter)); + + auto recordable = processor.MakeRecordable(); + + processor.OnStart(*recordable); + ASSERT_EQ(*span_received, false); + + processor.OnEnd(std::move(recordable)); + ASSERT_EQ(*span_received, true); + + processor.Shutdown(); +} diff --git a/sdk/test/trace/span_data_test.cc b/sdk/test/trace/span_data_test.cc new file mode 100644 index 0000000000..c2e6b4d5ba --- /dev/null +++ b/sdk/test/trace/span_data_test.cc @@ -0,0 +1,50 @@ +#include "opentelemetry/sdk/trace/span_data.h" +#include "opentelemetry/trace/span_id.h" +#include "opentelemetry/trace/trace_id.h" + +#include + +using opentelemetry::sdk::trace::SpanData; + +TEST(SpanData, DefaultValues) +{ + opentelemetry::trace::TraceId zero_trace_id; + opentelemetry::trace::SpanId zero_span_id; + SpanData data; + + ASSERT_EQ(data.GetTraceId(), zero_trace_id); + ASSERT_EQ(data.GetSpanId(), zero_span_id); + ASSERT_EQ(data.GetParentSpanId(), zero_span_id); + ASSERT_EQ(data.GetName(), ""); + ASSERT_EQ(data.GetStatus(), opentelemetry::trace::CanonicalCode::OK); + ASSERT_EQ(data.GetDescription(), ""); + ASSERT_EQ(data.GetStartTime().time_since_epoch(), std::chrono::nanoseconds(0)); + ASSERT_EQ(data.GetDuration(), std::chrono::nanoseconds(0)); +} + +TEST(SpanData, Set) +{ + opentelemetry::trace::TraceId trace_id; + opentelemetry::trace::SpanId span_id; + opentelemetry::trace::SpanId parent_span_id; + opentelemetry::core::SystemTimestamp now(std::chrono::system_clock::now()); + + SpanData data; + data.SetTraceId(trace_id); + data.SetSpanId(span_id); + data.SetParentSpanId(parent_span_id); + data.SetName("span name"); + data.SetStatus(opentelemetry::trace::CanonicalCode::UNKNOWN, "description"); + data.SetStartTime(now); + data.SetDuration(std::chrono::nanoseconds(1000000)); + data.AddEvent("event1", now); + + ASSERT_EQ(data.GetTraceId(), trace_id); + ASSERT_EQ(data.GetSpanId(), span_id); + ASSERT_EQ(data.GetParentSpanId(), parent_span_id); + ASSERT_EQ(data.GetName(), "span name"); + ASSERT_EQ(data.GetStatus(), opentelemetry::trace::CanonicalCode::UNKNOWN); + ASSERT_EQ(data.GetDescription(), "description"); + ASSERT_EQ(data.GetStartTime().time_since_epoch(), now.time_since_epoch()); + ASSERT_EQ(data.GetDuration(), std::chrono::nanoseconds(1000000)); +} From 62491c120bc65f9fdadebb2a80af5f56704d7ea6 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Wed, 1 Apr 2020 15:22:56 -0700 Subject: [PATCH 11/19] Remove obsolete headers --- sdk/include/opentelemetry/sdk/trace/exporter.h | 2 +- sdk/include/opentelemetry/sdk/trace/processor.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/exporter.h b/sdk/include/opentelemetry/sdk/trace/exporter.h index 9883ee652a..632f7fcde3 100644 --- a/sdk/include/opentelemetry/sdk/trace/exporter.h +++ b/sdk/include/opentelemetry/sdk/trace/exporter.h @@ -1,6 +1,6 @@ #pragma once -#include "opentelemetry/nostd/shared_ptr.h" +#include #include "opentelemetry/nostd/span.h" #include "opentelemetry/sdk/trace/recordable.h" diff --git a/sdk/include/opentelemetry/sdk/trace/processor.h b/sdk/include/opentelemetry/sdk/trace/processor.h index 979122eb19..bd2dd67585 100644 --- a/sdk/include/opentelemetry/sdk/trace/processor.h +++ b/sdk/include/opentelemetry/sdk/trace/processor.h @@ -3,7 +3,6 @@ #include #include #include "opentelemetry/sdk/trace/recordable.h" -#include "opentelemetry/trace/span.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk From 2e4d835ed5feb184d2901d4f592c2933d1081856 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 6 Apr 2020 13:13:33 -0700 Subject: [PATCH 12/19] PR comments --- .../opentelemetry/sdk/trace/span_data.h | 19 ++++++++++------- sdk/src/trace/simple_processor.h | 21 ++++++++++++------- sdk/test/trace/simple_processor_test.cc | 12 ++++++++--- 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/span_data.h b/sdk/include/opentelemetry/sdk/trace/span_data.h index 53ae0e7922..e4fa52522e 100644 --- a/sdk/include/opentelemetry/sdk/trace/span_data.h +++ b/sdk/include/opentelemetry/sdk/trace/span_data.h @@ -73,35 +73,38 @@ class SpanData final : public Recordable */ std::chrono::nanoseconds GetDuration() const noexcept { return duration_; } - void SetTraceId(opentelemetry::trace::TraceId trace_id) noexcept { trace_id_ = trace_id; } + void SetTraceId(opentelemetry::trace::TraceId trace_id) noexcept override + { + trace_id_ = trace_id; + } - void SetSpanId(opentelemetry::trace::SpanId span_id) noexcept { span_id_ = span_id; } + void SetSpanId(opentelemetry::trace::SpanId span_id) noexcept override { span_id_ = span_id; } - void SetParentSpanId(opentelemetry::trace::SpanId parent_span_id) noexcept + void SetParentSpanId(opentelemetry::trace::SpanId parent_span_id) noexcept override { parent_span_id_ = parent_span_id; } - void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept + void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept override { (void)name; (void)timestamp; } - void SetStatus(trace_api::CanonicalCode code, nostd::string_view description) noexcept + void SetStatus(trace_api::CanonicalCode code, nostd::string_view description) noexcept override { status_code_ = code; status_desc_ = std::string(description); } - void SetName(nostd::string_view name) noexcept { name_ = std::string(name); } + void SetName(nostd::string_view name) noexcept override { name_ = std::string(name); } - void SetStartTime(opentelemetry::core::SystemTimestamp start_time) noexcept + void SetStartTime(opentelemetry::core::SystemTimestamp start_time) noexcept override { start_time_ = start_time; } - void SetDuration(std::chrono::nanoseconds duration) noexcept { duration_ = duration; } + void SetDuration(std::chrono::nanoseconds duration) noexcept override { duration_ = duration; } private: opentelemetry::trace::TraceId trace_id_; diff --git a/sdk/src/trace/simple_processor.h b/sdk/src/trace/simple_processor.h index c62948aa8c..4c91782fc2 100644 --- a/sdk/src/trace/simple_processor.h +++ b/sdk/src/trace/simple_processor.h @@ -12,24 +12,27 @@ namespace trace * The simple span processor passes finished recordables to the configured * SpanExporter, as soon as they are finished. * - * OnEnd, ForceFlush and Shutdown are no-ops. + * OnEnd and ForceFlush are no-ops. */ -class SimpleSpanProcessor +class SimpleSpanProcessor : public SpanProcessor { public: /** * Initialize a simple span processor. * @param exporter the exporter used by the span processor */ - SimpleSpanProcessor(std::unique_ptr &&exporter) noexcept + explicit SimpleSpanProcessor(std::unique_ptr &&exporter) noexcept : exporter_(std::move(exporter)) {} - std::unique_ptr MakeRecordable() noexcept { return exporter_->MakeRecordable(); } + std::unique_ptr MakeRecordable() noexcept override + { + return exporter_->MakeRecordable(); + } - void OnStart(Recordable &span) noexcept {} + void OnStart(Recordable &span) noexcept override {} - void OnEnd(std::unique_ptr &&span) noexcept + void OnEnd(std::unique_ptr &&span) noexcept override { std::shared_ptr recordable{std::move(span)}; nostd::span> s(&recordable, 1); @@ -40,9 +43,11 @@ class SimpleSpanProcessor } } - void ForceFlush(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept {} + void ForceFlush( + std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override + {} - void Shutdown() noexcept {} + void Shutdown() noexcept override { exporter_->Shutdown(); } private: std::unique_ptr exporter_; diff --git a/sdk/test/trace/simple_processor_test.cc b/sdk/test/trace/simple_processor_test.cc index 4bfff91d13..1d632d4494 100644 --- a/sdk/test/trace/simple_processor_test.cc +++ b/sdk/test/trace/simple_processor_test.cc @@ -12,7 +12,10 @@ using namespace opentelemetry::sdk::trace; class MockSpanExporter final : public SpanExporter { public: - MockSpanExporter(std::shared_ptr span_received) noexcept : span_received_(span_received) {} + MockSpanExporter(std::shared_ptr span_received, + std::shared_ptr shutdown_called) noexcept + : span_received_(span_received), shutdown_called_(shutdown_called) + {} std::unique_ptr MakeRecordable() noexcept { @@ -31,16 +34,18 @@ class MockSpanExporter final : public SpanExporter return ExportResult::rSuccess; } - void Shutdown() noexcept {} + void Shutdown() noexcept { *shutdown_called_ = true; } private: std::shared_ptr span_received_; + std::shared_ptr shutdown_called_; }; TEST(SimpleSpanProcessor, ToMockSpanExporter) { std::shared_ptr span_received(new bool(false)); - std::unique_ptr exporter(new MockSpanExporter(span_received)); + std::shared_ptr shutdown_called(new bool(false)); + std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); SimpleSpanProcessor processor(std::move(exporter)); auto recordable = processor.MakeRecordable(); @@ -52,4 +57,5 @@ TEST(SimpleSpanProcessor, ToMockSpanExporter) ASSERT_EQ(*span_received, true); processor.Shutdown(); + ASSERT_EQ(*shutdown_called, true); } From 8574b667adea7a0ef6d38729d5180d14cd27ca9c Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 6 Apr 2020 13:46:30 -0700 Subject: [PATCH 13/19] PR comments --- sdk/include/opentelemetry/sdk/trace/exporter.h | 4 ++-- sdk/src/trace/simple_processor.h | 2 +- sdk/test/trace/simple_processor_test.cc | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/exporter.h b/sdk/include/opentelemetry/sdk/trace/exporter.h index 632f7fcde3..31246b9072 100644 --- a/sdk/include/opentelemetry/sdk/trace/exporter.h +++ b/sdk/include/opentelemetry/sdk/trace/exporter.h @@ -17,12 +17,12 @@ enum class ExportResult /** * Batch was successfully exported. */ - rSuccess = 0, + kSuccess = 0, /** * Exporting failed. The caller must not retry exporting the same batch; the * batch must be dropped. */ - rFailure + kFailure }; /** * SpanExporter defines the interface that protocol-specific span exporters must diff --git a/sdk/src/trace/simple_processor.h b/sdk/src/trace/simple_processor.h index 4c91782fc2..2dd0a41667 100644 --- a/sdk/src/trace/simple_processor.h +++ b/sdk/src/trace/simple_processor.h @@ -36,7 +36,7 @@ class SimpleSpanProcessor : public SpanProcessor { std::shared_ptr recordable{std::move(span)}; nostd::span> s(&recordable, 1); - if (exporter_->Export(s) == ExportResult::rFailure) + if (exporter_->Export(s) == ExportResult::kFailure) { /* Once it is defined how the SDK does logging, an error should be * logged in this case. */ diff --git a/sdk/test/trace/simple_processor_test.cc b/sdk/test/trace/simple_processor_test.cc index 1d632d4494..ead5a73854 100644 --- a/sdk/test/trace/simple_processor_test.cc +++ b/sdk/test/trace/simple_processor_test.cc @@ -31,7 +31,7 @@ class MockSpanExporter final : public SpanExporter *span_received_ = true; } } - return ExportResult::rSuccess; + return ExportResult::kSuccess; } void Shutdown() noexcept { *shutdown_called_ = true; } From d634bed406b732cbc16fd9ac3f3bc3a754325b06 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 6 Apr 2020 17:06:56 -0700 Subject: [PATCH 14/19] Add timeout and unique pointer --- sdk/include/opentelemetry/sdk/trace/exporter.h | 9 ++++++--- sdk/include/opentelemetry/sdk/trace/processor.h | 5 ++++- sdk/src/trace/simple_processor.h | 10 ++++++---- sdk/test/trace/simple_processor_test.cc | 16 +++++++++++----- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/exporter.h b/sdk/include/opentelemetry/sdk/trace/exporter.h index 31246b9072..d517c572b2 100644 --- a/sdk/include/opentelemetry/sdk/trace/exporter.h +++ b/sdk/include/opentelemetry/sdk/trace/exporter.h @@ -45,15 +45,18 @@ class SpanExporter /** * Exports a batch of span recordables. This method must not be called * concurrently for the same exporter instance. - * @param spans a span of shared pointers to span recordables + * @param spans a span of unique pointers to span recordables */ virtual ExportResult Export( - nostd::span> &spans) noexcept = 0; + nostd::span> &spans) noexcept = 0; /** * Shut down the exporter. + * @param timeout an optional timeout, the default timeout of 0 means that no + * timeout is applied. */ - virtual void Shutdown() noexcept = 0; + virtual void Shutdown( + std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept = 0; }; } // namespace trace } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/trace/processor.h b/sdk/include/opentelemetry/sdk/trace/processor.h index bd2dd67585..04f9f8ab9d 100644 --- a/sdk/include/opentelemetry/sdk/trace/processor.h +++ b/sdk/include/opentelemetry/sdk/trace/processor.h @@ -52,8 +52,11 @@ class SpanProcessor * exported before shutdown. After the call to Shutdown, subsequent calls to * OnStart, OnEnd, ForceFlush or Shutdown will return immediately without * doing anything. + * @param timeout an optional timeout, the default timeout of 0 means that no + * timeout is applied. */ - virtual void Shutdown() noexcept = 0; + virtual void Shutdown( + std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept = 0; }; } // namespace trace } // namespace sdk diff --git a/sdk/src/trace/simple_processor.h b/sdk/src/trace/simple_processor.h index 2dd0a41667..c9cc617fa6 100644 --- a/sdk/src/trace/simple_processor.h +++ b/sdk/src/trace/simple_processor.h @@ -34,9 +34,8 @@ class SimpleSpanProcessor : public SpanProcessor void OnEnd(std::unique_ptr &&span) noexcept override { - std::shared_ptr recordable{std::move(span)}; - nostd::span> s(&recordable, 1); - if (exporter_->Export(s) == ExportResult::kFailure) + nostd::span> batch(&span, 1); + if (exporter_->Export(batch) == ExportResult::kFailure) { /* Once it is defined how the SDK does logging, an error should be * logged in this case. */ @@ -47,7 +46,10 @@ class SimpleSpanProcessor : public SpanProcessor std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override {} - void Shutdown() noexcept override { exporter_->Shutdown(); } + void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override + { + exporter_->Shutdown(timeout); + } private: std::unique_ptr exporter_; diff --git a/sdk/test/trace/simple_processor_test.cc b/sdk/test/trace/simple_processor_test.cc index ead5a73854..9c767d45c5 100644 --- a/sdk/test/trace/simple_processor_test.cc +++ b/sdk/test/trace/simple_processor_test.cc @@ -17,24 +17,30 @@ class MockSpanExporter final : public SpanExporter : span_received_(span_received), shutdown_called_(shutdown_called) {} - std::unique_ptr MakeRecordable() noexcept + std::unique_ptr MakeRecordable() noexcept override { return std::unique_ptr(new SpanData); } - ExportResult Export(opentelemetry::nostd::span> &spans) noexcept + ExportResult Export( + opentelemetry::nostd::span> &spans) noexcept override { - for (auto span : spans) + for (auto span = spans.begin(); span != spans.end(); ++span) { - if (std::static_pointer_cast(span)) + auto raw = span->release(); + if (static_cast(raw)) { *span_received_ = true; } + delete raw; } return ExportResult::kSuccess; } - void Shutdown() noexcept { *shutdown_called_ = true; } + void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override + { + *shutdown_called_ = true; + } private: std::shared_ptr span_received_; From 454aa8fa5d855e45d50e4b4da3aa792abaa8d1dc Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Thu, 9 Apr 2020 10:16:25 -0700 Subject: [PATCH 15/19] Add SetIds, use in-class initialization --- .../opentelemetry/sdk/trace/recordable.h | 16 ++++--------- .../opentelemetry/sdk/trace/span_data.h | 23 ++++++------------- sdk/test/trace/span_data_test.cc | 4 +--- 3 files changed, 12 insertions(+), 31 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/recordable.h b/sdk/include/opentelemetry/sdk/trace/recordable.h index ec8de8805d..c102958fe3 100644 --- a/sdk/include/opentelemetry/sdk/trace/recordable.h +++ b/sdk/include/opentelemetry/sdk/trace/recordable.h @@ -24,22 +24,14 @@ class Recordable virtual ~Recordable() = default; /** - * Set a trace id for this span. + * Set a trace id, span id and parent span id for this span. * @param trace_id the trace id to set - */ - virtual void SetTraceId(opentelemetry::trace::TraceId trace_id) noexcept = 0; - - /** - * Set a span id for this span. * @param span_id the span id to set - */ - virtual void SetSpanId(opentelemetry::trace::SpanId span_id) noexcept = 0; - - /** - * Set a parent span id for this span. * @param parent_span_id the parent span id to set */ - virtual void SetParentSpanId(opentelemetry::trace::SpanId parent_span_id) noexcept = 0; + virtual void SetIds(opentelemetry::trace::TraceId trace_id, + opentelemetry::trace::SpanId span_id, + opentelemetry::trace::SpanId parent_span_id) noexcept = 0; /** * Add an event to a span. diff --git a/sdk/include/opentelemetry/sdk/trace/span_data.h b/sdk/include/opentelemetry/sdk/trace/span_data.h index e4fa52522e..8ac620ad1b 100644 --- a/sdk/include/opentelemetry/sdk/trace/span_data.h +++ b/sdk/include/opentelemetry/sdk/trace/span_data.h @@ -19,12 +19,6 @@ namespace trace class SpanData final : public Recordable { public: - /** - * Initialized all span fields with default values. Time stamps and duration - * are initialized with 0, the status of the span is initialized with OK. - */ - SpanData() noexcept : duration_(0), status_code_(opentelemetry::trace::CanonicalCode::OK) {} - /** * Get the trace id for this span * @return the trace id for this span @@ -73,15 +67,12 @@ class SpanData final : public Recordable */ std::chrono::nanoseconds GetDuration() const noexcept { return duration_; } - void SetTraceId(opentelemetry::trace::TraceId trace_id) noexcept override - { - trace_id_ = trace_id; - } - - void SetSpanId(opentelemetry::trace::SpanId span_id) noexcept override { span_id_ = span_id; } - - void SetParentSpanId(opentelemetry::trace::SpanId parent_span_id) noexcept override + void SetIds(opentelemetry::trace::TraceId trace_id, + opentelemetry::trace::SpanId span_id, + opentelemetry::trace::SpanId parent_span_id) noexcept override { + trace_id_ = trace_id; + span_id_ = span_id; parent_span_id_ = parent_span_id; } @@ -111,9 +102,9 @@ class SpanData final : public Recordable opentelemetry::trace::SpanId span_id_; opentelemetry::trace::SpanId parent_span_id_; core::SystemTimestamp start_time_; - std::chrono::nanoseconds duration_; + std::chrono::nanoseconds duration_{0}; std::string name_; - opentelemetry::trace::CanonicalCode status_code_; + opentelemetry::trace::CanonicalCode status_code_{opentelemetry::trace::CanonicalCode::OK}; std::string status_desc_; }; } // namespace trace diff --git a/sdk/test/trace/span_data_test.cc b/sdk/test/trace/span_data_test.cc index c2e6b4d5ba..ea8aab446c 100644 --- a/sdk/test/trace/span_data_test.cc +++ b/sdk/test/trace/span_data_test.cc @@ -30,9 +30,7 @@ TEST(SpanData, Set) opentelemetry::core::SystemTimestamp now(std::chrono::system_clock::now()); SpanData data; - data.SetTraceId(trace_id); - data.SetSpanId(span_id); - data.SetParentSpanId(parent_span_id); + data.SetIds(trace_id, span_id, parent_span_id); data.SetName("span name"); data.SetStatus(opentelemetry::trace::CanonicalCode::UNKNOWN, "description"); data.SetStartTime(now); From e1362d8714f7ffc813da550b1db3df6cecf07a44 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 13 Apr 2020 14:31:44 -0700 Subject: [PATCH 16/19] PR comments --- sdk/test/trace/simple_processor_test.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/sdk/test/trace/simple_processor_test.cc b/sdk/test/trace/simple_processor_test.cc index 9c767d45c5..b86a93f9c0 100644 --- a/sdk/test/trace/simple_processor_test.cc +++ b/sdk/test/trace/simple_processor_test.cc @@ -25,15 +25,14 @@ class MockSpanExporter final : public SpanExporter ExportResult Export( opentelemetry::nostd::span> &spans) noexcept override { - for (auto span = spans.begin(); span != spans.end(); ++span) + for (auto& span : spans) { - auto raw = span->release(); - if (static_cast(raw)) + if (span != nullptr) { *span_received_ = true; } - delete raw; } + return ExportResult::kSuccess; } From e3657678d5bdd1290fccedb81e15b4a1be1a02cc Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 13 Apr 2020 14:56:12 -0700 Subject: [PATCH 17/19] PR comments --- sdk/test/trace/simple_processor_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/test/trace/simple_processor_test.cc b/sdk/test/trace/simple_processor_test.cc index b86a93f9c0..13a06e675a 100644 --- a/sdk/test/trace/simple_processor_test.cc +++ b/sdk/test/trace/simple_processor_test.cc @@ -25,7 +25,7 @@ class MockSpanExporter final : public SpanExporter ExportResult Export( opentelemetry::nostd::span> &spans) noexcept override { - for (auto& span : spans) + for (auto &span : spans) { if (span != nullptr) { From a9e6cac4708b4bdf05c4efe2f887207e5642e66f Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 13 Apr 2020 15:15:30 -0700 Subject: [PATCH 18/19] Use specific test functions --- sdk/test/trace/simple_processor_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/test/trace/simple_processor_test.cc b/sdk/test/trace/simple_processor_test.cc index 13a06e675a..ddeba25fe6 100644 --- a/sdk/test/trace/simple_processor_test.cc +++ b/sdk/test/trace/simple_processor_test.cc @@ -56,11 +56,11 @@ TEST(SimpleSpanProcessor, ToMockSpanExporter) auto recordable = processor.MakeRecordable(); processor.OnStart(*recordable); - ASSERT_EQ(*span_received, false); + ASSERT_FALSE(*span_received); processor.OnEnd(std::move(recordable)); - ASSERT_EQ(*span_received, true); + ASSERT_TRUE(*span_received); processor.Shutdown(); - ASSERT_EQ(*shutdown_called, true); + ASSERT_TRUE(*shutdown_called); } From c4c1f0982f36364b0eb533246cd282cadc964d32 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 13 Apr 2020 15:53:08 -0700 Subject: [PATCH 19/19] PR comments --- sdk/test/trace/BUILD | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/test/trace/BUILD b/sdk/test/trace/BUILD index 8a27e5f717..7ba548fd53 100644 --- a/sdk/test/trace/BUILD +++ b/sdk/test/trace/BUILD @@ -1,5 +1,5 @@ cc_test( - name = "default_tracer_provider", + name = "default_tracer_provider_test", srcs = [ "default_tracer_provider_test.cc", ], @@ -10,7 +10,7 @@ cc_test( ) cc_test( - name = "span_data", + name = "span_data_test", srcs = [ "span_data_test.cc", ], @@ -21,7 +21,7 @@ cc_test( ) cc_test( - name = "simple_processor", + name = "simple_processor_test", srcs = [ "simple_processor_test.cc", ],