Add more baggage fixes and coverage

Signed-off-by: Isaac Hier <[email protected]>
jaegertracing · Apr 29, 2018 · ca6140e · ca6140e
1 parent 321a365
commit ca6140e
Show file tree

Hide file tree

Showing 4 changed files with 84 additions and 17 deletions.
diff --git a/src/jaegertracing/SpanContext.h b/src/jaegertracing/SpanContext.h
@@ -164,19 +164,24 @@ class SpanContext : public opentracing::SpanContext {
         forEachBaggageItem(f);
     }
 
-    bool operator==(const SpanContext& rhs) const
+    friend bool operator==(const SpanContext& lhs, const SpanContext& rhs)
     {
         {
-            std::lock(_mutex, rhs._mutex);
-            std::lock_guard<std::mutex> lock(_mutex, std::adopt_lock);
+            std::lock(lhs._mutex, rhs._mutex);
+            std::lock_guard<std::mutex> lhsLock(lhs._mutex, std::adopt_lock);
             std::lock_guard<std::mutex> rhsLock(rhs._mutex, std::adopt_lock);
-            if (_baggage != rhs._baggage) {
+            if (lhs._baggage != rhs._baggage) {
                 return false;
             }
         }
-        return _traceID == rhs._traceID && _spanID == rhs._spanID &&
-               _parentID == rhs._parentID && _flags == rhs._flags &&
-               _debugID == rhs._debugID;
+        return lhs._traceID == rhs._traceID && lhs._spanID == rhs._spanID &&
+               lhs._parentID == rhs._parentID && lhs._flags == rhs._flags &&
+               lhs._debugID == rhs._debugID;
+    }
+
+    friend bool operator!=(const SpanContext& lhs, const SpanContext& rhs)
+    {
+        return !operator==(lhs, rhs);
     }
 
   private:

diff --git a/src/jaegertracing/TracerTest.cpp b/src/jaegertracing/TracerTest.cpp
@@ -14,13 +14,13 @@
  * limitations under the License.
  */
 
+#include "jaegertracing/Tracer.h"
 #include "jaegertracing/Config.h"
 #include "jaegertracing/Constants.h"
 #include "jaegertracing/Span.h"
 #include "jaegertracing/SpanContext.h"
 #include "jaegertracing/Tag.h"
 #include "jaegertracing/TraceID.h"
-#include "jaegertracing/Tracer.h"
 #include "jaegertracing/baggage/RestrictionsConfig.h"
 #include "jaegertracing/net/IPAddress.h"
 #include "jaegertracing/propagation/HeadersConfig.h"
@@ -181,7 +181,7 @@ TEST(Tracer, testTracer)
     span->Log({ { "log-bool", true } });
     opentracing::FinishSpanOptions foptions;
     opentracing::LogRecord lr{};
-    lr.fields = { {"options-log", "yep"} };
+    lr.fields = { { "options-log", "yep" } };
     foptions.log_records.push_back(std::move(lr));
     lr.timestamp = opentracing::SystemClock::now();
     span->FinishWithOptions(foptions);
@@ -337,6 +337,65 @@ TEST(Tracer, testPropagation)
             static_cast<SpanContext*>(result->release()));
         ASSERT_TRUE(static_cast<bool>(extractedCtx));
         ASSERT_EQ(span->context(), *extractedCtx);
