From 081dcbd6a2643e3707e4cecc6715156ca09eac26 Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Tue, 21 Jul 2020 18:33:34 +0000 Subject: [PATCH 1/6] Add attributes to AddEvent --- .../otlp/include/opentelemetry/exporters/otlp/recordable.h | 4 +++- exporters/otlp/src/recordable.cc | 4 +++- exporters/otlp/test/recordable_test.cc | 3 ++- sdk/include/opentelemetry/sdk/trace/recordable.h | 6 +++++- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/recordable.h b/exporters/otlp/include/opentelemetry/exporters/otlp/recordable.h index 9d55dfd404..13f938803d 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/recordable.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/recordable.h @@ -21,7 +21,9 @@ class Recordable final : public sdk::trace::Recordable void SetAttribute(nostd::string_view key, const opentelemetry::common::AttributeValue &value) noexcept override; - void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept override; + void AddEvent(nostd::string_view name, + core::SystemTimestamp timestamp, + const trace::KeyValueIterable &attributes) noexcept override; void SetStatus(trace::CanonicalCode code, nostd::string_view description) noexcept override; diff --git a/exporters/otlp/src/recordable.cc b/exporters/otlp/src/recordable.cc index 26555ea0b7..08e88492ed 100644 --- a/exporters/otlp/src/recordable.cc +++ b/exporters/otlp/src/recordable.cc @@ -111,7 +111,9 @@ void Recordable::SetAttribute(nostd::string_view key, } } -void Recordable::AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept +void Recordable::AddEvent(nostd::string_view name, + core::SystemTimestamp timestamp, + const trace::KeyValueIterable &attributes) noexcept { auto *event = span_.add_events(); event->set_name(name.data(), name.size()); diff --git a/exporters/otlp/test/recordable_test.cc b/exporters/otlp/test/recordable_test.cc index ec1662662d..dc1606a19c 100644 --- a/exporters/otlp/test/recordable_test.cc +++ b/exporters/otlp/test/recordable_test.cc @@ -1,4 +1,5 @@ #include "opentelemetry/exporters/otlp/recordable.h" +#include "opentelemetry/trace/key_value_iterable_view.h" #include @@ -86,7 +87,7 @@ TEST(Recordable, AddEvents) std::chrono::system_clock::time_point event_time = std::chrono::system_clock::now(); core::SystemTimestamp event_timestamp(event_time); - rec.AddEvent(name, event_timestamp); + rec.AddEvent(name, event_timestamp, trace::KeyValueIterableView>({})); uint64_t unix_event_time = std::chrono::duration_cast(event_time.time_since_epoch()).count(); diff --git a/sdk/include/opentelemetry/sdk/trace/recordable.h b/sdk/include/opentelemetry/sdk/trace/recordable.h index a2ac8984dc..ef0f648511 100644 --- a/sdk/include/opentelemetry/sdk/trace/recordable.h +++ b/sdk/include/opentelemetry/sdk/trace/recordable.h @@ -6,6 +6,7 @@ #include "opentelemetry/trace/canonical_code.h" #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/trace_id.h" +#include "opentelemetry/trace/key_value_iterable.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -46,8 +47,11 @@ class Recordable * Add an event to a span. * @param name the name of the event * @param timestamp the timestamp of the event + * @param attributes the attributes associated with the event */ - virtual void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept = 0; + virtual void AddEvent(nostd::string_view name, + core::SystemTimestamp timestamp, + const trace_api::KeyValueIterable &attributes) noexcept = 0; /** * Set the status of the span. From 273d8e455edd98039d4358c6e98b93d7376edccd Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Tue, 21 Jul 2020 18:43:51 +0000 Subject: [PATCH 2/6] Update SpanData for new AddEvent --- exporters/otlp/src/recordable.cc | 1 + sdk/include/opentelemetry/sdk/trace/span_data.h | 5 ++++- sdk/test/trace/span_data_test.cc | 3 ++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/exporters/otlp/src/recordable.cc b/exporters/otlp/src/recordable.cc index 08e88492ed..6a7e94efde 100644 --- a/exporters/otlp/src/recordable.cc +++ b/exporters/otlp/src/recordable.cc @@ -118,6 +118,7 @@ void Recordable::AddEvent(nostd::string_view name, auto *event = span_.add_events(); event->set_name(name.data(), name.size()); event->set_time_unix_nano(timestamp.time_since_epoch().count()); + // TODO: handle attributes } void Recordable::SetStatus(trace::CanonicalCode code, nostd::string_view description) noexcept diff --git a/sdk/include/opentelemetry/sdk/trace/span_data.h b/sdk/include/opentelemetry/sdk/trace/span_data.h index ffe1db1878..42bde1cf07 100644 --- a/sdk/include/opentelemetry/sdk/trace/span_data.h +++ b/sdk/include/opentelemetry/sdk/trace/span_data.h @@ -194,9 +194,12 @@ class SpanData final : public Recordable attributes_[std::string(key)] = nostd::visit(converter_, value); } - void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept override + void AddEvent(nostd::string_view name, + core::SystemTimestamp timestamp, + const trace_api::KeyValueIterable &attributes) noexcept override { events_.push_back(SpanDataEvent(std::string(name), timestamp)); + // TODO: handle attributes } void SetStatus(trace_api::CanonicalCode code, nostd::string_view description) noexcept override diff --git a/sdk/test/trace/span_data_test.cc b/sdk/test/trace/span_data_test.cc index 41d7b2c670..ee30932812 100644 --- a/sdk/test/trace/span_data_test.cc +++ b/sdk/test/trace/span_data_test.cc @@ -2,6 +2,7 @@ #include "opentelemetry/nostd/variant.h" #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/trace_id.h" +#include "opentelemetry/trace/key_value_iterable_view.h" #include @@ -39,7 +40,7 @@ TEST(SpanData, Set) data.SetStartTime(now); data.SetDuration(std::chrono::nanoseconds(1000000)); data.SetAttribute("attr1", 314159); - data.AddEvent("event1", now); + data.AddEvent("event1", now, opentelemetry::trace::KeyValueIterableView>({})); ASSERT_EQ(data.GetTraceId(), trace_id); ASSERT_EQ(data.GetSpanId(), span_id); From 7c506b4a42e0356c1d42229af85d39d8bf7ebf37 Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Tue, 21 Jul 2020 19:08:34 +0000 Subject: [PATCH 3/6] Add AddLink to recordable --- .../opentelemetry/exporters/otlp/recordable.h | 4 ++++ exporters/otlp/src/recordable.cc | 9 +++++++++ sdk/include/opentelemetry/sdk/trace/recordable.h | 12 +++++++++++- sdk/include/opentelemetry/sdk/trace/span_data.h | 9 +++++++++ 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/recordable.h b/exporters/otlp/include/opentelemetry/exporters/otlp/recordable.h index 13f938803d..06ac93e139 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/recordable.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/recordable.h @@ -25,6 +25,10 @@ class Recordable final : public sdk::trace::Recordable core::SystemTimestamp timestamp, const trace::KeyValueIterable &attributes) noexcept override; + void AddLink(opentelemetry::trace::TraceId trace_id, + opentelemetry::trace::SpanId span_id, + const trace::KeyValueIterable &attributes) noexcept override; + void SetStatus(trace::CanonicalCode code, nostd::string_view description) noexcept override; void SetName(nostd::string_view name) noexcept override; diff --git a/exporters/otlp/src/recordable.cc b/exporters/otlp/src/recordable.cc index 6a7e94efde..65697b5b5c 100644 --- a/exporters/otlp/src/recordable.cc +++ b/exporters/otlp/src/recordable.cc @@ -121,6 +121,15 @@ void Recordable::AddEvent(nostd::string_view name, // TODO: handle attributes } +void Recordable::AddLink(opentelemetry::trace::TraceId trace_id, + opentelemetry::trace::SpanId span_id, + const trace::KeyValueIterable &attributes) noexcept +{ + (void)trace_id; + (void)span_id; + (void)attributes; +} + void Recordable::SetStatus(trace::CanonicalCode code, nostd::string_view description) noexcept { span_.mutable_status()->set_code(opentelemetry::proto::trace::v1::Status_StatusCode(code)); diff --git a/sdk/include/opentelemetry/sdk/trace/recordable.h b/sdk/include/opentelemetry/sdk/trace/recordable.h index ef0f648511..108ac688a9 100644 --- a/sdk/include/opentelemetry/sdk/trace/recordable.h +++ b/sdk/include/opentelemetry/sdk/trace/recordable.h @@ -4,9 +4,9 @@ #include "opentelemetry/core/timestamp.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/trace/canonical_code.h" +#include "opentelemetry/trace/key_value_iterable.h" #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/trace_id.h" -#include "opentelemetry/trace/key_value_iterable.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -53,6 +53,16 @@ class Recordable core::SystemTimestamp timestamp, const trace_api::KeyValueIterable &attributes) noexcept = 0; + /** + * Add a link to a span. + * @param trace_id the trace id of the linked span + * @param span_id the span id of the linked span + * @param attributes the attributes associated with the link + */ + virtual void AddLink(opentelemetry::trace::TraceId trace_id, + opentelemetry::trace::SpanId span_id, + const trace_api::KeyValueIterable &attributes) noexcept = 0; + /** * Set the status of the span. * @param code the status code diff --git a/sdk/include/opentelemetry/sdk/trace/span_data.h b/sdk/include/opentelemetry/sdk/trace/span_data.h index 42bde1cf07..31afa7a1bc 100644 --- a/sdk/include/opentelemetry/sdk/trace/span_data.h +++ b/sdk/include/opentelemetry/sdk/trace/span_data.h @@ -202,6 +202,15 @@ class SpanData final : public Recordable // TODO: handle attributes } + void AddLink(opentelemetry::trace::TraceId trace_id, + opentelemetry::trace::SpanId span_id, + const opentelemetry::trace::KeyValueIterable &attributes) noexcept override + { + (void)trace_id; + (void)span_id; + (void)attributes; + } + void SetStatus(trace_api::CanonicalCode code, nostd::string_view description) noexcept override { status_code_ = code; From 777971e69e1b5297f327af61b184764d1eb0c50d Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Tue, 21 Jul 2020 19:58:01 +0000 Subject: [PATCH 4/6] Add code coverage for SpanData --- sdk/test/trace/span_data_test.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sdk/test/trace/span_data_test.cc b/sdk/test/trace/span_data_test.cc index ee30932812..9a596f2b38 100644 --- a/sdk/test/trace/span_data_test.cc +++ b/sdk/test/trace/span_data_test.cc @@ -1,8 +1,8 @@ #include "opentelemetry/sdk/trace/span_data.h" #include "opentelemetry/nostd/variant.h" +#include "opentelemetry/trace/key_value_iterable_view.h" #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/trace_id.h" -#include "opentelemetry/trace/key_value_iterable_view.h" #include @@ -40,7 +40,10 @@ TEST(SpanData, Set) data.SetStartTime(now); data.SetDuration(std::chrono::nanoseconds(1000000)); data.SetAttribute("attr1", 314159); - data.AddEvent("event1", now, opentelemetry::trace::KeyValueIterableView>({})); + data.AddEvent("event1", now, + opentelemetry::trace::KeyValueIterableView>({})); + data.AddLink(trace_id, span_id, + opentelemetry::trace::KeyValueIterableView>({})); ASSERT_EQ(data.GetTraceId(), trace_id); ASSERT_EQ(data.GetSpanId(), span_id); From e80348459196e3ea84476faa7b3ff7520119337f Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Tue, 21 Jul 2020 23:10:57 +0000 Subject: [PATCH 5/6] Address review comments: add default parameters and update AddLink --- .../opentelemetry/exporters/otlp/recordable.h | 17 ++++++++------ exporters/otlp/src/recordable.cc | 6 ++--- .../opentelemetry/sdk/trace/recordable.h | 22 ++++++++++++------- .../opentelemetry/sdk/trace/span_data.h | 18 ++++++++------- sdk/test/trace/span_data_test.cc | 5 +---- 5 files changed, 37 insertions(+), 31 deletions(-) diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/recordable.h b/exporters/otlp/include/opentelemetry/exporters/otlp/recordable.h index 06ac93e139..57519bad01 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/recordable.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/recordable.h @@ -21,13 +21,16 @@ class Recordable final : public sdk::trace::Recordable void SetAttribute(nostd::string_view key, const opentelemetry::common::AttributeValue &value) noexcept override; - void AddEvent(nostd::string_view name, - core::SystemTimestamp timestamp, - const trace::KeyValueIterable &attributes) noexcept override; - - void AddLink(opentelemetry::trace::TraceId trace_id, - opentelemetry::trace::SpanId span_id, - const trace::KeyValueIterable &attributes) noexcept override; + void AddEvent( + nostd::string_view name, + core::SystemTimestamp timestamp = core::SystemTimestamp(std::chrono::system_clock::now()), + const trace::KeyValueIterable &attributes = + trace::KeyValueIterableView>({})) noexcept override; + + void AddLink( + opentelemetry::trace::SpanContext span_context, + const trace::KeyValueIterable &attributes = + trace::KeyValueIterableView>({})) noexcept override; void SetStatus(trace::CanonicalCode code, nostd::string_view description) noexcept override; diff --git a/exporters/otlp/src/recordable.cc b/exporters/otlp/src/recordable.cc index 65697b5b5c..c721313ce6 100644 --- a/exporters/otlp/src/recordable.cc +++ b/exporters/otlp/src/recordable.cc @@ -121,12 +121,10 @@ void Recordable::AddEvent(nostd::string_view name, // TODO: handle attributes } -void Recordable::AddLink(opentelemetry::trace::TraceId trace_id, - opentelemetry::trace::SpanId span_id, +void Recordable::AddLink(opentelemetry::trace::SpanContext span_context, const trace::KeyValueIterable &attributes) noexcept { - (void)trace_id; - (void)span_id; + (void)span_context; (void)attributes; } diff --git a/sdk/include/opentelemetry/sdk/trace/recordable.h b/sdk/include/opentelemetry/sdk/trace/recordable.h index 108ac688a9..489bfd5a03 100644 --- a/sdk/include/opentelemetry/sdk/trace/recordable.h +++ b/sdk/include/opentelemetry/sdk/trace/recordable.h @@ -5,10 +5,14 @@ #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/trace/canonical_code.h" #include "opentelemetry/trace/key_value_iterable.h" +#include "opentelemetry/trace/key_value_iterable_view.h" +#include "opentelemetry/trace/span_context.h" #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/trace_id.h" #include "opentelemetry/version.h" +#include + OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { @@ -49,19 +53,21 @@ class Recordable * @param timestamp the timestamp of the event * @param attributes the attributes associated with the event */ - virtual void AddEvent(nostd::string_view name, - core::SystemTimestamp timestamp, - const trace_api::KeyValueIterable &attributes) noexcept = 0; + virtual void AddEvent( + nostd::string_view name, + core::SystemTimestamp timestamp = core::SystemTimestamp(std::chrono::system_clock::now()), + const trace_api::KeyValueIterable &attributes = + trace_api::KeyValueIterableView>({})) noexcept = 0; /** * Add a link to a span. - * @param trace_id the trace id of the linked span - * @param span_id the span id of the linked span + * @param span_context the span context of the linked span * @param attributes the attributes associated with the link */ - virtual void AddLink(opentelemetry::trace::TraceId trace_id, - opentelemetry::trace::SpanId span_id, - const trace_api::KeyValueIterable &attributes) noexcept = 0; + virtual void AddLink( + opentelemetry::trace::SpanContext span_context, + const trace_api::KeyValueIterable &attributes = + trace_api::KeyValueIterableView>({})) noexcept = 0; /** * Set the status of the span. diff --git a/sdk/include/opentelemetry/sdk/trace/span_data.h b/sdk/include/opentelemetry/sdk/trace/span_data.h index 31afa7a1bc..47477fcaf7 100644 --- a/sdk/include/opentelemetry/sdk/trace/span_data.h +++ b/sdk/include/opentelemetry/sdk/trace/span_data.h @@ -194,20 +194,22 @@ class SpanData final : public Recordable attributes_[std::string(key)] = nostd::visit(converter_, value); } - void AddEvent(nostd::string_view name, - core::SystemTimestamp timestamp, - const trace_api::KeyValueIterable &attributes) noexcept override + void AddEvent( + nostd::string_view name, + core::SystemTimestamp timestamp = core::SystemTimestamp(std::chrono::system_clock::now()), + const trace_api::KeyValueIterable &attributes = + trace_api::KeyValueIterableView>({})) noexcept override { events_.push_back(SpanDataEvent(std::string(name), timestamp)); // TODO: handle attributes } - void AddLink(opentelemetry::trace::TraceId trace_id, - opentelemetry::trace::SpanId span_id, - const opentelemetry::trace::KeyValueIterable &attributes) noexcept override + void AddLink( + opentelemetry::trace::SpanContext span_context, + const trace_api::KeyValueIterable &attributes = + trace_api::KeyValueIterableView>({})) noexcept override { - (void)trace_id; - (void)span_id; + (void)span_context; (void)attributes; } diff --git a/sdk/test/trace/span_data_test.cc b/sdk/test/trace/span_data_test.cc index 9a596f2b38..8399d3ce71 100644 --- a/sdk/test/trace/span_data_test.cc +++ b/sdk/test/trace/span_data_test.cc @@ -40,10 +40,7 @@ TEST(SpanData, Set) data.SetStartTime(now); data.SetDuration(std::chrono::nanoseconds(1000000)); data.SetAttribute("attr1", 314159); - data.AddEvent("event1", now, - opentelemetry::trace::KeyValueIterableView>({})); - data.AddLink(trace_id, span_id, - opentelemetry::trace::KeyValueIterableView>({})); + data.AddEvent("event1", now); ASSERT_EQ(data.GetTraceId(), trace_id); ASSERT_EQ(data.GetSpanId(), span_id); From 961b59c0fe14e2009fc85d1a59183f82aa3bc68e Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Tue, 21 Jul 2020 23:22:45 +0000 Subject: [PATCH 6/6] Remove test changes --- exporters/otlp/test/recordable_test.cc | 3 +-- sdk/test/trace/span_data_test.cc | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/exporters/otlp/test/recordable_test.cc b/exporters/otlp/test/recordable_test.cc index dc1606a19c..ec1662662d 100644 --- a/exporters/otlp/test/recordable_test.cc +++ b/exporters/otlp/test/recordable_test.cc @@ -1,5 +1,4 @@ #include "opentelemetry/exporters/otlp/recordable.h" -#include "opentelemetry/trace/key_value_iterable_view.h" #include @@ -87,7 +86,7 @@ TEST(Recordable, AddEvents) std::chrono::system_clock::time_point event_time = std::chrono::system_clock::now(); core::SystemTimestamp event_timestamp(event_time); - rec.AddEvent(name, event_timestamp, trace::KeyValueIterableView>({})); + rec.AddEvent(name, event_timestamp); uint64_t unix_event_time = std::chrono::duration_cast(event_time.time_since_epoch()).count(); diff --git a/sdk/test/trace/span_data_test.cc b/sdk/test/trace/span_data_test.cc index 8399d3ce71..41d7b2c670 100644 --- a/sdk/test/trace/span_data_test.cc +++ b/sdk/test/trace/span_data_test.cc @@ -1,6 +1,5 @@ #include "opentelemetry/sdk/trace/span_data.h" #include "opentelemetry/nostd/variant.h" -#include "opentelemetry/trace/key_value_iterable_view.h" #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/trace_id.h"