From 04f9ee4baae46945ca9c0fadd0078dd71b088b95 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Wed, 8 Jul 2020 16:01:07 -0700 Subject: [PATCH 1/2] No const move for AttributeValue --- api/include/opentelemetry/plugin/tracer.h | 4 ++-- api/include/opentelemetry/trace/noop.h | 2 +- api/include/opentelemetry/trace/span.h | 2 +- examples/plugin/plugin/tracer.cc | 2 +- exporters/otlp/recordable.cc | 2 +- exporters/otlp/recordable.h | 2 +- sdk/include/opentelemetry/sdk/trace/recordable.h | 2 +- sdk/include/opentelemetry/sdk/trace/span_data.h | 2 +- sdk/src/trace/span.cc | 6 +++--- sdk/src/trace/span.h | 2 +- 10 files changed, 13 insertions(+), 13 deletions(-) diff --git a/api/include/opentelemetry/plugin/tracer.h b/api/include/opentelemetry/plugin/tracer.h index 9970e5a533..e2f8bc79f8 100644 --- a/api/include/opentelemetry/plugin/tracer.h +++ b/api/include/opentelemetry/plugin/tracer.h @@ -18,9 +18,9 @@ class Span final : public trace::Span {} // trace::Span - void SetAttribute(nostd::string_view name, const common::AttributeValue &&value) noexcept override + void SetAttribute(nostd::string_view name, const common::AttributeValue &value) noexcept override { - span_->SetAttribute(name, std::move(value)); + span_->SetAttribute(name, value); } void AddEvent(nostd::string_view name) noexcept override { span_->AddEvent(name); } diff --git a/api/include/opentelemetry/trace/noop.h b/api/include/opentelemetry/trace/noop.h index fbb0e0cb68..2d353489d0 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*/, - const common::AttributeValue && /*value*/) noexcept override + const common::AttributeValue & /*value*/) noexcept override {} void AddEvent(nostd::string_view /*name*/) noexcept override {} diff --git a/api/include/opentelemetry/trace/span.h b/api/include/opentelemetry/trace/span.h index 77265387e6..1f8ae415a9 100644 --- a/api/include/opentelemetry/trace/span.h +++ b/api/include/opentelemetry/trace/span.h @@ -77,7 +77,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, - const common::AttributeValue &&value) noexcept = 0; + const common::AttributeValue &value) noexcept = 0; // Adds an event to the Span. virtual void AddEvent(nostd::string_view name) noexcept = 0; diff --git a/examples/plugin/plugin/tracer.cc b/examples/plugin/plugin/tracer.cc index 3aa68e8b15..f054da7f03 100644 --- a/examples/plugin/plugin/tracer.cc +++ b/examples/plugin/plugin/tracer.cc @@ -25,7 +25,7 @@ class Span final : public trace::Span // opentelemetry::trace::Span void SetAttribute(nostd::string_view /*name*/, - const common::AttributeValue && /*value*/) noexcept override + const common::AttributeValue & /*value*/) noexcept override {} void AddEvent(nostd::string_view /*name*/) noexcept override {} diff --git a/exporters/otlp/recordable.cc b/exporters/otlp/recordable.cc index 1677bb6be1..6092c0f005 100644 --- a/exporters/otlp/recordable.cc +++ b/exporters/otlp/recordable.cc @@ -16,7 +16,7 @@ void Recordable::SetIds(trace::TraceId trace_id, } void Recordable::SetAttribute(nostd::string_view key, - const opentelemetry::common::AttributeValue &&value) noexcept + const opentelemetry::common::AttributeValue &value) noexcept { (void)key; (void)value; diff --git a/exporters/otlp/recordable.h b/exporters/otlp/recordable.h index 8490bc9141..9d55dfd404 100644 --- a/exporters/otlp/recordable.h +++ b/exporters/otlp/recordable.h @@ -19,7 +19,7 @@ class Recordable final : public sdk::trace::Recordable trace::SpanId parent_span_id) noexcept override; void SetAttribute(nostd::string_view key, - const opentelemetry::common::AttributeValue &&value) noexcept override; + const opentelemetry::common::AttributeValue &value) noexcept override; void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept override; diff --git a/sdk/include/opentelemetry/sdk/trace/recordable.h b/sdk/include/opentelemetry/sdk/trace/recordable.h index d901dbfa54..a2ac8984dc 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, - const 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 c3b557dc9b..54ef9bcd02 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, const 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/src/trace/span.cc b/sdk/src/trace/span.cc index dde8710ec2..96154a2799 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -57,7 +57,7 @@ Span::Span(std::shared_ptr &&tracer, recordable_->SetName(name); attributes.ForEachKeyValue([&](nostd::string_view key, common::AttributeValue value) noexcept { - recordable_->SetAttribute(key, std::move(value)); + recordable_->SetAttribute(key, value); return true; }); @@ -70,11 +70,11 @@ Span::~Span() End(); } -void Span::SetAttribute(nostd::string_view key, const common::AttributeValue &&value) noexcept +void Span::SetAttribute(nostd::string_view key, const common::AttributeValue &value) noexcept { std::lock_guard lock_guard{mu_}; - recordable_->SetAttribute(key, std::move(value)); + recordable_->SetAttribute(key, value); } void Span::AddEvent(nostd::string_view name) noexcept diff --git a/sdk/src/trace/span.h b/sdk/src/trace/span.h index db8d01ab4e..489833703c 100644 --- a/sdk/src/trace/span.h +++ b/sdk/src/trace/span.h @@ -24,7 +24,7 @@ class Span final : public trace_api::Span ~Span() override; // trace_api::Span - void SetAttribute(nostd::string_view key, const 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; From 1438b2ec92ca6511a2d26b5a04af332e0dba56cc Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Wed, 8 Jul 2020 17:04:11 -0700 Subject: [PATCH 2/2] Increase coverage --- api/test/trace/noop_test.cc | 2 ++ sdk/test/trace/tracer_test.cc | 24 ++++++++++++++++++++---- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/api/test/trace/noop_test.cc b/api/test/trace/noop_test.cc index 6dad65323b..945a333f33 100644 --- a/api/test/trace/noop_test.cc +++ b/api/test/trace/noop_test.cc @@ -25,4 +25,6 @@ TEST(NoopTest, UseNoopTracers) std::vector>> attributes3; s1->AddEvent("abc", attributes3); + + s1->SetAttribute("abc", 4); } diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index 2c6b891818..75011db536 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -1,7 +1,7 @@ #include "opentelemetry/sdk/trace/tracer.h" -#include "opentelemetry/sdk/trace/simple_processor.h" -#include "opentelemetry/sdk/trace/samplers/always_on.h" #include "opentelemetry/sdk/trace/samplers/always_off.h" +#include "opentelemetry/sdk/trace/samplers/always_on.h" +#include "opentelemetry/sdk/trace/simple_processor.h" #include "opentelemetry/sdk/trace/span_data.h" #include @@ -142,7 +142,6 @@ TEST(Tracer, StartSpanWithAttributes) ASSERT_EQ(3.0, nostd::get(span_data2->GetAttributes().at("attr3"))); } - TEST(Tracer, GetSampler) { // Create a Tracer with a default AlwaysOnSampler @@ -154,8 +153,25 @@ TEST(Tracer, GetSampler) // Create a Tracer with a AlwaysOffSampler std::shared_ptr processor_2(new SimpleSpanProcessor(nullptr)); - std::shared_ptr tracer_off(new Tracer(std::move(processor_2), std::make_shared())); + std::shared_ptr tracer_off( + new Tracer(std::move(processor_2), std::make_shared())); auto t2 = tracer_off->GetSampler(); ASSERT_EQ("AlwaysOffSampler", t2->GetDescription()); } + +TEST(Tracer, SpanSetAttribute) +{ + std::shared_ptr>> spans_received( + new std::vector>); + auto tracer = initTracer(spans_received); + + auto span = tracer->StartSpan("span 1"); + + span->SetAttribute("abc", 3.1); + + span->End(); + ASSERT_EQ(1, spans_received->size()); + auto &span_data = spans_received->at(0); + ASSERT_EQ(3.1, nostd::get(span_data->GetAttributes().at("abc"))); +}