From ca6140ee4d7d854f077b864ffce7cfcf9ab253c9 Mon Sep 17 00:00:00 2001 From: Isaac Hier Date: Sat, 28 Apr 2018 23:21:24 -0400 Subject: [PATCH] Add more baggage fixes and coverage Signed-off-by: Isaac Hier --- src/jaegertracing/SpanContext.h | 19 +++--- src/jaegertracing/TracerTest.cpp | 63 ++++++++++++++++++- src/jaegertracing/propagation/Propagator.h | 17 ++--- .../propagation/PropagatorTest.cpp | 2 +- 4 files changed, 84 insertions(+), 17 deletions(-) diff --git a/src/jaegertracing/SpanContext.h b/src/jaegertracing/SpanContext.h index bf8557e3..df78400a 100644 --- 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 lock(_mutex, std::adopt_lock); + std::lock(lhs._mutex, rhs._mutex); + std::lock_guard lhsLock(lhs._mutex, std::adopt_lock); std::lock_guard 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 index 529dff59..c072d9b5 100644 --- 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(result->release())); ASSERT_TRUE(static_cast(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(result)); + extractedCtx.reset(static_cast(result->release())); + ASSERT_TRUE(static_cast(extractedCtx)); + ASSERT_NE(span->context(), *extractedCtx); + SpanContext ctx( + span->context().traceID(), + span->context().spanID(), + span->context().parentID(), + span->context().flags() | + static_cast(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(result)); + extractedCtx.reset(static_cast(result->release())); + ASSERT_EQ(nullptr, extractedCtx.get()); + + // Test empty map. + headerMap.clear(); + result = tracer->Extract(headerReader); + ASSERT_TRUE(static_cast(result)); + extractedCtx.reset(static_cast(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(result)); + extractedCtx.reset(static_cast(result->release())); + ASSERT_TRUE(static_cast(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 index ec463678..bf04be81 100644 --- a/src/jaegertracing/propagation/Propagator.h +++ b/src/jaegertracing/propagation/Propagator.h @@ -69,7 +69,7 @@ class Propagator : public Extractor, public Injector { 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( opentracing::span_context_corrupted_error); } @@ -104,10 +104,15 @@ class Propagator : public Extractor, public Injector { return SpanContext(); } + int flags = ctx.flags(); + if (!debugID.empty()) { + flags |= static_cast(SpanContext::Flag::kDebug) | + static_cast(SpanContext::Flag::kSampled); + } return SpanContext(ctx.traceID(), ctx.spanID(), ctx.parentID(), - ctx.flags(), + flags, baggage, debugID); } @@ -146,11 +151,9 @@ class Propagator : public Extractor, public Injector { static StrMap parseCommaSeparatedMap(const std::string& escapedValue) { StrMap map; - const auto value = net::URI::queryUnescape(escapedValue); - for (auto pos = value.find(','), prev = static_cast(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 index 8f895158..05fc44c4 100644 --- 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 #include #include