From 39e26d28f58979d395363bd2eff1e94e5f935690 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 15 Jun 2020 17:53:23 -0700 Subject: [PATCH 01/18] Add SetAttribute method --- api/include/opentelemetry/plugin/tracer.h | 5 +++++ api/include/opentelemetry/trace/noop.h | 4 ++++ api/include/opentelemetry/trace/span.h | 7 ++----- sdk/include/opentelemetry/sdk/trace/recordable.h | 9 +++++++++ sdk/include/opentelemetry/sdk/trace/span_data.h | 6 ++++++ sdk/src/trace/span.cc | 6 ++++++ sdk/src/trace/span.h | 1 + 7 files changed, 33 insertions(+), 5 deletions(-) diff --git a/api/include/opentelemetry/plugin/tracer.h b/api/include/opentelemetry/plugin/tracer.h index 3d308e5826..a49a86f3b5 100644 --- a/api/include/opentelemetry/plugin/tracer.h +++ b/api/include/opentelemetry/plugin/tracer.h @@ -18,6 +18,11 @@ class Span final : public trace::Span {} // trace::Span + void SetAttribute(nostd::string_view name, common::AttributeValue &&value) noexcept override + { + span_->SetAttribute(name, std::move(value)); + } + void AddEvent(nostd::string_view name) noexcept override { span_->AddEvent(name); } void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept override diff --git a/api/include/opentelemetry/trace/noop.h b/api/include/opentelemetry/trace/noop.h index 50dcc8b558..59768bb263 100644 --- a/api/include/opentelemetry/trace/noop.h +++ b/api/include/opentelemetry/trace/noop.h @@ -24,6 +24,10 @@ class NoopSpan final : public Span public: explicit NoopSpan(const std::shared_ptr &tracer) noexcept : tracer_{tracer} {} + void SetAttribute(nostd::string_view /*key*/, + common::AttributeValue && /*value*/) noexcept override + {} + void AddEvent(nostd::string_view /*name*/) noexcept override {} void AddEvent(nostd::string_view /*name*/, core::SystemTimestamp /*timestamp*/) noexcept override diff --git a/api/include/opentelemetry/trace/span.h b/api/include/opentelemetry/trace/span.h index 21ed1aaaff..b53b40b305 100644 --- a/api/include/opentelemetry/trace/span.h +++ b/api/include/opentelemetry/trace/span.h @@ -2,6 +2,7 @@ #include +#include "opentelemetry/common/attribute_value.h" #include "opentelemetry/core/timestamp.h" #include "opentelemetry/nostd/span.h" #include "opentelemetry/nostd/string_view.h" @@ -74,13 +75,9 @@ class Span Span &operator=(const Span &) = delete; Span &operator=(Span &&) = delete; - // TODO // Sets an attribute on the Span. If the Span previously contained a mapping for // the key, the old value is replaced. - // - // If an empty string is used as the value, the attribute will be silently - // dropped. Note: this behavior could change in the future. - // virtual void SetAttribute(nostd::string_view key, AttributeValue&& value) = 0; + virtual void SetAttribute(nostd::string_view key, common::AttributeValue &&value) noexcept = 0; // Adds an event to the Span. virtual void AddEvent(nostd::string_view name) noexcept = 0; diff --git a/sdk/include/opentelemetry/sdk/trace/recordable.h b/sdk/include/opentelemetry/sdk/trace/recordable.h index c102958fe3..e48939903f 100644 --- a/sdk/include/opentelemetry/sdk/trace/recordable.h +++ b/sdk/include/opentelemetry/sdk/trace/recordable.h @@ -1,5 +1,6 @@ #pragma once +#include "opentelemetry/common/attribute_value.h" #include "opentelemetry/core/timestamp.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/trace/canonical_code.h" @@ -33,6 +34,14 @@ class Recordable opentelemetry::trace::SpanId span_id, opentelemetry::trace::SpanId parent_span_id) noexcept = 0; + /** + * Add an attribute to a span. + * @param name the name of the attribute + * @param value the attribute value + */ + virtual void SetAttribute(nostd::string_view key, + opentelemetry::common::AttributeValue &&value) noexcept = 0; + /** * Add an event to a span. * @param name the name of the event diff --git a/sdk/include/opentelemetry/sdk/trace/span_data.h b/sdk/include/opentelemetry/sdk/trace/span_data.h index 8ac620ad1b..8b9f5f7e77 100644 --- a/sdk/include/opentelemetry/sdk/trace/span_data.h +++ b/sdk/include/opentelemetry/sdk/trace/span_data.h @@ -76,6 +76,12 @@ class SpanData final : public Recordable parent_span_id_ = parent_span_id; } + void SetAttribute(nostd::string_view key, common::AttributeValue &&value) noexcept override + { + (void)key; + (void)value; + } + void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept override { (void)name; diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index a69e6673b0..ba5a281b64 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -64,6 +64,12 @@ Span::~Span() End(); } +void Span::SetAttribute(nostd::string_view key, common::AttributeValue &&value) noexcept +{ + (void)key; + (void)value; +} + void Span::AddEvent(nostd::string_view name) noexcept { (void)name; diff --git a/sdk/src/trace/span.h b/sdk/src/trace/span.h index e1a6f2084a..22e5486234 100644 --- a/sdk/src/trace/span.h +++ b/sdk/src/trace/span.h @@ -23,6 +23,7 @@ class Span final : public trace_api::Span ~Span() override; // trace_api::Span + void SetAttribute(nostd::string_view key, common::AttributeValue &&value) noexcept override; void AddEvent(nostd::string_view name) noexcept override; void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept override; From f9d8c7e4006256f07509643acffe0989914d855c Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 15 Jun 2020 18:17:46 -0700 Subject: [PATCH 02/18] Add to span data --- sdk/include/opentelemetry/sdk/trace/span_data.h | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/span_data.h b/sdk/include/opentelemetry/sdk/trace/span_data.h index 8b9f5f7e77..6e18c2dd26 100644 --- a/sdk/include/opentelemetry/sdk/trace/span_data.h +++ b/sdk/include/opentelemetry/sdk/trace/span_data.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include "opentelemetry/core/timestamp.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/trace/recordable.h" @@ -67,6 +68,15 @@ class SpanData final : public Recordable */ std::chrono::nanoseconds GetDuration() const noexcept { return duration_; } + /** + * Get the duration for this span + * @return the duration for this span + */ + const std::map& GetAttributes() const noexcept + { + return attributes_; + } + void SetIds(opentelemetry::trace::TraceId trace_id, opentelemetry::trace::SpanId span_id, opentelemetry::trace::SpanId parent_span_id) noexcept override @@ -78,8 +88,7 @@ class SpanData final : public Recordable void SetAttribute(nostd::string_view key, common::AttributeValue &&value) noexcept override { - (void)key; - (void)value; + attributes_[std::string(key)] = value; } void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept override @@ -112,6 +121,7 @@ class SpanData final : public Recordable std::string name_; opentelemetry::trace::CanonicalCode status_code_{opentelemetry::trace::CanonicalCode::OK}; std::string status_desc_; + std::map attributes_; }; } // namespace trace } // namespace sdk From 3de6cbffa7f1e2ecfe0f2de2af016ecb4ea11356 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Tue, 16 Jun 2020 11:36:20 -0700 Subject: [PATCH 03/18] Add SDK support for attributes --- sdk/include/opentelemetry/sdk/trace/span_data.h | 10 +++++----- sdk/src/trace/span.cc | 3 +-- sdk/src/trace/span.h | 1 + sdk/test/trace/span_data_test.cc | 4 ++++ 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/span_data.h b/sdk/include/opentelemetry/sdk/trace/span_data.h index 6e18c2dd26..e8ed974bf5 100644 --- a/sdk/include/opentelemetry/sdk/trace/span_data.h +++ b/sdk/include/opentelemetry/sdk/trace/span_data.h @@ -1,7 +1,7 @@ #pragma once #include -#include +#include #include "opentelemetry/core/timestamp.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/trace/recordable.h" @@ -69,10 +69,10 @@ class SpanData final : public Recordable std::chrono::nanoseconds GetDuration() const noexcept { return duration_; } /** - * Get the duration for this span - * @return the duration for this span + * Get the attributes for this span + * @return the attributes for this span */ - const std::map& GetAttributes() const noexcept + const std::unordered_map& GetAttributes() const noexcept { return attributes_; } @@ -121,7 +121,7 @@ class SpanData final : public Recordable std::string name_; opentelemetry::trace::CanonicalCode status_code_{opentelemetry::trace::CanonicalCode::OK}; std::string status_desc_; - std::map attributes_; + std::unordered_map attributes_; }; } // namespace trace } // namespace sdk diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index ba5a281b64..e774f7280d 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -66,8 +66,7 @@ Span::~Span() void Span::SetAttribute(nostd::string_view key, common::AttributeValue &&value) noexcept { - (void)key; - (void)value; + recordable_->SetAttribute(key, std::move(value)); } void Span::AddEvent(nostd::string_view name) noexcept diff --git a/sdk/src/trace/span.h b/sdk/src/trace/span.h index 22e5486234..bbbf2032cb 100644 --- a/sdk/src/trace/span.h +++ b/sdk/src/trace/span.h @@ -24,6 +24,7 @@ class Span final : public trace_api::Span // trace_api::Span void SetAttribute(nostd::string_view key, common::AttributeValue &&value) noexcept override; + void AddEvent(nostd::string_view name) noexcept override; void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept override; diff --git a/sdk/test/trace/span_data_test.cc b/sdk/test/trace/span_data_test.cc index ea8aab446c..6176042c03 100644 --- a/sdk/test/trace/span_data_test.cc +++ b/sdk/test/trace/span_data_test.cc @@ -1,3 +1,4 @@ +#include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/trace/span_data.h" #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/trace_id.h" @@ -20,6 +21,7 @@ TEST(SpanData, DefaultValues) ASSERT_EQ(data.GetDescription(), ""); ASSERT_EQ(data.GetStartTime().time_since_epoch(), std::chrono::nanoseconds(0)); ASSERT_EQ(data.GetDuration(), std::chrono::nanoseconds(0)); + ASSERT_EQ(data.GetAttributes().size(), 0); } TEST(SpanData, Set) @@ -35,6 +37,7 @@ TEST(SpanData, Set) data.SetStatus(opentelemetry::trace::CanonicalCode::UNKNOWN, "description"); data.SetStartTime(now); data.SetDuration(std::chrono::nanoseconds(1000000)); + data.SetAttribute("attr1", 314159); data.AddEvent("event1", now); ASSERT_EQ(data.GetTraceId(), trace_id); @@ -45,4 +48,5 @@ TEST(SpanData, Set) 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); } From c300da60aac5379bc2f91fb94b2a6bce02819b57 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Tue, 16 Jun 2020 13:33:12 -0700 Subject: [PATCH 04/18] Add tests --- api/include/opentelemetry/trace/span.h | 1 + sdk/src/trace/span.cc | 5 +++++ sdk/test/trace/tracer_test.cc | 27 +++++++++++++++++++++++++- 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/api/include/opentelemetry/trace/span.h b/api/include/opentelemetry/trace/span.h index b53b40b305..069592e63d 100644 --- a/api/include/opentelemetry/trace/span.h +++ b/api/include/opentelemetry/trace/span.h @@ -42,6 +42,7 @@ struct StartSpanOptions // SpanContext remote_parent; // Links // Attributes + nostd::span> attributes; SpanKind kind = SpanKind::kInternal; }; /** diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index e774f7280d..a9c6275ec7 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -55,6 +55,11 @@ Span::Span(std::shared_ptr &&tracer, processor_->OnStart(*recordable_); recordable_->SetName(name); + for (auto& attr : options.attributes) { + auto value = attr.second; + recordable_->SetAttribute(attr.first, std::move(value)); + } + recordable_->SetStartTime(NowOr(options.start_system_time)); start_steady_time = NowOr(options.start_steady_time); } diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index e0bc2afde6..a6ac26aa5a 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -91,7 +91,7 @@ TEST(Tracer, StartSpan) ASSERT_LT(std::chrono::nanoseconds(0), span_data->GetDuration()); } -TEST(Tracer, StartSpanWithOptions) +TEST(Tracer, StartSpanWithOptionsTime) { std::shared_ptr>> spans_received( new std::vector>); @@ -112,3 +112,28 @@ TEST(Tracer, StartSpanWithOptions) ASSERT_EQ(std::chrono::nanoseconds(300), span_data->GetStartTime().time_since_epoch()); ASSERT_EQ(std::chrono::nanoseconds(30), span_data->GetDuration()); } + +TEST(Tracer, StartSpanWithOptionsAttributes) +{ + std::shared_ptr>> spans_received( + new std::vector>); + auto tracer = initTracer(spans_received); + + opentelemetry::trace::StartSpanOptions start; + std::pair attributes[] = { + {"attr1", 314159}, + {"attr2", false}, + {"attr3", "hi"} + }; + start.attributes = attributes; + + tracer->StartSpan("span 1", start)->End(); + + ASSERT_EQ(1, spans_received->size()); + + auto &span_data = spans_received->at(0); + ASSERT_EQ(3, span_data->GetAttributes().size()); + ASSERT_EQ(314159, opentelemetry::nostd::get(span_data->GetAttributes().at("attr1"))); + ASSERT_EQ(false, opentelemetry::nostd::get(span_data->GetAttributes().at("attr2"))); + ASSERT_EQ("hi", opentelemetry::nostd::get(span_data->GetAttributes().at("attr3"))); +} From f07d893914d4a141eaf1fa28c90ddad993fc7897 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Tue, 16 Jun 2020 14:33:45 -0700 Subject: [PATCH 05/18] Set attributes on span start --- api/include/opentelemetry/trace/span.h | 5 +++-- examples/plugin/plugin/tracer.cc | 11 ++++++++--- .../opentelemetry/sdk/trace/span_data.h | 4 ++-- sdk/src/trace/span.cc | 7 ++++--- sdk/test/trace/span_data_test.cc | 2 +- sdk/test/trace/tracer_test.cc | 18 ++++++++---------- 6 files changed, 26 insertions(+), 21 deletions(-) diff --git a/api/include/opentelemetry/trace/span.h b/api/include/opentelemetry/trace/span.h index 069592e63d..8ecf192c56 100644 --- a/api/include/opentelemetry/trace/span.h +++ b/api/include/opentelemetry/trace/span.h @@ -37,12 +37,13 @@ struct StartSpanOptions core::SystemTimestamp start_system_time; core::SteadyTimestamp start_steady_time; + // Optionally set attributes at Span creation. + nostd::span> attributes; + // TODO: // Span(Context?) parent; // SpanContext remote_parent; // Links - // Attributes - nostd::span> attributes; SpanKind kind = SpanKind::kInternal; }; /** diff --git a/examples/plugin/plugin/tracer.cc b/examples/plugin/plugin/tracer.cc index c6e646d5d0..f62cb1fa4b 100644 --- a/examples/plugin/plugin/tracer.cc +++ b/examples/plugin/plugin/tracer.cc @@ -2,9 +2,10 @@ #include -namespace nostd = opentelemetry::nostd; -namespace core = opentelemetry::core; -namespace trace = opentelemetry::trace; +namespace nostd = opentelemetry::nostd; +namespace common = opentelemetry::common; +namespace core = opentelemetry::core; +namespace trace = opentelemetry::trace; namespace { @@ -22,6 +23,10 @@ class Span final : public trace::Span ~Span() { std::cout << "~Span\n"; } // opentelemetry::trace::Span + void SetAttribute(nostd::string_view /*name*/, + common::AttributeValue && /*value*/) noexcept override + {} + void AddEvent(nostd::string_view /*name*/) noexcept override {} void AddEvent(nostd::string_view /*name*/, core::SystemTimestamp /*timestamp*/) noexcept override diff --git a/sdk/include/opentelemetry/sdk/trace/span_data.h b/sdk/include/opentelemetry/sdk/trace/span_data.h index e8ed974bf5..5fab63c5ab 100644 --- a/sdk/include/opentelemetry/sdk/trace/span_data.h +++ b/sdk/include/opentelemetry/sdk/trace/span_data.h @@ -72,9 +72,9 @@ class SpanData final : public Recordable * Get the attributes for this span * @return the attributes for this span */ - const std::unordered_map& GetAttributes() const noexcept + const std::unordered_map &GetAttributes() const noexcept { - return attributes_; + return attributes_; } void SetIds(opentelemetry::trace::TraceId trace_id, diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index a9c6275ec7..923008c088 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -55,9 +55,10 @@ Span::Span(std::shared_ptr &&tracer, processor_->OnStart(*recordable_); recordable_->SetName(name); - for (auto& attr : options.attributes) { - auto value = attr.second; - recordable_->SetAttribute(attr.first, std::move(value)); + for (auto &attr : options.attributes) + { + auto value = attr.second; + recordable_->SetAttribute(attr.first, std::move(value)); } recordable_->SetStartTime(NowOr(options.start_system_time)); diff --git a/sdk/test/trace/span_data_test.cc b/sdk/test/trace/span_data_test.cc index 6176042c03..7bf6c34ec1 100644 --- a/sdk/test/trace/span_data_test.cc +++ b/sdk/test/trace/span_data_test.cc @@ -1,5 +1,5 @@ -#include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/trace/span_data.h" +#include "opentelemetry/nostd/variant.h" #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/trace_id.h" diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index a6ac26aa5a..227236f2f4 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -7,6 +7,8 @@ using namespace opentelemetry::sdk::trace; using opentelemetry::core::SteadyTimestamp; using opentelemetry::core::SystemTimestamp; +namespace nostd = opentelemetry::nostd; +namespace common = opentelemetry::common; /** * A mock exporter that switches a flag once a valid recordable was received. @@ -23,8 +25,7 @@ class MockSpanExporter final : public SpanExporter return std::unique_ptr(new SpanData); } - ExportResult Export( - const opentelemetry::nostd::span> &recordables) noexcept override + ExportResult Export(const nostd::span> &recordables) noexcept override { for (auto &recordable : recordables) { @@ -120,11 +121,8 @@ TEST(Tracer, StartSpanWithOptionsAttributes) auto tracer = initTracer(spans_received); opentelemetry::trace::StartSpanOptions start; - std::pair attributes[] = { - {"attr1", 314159}, - {"attr2", false}, - {"attr3", "hi"} - }; + std::pair attributes[] = { + {"attr1", 314159}, {"attr2", false}, {"attr3", "string"}}; start.attributes = attributes; tracer->StartSpan("span 1", start)->End(); @@ -133,7 +131,7 @@ TEST(Tracer, StartSpanWithOptionsAttributes) auto &span_data = spans_received->at(0); ASSERT_EQ(3, span_data->GetAttributes().size()); - ASSERT_EQ(314159, opentelemetry::nostd::get(span_data->GetAttributes().at("attr1"))); - ASSERT_EQ(false, opentelemetry::nostd::get(span_data->GetAttributes().at("attr2"))); - ASSERT_EQ("hi", opentelemetry::nostd::get(span_data->GetAttributes().at("attr3"))); + ASSERT_EQ(314159, nostd::get(span_data->GetAttributes().at("attr1"))); + ASSERT_EQ(false, nostd::get(span_data->GetAttributes().at("attr2"))); + ASSERT_EQ("string", nostd::get(span_data->GetAttributes().at("attr3"))); } From 08d24a2cbfde0264158391b82c437422756bed03 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Tue, 16 Jun 2020 15:20:03 -0700 Subject: [PATCH 06/18] Fix documentation --- api/include/opentelemetry/trace/span.h | 5 ++++- sdk/include/opentelemetry/sdk/trace/recordable.h | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/api/include/opentelemetry/trace/span.h b/api/include/opentelemetry/trace/span.h index 8ecf192c56..c12b8abb2a 100644 --- a/api/include/opentelemetry/trace/span.h +++ b/api/include/opentelemetry/trace/span.h @@ -37,7 +37,10 @@ struct StartSpanOptions core::SystemTimestamp start_system_time; core::SteadyTimestamp start_steady_time; - // Optionally set attributes at Span creation. + // Optionally set attributes at Span creation from the given key/value pairs.. + // + // Attributes will be processed in order, previous attributes with the same + // key will be overwritten. nostd::span> attributes; // TODO: diff --git a/sdk/include/opentelemetry/sdk/trace/recordable.h b/sdk/include/opentelemetry/sdk/trace/recordable.h index e48939903f..ebb698f9de 100644 --- a/sdk/include/opentelemetry/sdk/trace/recordable.h +++ b/sdk/include/opentelemetry/sdk/trace/recordable.h @@ -35,7 +35,7 @@ class Recordable opentelemetry::trace::SpanId parent_span_id) noexcept = 0; /** - * Add an attribute to a span. + * Set an attribute of a span. * @param name the name of the attribute * @param value the attribute value */ From d684dc6c4d1c10fdb717dc44775108f0bdee1a05 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Tue, 16 Jun 2020 15:24:42 -0700 Subject: [PATCH 07/18] Add test for overwriting attributes --- sdk/test/trace/tracer_test.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index 227236f2f4..f45344aea7 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -122,7 +122,7 @@ TEST(Tracer, StartSpanWithOptionsAttributes) opentelemetry::trace::StartSpanOptions start; std::pair attributes[] = { - {"attr1", 314159}, {"attr2", false}, {"attr3", "string"}}; + {"attr1", 314159}, {"attr2", false}, {"attr1", "string"}}; start.attributes = attributes; tracer->StartSpan("span 1", start)->End(); @@ -130,8 +130,7 @@ TEST(Tracer, StartSpanWithOptionsAttributes) ASSERT_EQ(1, spans_received->size()); auto &span_data = spans_received->at(0); - ASSERT_EQ(3, span_data->GetAttributes().size()); - ASSERT_EQ(314159, nostd::get(span_data->GetAttributes().at("attr1"))); + ASSERT_EQ(2, span_data->GetAttributes().size()); + ASSERT_EQ("string", nostd::get(span_data->GetAttributes().at("attr1"))); ASSERT_EQ(false, nostd::get(span_data->GetAttributes().at("attr2"))); - ASSERT_EQ("string", nostd::get(span_data->GetAttributes().at("attr3"))); } From f4d6661adbfc443f3e92031c1db02085d4ed3b97 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Tue, 16 Jun 2020 15:43:50 -0700 Subject: [PATCH 08/18] Update api/include/opentelemetry/trace/span.h Co-authored-by: Reiley Yang --- api/include/opentelemetry/trace/span.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/include/opentelemetry/trace/span.h b/api/include/opentelemetry/trace/span.h index c12b8abb2a..e706c66250 100644 --- a/api/include/opentelemetry/trace/span.h +++ b/api/include/opentelemetry/trace/span.h @@ -37,7 +37,7 @@ struct StartSpanOptions core::SystemTimestamp start_system_time; core::SteadyTimestamp start_steady_time; - // Optionally set attributes at Span creation from the given key/value pairs.. + // Optionally set attributes at Span creation from the given key/value pairs. // // Attributes will be processed in order, previous attributes with the same // key will be overwritten. From 852be31d89a6e0eec9c44793ddd5b8adee141f9f Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Tue, 16 Jun 2020 16:10:08 -0700 Subject: [PATCH 09/18] Add lock for setting attributes --- sdk/src/trace/span.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index 923008c088..4c660f533b 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -72,6 +72,8 @@ Span::~Span() void Span::SetAttribute(nostd::string_view key, common::AttributeValue &&value) noexcept { + std::lock_guard lock_guard{mu_}; + recordable_->SetAttribute(key, std::move(value)); } From 81b1f0323868eb2725862744db69dd8e6c39e3f8 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Wed, 17 Jun 2020 23:27:34 -0700 Subject: [PATCH 10/18] Use a struct for attribute key/value pairs --- api/include/opentelemetry/common/attribute_value.h | 8 ++++++++ api/include/opentelemetry/trace/span.h | 2 +- sdk/src/trace/span.cc | 4 ++-- sdk/test/trace/tracer_test.cc | 2 +- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/api/include/opentelemetry/common/attribute_value.h b/api/include/opentelemetry/common/attribute_value.h index 6340a4f97c..90a00ef14a 100644 --- a/api/include/opentelemetry/common/attribute_value.h +++ b/api/include/opentelemetry/common/attribute_value.h @@ -24,5 +24,13 @@ using AttributeValue = nostd::variant, nostd::span, nostd::span>; + +/** + * A key/value pair that can be used to set attributes. + */ +struct AttributeKeyValue { + nostd::string_view key; + AttributeValue value; +}; } // namespace common OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/trace/span.h b/api/include/opentelemetry/trace/span.h index e706c66250..d6cb5ddeb7 100644 --- a/api/include/opentelemetry/trace/span.h +++ b/api/include/opentelemetry/trace/span.h @@ -41,7 +41,7 @@ struct StartSpanOptions // // Attributes will be processed in order, previous attributes with the same // key will be overwritten. - nostd::span> attributes; + nostd::span attributes; // TODO: // Span(Context?) parent; diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index 4c660f533b..9f30a5c154 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -57,8 +57,8 @@ Span::Span(std::shared_ptr &&tracer, for (auto &attr : options.attributes) { - auto value = attr.second; - recordable_->SetAttribute(attr.first, std::move(value)); + auto value = attr.value; + recordable_->SetAttribute(attr.key, std::move(value)); } recordable_->SetStartTime(NowOr(options.start_system_time)); diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index f45344aea7..d8a8550d6f 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -121,7 +121,7 @@ TEST(Tracer, StartSpanWithOptionsAttributes) auto tracer = initTracer(spans_received); opentelemetry::trace::StartSpanOptions start; - std::pair attributes[] = { + common::AttributeKeyValue attributes[] = { {"attr1", 314159}, {"attr2", false}, {"attr1", "string"}}; start.attributes = attributes; From 9149aa12c8497abfd32dca42e253626acdc28ce8 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Fri, 19 Jun 2020 21:08:58 -0700 Subject: [PATCH 11/18] PR comments --- api/include/opentelemetry/common/attribute_value.h | 7 ++++--- sdk/src/trace/span.cc | 3 +-- sdk/test/trace/tracer_test.cc | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/api/include/opentelemetry/common/attribute_value.h b/api/include/opentelemetry/common/attribute_value.h index 90a00ef14a..8a6b0c1b39 100644 --- a/api/include/opentelemetry/common/attribute_value.h +++ b/api/include/opentelemetry/common/attribute_value.h @@ -28,9 +28,10 @@ using AttributeValue = nostd::variant &&tracer, for (auto &attr : options.attributes) { - auto value = attr.value; - recordable_->SetAttribute(attr.key, std::move(value)); + recordable_->SetAttribute(attr.key, std::move(attr.value)); } recordable_->SetStartTime(NowOr(options.start_system_time)); diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index d8a8550d6f..932bfafbfb 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -120,12 +120,12 @@ TEST(Tracer, StartSpanWithOptionsAttributes) new std::vector>); auto tracer = initTracer(spans_received); - opentelemetry::trace::StartSpanOptions start; + opentelemetry::trace::StartSpanOptions opts; common::AttributeKeyValue attributes[] = { {"attr1", 314159}, {"attr2", false}, {"attr1", "string"}}; - start.attributes = attributes; + opts.attributes = attributes; - tracer->StartSpan("span 1", start)->End(); + tracer->StartSpan("span 1", opts)->End(); ASSERT_EQ(1, spans_received->size()); From da1c5990e33ea8f085ed24d5de747093df1b6e49 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Sat, 20 Jun 2020 16:00:04 -0700 Subject: [PATCH 12/18] Attribute list --- api/include/opentelemetry/plugin/tracer.h | 3 ++- api/include/opentelemetry/trace/noop.h | 1 + api/include/opentelemetry/trace/span.h | 1 - api/include/opentelemetry/trace/tracer.h | 1 + examples/plugin/plugin/tracer.cc | 4 +++- examples/plugin/plugin/tracer.h | 3 ++- sdk/include/opentelemetry/sdk/trace/tracer.h | 1 + sdk/src/trace/span.cc | 3 ++- sdk/src/trace/span.h | 1 + sdk/src/trace/tracer.cc | 3 ++- sdk/test/trace/tracer_test.cc | 11 +++++------ 11 files changed, 20 insertions(+), 12 deletions(-) diff --git a/api/include/opentelemetry/plugin/tracer.h b/api/include/opentelemetry/plugin/tracer.h index a49a86f3b5..502a76e169 100644 --- a/api/include/opentelemetry/plugin/tracer.h +++ b/api/include/opentelemetry/plugin/tracer.h @@ -66,9 +66,10 @@ class Tracer final : public trace::Tracer, public std::enable_shared_from_this StartSpan( nostd::string_view name, + nostd::span attributes = {}, const trace::StartSpanOptions &options = {}) noexcept override { - auto span = tracer_handle_->tracer().StartSpan(name, options); + auto span = tracer_handle_->tracer().StartSpan(name, attributes, options); if (span == nullptr) { return nullptr; diff --git a/api/include/opentelemetry/trace/noop.h b/api/include/opentelemetry/trace/noop.h index 59768bb263..981b5540ff 100644 --- a/api/include/opentelemetry/trace/noop.h +++ b/api/include/opentelemetry/trace/noop.h @@ -60,6 +60,7 @@ class NoopTracer final : public Tracer, public std::enable_shared_from_this StartSpan(nostd::string_view /*name*/, + nostd::span /* attributes */, const StartSpanOptions & /*options*/) noexcept override { return nostd::unique_ptr{new (std::nothrow) NoopSpan{this->shared_from_this()}}; diff --git a/api/include/opentelemetry/trace/span.h b/api/include/opentelemetry/trace/span.h index d6cb5ddeb7..1ff8fe2999 100644 --- a/api/include/opentelemetry/trace/span.h +++ b/api/include/opentelemetry/trace/span.h @@ -41,7 +41,6 @@ struct StartSpanOptions // // Attributes will be processed in order, previous attributes with the same // key will be overwritten. - nostd::span attributes; // TODO: // Span(Context?) parent; diff --git a/api/include/opentelemetry/trace/tracer.h b/api/include/opentelemetry/trace/tracer.h index 29aa288123..c6a85994aa 100644 --- a/api/include/opentelemetry/trace/tracer.h +++ b/api/include/opentelemetry/trace/tracer.h @@ -24,6 +24,7 @@ class Tracer * Starts a span. */ virtual nostd::unique_ptr StartSpan(nostd::string_view name, + nostd::span attributes = {}, const StartSpanOptions &options = {}) noexcept = 0; /** diff --git a/examples/plugin/plugin/tracer.cc b/examples/plugin/plugin/tracer.cc index f62cb1fa4b..f6955a29a8 100644 --- a/examples/plugin/plugin/tracer.cc +++ b/examples/plugin/plugin/tracer.cc @@ -14,6 +14,7 @@ class Span final : public trace::Span public: Span(std::shared_ptr &&tracer, nostd::string_view name, + nostd::span /* attributes */, const trace::StartSpanOptions & /*options*/) noexcept : tracer_{std::move(tracer)}, name_{name} { @@ -58,8 +59,9 @@ class Span final : public trace::Span Tracer::Tracer(nostd::string_view /*output*/) {} nostd::unique_ptr Tracer::StartSpan(nostd::string_view name, + nostd::span attributes, const trace::StartSpanOptions &options) noexcept { return nostd::unique_ptr{ - new (std::nothrow) Span{this->shared_from_this(), name, options}}; + new (std::nothrow) Span{this->shared_from_this(), name, attributes, options}}; } diff --git a/examples/plugin/plugin/tracer.h b/examples/plugin/plugin/tracer.h index 4d653f32e9..de15c32a0b 100644 --- a/examples/plugin/plugin/tracer.h +++ b/examples/plugin/plugin/tracer.h @@ -13,7 +13,8 @@ class Tracer final : public opentelemetry::trace::Tracer, // opentelemetry::trace::Tracer opentelemetry::nostd::unique_ptr StartSpan( opentelemetry::nostd::string_view name, - const opentelemetry::trace::StartSpanOptions &options) noexcept override; + opentelemetry::nostd::span /* attributes */, + const opentelemetry::trace::StartSpanOptions & /*options */) noexcept override; void ForceFlushWithMicroseconds(uint64_t /*timeout*/) noexcept override {} diff --git a/sdk/include/opentelemetry/sdk/trace/tracer.h b/sdk/include/opentelemetry/sdk/trace/tracer.h index 06c83b1058..e096e6be3e 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer.h @@ -37,6 +37,7 @@ class Tracer final : public trace_api::Tracer, public std::enable_shared_from_th nostd::unique_ptr StartSpan( nostd::string_view name, + nostd::span attributes = {}, const trace_api::StartSpanOptions &options = {}) noexcept override; void ForceFlushWithMicroseconds(uint64_t timeout) noexcept override; diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index 7647adfee0..d9ef50d5b6 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -41,6 +41,7 @@ SteadyTimestamp NowOr(const SteadyTimestamp &steady) Span::Span(std::shared_ptr &&tracer, std::shared_ptr processor, nostd::string_view name, + nostd::span attributes, const trace_api::StartSpanOptions &options) noexcept : tracer_{std::move(tracer)}, processor_{processor}, @@ -55,7 +56,7 @@ Span::Span(std::shared_ptr &&tracer, processor_->OnStart(*recordable_); recordable_->SetName(name); - for (auto &attr : options.attributes) + for (auto &attr : attributes) { recordable_->SetAttribute(attr.key, std::move(attr.value)); } diff --git a/sdk/src/trace/span.h b/sdk/src/trace/span.h index bbbf2032cb..90cd673d4d 100644 --- a/sdk/src/trace/span.h +++ b/sdk/src/trace/span.h @@ -18,6 +18,7 @@ class Span final : public trace_api::Span explicit Span(std::shared_ptr &&tracer, std::shared_ptr processor, nostd::string_view name, + nostd::span attributes, const trace_api::StartSpanOptions &options) noexcept; ~Span() override; diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index 15e89ceffd..a27ee3f1bf 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -21,10 +21,11 @@ std::shared_ptr Tracer::GetProcessor() const noexcept nostd::unique_ptr Tracer::StartSpan( nostd::string_view name, + nostd::span attributes, const trace_api::StartSpanOptions &options) noexcept { return nostd::unique_ptr{ - new (std::nothrow) Span{this->shared_from_this(), processor_.load(), name, options}}; + new (std::nothrow) Span{this->shared_from_this(), processor_.load(), name, attributes, options}}; } void Tracer::ForceFlushWithMicroseconds(uint64_t timeout) noexcept diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index 932bfafbfb..ec58671f12 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -105,7 +105,7 @@ TEST(Tracer, StartSpanWithOptionsTime) opentelemetry::trace::EndSpanOptions end; end.end_steady_time = SteadyTimestamp(std::chrono::nanoseconds(40)); - tracer->StartSpan("span 1", start)->End(end); + tracer->StartSpan("span 1", {}, start)->End(end); ASSERT_EQ(1, spans_received->size()); @@ -120,12 +120,11 @@ TEST(Tracer, StartSpanWithOptionsAttributes) new std::vector>); auto tracer = initTracer(spans_received); - opentelemetry::trace::StartSpanOptions opts; - common::AttributeKeyValue attributes[] = { - {"attr1", 314159}, {"attr2", false}, {"attr1", "string"}}; - opts.attributes = attributes; - tracer->StartSpan("span 1", opts)->End(); + { + common::AttributeKeyValue attrs[] = {{"attr1", 314159}, {"attr2", false}, {"attr1", "string"}}; + tracer->StartSpan("span 1", attrs); + } ASSERT_EQ(1, spans_received->size()); From bfc7ae06bb7c570847f366180140d233d87c238d Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Sun, 21 Jun 2020 12:46:14 -0700 Subject: [PATCH 13/18] Const --- api/include/opentelemetry/plugin/tracer.h | 4 ++-- api/include/opentelemetry/trace/noop.h | 4 ++-- api/include/opentelemetry/trace/span.h | 2 +- api/include/opentelemetry/trace/tracer.h | 2 +- examples/plugin/plugin/tracer.cc | 6 +++--- examples/plugin/plugin/tracer.h | 2 +- sdk/include/opentelemetry/sdk/trace/recordable.h | 2 +- sdk/include/opentelemetry/sdk/trace/span_data.h | 2 +- sdk/include/opentelemetry/sdk/trace/tracer.h | 2 +- sdk/src/trace/span.cc | 4 ++-- sdk/src/trace/span.h | 4 ++-- sdk/src/trace/tracer.cc | 2 +- 12 files changed, 18 insertions(+), 18 deletions(-) diff --git a/api/include/opentelemetry/plugin/tracer.h b/api/include/opentelemetry/plugin/tracer.h index 502a76e169..b519706204 100644 --- a/api/include/opentelemetry/plugin/tracer.h +++ b/api/include/opentelemetry/plugin/tracer.h @@ -18,7 +18,7 @@ class Span final : public trace::Span {} // trace::Span - void SetAttribute(nostd::string_view name, common::AttributeValue &&value) noexcept override + void SetAttribute(nostd::string_view name, const common::AttributeValue &&value) noexcept override { span_->SetAttribute(name, std::move(value)); } @@ -66,7 +66,7 @@ class Tracer final : public trace::Tracer, public std::enable_shared_from_this StartSpan( nostd::string_view name, - nostd::span attributes = {}, + nostd::span attributes = {}, const trace::StartSpanOptions &options = {}) noexcept override { auto span = tracer_handle_->tracer().StartSpan(name, attributes, options); diff --git a/api/include/opentelemetry/trace/noop.h b/api/include/opentelemetry/trace/noop.h index 981b5540ff..0bc9f6ad81 100644 --- a/api/include/opentelemetry/trace/noop.h +++ b/api/include/opentelemetry/trace/noop.h @@ -25,7 +25,7 @@ class NoopSpan final : public Span explicit NoopSpan(const std::shared_ptr &tracer) noexcept : tracer_{tracer} {} void SetAttribute(nostd::string_view /*key*/, - common::AttributeValue && /*value*/) noexcept override + const common::AttributeValue && /*value*/) noexcept override {} void AddEvent(nostd::string_view /*name*/) noexcept override {} @@ -60,7 +60,7 @@ class NoopTracer final : public Tracer, public std::enable_shared_from_this StartSpan(nostd::string_view /*name*/, - nostd::span /* attributes */, + nostd::span /* attributes */, const StartSpanOptions & /*options*/) noexcept override { return nostd::unique_ptr{new (std::nothrow) NoopSpan{this->shared_from_this()}}; diff --git a/api/include/opentelemetry/trace/span.h b/api/include/opentelemetry/trace/span.h index 1ff8fe2999..e6540cd43e 100644 --- a/api/include/opentelemetry/trace/span.h +++ b/api/include/opentelemetry/trace/span.h @@ -81,7 +81,7 @@ class Span // Sets an attribute on the Span. If the Span previously contained a mapping for // the key, the old value is replaced. - virtual void SetAttribute(nostd::string_view key, common::AttributeValue &&value) noexcept = 0; + virtual void SetAttribute(nostd::string_view key, const common::AttributeValue &&value) noexcept = 0; // Adds an event to the Span. virtual void AddEvent(nostd::string_view name) noexcept = 0; diff --git a/api/include/opentelemetry/trace/tracer.h b/api/include/opentelemetry/trace/tracer.h index c6a85994aa..34230f1e25 100644 --- a/api/include/opentelemetry/trace/tracer.h +++ b/api/include/opentelemetry/trace/tracer.h @@ -24,7 +24,7 @@ class Tracer * Starts a span. */ virtual nostd::unique_ptr StartSpan(nostd::string_view name, - nostd::span attributes = {}, + nostd::span attributes = {}, const StartSpanOptions &options = {}) noexcept = 0; /** diff --git a/examples/plugin/plugin/tracer.cc b/examples/plugin/plugin/tracer.cc index f6955a29a8..2230f60d72 100644 --- a/examples/plugin/plugin/tracer.cc +++ b/examples/plugin/plugin/tracer.cc @@ -14,7 +14,7 @@ class Span final : public trace::Span public: Span(std::shared_ptr &&tracer, nostd::string_view name, - nostd::span /* attributes */, + nostd::span /* attributes */, const trace::StartSpanOptions & /*options*/) noexcept : tracer_{std::move(tracer)}, name_{name} { @@ -25,7 +25,7 @@ class Span final : public trace::Span // opentelemetry::trace::Span void SetAttribute(nostd::string_view /*name*/, - common::AttributeValue && /*value*/) noexcept override + const common::AttributeValue && /*value*/) noexcept override {} void AddEvent(nostd::string_view /*name*/) noexcept override {} @@ -59,7 +59,7 @@ class Span final : public trace::Span Tracer::Tracer(nostd::string_view /*output*/) {} nostd::unique_ptr Tracer::StartSpan(nostd::string_view name, - nostd::span attributes, + nostd::span attributes, const trace::StartSpanOptions &options) noexcept { return nostd::unique_ptr{ diff --git a/examples/plugin/plugin/tracer.h b/examples/plugin/plugin/tracer.h index de15c32a0b..07fdda9add 100644 --- a/examples/plugin/plugin/tracer.h +++ b/examples/plugin/plugin/tracer.h @@ -13,7 +13,7 @@ class Tracer final : public opentelemetry::trace::Tracer, // opentelemetry::trace::Tracer opentelemetry::nostd::unique_ptr StartSpan( opentelemetry::nostd::string_view name, - opentelemetry::nostd::span /* attributes */, + opentelemetry::nostd::span /* attributes */, const opentelemetry::trace::StartSpanOptions & /*options */) noexcept override; void ForceFlushWithMicroseconds(uint64_t /*timeout*/) noexcept override {} diff --git a/sdk/include/opentelemetry/sdk/trace/recordable.h b/sdk/include/opentelemetry/sdk/trace/recordable.h index ebb698f9de..d901dbfa54 100644 --- a/sdk/include/opentelemetry/sdk/trace/recordable.h +++ b/sdk/include/opentelemetry/sdk/trace/recordable.h @@ -40,7 +40,7 @@ class Recordable * @param value the attribute value */ virtual void SetAttribute(nostd::string_view key, - opentelemetry::common::AttributeValue &&value) noexcept = 0; + const opentelemetry::common::AttributeValue &&value) 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 5fab63c5ab..c3b557dc9b 100644 --- a/sdk/include/opentelemetry/sdk/trace/span_data.h +++ b/sdk/include/opentelemetry/sdk/trace/span_data.h @@ -86,7 +86,7 @@ class SpanData final : public Recordable parent_span_id_ = parent_span_id; } - void SetAttribute(nostd::string_view key, common::AttributeValue &&value) noexcept override + void SetAttribute(nostd::string_view key, const common::AttributeValue &&value) noexcept override { attributes_[std::string(key)] = value; } diff --git a/sdk/include/opentelemetry/sdk/trace/tracer.h b/sdk/include/opentelemetry/sdk/trace/tracer.h index e096e6be3e..10b44d9043 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer.h @@ -37,7 +37,7 @@ class Tracer final : public trace_api::Tracer, public std::enable_shared_from_th nostd::unique_ptr StartSpan( nostd::string_view name, - nostd::span attributes = {}, + nostd::span attributes = {}, const trace_api::StartSpanOptions &options = {}) noexcept override; void ForceFlushWithMicroseconds(uint64_t timeout) noexcept override; diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index d9ef50d5b6..05fe42b6de 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -41,7 +41,7 @@ SteadyTimestamp NowOr(const SteadyTimestamp &steady) Span::Span(std::shared_ptr &&tracer, std::shared_ptr processor, nostd::string_view name, - nostd::span attributes, + nostd::span attributes, const trace_api::StartSpanOptions &options) noexcept : tracer_{std::move(tracer)}, processor_{processor}, @@ -70,7 +70,7 @@ Span::~Span() End(); } -void Span::SetAttribute(nostd::string_view key, common::AttributeValue &&value) noexcept +void Span::SetAttribute(nostd::string_view key, const common::AttributeValue &&value) noexcept { std::lock_guard lock_guard{mu_}; diff --git a/sdk/src/trace/span.h b/sdk/src/trace/span.h index 90cd673d4d..e7ff256604 100644 --- a/sdk/src/trace/span.h +++ b/sdk/src/trace/span.h @@ -18,13 +18,13 @@ class Span final : public trace_api::Span explicit Span(std::shared_ptr &&tracer, std::shared_ptr processor, nostd::string_view name, - nostd::span attributes, + nostd::span attributes, const trace_api::StartSpanOptions &options) noexcept; ~Span() override; // trace_api::Span - void SetAttribute(nostd::string_view key, common::AttributeValue &&value) noexcept override; + void SetAttribute(nostd::string_view key, const common::AttributeValue &&value) noexcept override; void AddEvent(nostd::string_view name) noexcept override; diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index a27ee3f1bf..fb4673f912 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -21,7 +21,7 @@ std::shared_ptr Tracer::GetProcessor() const noexcept nostd::unique_ptr Tracer::StartSpan( nostd::string_view name, - nostd::span attributes, + nostd::span attributes, const trace_api::StartSpanOptions &options) noexcept { return nostd::unique_ptr{ From 0b952af4760f330a267400497dadc74c0586edb0 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Sun, 21 Jun 2020 19:40:20 -0700 Subject: [PATCH 14/18] Change to KeyValueIterable --- api/include/opentelemetry/plugin/factory.h | 4 +-- api/include/opentelemetry/plugin/tracer.h | 2 +- api/include/opentelemetry/trace/noop.h | 2 +- api/include/opentelemetry/trace/tracer.h | 23 +++++++++++++++- examples/plugin/load/main.cc | 1 + examples/plugin/plugin/tracer.cc | 4 +-- examples/plugin/plugin/tracer.h | 2 +- sdk/include/opentelemetry/sdk/trace/tracer.h | 2 +- sdk/src/trace/span.cc | 10 +++---- sdk/src/trace/span.h | 2 +- sdk/src/trace/tracer.cc | 2 +- sdk/test/trace/tracer_test.cc | 28 +++++++++++++++----- 12 files changed, 60 insertions(+), 22 deletions(-) diff --git a/api/include/opentelemetry/plugin/factory.h b/api/include/opentelemetry/plugin/factory.h index dd849335e7..60e8f5ab3b 100644 --- a/api/include/opentelemetry/plugin/factory.h +++ b/api/include/opentelemetry/plugin/factory.h @@ -36,7 +36,7 @@ class Factory final * @param error_message on failure this will contain an error message. * @return a Tracer on success or nullptr on failure. */ - std::shared_ptr MakeTracer(nostd::string_view tracer_config, + std::shared_ptr MakeTracer(nostd::string_view tracer_config, std::string &error_message) const noexcept { nostd::unique_ptr plugin_error_message; @@ -46,7 +46,7 @@ class Factory final detail::CopyErrorMessage(plugin_error_message.get(), error_message); return nullptr; } - return std::shared_ptr{new (std::nothrow) + return std::shared_ptr{new (std::nothrow) Tracer{library_handle_, std::move(tracer_handle)}}; } diff --git a/api/include/opentelemetry/plugin/tracer.h b/api/include/opentelemetry/plugin/tracer.h index b519706204..9970e5a533 100644 --- a/api/include/opentelemetry/plugin/tracer.h +++ b/api/include/opentelemetry/plugin/tracer.h @@ -66,7 +66,7 @@ class Tracer final : public trace::Tracer, public std::enable_shared_from_this StartSpan( nostd::string_view name, - nostd::span attributes = {}, + const trace::KeyValueIterable &attributes, const trace::StartSpanOptions &options = {}) noexcept override { auto span = tracer_handle_->tracer().StartSpan(name, attributes, options); diff --git a/api/include/opentelemetry/trace/noop.h b/api/include/opentelemetry/trace/noop.h index 0bc9f6ad81..423f40dca5 100644 --- a/api/include/opentelemetry/trace/noop.h +++ b/api/include/opentelemetry/trace/noop.h @@ -60,7 +60,7 @@ class NoopTracer final : public Tracer, public std::enable_shared_from_this StartSpan(nostd::string_view /*name*/, - nostd::span /* attributes */, + const KeyValueIterable & /*attributes*/, const StartSpanOptions & /*options*/) noexcept override { return nostd::unique_ptr{new (std::nothrow) NoopSpan{this->shared_from_this()}}; diff --git a/api/include/opentelemetry/trace/tracer.h b/api/include/opentelemetry/trace/tracer.h index 34230f1e25..8ea5359d13 100644 --- a/api/include/opentelemetry/trace/tracer.h +++ b/api/include/opentelemetry/trace/tracer.h @@ -24,9 +24,30 @@ class Tracer * Starts a span. */ virtual nostd::unique_ptr StartSpan(nostd::string_view name, - nostd::span attributes = {}, + const KeyValueIterable &attributes, const StartSpanOptions &options = {}) noexcept = 0; + nostd::unique_ptr StartSpan(nostd::string_view name, + const StartSpanOptions &options = {}) noexcept { + return this->StartSpan(name, {}, options); + } + + template ::value> * = nullptr> + nostd::unique_ptr StartSpan(nostd::string_view name, + const T &attributes, + const StartSpanOptions &options = {}) noexcept { + return this->StartSpan(name, KeyValueIterableView(attributes), options); + } + + nostd::unique_ptr StartSpan(nostd::string_view name, + std::initializer_list> + attributes, + const StartSpanOptions &options = {}) noexcept { + return this->StartSpan(name, nostd::span>{ + attributes.begin(), attributes.end()}, + options); + } + /** * Force any buffered spans to flush. * @param timeout to complete the flush diff --git a/examples/plugin/load/main.cc b/examples/plugin/load/main.cc index 3c6799fed0..71cb4626b3 100644 --- a/examples/plugin/load/main.cc +++ b/examples/plugin/load/main.cc @@ -1,4 +1,5 @@ #include "opentelemetry/plugin/dynamic_load.h" +#include "opentelemetry/trace/tracer.h" #include #include diff --git a/examples/plugin/plugin/tracer.cc b/examples/plugin/plugin/tracer.cc index 2230f60d72..51b2d85290 100644 --- a/examples/plugin/plugin/tracer.cc +++ b/examples/plugin/plugin/tracer.cc @@ -14,7 +14,7 @@ class Span final : public trace::Span public: Span(std::shared_ptr &&tracer, nostd::string_view name, - nostd::span /* attributes */, + const opentelemetry::trace::KeyValueIterable& /*attributes*/, const trace::StartSpanOptions & /*options*/) noexcept : tracer_{std::move(tracer)}, name_{name} { @@ -59,7 +59,7 @@ class Span final : public trace::Span Tracer::Tracer(nostd::string_view /*output*/) {} nostd::unique_ptr Tracer::StartSpan(nostd::string_view name, - nostd::span attributes, + const opentelemetry::trace::KeyValueIterable &attributes, const trace::StartSpanOptions &options) noexcept { return nostd::unique_ptr{ diff --git a/examples/plugin/plugin/tracer.h b/examples/plugin/plugin/tracer.h index 07fdda9add..c396cb83c0 100644 --- a/examples/plugin/plugin/tracer.h +++ b/examples/plugin/plugin/tracer.h @@ -13,7 +13,7 @@ class Tracer final : public opentelemetry::trace::Tracer, // opentelemetry::trace::Tracer opentelemetry::nostd::unique_ptr StartSpan( opentelemetry::nostd::string_view name, - opentelemetry::nostd::span /* attributes */, + const opentelemetry::trace::KeyValueIterable & /*attributes*/, const opentelemetry::trace::StartSpanOptions & /*options */) noexcept override; void ForceFlushWithMicroseconds(uint64_t /*timeout*/) noexcept override {} diff --git a/sdk/include/opentelemetry/sdk/trace/tracer.h b/sdk/include/opentelemetry/sdk/trace/tracer.h index 10b44d9043..c32751d83c 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer.h @@ -37,7 +37,7 @@ class Tracer final : public trace_api::Tracer, public std::enable_shared_from_th nostd::unique_ptr StartSpan( nostd::string_view name, - nostd::span attributes = {}, + const trace_api::KeyValueIterable &attributes, const trace_api::StartSpanOptions &options = {}) noexcept override; void ForceFlushWithMicroseconds(uint64_t timeout) noexcept override; diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index 05fe42b6de..630d77b89d 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -41,7 +41,7 @@ SteadyTimestamp NowOr(const SteadyTimestamp &steady) Span::Span(std::shared_ptr &&tracer, std::shared_ptr processor, nostd::string_view name, - nostd::span attributes, + const trace_api::KeyValueIterable &attributes, const trace_api::StartSpanOptions &options) noexcept : tracer_{std::move(tracer)}, processor_{processor}, @@ -56,10 +56,10 @@ Span::Span(std::shared_ptr &&tracer, processor_->OnStart(*recordable_); recordable_->SetName(name); - for (auto &attr : attributes) - { - recordable_->SetAttribute(attr.key, std::move(attr.value)); - } + attributes.ForEachKeyValue([&](nostd::string_view key, common::AttributeValue value) noexcept { + recordable_->SetAttribute(key, std::move(value)); + return true; + }); recordable_->SetStartTime(NowOr(options.start_system_time)); start_steady_time = NowOr(options.start_steady_time); diff --git a/sdk/src/trace/span.h b/sdk/src/trace/span.h index e7ff256604..0470b799d0 100644 --- a/sdk/src/trace/span.h +++ b/sdk/src/trace/span.h @@ -18,7 +18,7 @@ class Span final : public trace_api::Span explicit Span(std::shared_ptr &&tracer, std::shared_ptr processor, nostd::string_view name, - nostd::span attributes, + const trace_api::KeyValueIterable &attributes, const trace_api::StartSpanOptions &options) noexcept; ~Span() override; diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index fb4673f912..b83fb345b1 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -21,7 +21,7 @@ std::shared_ptr Tracer::GetProcessor() const noexcept nostd::unique_ptr Tracer::StartSpan( nostd::string_view name, - nostd::span attributes, + const trace_api::KeyValueIterable &attributes, const trace_api::StartSpanOptions &options) noexcept { return nostd::unique_ptr{ diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index ec58671f12..4aa00d0726 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -48,12 +48,12 @@ class MockSpanExporter final : public SpanExporter namespace { -std::shared_ptr initTracer( +std::shared_ptr initTracer( std::shared_ptr>> &received) { std::unique_ptr exporter(new MockSpanExporter(received)); std::shared_ptr processor(new SimpleSpanProcessor(std::move(exporter))); - return std::shared_ptr(new Tracer(processor)); + return std::shared_ptr(new Tracer(processor)); } } // namespace @@ -114,7 +114,7 @@ TEST(Tracer, StartSpanWithOptionsTime) ASSERT_EQ(std::chrono::nanoseconds(30), span_data->GetDuration()); } -TEST(Tracer, StartSpanWithOptionsAttributes) +TEST(Tracer, StartSpanWithAttributes) { std::shared_ptr>> spans_received( new std::vector>); @@ -122,14 +122,30 @@ TEST(Tracer, StartSpanWithOptionsAttributes) { - common::AttributeKeyValue attrs[] = {{"attr1", 314159}, {"attr2", false}, {"attr1", "string"}}; - tracer->StartSpan("span 1", attrs); + tracer->StartSpan("span 1", {{"attr1", 314159}, {"attr2", false}, {"attr1", "string"}}); + + std::map m; + m["attr3"] = 3.0; + tracer->StartSpan("span 2", m); + + char* local_value = new char[6]; + local_value[0] = '\0'; + tracer->StartSpan("span 3", {{"attr4", local_value}}); + delete local_value; } - ASSERT_EQ(1, spans_received->size()); + ASSERT_EQ(3, spans_received->size()); auto &span_data = spans_received->at(0); ASSERT_EQ(2, span_data->GetAttributes().size()); ASSERT_EQ("string", nostd::get(span_data->GetAttributes().at("attr1"))); ASSERT_EQ(false, nostd::get(span_data->GetAttributes().at("attr2"))); + + auto &span_data2 = spans_received->at(1); + ASSERT_EQ(1, span_data2->GetAttributes().size()); + ASSERT_EQ(3.0, nostd::get(span_data2->GetAttributes().at("attr3"))); + + auto &span_data3 = spans_received->at(2); + ASSERT_EQ(1, span_data2->GetAttributes().size()); + ASSERT_EQ("", nostd::get(span_data2->GetAttributes().at("attr4"))); } From d0a25f2c86dd487e5569cbf32d1b9d2270a5b20e Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 22 Jun 2020 08:55:49 -0700 Subject: [PATCH 15/18] Format --- api/include/opentelemetry/plugin/factory.h | 7 +++-- api/include/opentelemetry/trace/noop.h | 2 +- api/include/opentelemetry/trace/span.h | 3 +- api/include/opentelemetry/trace/tracer.h | 30 +++++++++++--------- examples/plugin/plugin/tracer.cc | 9 +++--- examples/plugin/plugin/tracer.h | 2 +- sdk/include/opentelemetry/sdk/trace/tracer.h | 2 +- sdk/src/trace/span.cc | 4 +-- sdk/src/trace/span.h | 2 +- sdk/src/trace/tracer.cc | 6 ++-- sdk/test/trace/tracer_test.cc | 17 ++++++----- 11 files changed, 45 insertions(+), 39 deletions(-) diff --git a/api/include/opentelemetry/plugin/factory.h b/api/include/opentelemetry/plugin/factory.h index 60e8f5ab3b..abec73867a 100644 --- a/api/include/opentelemetry/plugin/factory.h +++ b/api/include/opentelemetry/plugin/factory.h @@ -37,7 +37,8 @@ class Factory final * @return a Tracer on success or nullptr on failure. */ std::shared_ptr MakeTracer(nostd::string_view tracer_config, - std::string &error_message) const noexcept + std::string &error_message) const + noexcept { nostd::unique_ptr plugin_error_message; auto tracer_handle = factory_impl_->MakeTracerHandle(tracer_config, plugin_error_message); @@ -46,8 +47,8 @@ class Factory final detail::CopyErrorMessage(plugin_error_message.get(), error_message); return nullptr; } - return std::shared_ptr{new (std::nothrow) - Tracer{library_handle_, std::move(tracer_handle)}}; + return std::shared_ptr{ + new (std::nothrow) Tracer{library_handle_, std::move(tracer_handle)}}; } private: diff --git a/api/include/opentelemetry/trace/noop.h b/api/include/opentelemetry/trace/noop.h index 423f40dca5..fbb0e0cb68 100644 --- a/api/include/opentelemetry/trace/noop.h +++ b/api/include/opentelemetry/trace/noop.h @@ -60,7 +60,7 @@ class NoopTracer final : public Tracer, public std::enable_shared_from_this StartSpan(nostd::string_view /*name*/, - const KeyValueIterable & /*attributes*/, + const KeyValueIterable & /*attributes*/, const StartSpanOptions & /*options*/) noexcept override { return nostd::unique_ptr{new (std::nothrow) NoopSpan{this->shared_from_this()}}; diff --git a/api/include/opentelemetry/trace/span.h b/api/include/opentelemetry/trace/span.h index e6540cd43e..12b7bf5230 100644 --- a/api/include/opentelemetry/trace/span.h +++ b/api/include/opentelemetry/trace/span.h @@ -81,7 +81,8 @@ class Span // Sets an attribute on the Span. If the Span previously contained a mapping for // the key, the old value is replaced. - virtual void SetAttribute(nostd::string_view key, const common::AttributeValue &&value) noexcept = 0; + virtual void SetAttribute(nostd::string_view key, + const common::AttributeValue &&value) noexcept = 0; // Adds an event to the Span. virtual void AddEvent(nostd::string_view name) noexcept = 0; diff --git a/api/include/opentelemetry/trace/tracer.h b/api/include/opentelemetry/trace/tracer.h index 8ea5359d13..f488b8e613 100644 --- a/api/include/opentelemetry/trace/tracer.h +++ b/api/include/opentelemetry/trace/tracer.h @@ -24,28 +24,32 @@ class Tracer * Starts a span. */ virtual nostd::unique_ptr StartSpan(nostd::string_view name, - const KeyValueIterable &attributes, + const KeyValueIterable &attributes, const StartSpanOptions &options = {}) noexcept = 0; nostd::unique_ptr StartSpan(nostd::string_view name, - const StartSpanOptions &options = {}) noexcept { - return this->StartSpan(name, {}, options); + const StartSpanOptions &options = {}) noexcept + { + return this->StartSpan(name, {}, options); } template ::value> * = nullptr> nostd::unique_ptr StartSpan(nostd::string_view name, - const T &attributes, - const StartSpanOptions &options = {}) noexcept { - return this->StartSpan(name, KeyValueIterableView(attributes), options); + const T &attributes, + const StartSpanOptions &options = {}) noexcept + { + return this->StartSpan(name, KeyValueIterableView(attributes), options); } - nostd::unique_ptr StartSpan(nostd::string_view name, - std::initializer_list> - attributes, - const StartSpanOptions &options = {}) noexcept { - return this->StartSpan(name, nostd::span>{ - attributes.begin(), attributes.end()}, - options); + nostd::unique_ptr StartSpan( + nostd::string_view name, + std::initializer_list> attributes, + const StartSpanOptions &options = {}) noexcept + { + return this->StartSpan(name, + nostd::span>{ + attributes.begin(), attributes.end()}, + options); } /** diff --git a/examples/plugin/plugin/tracer.cc b/examples/plugin/plugin/tracer.cc index 51b2d85290..3aa68e8b15 100644 --- a/examples/plugin/plugin/tracer.cc +++ b/examples/plugin/plugin/tracer.cc @@ -14,7 +14,7 @@ class Span final : public trace::Span public: Span(std::shared_ptr &&tracer, nostd::string_view name, - const opentelemetry::trace::KeyValueIterable& /*attributes*/, + const opentelemetry::trace::KeyValueIterable & /*attributes*/, const trace::StartSpanOptions & /*options*/) noexcept : tracer_{std::move(tracer)}, name_{name} { @@ -58,9 +58,10 @@ class Span final : public trace::Span Tracer::Tracer(nostd::string_view /*output*/) {} -nostd::unique_ptr Tracer::StartSpan(nostd::string_view name, - const opentelemetry::trace::KeyValueIterable &attributes, - const trace::StartSpanOptions &options) noexcept +nostd::unique_ptr Tracer::StartSpan( + nostd::string_view name, + const opentelemetry::trace::KeyValueIterable &attributes, + const trace::StartSpanOptions &options) noexcept { return nostd::unique_ptr{ new (std::nothrow) Span{this->shared_from_this(), name, attributes, options}}; diff --git a/examples/plugin/plugin/tracer.h b/examples/plugin/plugin/tracer.h index c396cb83c0..ad1b98633d 100644 --- a/examples/plugin/plugin/tracer.h +++ b/examples/plugin/plugin/tracer.h @@ -13,7 +13,7 @@ class Tracer final : public opentelemetry::trace::Tracer, // opentelemetry::trace::Tracer opentelemetry::nostd::unique_ptr StartSpan( opentelemetry::nostd::string_view name, - const opentelemetry::trace::KeyValueIterable & /*attributes*/, + const opentelemetry::trace::KeyValueIterable & /*attributes*/, const opentelemetry::trace::StartSpanOptions & /*options */) noexcept override; void ForceFlushWithMicroseconds(uint64_t /*timeout*/) noexcept override {} diff --git a/sdk/include/opentelemetry/sdk/trace/tracer.h b/sdk/include/opentelemetry/sdk/trace/tracer.h index c32751d83c..577785878a 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer.h @@ -37,7 +37,7 @@ class Tracer final : public trace_api::Tracer, public std::enable_shared_from_th nostd::unique_ptr StartSpan( nostd::string_view name, - const trace_api::KeyValueIterable &attributes, + const trace_api::KeyValueIterable &attributes, const trace_api::StartSpanOptions &options = {}) noexcept override; void ForceFlushWithMicroseconds(uint64_t timeout) noexcept override; diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index 630d77b89d..dde8710ec2 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -41,7 +41,7 @@ SteadyTimestamp NowOr(const SteadyTimestamp &steady) Span::Span(std::shared_ptr &&tracer, std::shared_ptr processor, nostd::string_view name, - const trace_api::KeyValueIterable &attributes, + const trace_api::KeyValueIterable &attributes, const trace_api::StartSpanOptions &options) noexcept : tracer_{std::move(tracer)}, processor_{processor}, @@ -59,7 +59,7 @@ Span::Span(std::shared_ptr &&tracer, attributes.ForEachKeyValue([&](nostd::string_view key, common::AttributeValue value) noexcept { recordable_->SetAttribute(key, std::move(value)); return true; - }); + }); recordable_->SetStartTime(NowOr(options.start_system_time)); start_steady_time = NowOr(options.start_steady_time); diff --git a/sdk/src/trace/span.h b/sdk/src/trace/span.h index 0470b799d0..db8d01ab4e 100644 --- a/sdk/src/trace/span.h +++ b/sdk/src/trace/span.h @@ -18,7 +18,7 @@ class Span final : public trace_api::Span explicit Span(std::shared_ptr &&tracer, std::shared_ptr processor, nostd::string_view name, - const trace_api::KeyValueIterable &attributes, + const trace_api::KeyValueIterable &attributes, const trace_api::StartSpanOptions &options) noexcept; ~Span() override; diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index b83fb345b1..311459060b 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -21,11 +21,11 @@ std::shared_ptr Tracer::GetProcessor() const noexcept nostd::unique_ptr Tracer::StartSpan( nostd::string_view name, - const trace_api::KeyValueIterable &attributes, + const trace_api::KeyValueIterable &attributes, const trace_api::StartSpanOptions &options) noexcept { - return nostd::unique_ptr{ - new (std::nothrow) Span{this->shared_from_this(), processor_.load(), name, attributes, options}}; + return nostd::unique_ptr{new (std::nothrow) Span{ + this->shared_from_this(), processor_.load(), name, attributes, options}}; } void Tracer::ForceFlushWithMicroseconds(uint64_t timeout) noexcept diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index 4aa00d0726..b254dcf2e7 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -120,18 +120,17 @@ TEST(Tracer, StartSpanWithAttributes) new std::vector>); auto tracer = initTracer(spans_received); - { - tracer->StartSpan("span 1", {{"attr1", 314159}, {"attr2", false}, {"attr1", "string"}}); + tracer->StartSpan("span 1", {{"attr1", 314159}, {"attr2", false}, {"attr1", "string"}}); - std::map m; - m["attr3"] = 3.0; - tracer->StartSpan("span 2", m); + std::map m; + m["attr3"] = 3.0; + tracer->StartSpan("span 2", m); - char* local_value = new char[6]; - local_value[0] = '\0'; - tracer->StartSpan("span 3", {{"attr4", local_value}}); - delete local_value; + char *local_value = new char[6]; + local_value[0] = '\0'; + tracer->StartSpan("span 3", {{"attr4", local_value}}); + delete local_value; } ASSERT_EQ(3, spans_received->size()); From 21ef99fdfa3e76547dc74d4b28ac7a4d293bebd8 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 22 Jun 2020 09:37:46 -0700 Subject: [PATCH 16/18] Format --- api/include/opentelemetry/trace/span.h | 5 ----- api/include/opentelemetry/trace/tracer.h | 5 +++++ sdk/test/trace/tracer_test.cc | 11 +---------- 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/api/include/opentelemetry/trace/span.h b/api/include/opentelemetry/trace/span.h index 12b7bf5230..77265387e6 100644 --- a/api/include/opentelemetry/trace/span.h +++ b/api/include/opentelemetry/trace/span.h @@ -37,11 +37,6 @@ struct StartSpanOptions core::SystemTimestamp start_system_time; core::SteadyTimestamp start_steady_time; - // Optionally set attributes at Span creation from the given key/value pairs. - // - // Attributes will be processed in order, previous attributes with the same - // key will be overwritten. - // TODO: // Span(Context?) parent; // SpanContext remote_parent; diff --git a/api/include/opentelemetry/trace/tracer.h b/api/include/opentelemetry/trace/tracer.h index f488b8e613..825ff39fb1 100644 --- a/api/include/opentelemetry/trace/tracer.h +++ b/api/include/opentelemetry/trace/tracer.h @@ -22,6 +22,11 @@ class Tracer virtual ~Tracer() = default; /** * Starts a span. + * + * Optionally sets attributes at Span creation from the given key/value pairs. + * + * Attributes will be processed in order, previous attributes with the same + * key will be overwritten. */ virtual nostd::unique_ptr StartSpan(nostd::string_view name, const KeyValueIterable &attributes, diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index b254dcf2e7..366c6428a1 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -126,14 +126,9 @@ TEST(Tracer, StartSpanWithAttributes) std::map m; m["attr3"] = 3.0; tracer->StartSpan("span 2", m); - - char *local_value = new char[6]; - local_value[0] = '\0'; - tracer->StartSpan("span 3", {{"attr4", local_value}}); - delete local_value; } - ASSERT_EQ(3, spans_received->size()); + ASSERT_EQ(2, spans_received->size()); auto &span_data = spans_received->at(0); ASSERT_EQ(2, span_data->GetAttributes().size()); @@ -143,8 +138,4 @@ TEST(Tracer, StartSpanWithAttributes) auto &span_data2 = spans_received->at(1); ASSERT_EQ(1, span_data2->GetAttributes().size()); ASSERT_EQ(3.0, nostd::get(span_data2->GetAttributes().at("attr3"))); - - auto &span_data3 = spans_received->at(2); - ASSERT_EQ(1, span_data2->GetAttributes().size()); - ASSERT_EQ("", nostd::get(span_data2->GetAttributes().at("attr4"))); } From dd8e1b0f29a459eb618cd19862b08386b01a4ae8 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 22 Jun 2020 10:00:59 -0700 Subject: [PATCH 17/18] Format --- examples/plugin/load/main.cc | 1 - sdk/test/trace/tracer_test.cc | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/examples/plugin/load/main.cc b/examples/plugin/load/main.cc index 71cb4626b3..3c6799fed0 100644 --- a/examples/plugin/load/main.cc +++ b/examples/plugin/load/main.cc @@ -1,5 +1,4 @@ #include "opentelemetry/plugin/dynamic_load.h" -#include "opentelemetry/trace/tracer.h" #include #include diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index 366c6428a1..056227aa8d 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -105,7 +105,7 @@ TEST(Tracer, StartSpanWithOptionsTime) opentelemetry::trace::EndSpanOptions end; end.end_steady_time = SteadyTimestamp(std::chrono::nanoseconds(40)); - tracer->StartSpan("span 1", {}, start)->End(end); + tracer->StartSpan("span 1", start)->End(end); ASSERT_EQ(1, spans_received->size()); From 8723b37f923dda9d01d7ed6d0c5d11b7224aa8f5 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 22 Jun 2020 10:10:19 -0700 Subject: [PATCH 18/18] Remove unused struct --- api/include/opentelemetry/common/attribute_value.h | 9 --------- 1 file changed, 9 deletions(-) diff --git a/api/include/opentelemetry/common/attribute_value.h b/api/include/opentelemetry/common/attribute_value.h index 8a6b0c1b39..6340a4f97c 100644 --- a/api/include/opentelemetry/common/attribute_value.h +++ b/api/include/opentelemetry/common/attribute_value.h @@ -24,14 +24,5 @@ using AttributeValue = nostd::variant, nostd::span, nostd::span>; - -/** - * A key/value pair that can be used to set attributes. - */ -struct AttributeKeyValue -{ - nostd::string_view key; - AttributeValue value; -}; } // namespace common OPENTELEMETRY_END_NAMESPACE