Improve performance of span context propagation

cmake/Modules/LightStepClangTidy.cmake

@@ -13,9 +13,14 @@ else()
 -google-build-using-namespace,\
 -modernize-make-unique,\
 -hicpp-vararg,\
+-hicpp-signed-bitwise,\
+-hicpp-no-array-decay,\
 -cppcoreguidelines-owning-memory,\
 -cppcoreguidelines-pro-type-reinterpret-cast,\
 -cppcoreguidelines-pro-type-const-cast,\
+-cppcoreguidelines-pro-bounds-array-to-pointer-decay,\
+-cppcoreguidelines-pro-bounds-constant-array-index,\
+-cppcoreguidelines-pro-bounds-pointer-arithmetic,\
 -cppcoreguidelines-pro-type-vararg;\
 -warnings-as-errors=*")
 endif()

src/propagation.cpp

@@ -9,6 +9,7 @@
 #include <sstream>
 #include "in_memory_stream.h"
 #include "lightstep-tracer-common/lightstep_carrier.pb.h"
+#include "utility.h"
 
 namespace lightstep {
 #define PREFIX_TRACER_STATE "ot-tracer-"
@@ -22,26 +23,9 @@ const opentracing::string_view FieldNameSampled = PREFIX_TRACER_STATE "sampled";
 #undef PREFIX_TRACER_STATE
 
 const opentracing::string_view PropagationSingleKey = "x-ot-span-context";
-
-//------------------------------------------------------------------------------
-// Uint64ToHex
-//------------------------------------------------------------------------------
-static std::string Uint64ToHex(uint64_t u) {
-  std::ostringstream stream;
-  stream << std::setfill('0') << std::setw(16) << std::hex << u;
-  return stream.str();
-}
-
-//------------------------------------------------------------------------------
-// HexToUint64
-//------------------------------------------------------------------------------
-static uint64_t HexToUint64(opentracing::string_view s) {
-  in_memory_stream stream{s.data(), s.size()};
-  uint64_t x;
-  stream >> std::setw(16) >> std::hex >> x;
-  return x;
-}
-
+const opentracing::string_view TrueStr = "true";
+const opentracing::string_view FalseStr = "false";
+const opentracing::string_view ZeroStr = "0";
 //------------------------------------------------------------------------------
 // LookupKey
 //------------------------------------------------------------------------------
@@ -76,48 +60,58 @@ static opentracing::expected<opentracing::string_view> LookupKey(
 }
 
 //------------------------------------------------------------------------------
-// InjectSpanContextMultiKey
+// InjectSpanContextBaggage
 //------------------------------------------------------------------------------
-static opentracing::expected<void> InjectSpanContextMultiKey(
-    const opentracing::TextMapWriter& carrier, uint64_t trace_id,
-    uint64_t span_id, bool sampled, const BaggageMap& baggage) {
-  std::string trace_id_hex, span_id_hex, baggage_key;
+static opentracing::expected<void> InjectSpanContextBaggage(
+    const opentracing::TextMapWriter& carrier, const BaggageMap& baggage) {
+  std::string baggage_key;
   try {
-    trace_id_hex = Uint64ToHex(trace_id);
-    span_id_hex = Uint64ToHex(span_id);
     baggage_key = PrefixBaggage;
   } catch (const std::bad_alloc&) {
     return opentracing::make_unexpected(
         std::make_error_code(std::errc::not_enough_memory));
   }
-  auto result = carrier.Set(FieldNameTraceID, trace_id_hex);
+  for (const auto& baggage_item : baggage) {
+    try {
+      baggage_key.replace(std::begin(baggage_key) + PrefixBaggage.size(),
+                          std::end(baggage_key), baggage_item.first);
+    } catch (const std::bad_alloc&) {
+      return opentracing::make_unexpected(
+          std::make_error_code(std::errc::not_enough_memory));
+    }
+    auto result = carrier.Set(baggage_key, baggage_item.second);
+    if (!result) {
+      return result;
+    }
+  }
+  return {};
+}
+
+//------------------------------------------------------------------------------
+// InjectSpanContextMultiKey
+//------------------------------------------------------------------------------
+static opentracing::expected<void> InjectSpanContextMultiKey(
+    const opentracing::TextMapWriter& carrier, uint64_t trace_id,
+    uint64_t span_id, bool sampled, const BaggageMap& baggage) {
+  char data[16];
+  auto result = carrier.Set(FieldNameTraceID, Uint64ToHex(trace_id, data));
   if (!result) {
     return result;
   }
-  result = carrier.Set(FieldNameSpanID, span_id_hex);
+  result = carrier.Set(FieldNameSpanID, Uint64ToHex(span_id, data));
   if (!result) {
     return result;
   }
   if (sampled) {
-    result = carrier.Set(FieldNameSampled, "true");
+    result = carrier.Set(FieldNameSampled, TrueStr);
   } else {
-    result = carrier.Set(FieldNameSampled, "false");
+    result = carrier.Set(FieldNameSampled, FalseStr);
   }
   if (!result) {
     return result;
   }
-  for (const auto& baggage_item : baggage) {
-    try {
-      baggage_key.replace(std::begin(baggage_key) + PrefixBaggage.size(),
-                          std::end(baggage_key), baggage_item.first);
-    } catch (const std::bad_alloc&) {
-      return opentracing::make_unexpected(
-          std::make_error_code(std::errc::not_enough_memory));
-    }
-    result = carrier.Set(baggage_key, baggage_item.second);
-    if (!result) {
-      return result;
-    }
+  if (!baggage.empty()) {
+    return InjectSpanContextBaggage(carrier, baggage);
   }
   return {};
 }
@@ -224,13 +218,23 @@ static opentracing::expected<bool> ExtractSpanContextMultiKey(
                                        -> opentracing::expected<void> {
     try {
       if (key_compare(key, FieldNameTraceID)) {
-        trace_id = HexToUint64(value);
+        auto trace_id_maybe = HexToUint64(value);
+        if (!trace_id_maybe) {
+          return opentracing::make_unexpected(
+              opentracing::span_context_corrupted_error);
+        }
+        trace_id = *trace_id_maybe;
         count++;
       } else if (key_compare(key, FieldNameSpanID)) {
-        span_id = HexToUint64(value);
+        auto span_id_maybe = HexToUint64(value);
+        if (!span_id_maybe) {
+          return opentracing::make_unexpected(
+              opentracing::span_context_corrupted_error);
+        }
+        span_id = *span_id_maybe;
         count++;
       } else if (key_compare(key, FieldNameSampled)) {
-        sampled = !(value == "false" || value == "0");
+        sampled = !(value == FalseStr || value == ZeroStr);
         count++;
       } else if (key.length() > PrefixBaggage.size() &&
                  key_compare(

src/utility.cpp

@@ -5,11 +5,14 @@
 #include "lightstep-tracer-common/collector.pb.h"
 
 #include <unistd.h>
+#include <array>
+#include <cctype>
 #include <cmath>
 #include <iomanip>
 #include <random>
 #include <sstream>
 #include <stdexcept>
+#include <system_error>
 
 #include <pthread.h>
 
@@ -264,4 +267,114 @@ void LogReportResponse(Logger& logger, bool verbose,
     }
   }
 }
+
+//------------------------------------------------------------------------------
+// Uint64ToHex
+//------------------------------------------------------------------------------
+// This uses the lookup table solution described on this blog post
+// https://johnnylee-sde.github.io/Fast-unsigned-integer-to-hex-string/
+opentracing::string_view Uint64ToHex(uint64_t x, char* output) {
+  static const unsigned char digits[513] =
+      "000102030405060708090A0B0C0D0E0F"
+      "101112131415161718191A1B1C1D1E1F"
+      "202122232425262728292A2B2C2D2E2F"
+      "303132333435363738393A3B3C3D3E3F"
+      "404142434445464748494A4B4C4D4E4F"
+      "505152535455565758595A5B5C5D5E5F"
+      "606162636465666768696A6B6C6D6E6F"
+      "707172737475767778797A7B7C7D7E7F"
+      "808182838485868788898A8B8C8D8E8F"
+      "909192939495969798999A9B9C9D9E9F"
+      "A0A1A2A3A4A5A6A7A8A9AAABACADAEAF"
+      "B0B1B2B3B4B5B6B7B8B9BABBBCBDBEBF"
+      "C0C1C2C3C4C5C6C7C8C9CACBCCCDCECF"
+      "D0D1D2D3D4D5D6D7D8D9DADBDCDDDEDF"
+      "E0E1E2E3E4E5E6E7E8E9EAEBECEDEEEF"
+      "F0F1F2F3F4F5F6F7F8F9FAFBFCFDFEFF";
+  for (int i = 8; i-- > 0;) {
+    auto lookup_index = (x & 0xFF) * 2;
+    output[i * 2] = digits[lookup_index];
+    output[i * 2 + 1] = digits[lookup_index + 1];
+    x >>= 8;
+  }
+  return {output, 16};
+}
+
+//------------------------------------------------------------------------------
+// HexToUint64
+//------------------------------------------------------------------------------
+// Adopted from https://stackoverflow.com/a/11068850/4447365
+opentracing::expected<uint64_t> HexToUint64(opentracing::string_view s) {
+  static const unsigned char nil = std::numeric_limits<unsigned char>::max();
+  static const std::array<unsigned char, 256> hextable = {
+      {nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil,
+       nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil,
+       nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil,
+       nil, nil, nil, nil, nil, nil, 0,   1,   2,   3,   4,   5,   6,   7,
+       8,   9,   nil, nil, nil, nil, nil, nil, nil, 10,  11,  12,  13,  14,
+       15,  nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil,
+       nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, 10,
+       11,  12,  13,  14,  15,  nil, nil, nil, nil, nil, nil, nil, nil, nil,
+       nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil,
+       nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil,
+       nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil,
+       nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil,
+       nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil,
+       nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil,
+       nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil,
+       nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil,
+       nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil,
+       nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil,
+       nil, nil, nil, nil}};
+
+  auto i = s.data();
+  auto last = s.data() + s.size();
+
+  // Remove any leading spaces
+  while (i != last && static_cast<bool>(std::isspace(*i))) {
+    ++i;
+  }
+
+  // Remove any trailing spaces
+  while (i != last && static_cast<bool>(std::isspace(*(last - 1)))) {
+    --last;
+  }
+
+  auto first = i;
+
+  // Remove leading zeros
+  while (i != last && *i == '0') {
+    ++i;
+  }
+
+  auto length = std::distance(i, last);
+
+  // Check for overflow
+  if (length > 16) {
+    return opentracing::make_unexpected(
+        std::make_error_code(std::errc::value_too_large));
+  }
+
+  // Check for an empty string
+  if (length == 0) {
+    // Handle the case of the string being all zeros
+    if (first != i) {
+      return 0;
+    }
+    return opentracing::make_unexpected(
+        std::make_error_code(std::errc::invalid_argument));
+  }
+
+  uint64_t result = 0;
+  for (; i != last; ++i) {
+    auto value = hextable[*i];
+    if (value == nil) {
+      return opentracing::make_unexpected(
+          std::make_error_code(std::errc::invalid_argument));
+    }
+    result = (result << 4) | value;
+  }
+
+  return result;
+}
 }  // namespace lightstep

src/utility.h

@@ -28,4 +28,12 @@ collector::KeyValue ToKeyValue(opentracing::string_view key,
 // Logs any information returned by the collector.
 void LogReportResponse(Logger& logger, bool verbose,
                        const collector::ReportResponse& response);
+
+// Converts `x` to a hexidecimal, writes the results into `output` and returns
+// a string_view of the number.
+opentracing::string_view Uint64ToHex(uint64_t x, char* output);
+
+// Converts a hexidecimal number to a 64-bit integer. Either returns the number
+// or an error code.
+opentracing::expected<uint64_t> HexToUint64(opentracing::string_view s);
 }  // namespace lightstep

test/utility_test.cpp

@@ -55,3 +55,59 @@ TEST_CASE("Json") {
     CHECK(key_value1.json_value() == "[null]");
   }
 }
+
+TEST_CASE("hex-integer conversions") {
+  char data[16];
+
+  SECTION("Verify hex conversion and back against a range of values.") {
+    for (uint64_t x = 0; x < 1000; ++x) {
+      CHECK(x == *HexToUint64(Uint64ToHex(x, data)));
+      auto y = std::numeric_limits<uint64_t>::max() - x;
+      CHECK(y == *HexToUint64(Uint64ToHex(y, data)));
+    }
+  }
+
+  SECTION("Verify a few special values.") {
+    CHECK(Uint64ToHex(0, data) == "0000000000000000");
+    CHECK(Uint64ToHex(1, data) == "0000000000000001");
+    CHECK(Uint64ToHex(std::numeric_limits<uint64_t>::max(), data) ==
+          "FFFFFFFFFFFFFFFF");
+
+    CHECK(*HexToUint64("0") == 0);
+    CHECK(*HexToUint64("1") == 1);
+    CHECK(*HexToUint64("FFFFFFFFFFFFFFFF") ==
+          std::numeric_limits<uint64_t>::max());
+  }
+
+  SECTION("Leading or trailing spaces are ignored when converting from hex.") {
+    CHECK(*HexToUint64("  \tabc") == 0xabc);
+    CHECK(*HexToUint64("abc  \t") == 0xabc);
+    CHECK(*HexToUint64("  \tabc  \t") == 0xabc);
+  }
+
+  SECTION("Hex conversion works with both upper and lower case digits.") {
+    CHECK(*HexToUint64("ABCDEF") == 0xABCDEF);
+    CHECK(*HexToUint64("abcdef") == 0xABCDEF);
+  }
+
+  SECTION("Hex conversion with an empty string gives an error.") {
+    CHECK(!HexToUint64(""));
+    CHECK(!HexToUint64("  "));
+  }
+
+  SECTION(
+      "Hex conversion of a number bigger than "
+      "std::numeric_limits<uint64_t>::max() gives an error.") {
+    CHECK(!HexToUint64("1123456789ABCDEF1"));
+  }
+
+  SECTION(
+      "Hex conversion of a number within valid limits but with leading zeros "
+      "past 16 digits is successful.") {
+    CHECK(HexToUint64("0123456789ABCDEF1"));
+  }
+
+  SECTION("Hex conversion with invalid digits gives an error.") {
+    CHECK(!HexToUint64("abcHef"));
+  }
+}