Use a consistent key for the current span in the context (#480)

open-telemetry · Dec 23, 2020 · 2c97611 · 2c97611
1 parent ab7a8a2
commit 2c97611
Show file tree

Hide file tree

Showing 8 changed files with 59 additions and 13 deletions.
diff --git a/api/include/opentelemetry/trace/propagation/http_trace_context.h b/api/include/opentelemetry/trace/propagation/http_trace_context.h
@@ -77,17 +77,15 @@ class HttpTraceContext : public HTTPTextFormat<T>
                            const T &carrier,
                            context::Context &context) noexcept override
   {
-    SpanContext span_context    = ExtractImpl(getter, carrier);
-    nostd::string_view span_key = "current-span";
+    SpanContext span_context = ExtractImpl(getter, carrier);
     nostd::shared_ptr<Span> sp{new DefaultSpan(span_context)};
-    return context.SetValue(span_key, sp);
+    return context.SetValue(kSpanKey, sp);
   }
 
   static SpanContext GetCurrentSpan(const context::Context &context)
   {
-    const nostd::string_view span_key = "current-span";
     context::Context ctx(context);
-    context::ContextValue span = ctx.GetValue(span_key);
+    context::ContextValue span = ctx.GetValue(kSpanKey);
     if (nostd::holds_alternative<nostd::shared_ptr<Span>>(span))
     {
       return nostd::get<nostd::shared_ptr<Span>>(span).get()->GetContext();

diff --git a/api/include/opentelemetry/trace/scope.h b/api/include/opentelemetry/trace/scope.h
@@ -37,7 +37,7 @@ class Scope final
    */
   Scope(const nostd::shared_ptr<Span> &span) noexcept
       : token_(context::RuntimeContext::Attach(
-            context::RuntimeContext::GetCurrent().SetValue(SpanKey, span)))
+            context::RuntimeContext::GetCurrent().SetValue(kSpanKey, span)))
   {}
 
 private:

diff --git a/api/include/opentelemetry/trace/span.h b/api/include/opentelemetry/trace/span.h
@@ -14,11 +14,13 @@
 #include "opentelemetry/trace/span_context.h"
 #include "opentelemetry/version.h"
 
