From 67b0b807080a02f2de3e0741027187813a99e6c0 Mon Sep 17 00:00:00 2001 From: Evgeny Yakimov Date: Fri, 9 Apr 2021 10:38:20 +0100 Subject: [PATCH] Add SpanContext (and TraceState) to Recordable - Updated Recordable interface to take a span context and parent span id rather than just the trace/span ids. - Updated SpanData to new interface and added new getters for accessing SpanContext - Updated ThreadsafeSpanData with the same - Updated tests for both (and made consistent) - Updated OStreamSpanExporter to print tracestate and tests --- .../opentelemetry/common/kv_properties.h | 4 ++- exporters/ostream/src/span_exporter.cc | 1 + exporters/ostream/test/ostream_span_test.cc | 29 +++++++++++----- .../ext/zpages/threadsafe_span_data.h | 32 ++++++++++------- ext/test/zpages/threadsafe_span_data_test.cc | 34 +++++++++++++------ .../opentelemetry/sdk/trace/recordable.h | 11 +++--- .../opentelemetry/sdk/trace/span_data.h | 21 +++++++----- sdk/src/trace/span.cc | 14 ++++---- sdk/test/trace/span_data_test.cc | 27 +++++++++++---- 9 files changed, 112 insertions(+), 61 deletions(-) diff --git a/api/include/opentelemetry/common/kv_properties.h b/api/include/opentelemetry/common/kv_properties.h index 2124cd1528..18c517f480 100644 --- a/api/include/opentelemetry/common/kv_properties.h +++ b/api/include/opentelemetry/common/kv_properties.h @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +#pragma once + #include "opentelemetry/common/key_value_iterable_view.h" #include "opentelemetry/common/string_util.h" #include "opentelemetry/nostd/function_ref.h" @@ -267,4 +269,4 @@ class KeyValueProperties size_t Size() const noexcept { return num_entries_; } }; } // namespace common -OPENTELEMETRY_END_NAMESPACE +OPENTELEMETRY_END_NAMESPACE \ No newline at end of file diff --git a/exporters/ostream/src/span_exporter.cc b/exporters/ostream/src/span_exporter.cc index 724b21e07c..09562ec703 100644 --- a/exporters/ostream/src/span_exporter.cc +++ b/exporters/ostream/src/span_exporter.cc @@ -64,6 +64,7 @@ sdktrace::ExportResult OStreamSpanExporter::Export( << "\n name : " << span->GetName() << "\n trace_id : " << std::string(trace_id, 32) << "\n span_id : " << std::string(span_id, 16) + << "\n tracestate : " << span->GetSpanContext().trace_state()->ToHeader() << "\n parent_span_id: " << std::string(parent_span_id, 16) << "\n start : " << span->GetStartTime().time_since_epoch().count() << "\n duration : " << span->GetDuration().count() diff --git a/exporters/ostream/test/ostream_span_test.cc b/exporters/ostream/test/ostream_span_test.cc index 4ff59dcabc..c955544f5c 100644 --- a/exporters/ostream/test/ostream_span_test.cc +++ b/exporters/ostream/test/ostream_span_test.cc @@ -48,6 +48,7 @@ constexpr const char *kDefaultSpanPrinted = " name : \n" " trace_id : 00000000000000000000000000000000\n" " span_id : 0000000000000000\n" + " tracestate : \n" " parent_span_id: 0000000000000000\n" " start : 0\n" " duration : 0\n" @@ -85,16 +86,21 @@ TEST(OStreamSpanExporter, PrintSpanWithBasicFields) auto recordable = processor->MakeRecordable(); + constexpr uint8_t trace_id_buf[] = {1, 2, 3, 4, 5, 6, 7, 8, 1, 2, 3, 4, 5, 6, 7, 8}; + constexpr uint8_t span_id_buf[] = {1, 2, 3, 4, 5, 6, 7, 8}; + constexpr uint8_t parent_span_id_buf[] = {8, 7, 6, 5, 4, 3, 2, 1}; + opentelemetry::trace::TraceId trace_id{trace_id_buf}; + opentelemetry::trace::SpanId span_id{span_id_buf}; + opentelemetry::trace::SpanId parent_span_id{parent_span_id_buf}; + const auto trace_state = opentelemetry::trace::TraceState::GetDefault()->Set("state1", "value"); + const opentelemetry::trace::SpanContext span_context{ + trace_id, span_id, + opentelemetry::trace::TraceFlags{opentelemetry::trace::TraceFlags::kIsSampled}, true, + trace_state}; + + recordable->SetIdentity(span_context, parent_span_id); recordable->SetName("Test Span"); opentelemetry::core::SystemTimestamp now(std::chrono::system_clock::now()); - - constexpr uint8_t trace_id_buf[] = {1, 2, 3, 4, 5, 6, 7, 8, 1, 2, 3, 4, 5, 6, 7, 8}; - opentelemetry::trace::TraceId t_id(trace_id_buf); - constexpr uint8_t span_id_buf[] = {1, 2, 3, 4, 5, 6, 7, 8}; - opentelemetry::trace::SpanId s_id(span_id_buf); - - recordable->SetIds(t_id, s_id, s_id); - recordable->SetStartTime(now); recordable->SetDuration(std::chrono::nanoseconds(100)); recordable->SetStatus(opentelemetry::trace::StatusCode::kOk, "Test Description"); @@ -109,7 +115,8 @@ TEST(OStreamSpanExporter, PrintSpanWithBasicFields) " name : Test Span\n" " trace_id : 01020304050607080102030405060708\n" " span_id : 0102030405060708\n" - " parent_span_id: 0102030405060708\n" + " tracestate : state1=value\n" + " parent_span_id: 0807060504030201\n" " start : " + start + "\n" @@ -143,6 +150,7 @@ TEST(OStreamSpanExporter, PrintSpanWithAttribute) " name : \n" " trace_id : 00000000000000000000000000000000\n" " span_id : 0000000000000000\n" + " tracestate : \n" " parent_span_id: 0000000000000000\n" " start : 0\n" " duration : 0\n" @@ -178,6 +186,7 @@ TEST(OStreamSpanExporter, PrintSpanWithArrayAttribute) " name : \n" " trace_id : 00000000000000000000000000000000\n" " span_id : 0000000000000000\n" + " tracestate : \n" " parent_span_id: 0000000000000000\n" " start : 0\n" " duration : 0\n" @@ -219,6 +228,7 @@ TEST(OStreamSpanExporter, PrintSpanWithEvents) " name : \n" " trace_id : 00000000000000000000000000000000\n" " span_id : 0000000000000000\n" + " tracestate : \n" " parent_span_id: 0000000000000000\n" " start : 0\n" " duration : 0\n" @@ -291,6 +301,7 @@ TEST(OStreamSpanExporter, PrintSpanWithLinks) " name : \n" " trace_id : 00000000000000000000000000000000\n" " span_id : 0000000000000000\n" + " tracestate : \n" " parent_span_id: 0000000000000000\n" " start : 0\n" " duration : 0\n" diff --git a/ext/include/opentelemetry/ext/zpages/threadsafe_span_data.h b/ext/include/opentelemetry/ext/zpages/threadsafe_span_data.h index 3bb0326130..0d3b0488a1 100644 --- a/ext/include/opentelemetry/ext/zpages/threadsafe_span_data.h +++ b/ext/include/opentelemetry/ext/zpages/threadsafe_span_data.h @@ -33,7 +33,7 @@ class ThreadsafeSpanData final : public opentelemetry::sdk::trace::Recordable opentelemetry::trace::TraceId GetTraceId() const noexcept { std::lock_guard lock(mutex_); - return trace_id_; + return span_context_.trace_id(); } /** @@ -43,7 +43,17 @@ class ThreadsafeSpanData final : public opentelemetry::sdk::trace::Recordable opentelemetry::trace::SpanId GetSpanId() const noexcept { std::lock_guard lock(mutex_); - return span_id_; + return span_context_.span_id(); + } + + /** + * Get the span context for this span + * @return the span context for this span + */ + const opentelemetry::trace::SpanContext &GetSpanContext() const noexcept + { + std::lock_guard lock(mutex_); + return span_context_; } /** @@ -110,20 +120,18 @@ class ThreadsafeSpanData final : public opentelemetry::sdk::trace::Recordable * Get the attributes for this span * @return the attributes for this span */ - const std::unordered_map - GetAttributes() const noexcept + std::unordered_map GetAttributes() + const noexcept { std::lock_guard lock(mutex_); return attributes_; } - void SetIds(opentelemetry::trace::TraceId trace_id, - opentelemetry::trace::SpanId span_id, - opentelemetry::trace::SpanId parent_span_id) noexcept override + void SetIdentity(const opentelemetry::trace::SpanContext &span_context, + opentelemetry::trace::SpanId parent_span_id) noexcept override { std::lock_guard lock(mutex_); - trace_id_ = trace_id; - span_id_ = span_id; + span_context_ = span_context; parent_span_id_ = parent_span_id; } @@ -195,8 +203,7 @@ class ThreadsafeSpanData final : public opentelemetry::sdk::trace::Recordable private: ThreadsafeSpanData(const ThreadsafeSpanData &threadsafe_span_data, const std::lock_guard &) - : trace_id_(threadsafe_span_data.trace_id_), - span_id_(threadsafe_span_data.span_id_), + : span_context_(threadsafe_span_data.span_context_), parent_span_id_(threadsafe_span_data.parent_span_id_), start_time_(threadsafe_span_data.start_time_), duration_(threadsafe_span_data.duration_), @@ -209,8 +216,7 @@ class ThreadsafeSpanData final : public opentelemetry::sdk::trace::Recordable {} mutable std::mutex mutex_; - opentelemetry::trace::TraceId trace_id_; - opentelemetry::trace::SpanId span_id_; + opentelemetry::trace::SpanContext span_context_{false, false}; opentelemetry::trace::SpanId parent_span_id_; core::SystemTimestamp start_time_; std::chrono::nanoseconds duration_{0}; diff --git a/ext/test/zpages/threadsafe_span_data_test.cc b/ext/test/zpages/threadsafe_span_data_test.cc index 306e23a4bb..18346020b4 100644 --- a/ext/test/zpages/threadsafe_span_data_test.cc +++ b/ext/test/zpages/threadsafe_span_data_test.cc @@ -12,12 +12,13 @@ using opentelemetry::sdk::common::OwnedAttributeValue; TEST(ThreadsafeSpanData, DefaultValues) { - opentelemetry::trace::TraceId zero_trace_id; + opentelemetry::trace::SpanContext empty_span_context{false, false}; opentelemetry::trace::SpanId zero_span_id; ThreadsafeSpanData data; - ASSERT_EQ(data.GetTraceId(), zero_trace_id); - ASSERT_EQ(data.GetSpanId(), zero_span_id); + ASSERT_EQ(data.GetTraceId(), empty_span_context.trace_id()); + ASSERT_EQ(data.GetSpanId(), empty_span_context.span_id()); + ASSERT_EQ(data.GetSpanContext(), empty_span_context); ASSERT_EQ(data.GetParentSpanId(), zero_span_id); ASSERT_EQ(data.GetName(), ""); ASSERT_EQ(data.GetStatus(), opentelemetry::trace::StatusCode::kUnset); @@ -29,27 +30,40 @@ TEST(ThreadsafeSpanData, DefaultValues) TEST(ThreadsafeSpanData, Set) { - opentelemetry::trace::TraceId trace_id; - opentelemetry::trace::SpanId span_id; - opentelemetry::trace::SpanId parent_span_id; + constexpr uint8_t trace_id_buf[] = {1, 2, 3, 4, 5, 6, 7, 8, 1, 2, 3, 4, 5, 6, 7, 8}; + constexpr uint8_t span_id_buf[] = {1, 2, 3, 4, 5, 6, 7, 8}; + constexpr uint8_t parent_span_id_buf[] = {8, 7, 6, 5, 4, 3, 2, 1}; + opentelemetry::trace::TraceId trace_id{trace_id_buf}; + opentelemetry::trace::SpanId span_id{span_id_buf}; + opentelemetry::trace::SpanId parent_span_id{parent_span_id_buf}; + const auto trace_state = opentelemetry::trace::TraceState::GetDefault()->Set("key1", "value"); + const opentelemetry::trace::SpanContext span_context{ + trace_id, span_id, + opentelemetry::trace::TraceFlags{opentelemetry::trace::TraceFlags::kIsSampled}, true, + trace_state}; opentelemetry::core::SystemTimestamp now(std::chrono::system_clock::now()); ThreadsafeSpanData data; - data.SetIds(trace_id, span_id, parent_span_id); + data.SetIdentity(span_context, parent_span_id); data.SetName("span name"); + data.SetSpanKind(opentelemetry::trace::SpanKind::kServer); data.SetStatus(opentelemetry::trace::StatusCode::kOk, "description"); data.SetStartTime(now); data.SetDuration(std::chrono::nanoseconds(1000000)); - data.SetAttribute("attr1", 314159); - data.AddEvent("event1", now); + data.SetAttribute("attr1", (int64_t)314159); + data.opentelemetry::sdk::trace::Recordable::AddEvent("event1", now); ASSERT_EQ(data.GetTraceId(), trace_id); ASSERT_EQ(data.GetSpanId(), span_id); + ASSERT_EQ(data.GetSpanContext(), span_context); + std::string trace_state_key1_value; + ASSERT_EQ(data.GetSpanContext().trace_state()->Get("key1", trace_state_key1_value), true); + ASSERT_EQ(trace_state_key1_value, "value"); ASSERT_EQ(data.GetParentSpanId(), parent_span_id); ASSERT_EQ(data.GetName(), "span name"); ASSERT_EQ(data.GetStatus(), opentelemetry::trace::StatusCode::kOk); ASSERT_EQ(data.GetDescription(), "description"); ASSERT_EQ(data.GetStartTime().time_since_epoch(), now.time_since_epoch()); ASSERT_EQ(data.GetDuration(), std::chrono::nanoseconds(1000000)); - ASSERT_EQ(opentelemetry::nostd::get(data.GetAttributes().at("attr1")), 314159); + ASSERT_EQ(opentelemetry::nostd::get(data.GetAttributes().at("attr1")), 314159); } diff --git a/sdk/include/opentelemetry/sdk/trace/recordable.h b/sdk/include/opentelemetry/sdk/trace/recordable.h index e179f440bb..ef5678d921 100644 --- a/sdk/include/opentelemetry/sdk/trace/recordable.h +++ b/sdk/include/opentelemetry/sdk/trace/recordable.h @@ -32,14 +32,12 @@ class Recordable virtual ~Recordable() = default; /** - * Set a trace id, span id and parent span id for this span. - * @param trace_id the trace id to set - * @param span_id the span id to set + * Set the span context and parent span id + * @param span_context the span context to set * @param parent_span_id the parent span id to set */ - virtual void SetIds(opentelemetry::trace::TraceId trace_id, - opentelemetry::trace::SpanId span_id, - opentelemetry::trace::SpanId parent_span_id) noexcept = 0; + virtual void SetIdentity(const opentelemetry::trace::SpanContext &span_context, + opentelemetry::trace::SpanId parent_span_id) noexcept = 0; /** * Set an attribute of a span. @@ -115,6 +113,7 @@ class Recordable * @param span_kind the spankind to set */ virtual void SetSpanKind(opentelemetry::trace::SpanKind span_kind) noexcept = 0; + /** * Set the start time of the span. * @param start_time the start time to set diff --git a/sdk/include/opentelemetry/sdk/trace/span_data.h b/sdk/include/opentelemetry/sdk/trace/span_data.h index 081c907394..3b7a523a37 100644 --- a/sdk/include/opentelemetry/sdk/trace/span_data.h +++ b/sdk/include/opentelemetry/sdk/trace/span_data.h @@ -100,13 +100,19 @@ class SpanData final : public Recordable * Get the trace id for this span * @return the trace id for this span */ - opentelemetry::trace::TraceId GetTraceId() const noexcept { return trace_id_; } + opentelemetry::trace::TraceId GetTraceId() const noexcept { return span_context_.trace_id(); } /** * Get the span id for this span * @return the span id for this span */ - opentelemetry::trace::SpanId GetSpanId() const noexcept { return span_id_; } + opentelemetry::trace::SpanId GetSpanId() const noexcept { return span_context_.span_id(); } + + /** + * Get the span context for this span + * @return the span context for this span + */ + const opentelemetry::trace::SpanContext &GetSpanContext() const noexcept { return span_context_; } /** * Get the parent span id for this span @@ -171,12 +177,10 @@ class SpanData final : public Recordable */ const std::vector &GetLinks() const noexcept { return links_; } - void SetIds(opentelemetry::trace::TraceId trace_id, - opentelemetry::trace::SpanId span_id, - opentelemetry::trace::SpanId parent_span_id) noexcept override + void SetIdentity(const opentelemetry::trace::SpanContext &span_context, + opentelemetry::trace::SpanId parent_span_id) noexcept override { - trace_id_ = trace_id; - span_id_ = span_id; + span_context_ = span_context; parent_span_id_ = parent_span_id; } @@ -226,8 +230,7 @@ class SpanData final : public Recordable void SetDuration(std::chrono::nanoseconds duration) noexcept override { duration_ = duration; } private: - opentelemetry::trace::TraceId trace_id_; - opentelemetry::trace::SpanId span_id_; + opentelemetry::trace::SpanContext span_context_{false, false}; opentelemetry::trace::SpanId parent_span_id_; core::SystemTimestamp start_time_; std::chrono::nanoseconds duration_{0}; diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index 576d54f38c..ad4f8fe12c 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -81,27 +81,29 @@ Span::Span(std::shared_ptr &&tracer, trace_api::TraceId trace_id; trace_api::SpanId span_id = GenerateRandomSpanId(); + trace_api::SpanId parent_span_id; bool is_parent_span_valid = false; if (parent_span_context.IsValid()) { - trace_id = parent_span_context.trace_id(); - recordable_->SetIds(trace_id, span_id, parent_span_context.span_id()); + trace_id = parent_span_context.trace_id(); + parent_span_id = parent_span_context.span_id(); is_parent_span_valid = true; } else { trace_id = GenerateRandomTraceId(); - recordable_->SetIds(trace_id, span_id, trace_api::SpanId()); } span_context_ = std::unique_ptr(new trace_api::SpanContext( trace_id, span_id, sampled ? trace_api::TraceFlags{trace_api::TraceFlags::kIsSampled} : trace_api::TraceFlags{}, false, - trace_state ? trace_state - : is_parent_span_valid ? parent_span_context.trace_state() - : trace_api::TraceState::GetDefault())); + trace_state ? trace_state + : is_parent_span_valid ? parent_span_context.trace_state() + : trace_api::TraceState::GetDefault())); + + recordable_->SetIdentity(*span_context_, parent_span_id); attributes.ForEachKeyValue( [&](nostd::string_view key, opentelemetry::common::AttributeValue value) noexcept { diff --git a/sdk/test/trace/span_data_test.cc b/sdk/test/trace/span_data_test.cc index f04fe0516a..f02d1b9070 100644 --- a/sdk/test/trace/span_data_test.cc +++ b/sdk/test/trace/span_data_test.cc @@ -10,12 +10,13 @@ using opentelemetry::sdk::trace::SpanData; TEST(SpanData, DefaultValues) { - opentelemetry::trace::TraceId zero_trace_id; + opentelemetry::trace::SpanContext empty_span_context{false, false}; 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.GetTraceId(), empty_span_context.trace_id()); + ASSERT_EQ(data.GetSpanId(), empty_span_context.span_id()); + ASSERT_EQ(data.GetSpanContext(), empty_span_context); ASSERT_EQ(data.GetParentSpanId(), zero_span_id); ASSERT_EQ(data.GetName(), ""); ASSERT_EQ(data.GetStatus(), opentelemetry::trace::StatusCode::kUnset); @@ -28,13 +29,21 @@ TEST(SpanData, DefaultValues) TEST(SpanData, Set) { - opentelemetry::trace::TraceId trace_id; - opentelemetry::trace::SpanId span_id; - opentelemetry::trace::SpanId parent_span_id; + constexpr uint8_t trace_id_buf[] = {1, 2, 3, 4, 5, 6, 7, 8, 1, 2, 3, 4, 5, 6, 7, 8}; + constexpr uint8_t span_id_buf[] = {1, 2, 3, 4, 5, 6, 7, 8}; + constexpr uint8_t parent_span_id_buf[] = {8, 7, 6, 5, 4, 3, 2, 1}; + opentelemetry::trace::TraceId trace_id{trace_id_buf}; + opentelemetry::trace::SpanId span_id{span_id_buf}; + opentelemetry::trace::SpanId parent_span_id{parent_span_id_buf}; + const auto trace_state = opentelemetry::trace::TraceState::GetDefault()->Set("key1", "value"); + const opentelemetry::trace::SpanContext span_context{ + trace_id, span_id, + opentelemetry::trace::TraceFlags{opentelemetry::trace::TraceFlags::kIsSampled}, true, + trace_state}; opentelemetry::core::SystemTimestamp now(std::chrono::system_clock::now()); SpanData data; - data.SetIds(trace_id, span_id, parent_span_id); + data.SetIdentity(span_context, parent_span_id); data.SetName("span name"); data.SetSpanKind(opentelemetry::trace::SpanKind::kServer); data.SetStatus(opentelemetry::trace::StatusCode::kOk, "description"); @@ -45,6 +54,10 @@ TEST(SpanData, Set) ASSERT_EQ(data.GetTraceId(), trace_id); ASSERT_EQ(data.GetSpanId(), span_id); + ASSERT_EQ(data.GetSpanContext(), span_context); + std::string trace_state_key1_value; + ASSERT_EQ(data.GetSpanContext().trace_state()->Get("key1", trace_state_key1_value), true); + ASSERT_EQ(trace_state_key1_value, "value"); ASSERT_EQ(data.GetParentSpanId(), parent_span_id); ASSERT_EQ(data.GetName(), "span name"); ASSERT_EQ(data.GetSpanKind(), opentelemetry::trace::SpanKind::kServer);