Better code for setting and getting traceids and spanids from EDNS

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

diff --git a/pdns/dnsdistdist/dnsdist-actions-factory.cc b/pdns/dnsdistdist/dnsdist-actions-factory.cc
index e8f5d7af20fc2782766d02af1aafbef02aa63343..07e8ea9b19d722fd3135953eb459df80fb0e7edc 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-actions-factory.cc
+++ b/pdns/dnsdistdist/dnsdist-actions-factory.cc
@@ -1521,7 +1521,7 @@ public:
 
     auto [protocol, httpProtocol] = ProtocolToDNSTap(dnsquestion->getProtocol());
     // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
-    DnstapMessagemessage(std::move(data), !dnsquestion->getHeader()->qr ? DnstapMessage::MessageType::client_query : DnstapMessage::MessageType::client_response, d_identity, &dnsquestion->ids.origRemote, &dnsquestion->ids.origDest, protocol, reinterpret_cast<const char*>(dnsquestion->getData().data()), dnsquestion->getData().size(), &dnsquestion->getQueryRealTime(), nullptr, boost::none, httpProtocol);
+    DnstapMessage message(std::move(data), !dnsquestion->getHeader()->qr ? DnstapMessage::MessageType::client_query : DnstapMessage::MessageType::client_response, d_identity, &dnsquestion->ids.origRemote, &dnsquestion->ids.origDest, protocol, reinterpret_cast<const char*>(dnsquestion->getData().data()), dnsquestion->getData().size(), &dnsquestion->getQueryRealTime(), nullptr, boost::none, httpProtocol);
     {
       if (d_alterFunc) {
         auto lock = g_lua.lock();

diff --git a/pdns/protozero-trace.cc b/pdns/protozero-trace.cc
index c4b4a7228561aee780bab3e461237c532c59793e..5b1fa712f4817e7cdabac274027d69c50ad5da33 100644 (file)
--- a/pdns/protozero-trace.cc
+++ b/pdns/protozero-trace.cc
@@ -512,18 +512,17 @@ void extractOTraceIDs(const EDNSOptionViewMap& map, pdns::trace::InitialSpanInfo
   // parent_span_id gets set from edns options (if available and well-formed, otherwise it remains cleared (no parent))
   // span_id gets inited randomly
   bool traceidset = false;
-  const auto traceIDSize = span.trace_id.size();
 
   if (const auto& option = map.find(EDNSOptionCode::OTTRACEIDS); option != map.end()) {
-    // 1 byte version, then tracid  then optinal spanid
-    if (option->second.values.size() > 0) {
-      if (option->second.values.at(0).size >= 1 + traceIDSize) {
+    const auto& value = option->second.values.at(0);
+    const EDNSOTTraceRecordView data{reinterpret_cast<const uint8_t*>(value.content), value.size}; // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast)
+    // 1 byte version, then tracid then optional spanid
+    uint8_t version{};
+    if (data.getVersion(version) && version == 0) {
+      if (data.getTraceID(span.trace_id)) {
         traceidset = true;
-        pdns::trace::fill(span.trace_id, &option->second.values.at(0).content[1], traceIDSize);  // NOLINT(cppcoreguidelines-pro-bounds-pointer-arithmetic) it's the API
-      }
-      if (option->second.values.at(0).size == 1 + traceIDSize + span.parent_span_id.size()) {
-        pdns::trace::fill(span.parent_span_id, &option->second.values.at(0).content[traceIDSize + 1], span.parent_span_id.size()); // NOLINT(cppcoreguidelines-pro-bounds-pointer-arithmetic) it's the API
       }
+      (void)data.getSpanID(span.parent_span_id);
     }
   }
   if (!traceidset) {

diff --git a/pdns/protozero-trace.hh b/pdns/protozero-trace.hh
index 1c1250adb69ccec424e865f3ad65bdbb3c956ffb..a66328655e2647387f07c0e492cda315a121c0d9 100644 (file)
--- a/pdns/protozero-trace.hh
+++ b/pdns/protozero-trace.hh
@@ -712,6 +712,70 @@ inline KeyValueList KeyValueList::decode(protozero::pbf_reader& reader)
   return pdns::trace::decode<KeyValueList, KeyValue>(reader);
 }
 
+struct EDNSOTTraceRecord
+{
+  // 1 byte version 16 bytes traceid, optional 8 bytes spanid
+  static constexpr size_t fullSize = 1 + 16 + 8;
+  static constexpr size_t sizeNoSpanID = 1 + 16;
+  static constexpr size_t traceIDOffset = 1;
+  static constexpr size_t spanIDOffset = 1 + 16;
+
+  EDNSOTTraceRecord(uint8_t* arg) :
+    data(arg) {}
+  // NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+  void setVersion(uint8_t version)
+  {
+    data[0] = version;
+  }
+  void setTraceID(const TraceID& traceid)
+  {
+    std::copy(traceid.begin(), traceid.end(), &data[traceIDOffset]);
+  }
+  void setSpanID(const SpanID& spanid)
+  {
+    std::copy(spanid.begin(), spanid.end(), &data[spanIDOffset]);
+  }
+  // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+private:
+  uint8_t* data;
+};
+
+struct EDNSOTTraceRecordView
+{
+  EDNSOTTraceRecordView(const uint8_t* arg, size_t argsize) :
+    data(arg), size(argsize) {}
+
+  // NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+  [[nodiscard]] bool getVersion(uint8_t& version) const
+  {
+    if (size > 0) {
+      version = data[0];
+      return true;
+    }
+    return false;
+  }
+  [[nodiscard]] bool getTraceID(TraceID& traceid) const
+  {
+    if (size >= pdns::trace::EDNSOTTraceRecord::sizeNoSpanID) {
+      std::copy(&data[EDNSOTTraceRecord::traceIDOffset], &data[EDNSOTTraceRecord::traceIDOffset + traceid.size()], traceid.begin());
+      return true;
+    }
+    return false;
+  }
+  [[nodiscard]] bool getSpanID(SpanID& spanid) const
+  {
+    if (size == pdns::trace::EDNSOTTraceRecord::fullSize) {
+      std::copy(&data[EDNSOTTraceRecord::spanIDOffset], &data[EDNSOTTraceRecord::spanIDOffset + spanid.size()], spanid.begin());
+      return true;
+    }
+    return false;
+  }
+  // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+private:
+  const uint8_t* const data;
+  const size_t size;
+};
+
 void extractOTraceIDs(const EDNSOptionViewMap& map, pdns::trace::InitialSpanInfo& span);
 
 } // namespace pdns::trace

diff --git a/pdns/sdig.cc b/pdns/sdig.cc
index 98cbc91c1029ec28e517c21a7c01f9dc94bc7008..887dafb38eca6bdf9b465a8d51eeef22e14a3acb 100644 (file)
--- a/pdns/sdig.cc
+++ b/pdns/sdig.cc
@@ -97,10 +97,11 @@ static void fillPacket(vector<uint8_t>& packet, const string& q, const string& t
     if (otids) {
       const auto traceid = otids->first;
       const auto spanid = otids->second;
-      std::array<uint8_t, 1 + traceid.size() + spanid.size()> data{};
-      data.at(0) = 0; // version
-      std::copy(traceid.begin(), traceid.end(), &data.at(1));
-      std::copy(spanid.begin(), spanid.end(), &data.at(1 + traceid.size()));
+      std::array<uint8_t, pdns::trace::EDNSOTTraceRecord::fullSize> data{};
+      pdns::trace::EDNSOTTraceRecord record{data.data()};
+      record.setVersion(0);
+      record.setTraceID(traceid);
+      record.setSpanID(spanid);
       opts.emplace_back(EDNSOptionCode::OTTRACEIDS, std::string_view(reinterpret_cast<const char*>(data.data()), data.size())); // NOLINT
     }
     pw.addOpt(bufsize, 0, dnssec ? EDNSOpts::DNSSECOK : 0, opts);