-constexpr char SpanKey[] = "span_key";
-
 OPENTELEMETRY_BEGIN_NAMESPACE
 namespace trace
 {
+
+// The key identifies the active span in the current context.
+constexpr char kSpanKey[] = "active_span";
+
 enum class SpanKind
 {
   kInternal,

diff --git a/api/include/opentelemetry/trace/tracer.h b/api/include/opentelemetry/trace/tracer.h
@@ -144,7 +144,7 @@ class Tracer
    */
   nostd::shared_ptr<Span> GetCurrentSpan() noexcept
   {
-    context::ContextValue active_span = context::RuntimeContext::GetValue(SpanKey);
+    context::ContextValue active_span = context::RuntimeContext::GetValue(kSpanKey);
     if (nostd::holds_alternative<nostd::shared_ptr<Span>>(active_span))
     {
       return nostd::get<nostd::shared_ptr<Span>>(active_span);

diff --git a/api/test/trace/CMakeLists.txt b/api/test/trace/CMakeLists.txt
@@ -1,3 +1,5 @@
+add_subdirectory(propagation)
+
 foreach(
   testname
   key_value_iterable_view_test

diff --git a/api/test/trace/propagation/http_text_format_test.cc b/api/test/trace/propagation/http_text_format_test.cc
@@ -3,6 +3,7 @@
 #include "opentelemetry/nostd/span.h"
 #include "opentelemetry/nostd/string_view.h"
 #include "opentelemetry/trace/default_span.h"
+#include "opentelemetry/trace/noop.h"
 #include "opentelemetry/trace/span.h"
 #include "opentelemetry/trace/span_context.h"
 #include "opentelemetry/trace/trace_id.h"
@@ -20,6 +21,14 @@
 
 using namespace opentelemetry;
 
+template <typename T>
+static std::string Hex(const T &id_item)
+{
+  char buf[T::kSize * 2];
+  id_item.ToLowerBase16(buf);
+  return std::string(buf, sizeof(buf));
+}
+
 static nostd::string_view Getter(const std::map<std::string, std::string> &carrier,
                                  nostd::string_view trace_type = "traceparent")
 {
@@ -86,3 +95,37 @@ TEST(HTTPTextFormatTest, PropagateInvalidContext)
   format.Inject(Setter, carrier, ctx);
   EXPECT_TRUE(carrier.count("traceparent") == 0);
 }
+
+TEST(HTTPTextFormatTest, SetRemoteSpan)
+{
+  const std::map<std::string, std::string> carrier = {
+      {"traceparent", "00-4bf92f3577b34da6a3ce929d0e0e4736-0102030405060708-01"}};
+  context::Context ctx1 = context::Context{};
+  context::Context ctx2 = format.Extract(Getter, carrier, ctx1);
+
+  auto ctx2_span = ctx2.GetValue(trace::kSpanKey);
+  EXPECT_TRUE(nostd::holds_alternative<nostd::shared_ptr<trace::Span>>(ctx2_span));
+
+  auto span = nostd::get<nostd::shared_ptr<trace::Span>>(ctx2_span);
+
+  EXPECT_EQ(Hex(span->GetContext().trace_id()), "4bf92f3577b34da6a3ce929d0e0e4736");
+  EXPECT_EQ(Hex(span->GetContext().span_id()), "0102030405060708");
+  EXPECT_EQ(span->GetContext().IsSampled(), true);
+  EXPECT_EQ(span->GetContext().HasRemoteParent(), true);
+}
+
+TEST(HTTPTextFormatTest, GetCurrentSpan)
+{
+  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};
+  trace::SpanContext span_context{trace::TraceId{buf_trace}, trace::SpanId{buf_span},
+                                  trace::TraceFlags{true}, false};
+  nostd::shared_ptr<trace::Span> sp{new trace::DefaultSpan{span_context}};
+
+  // Set `sp` as the currently active span, which must be used by `Inject`.
+  trace::Scope scoped_span{sp};
+
+  std::map<std::string, std::string> headers = {};
+  format.Inject(Setter, headers, context::RuntimeContext::GetCurrent());
+  EXPECT_EQ(headers["traceparent"], "00-0102030405060708090a0b0c0d0e0f10-0102030405060708-01");
+}
diff --git a/api/test/trace/scope_test.cc b/api/test/trace/scope_test.cc
@@ -5,6 +5,7 @@
 
 #include <gtest/gtest.h>
 
+using opentelemetry::trace::kSpanKey;
 using opentelemetry::trace::NoopSpan;
 using opentelemetry::trace::Scope;
 using opentelemetry::trace::Span;
@@ -16,7 +17,7 @@ TEST(ScopeTest, Construct)
   nostd::shared_ptr<Span> span(new NoopSpan(nullptr));
   Scope scope(span);
 
-  context::ContextValue active_span_value = context::RuntimeContext::GetValue(SpanKey);
+  context::ContextValue active_span_value = context::RuntimeContext::GetValue(kSpanKey);
   ASSERT_TRUE(nostd::holds_alternative<nostd::shared_ptr<Span>>(active_span_value));
 
   auto active_span = nostd::get<nostd::shared_ptr<Span>>(active_span_value);
@@ -32,14 +33,14 @@ TEST(ScopeTest, Destruct)
     nostd::shared_ptr<Span> span_nested(new NoopSpan(nullptr));
     Scope scope_nested(span_nested);
 
-    context::ContextValue active_span_value = context::RuntimeContext::GetValue(SpanKey);
+    context::ContextValue active_span_value = context::RuntimeContext::GetValue(kSpanKey);
     ASSERT_TRUE(nostd::holds_alternative<nostd::shared_ptr<Span>>(active_span_value));
 
     auto active_span = nostd::get<nostd::shared_ptr<Span>>(active_span_value);
     ASSERT_EQ(active_span, span_nested);
   }
 
-  context::ContextValue active_span_value = context::RuntimeContext::GetValue(SpanKey);
+  context::ContextValue active_span_value = context::RuntimeContext::GetValue(kSpanKey);
   ASSERT_TRUE(nostd::holds_alternative<nostd::shared_ptr<Span>>(active_span_value));
 
   auto active_span = nostd::get<nostd::shared_ptr<Span>>(active_span_value);

diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc
@@ -38,7 +38,7 @@ trace_api::SpanContext GetCurrentSpanContext(const trace_api::SpanContext &expli
   }
 
   // Use the currently active span, if there's one.
-  auto curr_span_context = context::RuntimeContext::GetValue(SpanKey);
+  auto curr_span_context = context::RuntimeContext::GetValue(trace_api::kSpanKey);
 
   if (nostd::holds_alternative<nostd::shared_ptr<trace_api::Span>>(curr_span_context))
   {