+
+        // Test debug header.
+        headerMap.clear();
+        headerMap[kJaegerDebugHeader] = "yes";
+        tracer->Inject(span->context(), headerWriter);
+        result = tracer->Extract(headerReader);
+        ASSERT_TRUE(static_cast<bool>(result));
+        extractedCtx.reset(static_cast<SpanContext*>(result->release()));
+        ASSERT_TRUE(static_cast<bool>(extractedCtx));
+        ASSERT_NE(span->context(), *extractedCtx);
+        SpanContext ctx(
+            span->context().traceID(),
+            span->context().spanID(),
+            span->context().parentID(),
+            span->context().flags() |
+                static_cast<unsigned char>(SpanContext::Flag::kDebug),
+            span->context().baggage(),
+            "yes");
+        ASSERT_EQ(ctx, *extractedCtx);
+
+        // Test bad trace context.
+        headerMap.clear();
+        headerMap[kTraceContextHeaderName] = "12345678";
+        result = tracer->Extract(headerReader);
+        ASSERT_TRUE(static_cast<bool>(result));
+        extractedCtx.reset(static_cast<SpanContext*>(result->release()));
+        ASSERT_EQ(nullptr, extractedCtx.get());
+
+        // Test empty map.
+        headerMap.clear();
+        result = tracer->Extract(headerReader);
+        ASSERT_TRUE(static_cast<bool>(result));
+        extractedCtx.reset(static_cast<SpanContext*>(result->release()));
+        ASSERT_EQ(nullptr, extractedCtx.get());
+
+        // Test alternative baggage format.
+        headerMap.clear();
+        ctx = SpanContext(span->context().traceID(),
+                          span->context().spanID(),
+                          span->context().parentID(),
+                          span->context().flags(),
+                          { { "a", "x" }, { "b", "y" }, { "c", "z" } });
+        tracer->Inject(ctx, headerWriter);
+        for (auto itr = std::begin(headerMap); itr != std::end(headerMap);) {
+            if (itr->first.substr(0, std::strlen(kTraceBaggageHeaderPrefix)) ==
+                kTraceBaggageHeaderPrefix) {
+                itr = headerMap.erase(itr);
+            }
+            else {
+                ++itr;
+            }
+        }
+        headerMap[kJaegerBaggageHeader] = "a=x,b=y,c=z";
+        result = tracer->Extract(headerReader);
+        ASSERT_TRUE(static_cast<bool>(result));
+        extractedCtx.reset(static_cast<SpanContext*>(result->release()));
+        ASSERT_TRUE(static_cast<bool>(extractedCtx));
+        ASSERT_EQ(3, extractedCtx->baggage().size());
+        ASSERT_EQ(ctx, *extractedCtx);
     }
 }
 

diff --git a/src/jaegertracing/propagation/Propagator.h b/src/jaegertracing/propagation/Propagator.h
@@ -69,7 +69,7 @@ class Propagator : public Extractor<ReaderType>, public Injector<WriterType> {
                 if (key == _headerKeys.traceContextHeaderName()) {
                     const auto safeValue = decodeValue(value);
                     std::istringstream iss(safeValue);
-                    if (!(iss >> ctx)) {
+                    if (!(iss >> ctx) || ctx == SpanContext()) {
                         return opentracing::make_expected_from_error<void>(
                             opentracing::span_context_corrupted_error);
                     }
@@ -104,10 +104,15 @@ class Propagator : public Extractor<ReaderType>, public Injector<WriterType> {
             return SpanContext();
         }
 
+        int flags = ctx.flags();
+        if (!debugID.empty()) {
+            flags |= static_cast<unsigned char>(SpanContext::Flag::kDebug) |
+                     static_cast<unsigned char>(SpanContext::Flag::kSampled);
+        }
         return SpanContext(ctx.traceID(),
                            ctx.spanID(),
                            ctx.parentID(),
-                           ctx.flags(),
+                           flags,
                            baggage,
                            debugID);
     }
@@ -146,11 +151,9 @@ class Propagator : public Extractor<ReaderType>, public Injector<WriterType> {
     static StrMap parseCommaSeparatedMap(const std::string& escapedValue)
     {
         StrMap map;
-        const auto value = net::URI::queryUnescape(escapedValue);
-        for (auto pos = value.find(','), prev = static_cast<size_t>(0);
-             pos != std::string::npos;
-             prev = pos, pos = value.find(',', pos + 1)) {
-            const auto piece = value.substr(prev, pos);
+        std::istringstream iss(net::URI::queryUnescape(escapedValue));
+        std::string piece;
+        while (std::getline(iss, piece, ',')) {
             const auto eqPos = piece.find('=');
             if (eqPos != std::string::npos) {
                 const auto key = piece.substr(0, eqPos);

diff --git a/src/jaegertracing/propagation/PropagatorTest.cpp b/src/jaegertracing/propagation/PropagatorTest.cpp
@@ -14,9 +14,9 @@
  * limitations under the License.
  */
 
+#include "jaegertracing/propagation/Propagator.h"
 #include "jaegertracing/SpanContext.h"
 #include "jaegertracing/TraceID.h"
-#include "jaegertracing/propagation/Propagator.h"
 #include <gtest/gtest.h>
 #include <iosfwd>
 #include <random>