From 40026e2c5d13c7755102be83017390dcbfe2f0c8 Mon Sep 17 00:00:00 2001 From: Oblivion Date: Wed, 23 Mar 2022 15:35:05 +0000 Subject: [PATCH 1/4] span SetAttribute crash --- sdk/src/trace/span.cc | 5 ++++- sdk/test/trace/tracer_test.cc | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index dff84a22b7..09c9e949b9 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -94,7 +94,10 @@ Span::~Span() void Span::SetAttribute(nostd::string_view key, const common::AttributeValue &value) noexcept { std::lock_guard lock_guard{mu_}; - + if (recordable_ == nullptr) + { + return; + } recordable_->SetAttribute(key, value); } diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index 9d1029cd02..138cc3b191 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -383,6 +383,24 @@ TEST(Tracer, SpanSetAttribute) ASSERT_EQ(3.1, nostd::get(cur_span_data->GetAttributes().at("abc"))); } +TEST(Tracer, SpanSetAttributeAfterEnd) +{ + std::unique_ptr exporter(new InMemorySpanExporter()); + std::shared_ptr span_data = exporter->GetData(); + auto tracer = initTracer(std::move(exporter)); + auto span = tracer->StartSpan("span 1"); + span->SetAttribute("abc", 3.1); + + span->End(); + + span->SetAttribute("testing null recordable", 3.1); + + auto spans = span_data->GetSpans(); + ASSERT_EQ(1, spans.size()); + auto &cur_span_data = spans.at(0); + ASSERT_EQ(3.1, nostd::get(cur_span_data->GetAttributes().at("abc"))); +} + TEST(Tracer, SpanSetEvents) { std::unique_ptr exporter(new InMemorySpanExporter()); From 6960461676be97d43281431c30483dba70329144 Mon Sep 17 00:00:00 2001 From: Ehsan Saei <71217171+esigo@users.noreply.github.com> Date: Wed, 23 Mar 2022 16:45:31 +0100 Subject: [PATCH 2/4] comments --- CHANGELOG.md | 1 + sdk/src/trace/span.cc | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f66ec617a..bd41c75626 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Increment the: ## [Unreleased] +* [SDK] Bugfix: span SetAttribute crash ([#1283](https://github.com/open-telemetry/opentelemetry-cpp/pull/1283)) * [EXPORTER] Jaeger Exporter - Populate Span Links ([#1251](https://github.com/open-telemetry/opentelemetry-cpp/pull/1251)) ## [1.2.0] 2022-01-31 diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index 09c9e949b9..13ea99126c 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -98,6 +98,7 @@ void Span::SetAttribute(nostd::string_view key, const common::AttributeValue &va { return; } + recordable_->SetAttribute(key, value); } From 5ff257d608e28ef18181d0f63b5f1885a2fc6319 Mon Sep 17 00:00:00 2001 From: Oblivion Date: Thu, 24 Mar 2022 17:45:18 +0000 Subject: [PATCH 3/4] test after end --- sdk/test/trace/tracer_test.cc | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index 138cc3b191..d946da6f81 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -383,7 +383,7 @@ TEST(Tracer, SpanSetAttribute) ASSERT_EQ(3.1, nostd::get(cur_span_data->GetAttributes().at("abc"))); } -TEST(Tracer, SpanSetAttributeAfterEnd) +TEST(Tracer, TestAfterEnd) { std::unique_ptr exporter(new InMemorySpanExporter()); std::shared_ptr span_data = exporter->GetData(); @@ -393,7 +393,17 @@ TEST(Tracer, SpanSetAttributeAfterEnd) span->End(); - span->SetAttribute("testing null recordable", 3.1); + // test after end + EXPECT_NO_THROW(span->SetAttribute("testing null recordable", 3.1)); + EXPECT_NO_THROW(span->AddEvent("event 1")); + EXPECT_NO_THROW(span->AddEvent("event 2", std::chrono::system_clock::now())); + EXPECT_NO_THROW(span->AddEvent("event 3", std::chrono::system_clock::now(), {{"attr1", 1}})); + std::string new_name{"new name"}; + EXPECT_NO_THROW(span->UpdateName(new_name)); + EXPECT_NO_THROW(span->SetAttribute("attr1", 3.1)); + std::string description{"description"}; + EXPECT_NO_THROW(span->SetStatus(opentelemetry::trace::StatusCode::kError, description)); + EXPECT_NO_THROW(span->End()); auto spans = span_data->GetSpans(); ASSERT_EQ(1, spans.size()); From b73b837df6c88b3a5e3565f4b566fc24e537285f Mon Sep 17 00:00:00 2001 From: Oblivion Date: Thu, 24 Mar 2022 18:57:00 +0000 Subject: [PATCH 4/4] fix CI --- sdk/test/trace/tracer_test.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index d946da6f81..dc806b079f 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -394,16 +394,16 @@ TEST(Tracer, TestAfterEnd) span->End(); // test after end - EXPECT_NO_THROW(span->SetAttribute("testing null recordable", 3.1)); - EXPECT_NO_THROW(span->AddEvent("event 1")); - EXPECT_NO_THROW(span->AddEvent("event 2", std::chrono::system_clock::now())); - EXPECT_NO_THROW(span->AddEvent("event 3", std::chrono::system_clock::now(), {{"attr1", 1}})); + span->SetAttribute("testing null recordable", 3.1); + span->AddEvent("event 1"); + span->AddEvent("event 2", std::chrono::system_clock::now()); + span->AddEvent("event 3", std::chrono::system_clock::now(), {{"attr1", 1}}); std::string new_name{"new name"}; - EXPECT_NO_THROW(span->UpdateName(new_name)); - EXPECT_NO_THROW(span->SetAttribute("attr1", 3.1)); + span->UpdateName(new_name); + span->SetAttribute("attr1", 3.1); std::string description{"description"}; - EXPECT_NO_THROW(span->SetStatus(opentelemetry::trace::StatusCode::kError, description)); - EXPECT_NO_THROW(span->End()); + span->SetStatus(opentelemetry::trace::StatusCode::kError, description); + span->End(); auto spans = span_data->GetSpans(); ASSERT_EQ(1, spans.size());