diff --git a/api/include/opentelemetry/trace/default_span.h b/api/include/opentelemetry/trace/default_span.h index 9dbe682bc7..bfe46ae2ca 100644 --- a/api/include/opentelemetry/trace/default_span.h +++ b/api/include/opentelemetry/trace/default_span.h @@ -41,8 +41,6 @@ class DefaultSpan : public Span nostd::string_view ToString() { return "DefaultSpan"; } - DefaultSpan() = default; - DefaultSpan(SpanContext span_context) : span_context_(span_context) {} // movable and copiable diff --git a/api/include/opentelemetry/trace/noop.h b/api/include/opentelemetry/trace/noop.h index 533225e851..b4d950fd60 100644 --- a/api/include/opentelemetry/trace/noop.h +++ b/api/include/opentelemetry/trace/noop.h @@ -24,7 +24,9 @@ namespace trace class NoopSpan final : public Span { public: - explicit NoopSpan(const std::shared_ptr &tracer) noexcept : tracer_{tracer} {} + explicit NoopSpan(const std::shared_ptr &tracer) noexcept + : tracer_{tracer}, span_context_{SpanContext::GetInvalid()} + {} void SetAttribute(nostd::string_view /*key*/, const common::AttributeValue & /*value*/) noexcept override diff --git a/api/include/opentelemetry/trace/propagation/http_trace_context.h b/api/include/opentelemetry/trace/propagation/http_trace_context.h index 259bbcbc3b..07467b7ef6 100644 --- a/api/include/opentelemetry/trace/propagation/http_trace_context.h +++ b/api/include/opentelemetry/trace/propagation/http_trace_context.h @@ -92,7 +92,7 @@ class HttpTraceContext : public HTTPTextFormat { return nostd::get>(span).get()->GetContext(); } - return SpanContext(); + return SpanContext::GetInvalid(); } static TraceId GenerateTraceIdFromString(nostd::string_view trace_id) diff --git a/api/include/opentelemetry/trace/span.h b/api/include/opentelemetry/trace/span.h index 922f38fb92..016cd557e5 100644 --- a/api/include/opentelemetry/trace/span.h +++ b/api/include/opentelemetry/trace/span.h @@ -43,8 +43,13 @@ struct StartSpanOptions core::SystemTimestamp start_system_time; core::SteadyTimestamp start_steady_time; + // Explicitely set the parent of a Span. + // + // This defaults to an invalid span context. In this case, the Span is + // automatically parented to the currently active span. + SpanContext parent = SpanContext::GetInvalid(); + // TODO: - // Span(Context?) parent; // SpanContext remote_parent; // Links SpanKind kind = SpanKind::kInternal; diff --git a/api/include/opentelemetry/trace/span_context.h b/api/include/opentelemetry/trace/span_context.h index 6228bf215c..6add265eed 100644 --- a/api/include/opentelemetry/trace/span_context.h +++ b/api/include/opentelemetry/trace/span_context.h @@ -33,10 +33,6 @@ namespace trace_api = opentelemetry::trace; class SpanContext final { public: - // An invalid SpanContext. - SpanContext() noexcept - : trace_flags_(trace::TraceFlags((uint8_t) false)), remote_parent_(false){}; - /* A temporary constructor for an invalid SpanContext. * Trace id and span id are set to invalid (all zeros). * @@ -71,31 +67,9 @@ class SpanContext final remote_parent_(has_remote_parent) {} - SpanContext(SpanContext &&ctx) - : trace_id_(ctx.trace_id()), span_id_(ctx.span_id()), trace_flags_(ctx.trace_flags()) - {} + SpanContext(const SpanContext &ctx) = default; - SpanContext(const SpanContext &ctx) - : trace_id_(ctx.trace_id()), span_id_(ctx.span_id()), trace_flags_(ctx.trace_flags()) - {} - // - // SpanContext &operator=(const SpanContext &ctx) - // { - // SpanContext *spn_ctx = - // new SpanContext(ctx.trace_id(), ctx.span_id(), ctx.trace_flags(), - // ctx.HasRemoteParent()); - // this = spn_ctx; - // return *this; - // }; - // - // SpanContext &operator=(SpanContext &&ctx) - // { - // SpanContext *spn_ctx = - // new SpanContext(ctx.trace_id(), ctx.span_id(), ctx.trace_flags(), - // ctx.HasRemoteParent()); - // this = spn_ctx; - // return *this; - // }; + SpanContext &operator=(const SpanContext &ctx) = default; bool operator==(const SpanContext &that) const noexcept { @@ -110,10 +84,10 @@ class SpanContext final bool IsSampled() const noexcept { return trace_flags_.IsSampled(); } private: - const trace_api::TraceId trace_id_; - const trace_api::SpanId span_id_; - const trace_api::TraceFlags trace_flags_; - const bool remote_parent_ = false; + trace_api::TraceId trace_id_; + trace_api::SpanId span_id_; + trace_api::TraceFlags trace_flags_; + bool remote_parent_ = false; }; } // namespace trace OPENTELEMETRY_END_NAMESPACE diff --git a/api/test/trace/propagation/http_text_format_test.cc b/api/test/trace/propagation/http_text_format_test.cc index c6262a1606..bbdba64d36 100644 --- a/api/test/trace/propagation/http_text_format_test.cc +++ b/api/test/trace/propagation/http_text_format_test.cc @@ -66,8 +66,9 @@ TEST(HTTPTextFormatTest, NoSendEmptyTraceState) // If the trace state is empty, do not set the header. const std::map carrier = { {"traceparent", "00-4bf92f3577b34da6a3ce929d0e0e4736-0102030405060708-01"}}; - context::Context ctx1 = - context::Context("current-span", nostd::shared_ptr(new trace::DefaultSpan())); + context::Context ctx1 = context::Context{ + "current-span", + nostd::shared_ptr(new trace::DefaultSpan(trace::SpanContext::GetInvalid()))}; context::Context ctx2 = format.Extract(Getter, carrier, ctx1); std::map c2 = {}; format.Inject(Setter, c2, ctx2); diff --git a/api/test/trace/span_context_test.cc b/api/test/trace/span_context_test.cc index b364348ccd..1a4b7c62a2 100644 --- a/api/test/trace/span_context_test.cc +++ b/api/test/trace/span_context_test.cc @@ -42,7 +42,7 @@ TEST(SpanContextTest, TraceFlags) // Test that SpanContext is invalid TEST(SpanContextTest, Invalid) { - SpanContext s1(false, false); + SpanContext s1 = SpanContext::GetInvalid(); EXPECT_FALSE(s1.IsValid()); // Test that trace id and span id are invalid diff --git a/examples/plugin/plugin/tracer.cc b/examples/plugin/plugin/tracer.cc index e8f324c9e7..31b9367c7c 100644 --- a/examples/plugin/plugin/tracer.cc +++ b/examples/plugin/plugin/tracer.cc @@ -20,7 +20,7 @@ class Span final : public trace::Span nostd::string_view name, const opentelemetry::trace::KeyValueIterable & /*attributes*/, const trace::StartSpanOptions & /*options*/) noexcept - : tracer_{std::move(tracer)}, name_{name} + : tracer_{std::move(tracer)}, name_{name}, span_context_{trace::SpanContext::GetInvalid()} { std::cout << "StartSpan: " << name << "\n"; } diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index 70a7b42109..ee3d74ce1e 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -30,18 +30,25 @@ std::shared_ptr Tracer::GetSampler() const noexcept return sampler_; } -// Helper function to extract the current span context from the runtime context. -// Returns an invalid span context if the runtime context doesn't contain a span. -trace_api::SpanContext GetCurrentSpanContext() +trace_api::SpanContext GetCurrentSpanContext(const trace_api::SpanContext &explicit_parent) { - context::ContextValue curr_span_context = context::RuntimeContext::GetValue(SpanKey); + // Use the explicit parent, if it's valid. + if (explicit_parent.IsValid()) + { + return explicit_parent; + } + + // Use the currently active span, if there's one. + auto curr_span_context = context::RuntimeContext::GetValue(SpanKey); if (nostd::holds_alternative>(curr_span_context)) { auto curr_span = nostd::get>(curr_span_context); return curr_span->GetContext(); } - return trace_api::SpanContext(); + + // Otherwise return an invalid SpanContext. + return trace_api::SpanContext::GetInvalid(); } nostd::shared_ptr Tracer::StartSpan( @@ -49,9 +56,11 @@ nostd::shared_ptr Tracer::StartSpan( const trace_api::KeyValueIterable &attributes, const trace_api::StartSpanOptions &options) noexcept { + trace_api::SpanContext parent = GetCurrentSpanContext(options.parent); + // TODO: replace nullptr with parent context in span context auto sampling_result = - sampler_->ShouldSample(nullptr, trace_api::TraceId(), name, options.kind, attributes); + sampler_->ShouldSample(nullptr, parent.trace_id(), name, options.kind, attributes); if (sampling_result.decision == Decision::DROP) { // Don't allocate a no-op span for every DROP decision, but use a static @@ -63,9 +72,8 @@ nostd::shared_ptr Tracer::StartSpan( } else { - auto span = nostd::shared_ptr{ - new (std::nothrow) Span{this->shared_from_this(), processor_.load(), name, attributes, - options, GetCurrentSpanContext()}}; + auto span = nostd::shared_ptr{new (std::nothrow) Span{ + this->shared_from_this(), processor_.load(), name, attributes, options, parent}}; // if the attributes is not nullptr, add attributes to the span. if (sampling_result.attributes) diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index 98a18ea688..1e70af316c 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -442,3 +442,38 @@ TEST(Tracer, WithActiveSpan) ASSERT_EQ(1, spans.size()); EXPECT_EQ("span 1", spans.at(0)->GetName()); } + +TEST(Tracer, ExpectParent) +{ + std::unique_ptr exporter(new InMemorySpanExporter()); + std::shared_ptr span_data = exporter->GetData(); + auto tracer = initTracer(std::move(exporter)); + auto spans = span_data.get()->GetSpans(); + + ASSERT_EQ(0, spans.size()); + + auto span_first = tracer->StartSpan("span 1"); + + trace_api::StartSpanOptions options; + options.parent = span_first->GetContext(); + auto span_second = tracer->StartSpan("span 2", options); + + options.parent = span_second->GetContext(); + auto span_third = tracer->StartSpan("span 3", options); + + span_third->End(); + span_second->End(); + span_first->End(); + + spans = span_data->GetSpans(); + ASSERT_EQ(3, spans.size()); + auto spandata_first = std::move(spans.at(2)); + auto spandata_second = std::move(spans.at(1)); + auto spandata_third = std::move(spans.at(0)); + EXPECT_EQ("span 1", spandata_first->GetName()); + EXPECT_EQ("span 2", spandata_second->GetName()); + EXPECT_EQ("span 3", spandata_third->GetName()); + + EXPECT_EQ(spandata_first->GetSpanId(), spandata_second->GetParentSpanId()); + EXPECT_EQ(spandata_second->GetSpanId(), spandata_third->GetParentSpanId()); +}