diff --git a/api/include/opentelemetry/trace/noop.h b/api/include/opentelemetry/trace/noop.h index ad7f05273a..76c0fe0749 100644 --- a/api/include/opentelemetry/trace/noop.h +++ b/api/include/opentelemetry/trace/noop.h @@ -29,7 +29,12 @@ class NoopSpan final : public Span { public: explicit NoopSpan(const std::shared_ptr &tracer) noexcept - : tracer_{tracer}, span_context_{SpanContext::GetInvalid()} + : tracer_{tracer}, span_context_{new SpanContext(false, false)} + {} + + explicit NoopSpan(const std::shared_ptr &tracer, + nostd::unique_ptr span_context) noexcept + : tracer_{tracer}, span_context_{std::move(span_context)} {} void SetAttribute(nostd::string_view /*key*/, @@ -55,11 +60,11 @@ class NoopSpan final : public Span bool IsRecording() const noexcept override { return false; } - SpanContext GetContext() const noexcept override { return span_context_; } + SpanContext GetContext() const noexcept override { return *span_context_.get(); } private: std::shared_ptr tracer_; - SpanContext span_context_; + nostd::unique_ptr span_context_; }; /** diff --git a/api/test/trace/noop_test.cc b/api/test/trace/noop_test.cc index 5414a5c726..b17faf64d4 100644 --- a/api/test/trace/noop_test.cc +++ b/api/test/trace/noop_test.cc @@ -10,14 +10,13 @@ #include -using opentelemetry::common::SystemTimestamp; -using opentelemetry::trace::NoopTracer; -using opentelemetry::trace::SpanContext; -using opentelemetry::trace::Tracer; +namespace trace_api = opentelemetry::trace; +namespace nonstd = opentelemetry::nostd; +namespace common = opentelemetry::common; TEST(NoopTest, UseNoopTracers) { - std::shared_ptr tracer{new NoopTracer{}}; + std::shared_ptr tracer{new trace_api::NoopTracer{}}; auto s1 = tracer->StartSpan("abc"); std::map attributes1; @@ -41,7 +40,7 @@ TEST(NoopTest, UseNoopTracers) s1->UpdateName("test_name"); - SystemTimestamp t1; + common::SystemTimestamp t1; s1->AddEvent("test_time_stamp", t1); s1->GetContext(); @@ -49,12 +48,34 @@ TEST(NoopTest, UseNoopTracers) TEST(NoopTest, StartSpan) { - std::shared_ptr tracer{new NoopTracer{}}; + std::shared_ptr tracer{new trace_api::NoopTracer{}}; - std::map attrs = {{"a", "3"}}; - std::vector>> links = { - {SpanContext(false, false), attrs}}; + std::map attrs = {{"a", "3"}}; + std::vector>> links = { + {trace_api::SpanContext(false, false), attrs}}; auto s1 = tracer->StartSpan("abc", attrs, links); - auto s2 = tracer->StartSpan("efg", {{"a", 3}}, {{SpanContext(false, false), {{"b", 4}}}}); + auto s2 = + tracer->StartSpan("efg", {{"a", 3}}, {{trace_api::SpanContext(false, false), {{"b", 4}}}}); +} + +TEST(NoopTest, CreateSpanValidSpanContext) +{ + // Create valid spancontext for NoopSpan + + constexpr uint8_t buf_span[] = {1, 2, 3, 4, 5, 6, 7, 8}; + constexpr uint8_t buf_trace[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; + auto trace_id = trace_api::TraceId{buf_trace}; + auto span_id = trace_api::SpanId{buf_span}; + auto span_context = nonstd::unique_ptr( + new trace_api::SpanContext{trace_id, span_id, trace_api::TraceFlags{true}, false}); + std::shared_ptr tracer{new trace_api::NoopTracer{}}; + auto s1 = + nonstd::shared_ptr(new trace_api::NoopSpan(tracer, std::move(span_context))); + auto stored_span_context = s1->GetContext(); + EXPECT_EQ(stored_span_context.span_id(), span_id); + EXPECT_EQ(stored_span_context.trace_id(), trace_id); + + s1->AddEvent("even1"); // noop + s1->End(); // noop } diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index 149684bc8e..c84d847bd9 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -46,25 +46,27 @@ nostd::shared_ptr Tracer::StartSpan( auto sampling_result = context_->GetSampler().ShouldSample(parent_context, trace_id, name, options.kind, attributes, links); + auto trace_flags = sampling_result.decision == Decision::DROP + ? trace_api::TraceFlags{} + : trace_api::TraceFlags{trace_api::TraceFlags::kIsSampled}; + + auto span_context = std::unique_ptr(new trace_api::SpanContext( + trace_id, span_id, trace_flags, false, + sampling_result.trace_state ? sampling_result.trace_state + : is_parent_span_valid ? parent_context.trace_state() + : trace_api::TraceState::GetDefault())); if (sampling_result.decision == Decision::DROP) { - // Don't allocate a no-op span for every DROP decision, but use a static - // singleton for this case. - static nostd::shared_ptr noop_span( - new trace_api::NoopSpan{this->shared_from_this()}); + // create no-op span with valid span-context. + auto noop_span = nostd::shared_ptr{ + new (std::nothrow) trace_api::NoopSpan(this->shared_from_this(), std::move(span_context))}; return noop_span; } else { - auto span_context = std::unique_ptr(new trace_api::SpanContext( - trace_id, span_id, trace_api::TraceFlags{trace_api::TraceFlags::kIsSampled}, false, - sampling_result.trace_state ? sampling_result.trace_state - : is_parent_span_valid ? parent_context.trace_state() - : trace_api::TraceState::GetDefault())); - auto span = nostd::shared_ptr{ new (std::nothrow) Span{this->shared_from_this(), name, attributes, links, options, parent_context, std::move(span_context)}}; diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index 0c5cc397d8..2e9dd73413 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -165,8 +165,14 @@ TEST(Tracer, StartSpanSampleOff) auto tracer_off = initTracer(std::move(exporter), new AlwaysOffSampler()); // This span will not be recorded. - tracer_off->StartSpan("span 2")->End(); + auto span = tracer_off->StartSpan("span 2"); + // Always generate a valid span-context (span-id) + auto context = span->GetContext(); + EXPECT_TRUE(context.IsValid()); + EXPECT_FALSE(context.IsSampled()); + + span->End(); // The span doesn't write any span data because the sampling decision is alway // DROP. ASSERT_EQ(0, span_data->GetSpans().size());