Fix: ETW span ownership lifetime

exporters/etw/README.md

@@ -12,5 +12,13 @@ subsequent data recording or forwarding to alternate pipelines and flows.
 Windows Event Tracing infrastructure is available to any vendor or application
 being deployed on Windows.
 
+## Tracer lifetime and reuse
+
+For ETW, the tracer name maps to the ETW provider name/GUID. Applications
+should create one tracer per provider name (and encoding) and reuse it for the
+process or component lifetime. `TracerProvider::GetTracer()` may cache tracers
+internally, so avoid calling `GetTracer()->StartSpan()` in hot paths. Instead,
+obtain the tracer once during setup and reuse it for span creation.
+
 It is recommended to consume this exporter via
 [vcpkg](https://vcpkg.io/en/package/opentelemetry-cpp).

exporters/etw/include/opentelemetry/exporters/etw/etw_tracer.h

@@ -6,6 +6,7 @@
 #include <algorithm>
 #include <atomic>
 
+#include <cctype>
 #include <cstdint>
 #include <cstdio>
 #include <cstdlib>
@@ -17,6 +18,7 @@
 #include <memory>
 #include <mutex>
 #include <sstream>
+#include <unordered_map>
 #include <vector>
 
 #include "opentelemetry/nostd/shared_ptr.h"
@@ -159,8 +161,7 @@ void UpdateStatus(T &t, Properties &props)
 /**
  * @brief Tracer class that allows to send spans to ETW Provider.
  */
-class Tracer : public opentelemetry::trace::Tracer,
-               public std::enable_shared_from_this<opentelemetry::trace::Tracer>
+class Tracer : public opentelemetry::trace::Tracer
 {
 public:
   /**
@@ -169,6 +170,8 @@ class Tracer : public opentelemetry::trace::Tracer,
   bool IsClosed() const noexcept { return isClosed_.load(); }
 
 private:
+  friend class TracerProvider;
+
   /**
    * @brief Parent provider of this Tracer
    */
@@ -510,7 +513,7 @@ class Tracer : public opentelemetry::trace::Tracer,
     {
       auto noopSpan = nostd::shared_ptr<opentelemetry::trace::Span>{
           new (std::nothrow)
-              opentelemetry::trace::NoopSpan(this->shared_from_this(), std::move(spanContext))};
+              opentelemetry::trace::NoopSpan(std::shared_ptr<Tracer>{}, std::move(spanContext))};
       return noopSpan;
     }
 
@@ -1059,6 +1062,54 @@ class Span : public opentelemetry::trace::Span
   opentelemetry::trace::Tracer &tracer() const noexcept { return this->owner_; }
 };
 
+static inline std::string TrimWhitespace(std::string input)
+{
+  auto is_space = [](unsigned char c) { return std::isspace(c) != 0; };
+  auto begin    = std::find_if_not(input.begin(), input.end(), is_space);
+  if (begin == input.end())
+  {
+    return std::string{};
+  }
+  auto end = std::find_if_not(input.rbegin(), input.rend(), is_space).base();
+  return std::string(begin, end);
+}
+
+static inline ETWProvider::EventFormat ParseEncodingArg(nostd::string_view args,
+                                                        ETWProvider::EventFormat fallback)
+{
+  if (args.size() == 0)
+  {
+    return fallback;
+  }
+
+  std::string arg(args.data(), args.size());
+  arg = TrimWhitespace(std::move(arg));
+  if (arg.empty())
+  {
+    return fallback;
+  }
+
+  std::transform(arg.begin(), arg.end(), arg.begin(),
+                 [](unsigned char c) { return static_cast<char>(std::toupper(c)); });
+
+  if (arg == "MSGPACK" || arg == "MESSAGEPACK")
+  {
+    return ETWProvider::EventFormat::ETW_MSGPACK;
+  }
+
+  if (arg == "XML")
+  {
+    return ETWProvider::EventFormat::ETW_XML;
+  }
+
+  if (arg == "ETW" || arg == "TLD")
+  {
+    return ETWProvider::EventFormat::ETW_MANIFEST;
+  }
+
+  return fallback;
+}
+
 /**
  * @brief ETW TracerProvider
  */
