Merge branch 'main' into http-propagate

open-telemetry · May 7, 2021 · 6ea9092 · 6ea9092
2 parents b0865c7 + 5a97750
commit 6ea9092
Show file tree

Hide file tree

Showing 12 changed files with 179 additions and 112 deletions.
diff --git 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<Span> 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
@@ -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<trace::Span> GetSpan(const context::Context &context)
 {
   context::ContextValue span = context.GetValue(trace::kSpanKey);
   if (nostd::holds_alternative<nostd::shared_ptr<trace::Span>>(span))
   {
-    return nostd::get<nostd::shared_ptr<trace::Span>>(span).get()->GetContext();
+    return nostd::get<nostd::shared_ptr<trace::Span>>(span);
   }
+  return nostd::shared_ptr<trace::Span>(new DefaultSpan(SpanContext::GetInvalid()));
+}
+
+inline context::Context SetSpan(context::Context &context, nostd::shared_ptr<trace::Span> 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
@@ -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<Span> 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
@@ -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<Span> 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
@@ -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<Scope> WithActiveSpan(nostd::shared_ptr<Span> &span) noexcept
+  static nostd::unique_ptr<Scope> WithActiveSpan(nostd::shared_ptr<Span> &span) noexcept
   {
     return nostd::unique_ptr<Scope>(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<Span> GetCurrentSpan() noexcept
+  static nostd::shared_ptr<Span> GetCurrentSpan() noexcept
   {
     context::ContextValue active_span = context::RuntimeContext::GetValue(kSpanKey);
     if (nostd::holds_alternative<nostd::shared_ptr<Span>>(active_span))

diff --git 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
@@ -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/exporters/etw/include/opentelemetry/exporters/etw/etw_properties.h b/exporters/etw/include/opentelemetry/exporters/etw/etw_properties.h
@@ -22,6 +22,10 @@
 #include <string>
 #include <vector>
 
+#ifdef _WIN32
+#  include <Windows.h>
+#endif
+
 OPENTELEMETRY_BEGIN_NAMESPACE
 namespace exporter
 {
@@ -178,7 +182,7 @@ class PropertyValue : public PropertyVariant
    * @param v
    * @return
    */
-  PropertyValue(const char *value) : PropertyVariant(value){};
+  PropertyValue(const char *value) : PropertyVariant(std::string(value)){};
 
   /**
    * @brief PropertyValue from string.
@@ -309,6 +313,7 @@ class PropertyValue : public PropertyVariant
       case PropertyType::kTypeSpanBool: {
         const auto &vec = nostd::get<std::vector<bool>>(*this);
         // FIXME: sort out how to remap from vector<bool> to span<bool>
+        UNREFERENCED_PARAMETER(vec);
         break;
       }
       case PropertyType::kTypeSpanInt:

diff --git a/exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h b/exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h
@@ -22,6 +22,7 @@
 #  pragma warning(push)
 #  pragma warning(disable : 4459)
 #  pragma warning(disable : 4018)
+#  pragma warning(disable : 5054)
 #endif
 
 #include "opentelemetry/exporters/etw/etw_properties.h"
@@ -52,10 +53,9 @@
 
 #define MICROSOFT_EVENTTAG_NORMAL_PERSISTENCE 0x01000000
 
-OPENTELEMETRY_BEGIN_NAMESPACE
+using namespace OPENTELEMETRY_NAMESPACE::exporter::etw;
 
-using Properties   = exporter::etw::Properties;
-using PropertyType = exporter::etw::PropertyType;
+OPENTELEMETRY_BEGIN_NAMESPACE
 
 class ETWProvider
 {
@@ -137,7 +137,6 @@ class ETWProvider
 
     switch (format)
     {
-#ifdef HAVE_TLD
       // Register with TraceLoggingDynamic facility - dynamic manifest ETW events.
       case EventFormat::ETW_MANIFEST: {
         tld::ProviderMetadataBuilder<std::vector<BYTE>> providerMetaBuilder(
@@ -164,7 +163,6 @@ class ETWProvider
         };
       };
       break;
-#endif
 
 #ifdef HAVE_MSGPACK
       // Register for MsgPack payload ETW events.
@@ -217,14 +215,12 @@ class ETWProvider
         data.refCount--;
         if (data.refCount == 0)
         {
-#ifdef HAVE_TLD
           if (data.providerMetaVector.size())
           {
             // ETW/TraceLoggingDynamic provider
             result = tld::UnregisterProvider(data.providerHandle);
           }
           else
-#endif
           {
             // Other provider types, e.g. ETW/MsgPack
             result = EventUnregister(data.providerHandle);
@@ -412,6 +408,11 @@ class ETWProvider
     };
     return (unsigned long)(writeResponse);
 #else
+    UNREFERENCED_PARAMETER(providerData);
+    UNREFERENCED_PARAMETER(eventData);
+    UNREFERENCED_PARAMETER(ActivityId);
+    UNREFERENCED_PARAMETER(RelatedActivityId);
+    UNREFERENCED_PARAMETER(Opcode);
     return STATUS_ERROR;
 #endif
   }
@@ -428,7 +429,6 @@ class ETWProvider
                          LPCGUID RelatedActivityId = nullptr,
                          uint8_t Opcode            = 0 /* Information */)
   {
-#ifdef HAVE_TLD
     // Make sure you stop sending event before register unregistering providerData
     if (providerData.providerHandle == INVALID_HANDLE)
     {
@@ -465,10 +465,7 @@ class ETWProvider
     for (auto &kv : eventData)
     {
       const char *name = kv.first.data();
-      // Don't include event name field in the payload
-      if (EVENT_NAME == name)
-        continue;
-      auto &value = kv.second;
+      auto &value      = kv.second;
       switch (value.index())
       {
         case PropertyType::kTypeBool: {
@@ -518,15 +515,15 @@ class ETWProvider
           dbuilder.AddString(temp);
           break;
         }
-#  if HAVE_TYPE_GUID
+#if HAVE_TYPE_GUID
           // TODO: consider adding UUID/GUID to spec
         case PropertyType::kGUID: {
           builder.AddField(name.c_str(), TypeGuid);
           auto temp = nostd::get<GUID>(value);
           dbuilder.AddBytes(&temp, sizeof(GUID));
           break;
         }
-#  endif
+#endif
 
         // TODO: arrays are not supported
         case PropertyType::kTypeSpanByte:
@@ -577,9 +574,6 @@ class ETWProvider
     };
 
     return (unsigned long)(writeResponse);
-#else
-    return STATUS_ERROR;
-#endif
   }
 
   unsigned long write(Handle &providerData,