Fix: ETW span ownership lifetime

exporters/etw/README.md

@@ -12,5 +12,23 @@ 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 reuse it for the process or
+component lifetime. `TracerProvider::GetTracer()` caches tracers internally,
+so avoid calling `GetTracer()->StartSpan()` in hot paths. Instead, obtain the
+tracer once during setup and reuse it for span creation.
+
+Encoding (ETW/TLD, MSGPACK, XML) is configured via `TelemetryProviderOptions`
+passed to the `TracerProvider` constructor:
+
+```cpp
+TelemetryProviderOptions opts;
+opts["encoding"] = std::string("MsgPack");
+etw::TracerProvider tp(opts);
+auto tracer = tp.GetTracer("MyProvider");
+```
+
 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

@@ -3,7 +3,6 @@
 
 #pragma once
 
-#include <algorithm>
 #include <atomic>
 
 #include <cstdint>
@@ -17,6 +16,7 @@
 #include <memory>
 #include <mutex>
 #include <sstream>
+#include <unordered_map>
 #include <vector>
 
 #include "opentelemetry/nostd/shared_ptr.h"
@@ -159,8 +159,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 +168,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 +511,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;
     }
 
@@ -1106,6 +1107,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 +1146,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;
@@ -1156,11 +1161,12 @@ class TracerProvider : public opentelemetry::trace::TracerProvider
    * @brief Obtain ETW Tracer.
    * @param name ProviderId (instrumentation name) - Name or GUID
    *
-   * @param args Additional arguments that controls `codec` of the provider.
-   * Possible values are:
-   * - "ETW"            - 'classic' Trace Logging Dynamic manifest ETW events.
-   * - "MSGPACK"        - MessagePack-encoded binary payload ETW events.
-   * - "XML"            - XML events (reserved for future use)
+   * @param args Instrumentation version (unused by ETW).
+   * @param schema_url Schema URL (unused by ETW).
+   *
+   * Encoding is controlled via TelemetryProviderOptions["encoding"] passed
+   * to the TracerProvider constructor. Valid values: "ETW"/"TLD" (default),
+   * "MSGPACK", "XML".
    * @return
    */
   nostd::shared_ptr<opentelemetry::trace::Tracer> GetTracer(
@@ -1177,10 +1183,54 @@ class TracerProvider : public opentelemetry::trace::TracerProvider
     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};
+    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