@@ -1106,6 +1157,8 @@ class TracerProvider : public opentelemetry::trace::TracerProvider
         id_generator_{std::move(id_generator)},
         tail_sampler_{std::move(tail_sampler)}
   {
+    (void)Tracer::etwProvider().is_registered(std::string{});
+
     // By default we ensure that all events carry their with TraceId and SpanId
     GetOption(options, "enableTraceId", config_.enableTraceId, true);
     GetOption(options, "enableSpanId", config_.enableSpanId, true);
@@ -1143,6 +1196,8 @@ class TracerProvider : public opentelemetry::trace::TracerProvider
         tail_sampler_{
             std::unique_ptr<opentelemetry::exporter::etw::TailSampler>(new AlwaysOnTailSampler())}
   {
+    (void)Tracer::etwProvider().is_registered(std::string{});
+
     config_.enableTraceId           = true;
     config_.enableSpanId            = true;
     config_.enableActivityId        = false;
@@ -1174,13 +1229,56 @@ class TracerProvider : public opentelemetry::trace::TracerProvider
 #endif
       ) noexcept override
   {
-    UNREFERENCED_PARAMETER(args);
     UNREFERENCED_PARAMETER(schema_url);
-    ETWProvider::EventFormat evtFmt = config_.encoding;
-    std::shared_ptr<opentelemetry::trace::Tracer> tracer{new (std::nothrow)
-                                                             Tracer(*this, name, evtFmt)};
-    return nostd::shared_ptr<opentelemetry::trace::Tracer>{tracer};
+    ETWProvider::EventFormat evtFmt = ParseEncodingArg(args, config_.encoding);
+    TracerCacheKey key{name.data(), name.size(), evtFmt};
+
+    std::lock_guard<std::mutex> lock(tracers_mu_);
+    auto it = tracers_.find(key);
+    if (it != tracers_.end())
+    {
+      return nostd::shared_ptr<opentelemetry::trace::Tracer>{
+          std::static_pointer_cast<opentelemetry::trace::Tracer>(it->second)};
+    }
+
+    std::shared_ptr<Tracer> tracer{new (std::nothrow) Tracer(*this, name, evtFmt)};
+    tracers_.emplace(std::move(key), tracer);
+    return nostd::shared_ptr<opentelemetry::trace::Tracer>{
+        std::static_pointer_cast<opentelemetry::trace::Tracer>(tracer)};
   }
+
+private:
+  struct TracerCacheKey
+  {
+    std::string name;
+    ETWProvider::EventFormat encoding;
+
+    TracerCacheKey(std::string value, ETWProvider::EventFormat format)
+        : name(std::move(value)), encoding(format)
+    {}
+
+    TracerCacheKey(const char *value, size_t size, ETWProvider::EventFormat format)
+        : name(value, size), encoding(format)
+    {}
+
+    bool operator==(const TracerCacheKey &other) const
+    {
+      return encoding == other.encoding && name == other.name;
+    }
+  };
+
+  struct TracerCacheKeyHash
+  {
+    size_t operator()(const TracerCacheKey &key) const noexcept
+    {
+      size_t name_hash = std::hash<std::string>{}(key.name);
+      size_t fmt_hash  = static_cast<size_t>(key.encoding);
+      return name_hash ^ (fmt_hash + 0x9e3779b97f4a7c15ULL + (name_hash << 6) + (name_hash >> 2));
+    }
+  };
+
+  std::mutex tracers_mu_;
+  std::unordered_map<TracerCacheKey, std::shared_ptr<Tracer>, TracerCacheKeyHash> tracers_;
 };
 
 }  // namespace etw

exporters/etw/test/etw_tracer_test.cc

@@ -392,7 +392,7 @@ TEST(ETWTracer, GlobalSingletonTracer)
 }
 */
 
-  // Obtain a different tracer withs its own trace-id.
+  // Obtain the same tracer instance with the same trace-id as before.
   auto localTracer = GetGlobalTracerProvider().GetTracer(kGlobalProviderName);
   auto s2 = localTracer->StartSpan("Span2");
   auto traceId2 = s2->GetContext().trace_id();
@@ -455,7 +455,7 @@ TEST(ETWTracer, GlobalSingletonTracer)
   }
 }
 */
-  EXPECT_NE(traceId1, traceId2);
+  EXPECT_EQ(traceId1, traceId2);
   EXPECT_EQ(traceId1, traceId3);
 
 #  if OPENTELEMETRY_ABI_VERSION_NO == 1
@@ -503,6 +503,17 @@ TEST(ETWTracer, AlwayOffTailSampler)
   auto tracer = tp.GetTracer(providerName);
 }
 
+TEST(ETWTracer, SpanSurvivesTracerReassignment)
+{
+  exporter::etw::TracerProvider tp;
+  auto tracer = tp.GetTracer("Geneva-Tracer-Foo");
+  auto spanFoo = tracer->StartSpan("Span-Foo");
+  tracer = tp.GetTracer("Geneva-Tracer-Bar");
+  auto spanBar = tracer->StartSpan("Span-Bar");
+  EXPECT_NO_THROW(spanFoo->End());
+  EXPECT_NO_THROW(spanBar->End());
+}
+
 TEST(ETWTracer, CustomIdGenerator)
 {
   std::string providerName = kGlobalProviderName; // supply unique instrumentation name here