From 5a97750c6e22af63da894829843eb5694e078413 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 7 May 2021 09:54:23 +0530 Subject: [PATCH] Cleanup: Get Span from Context, Set Span to Context (#723) --- .../trace/propagation/b3_propagator.h | 6 ++--- .../trace/propagation/detail/context.h | 15 +++++++----- .../trace/propagation/http_trace_context.h | 4 ++-- .../opentelemetry/trace/propagation/jaeger.h | 4 ++-- api/include/opentelemetry/trace/tracer.h | 4 ++-- .../propagation/http_text_format_test.cc | 2 +- .../propagation/jaeger_propagation_test.cc | 4 ++-- sdk/src/trace/tracer.cc | 24 ++----------------- 8 files changed, 23 insertions(+), 40 deletions(-) diff --git a/api/include/opentelemetry/trace/propagation/b3_propagator.h b/api/include/opentelemetry/trace/propagation/b3_propagator.h index 93cfed66edd..b8c1db5a01c 100644 --- a/api/include/opentelemetry/trace/propagation/b3_propagator.h +++ b/api/include/opentelemetry/trace/propagation/b3_propagator.h @@ -47,7 +47,7 @@ class B3PropagatorExtractor : public opentelemetry::context::propagation::TextMa { SpanContext span_context = ExtractImpl(carrier); nostd::shared_ptr sp{new DefaultSpan(span_context)}; - return context.SetValue(kSpanKey, sp); + return SetSpan(context, sp); } static TraceId TraceIdFromHex(nostd::string_view trace_id) @@ -128,7 +128,7 @@ class B3Propagator : public B3PropagatorExtractor void Inject(opentelemetry::context::propagation::TextMapCarrier &carrier, const context::Context &context) noexcept override { - SpanContext span_context = detail::GetCurrentSpan(context); + SpanContext span_context = GetSpan(context)->GetContext(); if (!span_context.IsValid()) { return; @@ -155,7 +155,7 @@ class B3PropagatorMultiHeader : public B3PropagatorExtractor void Inject(opentelemetry::context::propagation::TextMapCarrier &carrier, const context::Context &context) noexcept override { - SpanContext span_context = detail::GetCurrentSpan(context); + SpanContext span_context = GetSpan(context)->GetContext(); if (!span_context.IsValid()) { return; diff --git a/api/include/opentelemetry/trace/propagation/detail/context.h b/api/include/opentelemetry/trace/propagation/detail/context.h index 5a5e454bb9b..da0737600f7 100644 --- a/api/include/opentelemetry/trace/propagation/detail/context.h +++ b/api/include/opentelemetry/trace/propagation/detail/context.h @@ -1,26 +1,29 @@ #pragma once #include "opentelemetry/context/context.h" +#include "opentelemetry/trace/default_span.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace trace { namespace propagation { -namespace detail -{ -inline trace::SpanContext GetCurrentSpan(const context::Context &context) +inline nostd::shared_ptr GetSpan(const context::Context &context) { context::ContextValue span = context.GetValue(trace::kSpanKey); if (nostd::holds_alternative>(span)) { - return nostd::get>(span).get()->GetContext(); + return nostd::get>(span); } + return nostd::shared_ptr(new DefaultSpan(SpanContext::GetInvalid())); +} + +inline context::Context SetSpan(context::Context &context, nostd::shared_ptr span) +{ - return trace::SpanContext::GetInvalid(); + return context.SetValue(kSpanKey, span); } -} // namespace detail } // namespace propagation } // namespace trace OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/trace/propagation/http_trace_context.h b/api/include/opentelemetry/trace/propagation/http_trace_context.h index 250737eba0c..cc3527c2867 100644 --- a/api/include/opentelemetry/trace/propagation/http_trace_context.h +++ b/api/include/opentelemetry/trace/propagation/http_trace_context.h @@ -45,7 +45,7 @@ class HttpTraceContext : public opentelemetry::context::propagation::TextMapProp void Inject(opentelemetry::context::propagation::TextMapCarrier &carrier, const context::Context &context) noexcept override { - SpanContext span_context = detail::GetCurrentSpan(context); + SpanContext span_context = GetSpan(context)->GetContext(); if (!span_context.IsValid()) { return; @@ -58,7 +58,7 @@ class HttpTraceContext : public opentelemetry::context::propagation::TextMapProp { SpanContext span_context = ExtractImpl(carrier); nostd::shared_ptr sp{new DefaultSpan(span_context)}; - return context.SetValue(kSpanKey, sp); + return SetSpan(context, sp); } static TraceId TraceIdFromHex(nostd::string_view trace_id) diff --git a/api/include/opentelemetry/trace/propagation/jaeger.h b/api/include/opentelemetry/trace/propagation/jaeger.h index 2c44b8d7115..4cdc4354d6b 100644 --- a/api/include/opentelemetry/trace/propagation/jaeger.h +++ b/api/include/opentelemetry/trace/propagation/jaeger.h @@ -34,7 +34,7 @@ class JaegerPropagator : public context::propagation::TextMapPropagator void Inject(context::propagation::TextMapCarrier &carrier, const context::Context &context) noexcept override { - SpanContext span_context = detail::GetCurrentSpan(context); + SpanContext span_context = GetSpan(context)->GetContext(); if (!span_context.IsValid()) { return; @@ -64,7 +64,7 @@ class JaegerPropagator : public context::propagation::TextMapPropagator { SpanContext span_context = ExtractImpl(carrier); nostd::shared_ptr sp{new DefaultSpan(span_context)}; - return context.SetValue(kSpanKey, sp); + return SetSpan(context, sp); } private: diff --git a/api/include/opentelemetry/trace/tracer.h b/api/include/opentelemetry/trace/tracer.h index ab9c6f16752..168cf5e5839 100644 --- a/api/include/opentelemetry/trace/tracer.h +++ b/api/include/opentelemetry/trace/tracer.h @@ -140,7 +140,7 @@ class Tracer * @param span the span that should be set as the new active span. * @return a Scope that controls how long the span will be active. */ - nostd::unique_ptr WithActiveSpan(nostd::shared_ptr &span) noexcept + static nostd::unique_ptr WithActiveSpan(nostd::shared_ptr &span) noexcept { return nostd::unique_ptr(new Scope{span}); } @@ -150,7 +150,7 @@ class Tracer * @return the currently active span, or an invalid default span if no span * is active. */ - nostd::shared_ptr GetCurrentSpan() noexcept + static nostd::shared_ptr GetCurrentSpan() noexcept { context::ContextValue active_span = context::RuntimeContext::GetValue(kSpanKey); if (nostd::holds_alternative>(active_span)) diff --git a/api/test/trace/propagation/http_text_format_test.cc b/api/test/trace/propagation/http_text_format_test.cc index 48e9ca95013..8ed75ef6488 100644 --- a/api/test/trace/propagation/http_text_format_test.cc +++ b/api/test/trace/propagation/http_text_format_test.cc @@ -147,7 +147,7 @@ TEST(TextMapPropagatorTest, InvalidIdentitiesAreNotExtracted) context::Context ctx1 = context::Context{}; context::Context ctx2 = format.Extract(carrier, ctx1); - auto span = trace::propagation::detail::GetCurrentSpan(ctx2); + auto span = trace::propagation::GetSpan(ctx2)->GetContext(); EXPECT_FALSE(span.IsValid()); } } diff --git a/api/test/trace/propagation/jaeger_propagation_test.cc b/api/test/trace/propagation/jaeger_propagation_test.cc index 2f11b2cb5bc..70bd8d475ba 100644 --- a/api/test/trace/propagation/jaeger_propagation_test.cc +++ b/api/test/trace/propagation/jaeger_propagation_test.cc @@ -95,7 +95,7 @@ TEST(JaegerPropagatorTest, ExtractValidSpans) context::Context ctx1 = context::Context{}; context::Context ctx2 = format.Extract(carrier, ctx1); - auto span = trace::propagation::detail::GetCurrentSpan(ctx2); + auto span = trace::propagation::GetSpan(ctx2)->GetContext(); EXPECT_TRUE(span.IsValid()); EXPECT_EQ(Hex(span.trace_id()), test_trace.expected_trace_id); @@ -126,7 +126,7 @@ TEST(JaegerPropagatorTest, ExctractInvalidSpans) context::Context ctx1 = context::Context{}; context::Context ctx2 = format.Extract(carrier, ctx1); - auto span = trace::propagation::detail::GetCurrentSpan(ctx2); + auto span = trace::propagation::GetSpan(ctx2)->GetContext(); EXPECT_FALSE(span.IsValid()); } } diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index e111357ec50..57558def28b 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -16,34 +16,14 @@ Tracer::Tracer(std::shared_ptr context, : context_{context}, instrumentation_library_{std::move(instrumentation_library)} {} -trace_api::SpanContext GetCurrentSpanContext(const trace_api::SpanContext &explicit_parent) -{ - // 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(trace_api::kSpanKey); - - if (nostd::holds_alternative>(curr_span_context)) - { - auto curr_span = nostd::get>(curr_span_context); - return curr_span->GetContext(); - } - - // Otherwise return an invalid SpanContext. - return trace_api::SpanContext::GetInvalid(); -} - nostd::shared_ptr Tracer::StartSpan( nostd::string_view name, const opentelemetry::common::KeyValueIterable &attributes, const trace_api::SpanContextKeyValueIterable &links, const trace_api::StartSpanOptions &options) noexcept { - trace_api::SpanContext parent = GetCurrentSpanContext(options.parent); + trace_api::SpanContext parent = + options.parent.IsValid() ? options.parent : GetCurrentSpan()->GetContext(); auto sampling_result = context_->GetSampler().ShouldSample(parent, parent.trace_id(), name, options.kind, attributes, links);