dnsdist: Prevent copies of OT closers

Moving them is OK, duplicating them isn't otherwise we might close
the same span several times which is bad.

Signed-off-by: Remi Gacogne <remi.gacogne@powerdns.com>

diff --git a/pdns/dnsdistdist/dnsdist-idstate.cc b/pdns/dnsdistdist/dnsdist-idstate.cc
index f9e21983be61fad0c48cf0057e2a3bc42307468d..cdf09e5821e13fb312e1141f349cb1599d5e953c 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-idstate.cc
+++ b/pdns/dnsdistdist/dnsdist-idstate.cc
@@ -112,13 +112,13 @@ InternalQueryState::~InternalQueryState()
 
 std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getCloser([[maybe_unused]] const std::string_view& name, [[maybe_unused]] const SpanID& parentSpanID)
 {
-  std::optional<pdns::trace::dnsdist::Tracer::Closer> ret(std::nullopt);
+  std::optional<pdns::trace::dnsdist::Tracer::Closer> ret{std::nullopt};
 #ifndef DISABLE_PROTOBUF
   // getTracer returns a Tracer when tracing is globally enabled
   // tracingEnabled tells us whether or not tracing is enabled for this query
   // Should tracing be disabled, *but* we have not processed query rules, we will still return a closer if tracing is globally enabled
   if (auto tracer = getTracer(); tracer != nullptr && (tracingEnabled || !rulesAppliedToQuery)) {
-    ret = std::optional<pdns::trace::dnsdist::Tracer::Closer>(d_OTTracer->openSpan(std::string(name), parentSpanID));
+    ret = d_OTTracer->openSpan(std::string(name), parentSpanID);
   }
 #endif
   return ret;
@@ -126,11 +126,11 @@ std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getClose
 
 std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getCloser([[maybe_unused]] const std::string_view& name, [[maybe_unused]] const std::string_view& parentSpanName)
 {
-  std::optional<pdns::trace::dnsdist::Tracer::Closer> ret(std::nullopt);
+  std::optional<pdns::trace::dnsdist::Tracer::Closer> ret{std::nullopt};
 #ifndef DISABLE_PROTOBUF
   if (auto tracer = getTracer(); tracer != nullptr) {
     auto parentSpanID = d_OTTracer->getLastSpanIDForName(std::string(parentSpanName));
-    return getCloser(name, parentSpanID);
+    ret = getCloser(name, parentSpanID);
   }
 #endif
   return ret;
@@ -138,10 +138,10 @@ std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getClose
 
 std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getCloser([[maybe_unused]] const std::string_view& name)
 {
-  std::optional<pdns::trace::dnsdist::Tracer::Closer> ret(std::nullopt);
+  std::optional<pdns::trace::dnsdist::Tracer::Closer> ret{std::nullopt};
 #ifndef DISABLE_PROTOBUF
   if (auto tracer = getTracer(); tracer != nullptr) {
-    return getCloser(std::string(name), tracer->getLastSpanID());
+    ret = getCloser(std::string(name), tracer->getLastSpanID());
   }
 #endif
   return ret;
@@ -149,7 +149,7 @@ std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getClose
 
 std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getRulesCloser([[maybe_unused]] const std::string_view& ruleName, [[maybe_unused]] const std::string& ruleType)
 {
-  std::optional<pdns::trace::dnsdist::Tracer::Closer> ret(std::nullopt);
+  std::optional<pdns::trace::dnsdist::Tracer::Closer> ret{std::nullopt};
 #ifndef DISABLE_PROTOBUF
   static const std::string prefix = "Rule: ";
   // getTracer returns a Tracer when tracing is globally enabled
@@ -158,7 +158,7 @@ std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getRules
   if (auto tracer = getTracer(); tracer != nullptr && (tracingEnabled || !rulesAppliedToQuery)) {
     auto parentSpanID = tracer->getLastSpanID();
     auto name = ruleType + prefix + std::string(ruleName);
-    ret = std::optional<pdns::trace::dnsdist::Tracer::Closer>(tracer->openSpan(name, parentSpanID));
+    ret = tracer->openSpan(name, parentSpanID);
   }
 #endif
   return ret;

diff --git a/pdns/dnsdistdist/dnsdist-opentelemetry.hh b/pdns/dnsdistdist/dnsdist-opentelemetry.hh
index 138f4a7efb2a463c747d4c3d92b3207bc622bcc9..db1bc69c73f53d7cbc606467139e912b33c18f1b 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-opentelemetry.hh
+++ b/pdns/dnsdistdist/dnsdist-opentelemetry.hh
@@ -214,11 +214,30 @@ public:
         d_tracer->closeSpan(d_spanID);
       }
 #endif
-    };
-    Closer(const Closer&) = default;
-    Closer& operator=(const Closer&) = default;
-    Closer& operator=(Closer&&) noexcept = default;
-    Closer(Closer&&) = default;
+    }
+    Closer(const Closer&) = delete;
+    Closer& operator=(const Closer&) = delete;
+    Closer& operator=(Closer&& rhs) noexcept
+    {
+#ifndef DISABLE_PROTOBUF
+      this->d_tracer = std::move(rhs.d_tracer);
+      this->d_spanID = rhs.d_spanID;
+      /* we wouldn't want to close it twice */
+      rhs.d_tracer.reset();
+      rhs.d_spanID.clear();
+#endif
+      return *this;
+    }
+    Closer(Closer&& rhs)
+    {
+#ifndef DISABLE_PROTOBUF
+      this->d_tracer = std::move(rhs.d_tracer);
+      this->d_spanID = rhs.d_spanID;
+      /* we wouldn't want to close it twice */
+      rhs.d_tracer.reset();
+      rhs.d_spanID.clear();
+#endif
+    }
 
     /**
      * @brief Get the SpanID