From bcdbbcc7ad5dc2d19dec505c1f289dad9319718d Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 18 Nov 2019 21:39:46 -0800 Subject: [PATCH 01/42] Work on trace-context propagator. --- src/tracer/immutable_span_context.h | 1 + src/tracer/lightstep_span_context.h | 4 ++ src/tracer/propagation/BUILD | 15 ++++++ src/tracer/propagation/trace_context.cpp | 4 ++ src/tracer/propagation/trace_context.h | 13 +++++ .../propagation/trace_context_propagator.cpp | 49 +++++++++++++++++++ .../propagation/trace_context_propagator.h | 25 ++++++++++ 7 files changed, 111 insertions(+) create mode 100644 src/tracer/propagation/trace_context.cpp create mode 100644 src/tracer/propagation/trace_context.h create mode 100644 src/tracer/propagation/trace_context_propagator.cpp create mode 100644 src/tracer/propagation/trace_context_propagator.h diff --git a/src/tracer/immutable_span_context.h b/src/tracer/immutable_span_context.h index 86636058..d09769d5 100644 --- a/src/tracer/immutable_span_context.h +++ b/src/tracer/immutable_span_context.h @@ -57,6 +57,7 @@ class ImmutableSpanContext final : public LightStepSpanContext { uint64_t span_id_; bool sampled_; BaggageProtobufMap baggage_; + std::string trace_state_; template opentracing::expected InjectImpl( diff --git a/src/tracer/lightstep_span_context.h b/src/tracer/lightstep_span_context.h index b73cce97..101128c8 100644 --- a/src/tracer/lightstep_span_context.h +++ b/src/tracer/lightstep_span_context.h @@ -19,6 +19,10 @@ class LightStepSpanContext : public opentracing::SpanContext { virtual bool sampled() const noexcept = 0; + virtual opentracing::string_view trace_state() const noexcept { + return {}; + } + virtual opentracing::expected Inject( const PropagationOptions& propagation_options, std::ostream& writer) const = 0; diff --git a/src/tracer/propagation/BUILD b/src/tracer/propagation/BUILD index daa257f4..39a49983 100644 --- a/src/tracer/propagation/BUILD +++ b/src/tracer/propagation/BUILD @@ -89,6 +89,21 @@ lightstep_cc_library( ], ) +lightstep_cc_library( + name = "trace_context_propagator_lib", + private_hdrs = [ + "trace_context_propagator.h", + ], + srcs = [ + "trace_context_propagator.cpp", + ], + deps = [ + "//src/common:hex_conversion_lib", + "//src/common:utility_lib", + ":propagator_interface", + ], +) + lightstep_cc_library( name = "baggage_propagator_lib", private_hdrs = [ diff --git a/src/tracer/propagation/trace_context.cpp b/src/tracer/propagation/trace_context.cpp new file mode 100644 index 00000000..31177e17 --- /dev/null +++ b/src/tracer/propagation/trace_context.cpp @@ -0,0 +1,4 @@ +#include "tracer/propagation/trace_context.h" + +namespace lightstep { +} // namespace lightstep diff --git a/src/tracer/propagation/trace_context.h b/src/tracer/propagation/trace_context.h new file mode 100644 index 00000000..2d7b7a59 --- /dev/null +++ b/src/tracer/propagation/trace_context.h @@ -0,0 +1,13 @@ +#pragma once + +#include + +namespace lightstep { +struct TraceContext { + uint8_t version; + uint64_t trace_id_high; + uint64_t trace_id_low; + uint64_t span_id; + uint8_t flags; +}; +} // namespace lightstep diff --git a/src/tracer/propagation/trace_context_propagator.cpp b/src/tracer/propagation/trace_context_propagator.cpp new file mode 100644 index 00000000..81565d2c --- /dev/null +++ b/src/tracer/propagation/trace_context_propagator.cpp @@ -0,0 +1,49 @@ +#include "tracer/propagation/trace_context_propagator.h" + +namespace lightstep { +//-------------------------------------------------------------------------------------------------- +// InjectSpanContext +//-------------------------------------------------------------------------------------------------- +opentracing::expected TraceContextPropagator::InjectSpanContext( + const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, + uint64_t trace_id_low, uint64_t span_id, bool sampled, + const BaggageProtobufMap& baggage) const { + (void)carrier; + (void)trace_id_high; + (void)trace_id_low; + (void)span_id; + (void)sampled; + (void)baggage; + return {}; +} + +opentracing::expected TraceContextPropagator::InjectSpanContext( + const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, + uint64_t trace_id_low, uint64_t span_id, bool sampled, + const BaggageFlatMap& baggage) const { + (void)carrier; + (void)trace_id_high; + (void)trace_id_low; + (void)span_id; + (void)sampled; + (void)baggage; + return {}; +} + +//-------------------------------------------------------------------------------------------------- +// ExtractSpanContext +//-------------------------------------------------------------------------------------------------- +opentracing::expected TraceContextPropagator::ExtractSpanContext( + const opentracing::TextMapReader& carrier, bool case_sensitive, + uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, + bool& sampled, BaggageProtobufMap& baggage) const { + (void)carrier; + (void)case_sensitive; + (void)trace_id_high; + (void)trace_id_low; + (void)span_id; + (void)sampled; + (void)baggage; + return true; +} +} // namespace lightstep diff --git a/src/tracer/propagation/trace_context_propagator.h b/src/tracer/propagation/trace_context_propagator.h new file mode 100644 index 00000000..5a9c11e5 --- /dev/null +++ b/src/tracer/propagation/trace_context_propagator.h @@ -0,0 +1,25 @@ +#pragma once + +#include "tracer/propagation/propagator.h" + +namespace lightstep { +class TraceContextPropagator final : Propagator { + public: + // Propagator + opentracing::expected InjectSpanContext( + const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, + uint64_t trace_id_low, uint64_t span_id, bool sampled, + const BaggageProtobufMap& baggage) const override; + + opentracing::expected InjectSpanContext( + const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, + uint64_t trace_id_low, uint64_t span_id, bool sampled, + const BaggageFlatMap& baggage) const override; + + opentracing::expected ExtractSpanContext( + const opentracing::TextMapReader& carrier, bool case_sensitive, + uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, + bool& sampled, BaggageProtobufMap& baggage) const override; + private: +}; +} // namespace lightstep From c89155b297a69e5f07a65deddb7a973a2fbe3f61 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 6 Dec 2019 12:45:50 -0800 Subject: [PATCH 02/42] Add support for 8-bit hex conversions. --- src/common/hex_conversion.cpp | 25 +++++++++++++++++------- src/common/hex_conversion.h | 17 ++++++++++++++++ src/tracer/propagation/trace_context.cpp | 7 +++++++ src/tracer/propagation/trace_context.h | 7 ++++++- test/common/hex_conversion_test.cpp | 14 +++++++++++++ 5 files changed, 62 insertions(+), 8 deletions(-) diff --git a/src/common/hex_conversion.cpp b/src/common/hex_conversion.cpp index 2ea3b456..82aa05c2 100644 --- a/src/common/hex_conversion.cpp +++ b/src/common/hex_conversion.cpp @@ -1,11 +1,14 @@ #include "common/hex_conversion.h" +#include + namespace lightstep { //-------------------------------------------------------------------------------------------------- -// HexToUint64Impl +// HexToUintImpl //-------------------------------------------------------------------------------------------------- -static opentracing::expected HexToUint64Impl( +template +static opentracing::expected HexToUintImpl( const char* i, const char* last) noexcept { static const unsigned char nil = std::numeric_limits::max(); static const std::array hextable = { @@ -28,7 +31,7 @@ static opentracing::expected HexToUint64Impl( 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}}; - uint64_t result = 0; + T result = 0; for (; i != last; ++i) { auto value = hextable[*i]; if (value == nil) { @@ -114,7 +117,7 @@ opentracing::expected HexToUint64( std::make_error_code(std::errc::invalid_argument)); } - return HexToUint64Impl(i, last); + return HexToUintImpl(i, last); } //-------------------------------------------------------------------------------------------------- @@ -158,7 +161,7 @@ opentracing::expected HexToUint128(opentracing::string_view s, // handle the case when we have a number that fits in a 64-bit integer if (length <= Num64BitHexDigits) { x_high = 0; - auto x_maybe = HexToUint64Impl(i, last); + auto x_maybe = HexToUintImpl(i, last); if (!x_maybe) { return opentracing::make_unexpected(x_maybe.error()); } @@ -169,17 +172,25 @@ opentracing::expected HexToUint128(opentracing::string_view s, // handle the case when we have a number that requires more than a single // 64-bit integer auto boundary = i + (length - Num64BitHexDigits); - auto x_high_maybe = HexToUint64Impl(i, boundary); + auto x_high_maybe = HexToUintImpl(i, boundary); if (!x_high_maybe) { return opentracing::make_unexpected(x_high_maybe.error()); } x_high = *x_high_maybe; - auto x_low_maybe = HexToUint64Impl(boundary, last); + auto x_low_maybe = HexToUintImpl(boundary, last); if (!x_low_maybe) { return opentracing::make_unexpected(x_low_maybe.error()); } x_low = *x_low_maybe; return {}; } + +//-------------------------------------------------------------------------------------------------- +// NormalizedHexToUint8 +//-------------------------------------------------------------------------------------------------- +opentracing::expected NormalizedHexToUint8(opentracing::string_view s) noexcept { + assert(s.size() == Num8BitHexDigits); + return HexToUintImpl(s.data(), s.data() + Num8BitHexDigits); +} } // namespace lightstep diff --git a/src/common/hex_conversion.h b/src/common/hex_conversion.h index cd3e4520..eb38ca5c 100644 --- a/src/common/hex_conversion.h +++ b/src/common/hex_conversion.h @@ -12,6 +12,8 @@ const size_t Num64BitHexDigits = std::numeric_limits::digits / 4; const size_t Num32BitHexDigits = std::numeric_limits::digits / 4; +const size_t Num8BitHexDigits = std::numeric_limits::digits / 4; + extern const unsigned char HexDigitLookupTable[513]; /** @@ -46,6 +48,19 @@ inline opentracing::string_view Uint32ToHex(uint32_t x, char* output) noexcept { return {output, Num32BitHexDigits}; } +/** + * Writes a 8-bit number in hex. + * @param x the number to write + * @param output where to output the number + * @return x as a hex string + */ +inline opentracing::string_view Uint8ToHex(uint8_t x, char* output) noexcept { + auto lookup_index = static_cast(x) * 2; + output[0] = HexDigitLookupTable[lookup_index]; + output[1] = HexDigitLookupTable[lookup_index + 1]; + return {output, Num8BitHexDigits}; +} + // Converts a hexidecimal number to a 64-bit integer. Either returns the number // or an error code. opentracing::expected HexToUint64( @@ -55,6 +70,8 @@ opentracing::expected HexToUint128(opentracing::string_view s, uint64_t& x_high, uint64_t& x_low) noexcept; +opentracing::expected NormalizedHexToUint8(opentracing::string_view s) noexcept; + /** * Serialize integers to hex */ diff --git a/src/tracer/propagation/trace_context.cpp b/src/tracer/propagation/trace_context.cpp index 31177e17..8f9b0983 100644 --- a/src/tracer/propagation/trace_context.cpp +++ b/src/tracer/propagation/trace_context.cpp @@ -1,4 +1,11 @@ #include "tracer/propagation/trace_context.h" namespace lightstep { +//-------------------------------------------------------------------------------------------------- +// ParseTraceContext +//-------------------------------------------------------------------------------------------------- +opentracing::expected ParseTraceContext(opentracing::string_view s) noexcept { + (void)s; + return {}; +} } // namespace lightstep diff --git a/src/tracer/propagation/trace_context.h b/src/tracer/propagation/trace_context.h index 2d7b7a59..19431083 100644 --- a/src/tracer/propagation/trace_context.h +++ b/src/tracer/propagation/trace_context.h @@ -2,12 +2,17 @@ #include +#include +#include + namespace lightstep { struct TraceContext { - uint8_t version; uint64_t trace_id_high; uint64_t trace_id_low; uint64_t span_id; uint8_t flags; + uint8_t version; }; + +opentracing::expected ParseTraceContext(opentracing::string_view s) noexcept; } // namespace lightstep diff --git a/test/common/hex_conversion_test.cpp b/test/common/hex_conversion_test.cpp index 4d53341d..bbd8341f 100644 --- a/test/common/hex_conversion_test.cpp +++ b/test/common/hex_conversion_test.cpp @@ -19,6 +19,20 @@ static void Generate128BitTestCases(uint64_t x, F f) { f(max - x, max - x); } +TEST_CASE("hex-integer conversions (8-bit)") { + char data[Num8BitHexDigits]; + + SECTION("Verify hex conversion and back against a range of values.") { + for (uint8_t x = 0; x < std::numeric_limits::max(); ++x) { + { + REQUIRE(x == *NormalizedHexToUint8(Uint8ToHex(x, data))); + auto y = std::numeric_limits::max() - x; + REQUIRE(y == *NormalizedHexToUint8(Uint8ToHex(y, data))); + } + } + } +} + TEST_CASE("hex-integer conversions (64-bit)") { char data[Num64BitHexDigits]; From 9aca0b4b1c93658ee71460490611d30b035e3ac8 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 6 Dec 2019 12:47:11 -0800 Subject: [PATCH 03/42] Fix typo. --- test/common/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/BUILD b/test/common/BUILD index a34070a4..1d3a3265 100644 --- a/test/common/BUILD +++ b/test/common/BUILD @@ -18,7 +18,7 @@ lightstep_catch_test( ) lightstep_catch_test( - name = "spin_lock_mute_test", + name = "spin_lock_mutex_test", srcs = [ "spin_lock_mutex_test.cpp", ], From 474e64a2e44bbe5bdef24572ec7db14fe9049612 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 10 Dec 2019 09:02:49 -0800 Subject: [PATCH 04/42] Add code to parse a trace context. --- src/common/hex_conversion.cpp | 33 +++++++++++- src/common/hex_conversion.h | 6 +++ src/tracer/propagation/trace_context.cpp | 65 ++++++++++++++++++++++-- src/tracer/propagation/trace_context.h | 9 ++-- 4 files changed, 105 insertions(+), 8 deletions(-) diff --git a/src/common/hex_conversion.cpp b/src/common/hex_conversion.cpp index 82aa05c2..8b843dab 100644 --- a/src/common/hex_conversion.cpp +++ b/src/common/hex_conversion.cpp @@ -191,6 +191,37 @@ opentracing::expected HexToUint128(opentracing::string_view s, //-------------------------------------------------------------------------------------------------- opentracing::expected NormalizedHexToUint8(opentracing::string_view s) noexcept { assert(s.size() == Num8BitHexDigits); - return HexToUintImpl(s.data(), s.data() + Num8BitHexDigits); + return HexToUintImpl(s.data(), s.data() + s.size()); +} + +//-------------------------------------------------------------------------------------------------- +// NormalizedHexToUint64 +//-------------------------------------------------------------------------------------------------- +opentracing::expected NormalizedHexToUint64( + opentracing::string_view s) noexcept { + assert(s.size() == Num64BitHexDigits); + return HexToUintImpl(s.data(), s.data() + s.size()); +} + +//-------------------------------------------------------------------------------------------------- +// NormalizedHexToUint128 +//-------------------------------------------------------------------------------------------------- +opentracing::expected NormalizedHexToUint128(opentracing::string_view s, + uint64_t& x_high, + uint64_t& x_low) noexcept { + assert(s.size() == 2 * Num64BitHexDigits); + auto x_maybe = + HexToUintImpl(s.data(), s.data() + Num64BitHexDigits); + if (!x_maybe) { + return opentracing::make_unexpected(x_maybe.error()); + } + x_high = *x_maybe; + x_maybe = HexToUintImpl(s.data() + Num64BitHexDigits, + s.data() + 2 * Num64BitHexDigits); + if (!x_maybe) { + return opentracing::make_unexpected(x_maybe.error()); + } + x_low = *x_maybe; + return {}; } } // namespace lightstep diff --git a/src/common/hex_conversion.h b/src/common/hex_conversion.h index eb38ca5c..3dcba1b5 100644 --- a/src/common/hex_conversion.h +++ b/src/common/hex_conversion.h @@ -72,6 +72,12 @@ opentracing::expected HexToUint128(opentracing::string_view s, opentracing::expected NormalizedHexToUint8(opentracing::string_view s) noexcept; +opentracing::expected NormalizedHexToUint64(opentracing::string_view s) noexcept; + +opentracing::expected NormalizedHexToUint128(opentracing::string_view s, + uint64_t& x_high, + uint64_t& x_low) noexcept; + /** * Serialize integers to hex */ diff --git a/src/tracer/propagation/trace_context.cpp b/src/tracer/propagation/trace_context.cpp index 8f9b0983..5e2d7775 100644 --- a/src/tracer/propagation/trace_context.cpp +++ b/src/tracer/propagation/trace_context.cpp @@ -1,11 +1,70 @@ #include "tracer/propagation/trace_context.h" +const size_t TraceContextMinLength = 55; + namespace lightstep { //-------------------------------------------------------------------------------------------------- // ParseTraceContext //-------------------------------------------------------------------------------------------------- -opentracing::expected ParseTraceContext(opentracing::string_view s) noexcept { - (void)s; +opentracing::expected ParseTraceContext( + opentracing::string_view s, TraceContext& trace_context) noexcept { + if (s.size() < TraceContextMinLength) { + return std::make_error_code(std::errc::invalid_argument); + } + size_t offset = 0; + + // version + auto version_maybe = NormalizedHexToUint8( + opentracing::string_view{s.data() + offset, Num8BitHexDigits}); + if (!version_maybe) { + return opentracing::make_unexpected(version_maybe.error()); + } + trace_context.version = *version_maybe; + offset += Num8BitHexDigits; + if (s[offset] != '-') { + return std::make_error_code(std::errc::invalid_argument); + } + ++offset; + + // trace-id + auto error_maybe = NormalizedHexToUint128( + opentracing::string_view{s.data() + offset, Num128BitHexDigits}, + trace_context.trace_id_high, trace_context.trace_id_low); + if (!was_successful) { + return error_maybe; + } + offset += Num128BitHexDigits; + if (s[offset] != '-') { + return std::make_error_code(std::errc::invalid_argument); + } + ++offset; + + // parent-id + auto parent_id_maybe = NormalizedHexToUint64( + opentracing::string_view{s.data() + offset, Num64BitHexDigits}); + if (!parent_id_maybe) { + return opentracing::make_unexpected(parent_id_maybe.error()); + } + trace_context.parent_id = *parent_id_maybe; + offset += Num64BitHexDigits; + if (s[offset] != '-') { + return std::make_error_code(std::errc::invalid_argument); + } + ++offset; + + + // trace-flags + auto trace_flags_maybe = NormalizedHexToUint8( + opentracing::string_view{s.data() + offset, Num8BitHexDigits}); + if (!trace_flags_maybe) { + return opentracing::make_unexpected(trace_flags_maybe.error()); + } + trace_context.trace_flags = *trace_flags_maybe; + offset += Num8BitHexDigits; + if (s[offset] != '-') { + return std::make_error_code(std::errc::invalid_argument); + } + return {}; } -} // namespace lightstep +} // namespace lightstep diff --git a/src/tracer/propagation/trace_context.h b/src/tracer/propagation/trace_context.h index 19431083..7c4a8bf1 100644 --- a/src/tracer/propagation/trace_context.h +++ b/src/tracer/propagation/trace_context.h @@ -9,10 +9,11 @@ namespace lightstep { struct TraceContext { uint64_t trace_id_high; uint64_t trace_id_low; - uint64_t span_id; - uint8_t flags; + uint64_t parent_id; + uint8_t trace_flags; uint8_t version; }; -opentracing::expected ParseTraceContext(opentracing::string_view s) noexcept; -} // namespace lightstep +opentracing::expected ParseTraceContext( + opentracing::string_view s, TraceContext& trace_context) noexcept; +} // namespace lightstep From d801ac58beca09807c62abf64a65f75eb5a9b453 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 12 Dec 2019 10:32:36 -0800 Subject: [PATCH 05/42] Set up trace-context test. --- src/common/hex_conversion.h | 2 ++ src/tracer/propagation/BUILD | 14 ++++++++++++++ src/tracer/propagation/trace_context.cpp | 18 +++++++++++------- test/tracer/propagation/BUILD | 10 ++++++++++ test/tracer/propagation/trace_context_test.cpp | 4 ++++ 5 files changed, 41 insertions(+), 7 deletions(-) create mode 100644 test/tracer/propagation/trace_context_test.cpp diff --git a/src/common/hex_conversion.h b/src/common/hex_conversion.h index 3dcba1b5..8d1af56f 100644 --- a/src/common/hex_conversion.h +++ b/src/common/hex_conversion.h @@ -10,6 +10,8 @@ namespace lightstep { const size_t Num64BitHexDigits = std::numeric_limits::digits / 4; +const size_t Num128BitHexDigits = 2 * Num64BitHexDigits; + const size_t Num32BitHexDigits = std::numeric_limits::digits / 4; const size_t Num8BitHexDigits = std::numeric_limits::digits / 4; diff --git a/src/tracer/propagation/BUILD b/src/tracer/propagation/BUILD index 39a49983..03b9f2b4 100644 --- a/src/tracer/propagation/BUILD +++ b/src/tracer/propagation/BUILD @@ -89,6 +89,20 @@ lightstep_cc_library( ], ) +lightstep_cc_library( + name = "trace_context_lib", + private_hdrs = [ + "trace_context.h", + ], + srcs = [ + "trace_context.cpp", + ], + deps = [ + "//src/common:hex_conversion_lib", + "@io_opentracing_cpp//:opentracing", + ], +) + lightstep_cc_library( name = "trace_context_propagator_lib", private_hdrs = [ diff --git a/src/tracer/propagation/trace_context.cpp b/src/tracer/propagation/trace_context.cpp index 5e2d7775..8466a19c 100644 --- a/src/tracer/propagation/trace_context.cpp +++ b/src/tracer/propagation/trace_context.cpp @@ -1,5 +1,7 @@ #include "tracer/propagation/trace_context.h" +#include "common/hex_conversion.h" + const size_t TraceContextMinLength = 55; namespace lightstep { @@ -9,7 +11,8 @@ namespace lightstep { opentracing::expected ParseTraceContext( opentracing::string_view s, TraceContext& trace_context) noexcept { if (s.size() < TraceContextMinLength) { - return std::make_error_code(std::errc::invalid_argument); + return opentracing::make_unexpected( + std::make_error_code(std::errc::invalid_argument)); } size_t offset = 0; @@ -22,7 +25,8 @@ opentracing::expected ParseTraceContext( trace_context.version = *version_maybe; offset += Num8BitHexDigits; if (s[offset] != '-') { - return std::make_error_code(std::errc::invalid_argument); + return opentracing::make_unexpected( + std::make_error_code(std::errc::invalid_argument)); } ++offset; @@ -30,12 +34,13 @@ opentracing::expected ParseTraceContext( auto error_maybe = NormalizedHexToUint128( opentracing::string_view{s.data() + offset, Num128BitHexDigits}, trace_context.trace_id_high, trace_context.trace_id_low); - if (!was_successful) { + if (!error_maybe) { return error_maybe; } offset += Num128BitHexDigits; if (s[offset] != '-') { - return std::make_error_code(std::errc::invalid_argument); + return opentracing::make_unexpected( + std::make_error_code(std::errc::invalid_argument)); } ++offset; @@ -48,11 +53,10 @@ opentracing::expected ParseTraceContext( trace_context.parent_id = *parent_id_maybe; offset += Num64BitHexDigits; if (s[offset] != '-') { - return std::make_error_code(std::errc::invalid_argument); + return opentracing::make_unexpected(parent_id_maybe.error()); } ++offset; - // trace-flags auto trace_flags_maybe = NormalizedHexToUint8( opentracing::string_view{s.data() + offset, Num8BitHexDigits}); @@ -62,7 +66,7 @@ opentracing::expected ParseTraceContext( trace_context.trace_flags = *trace_flags_maybe; offset += Num8BitHexDigits; if (s[offset] != '-') { - return std::make_error_code(std::errc::invalid_argument); + return opentracing::make_unexpected(parent_id_maybe.error()); } return {}; diff --git a/test/tracer/propagation/BUILD b/test/tracer/propagation/BUILD index 595ca67b..bd6eba64 100644 --- a/test/tracer/propagation/BUILD +++ b/test/tracer/propagation/BUILD @@ -94,6 +94,16 @@ lightstep_catch_test( ], ) +lightstep_catch_test( + name = "trace_context_test", + srcs = [ + "trace_context_test.cpp", + ], + deps = [ + "//src/tracer/propagation:trace_context_lib", + ], +) + lightstep_catch_test( name = "lightstep_propagation_test", srcs = [ diff --git a/test/tracer/propagation/trace_context_test.cpp b/test/tracer/propagation/trace_context_test.cpp new file mode 100644 index 00000000..37b77900 --- /dev/null +++ b/test/tracer/propagation/trace_context_test.cpp @@ -0,0 +1,4 @@ +#include "tracer/propagation/trace_context.h" + +#include "3rd_party/catch2/catch.hpp" +using namespace lightstep; From 97e5d76f987ea2f50efbf379c03e9752a6e6954b Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 12 Dec 2019 12:10:46 -0800 Subject: [PATCH 06/42] Add test case for parsing a trace context. --- benchmark/tracer_upload_bench/utility.cpp | 2 +- src/common/hex_conversion.cpp | 59 +++++++++++-------- src/common/hex_conversion.h | 6 +- src/tracer/lightstep_span_context.h | 4 +- src/tracer/propagation/trace_context.cpp | 4 -- .../propagation/trace_context_propagator.h | 3 +- .../tracer/propagation/trace_context_test.cpp | 15 +++++ 7 files changed, 56 insertions(+), 37 deletions(-) diff --git a/benchmark/tracer_upload_bench/utility.cpp b/benchmark/tracer_upload_bench/utility.cpp index ab2053ce..c1da90ca 100644 --- a/benchmark/tracer_upload_bench/utility.cpp +++ b/benchmark/tracer_upload_bench/utility.cpp @@ -7,8 +7,8 @@ #include #include -#include "tracer/lightstep_tracer_factory.h" #include "tracer/json_options.h" +#include "tracer/lightstep_tracer_factory.h" #include "google/protobuf/util/json_util.h" diff --git a/src/common/hex_conversion.cpp b/src/common/hex_conversion.cpp index 8b843dab..9da58553 100644 --- a/src/common/hex_conversion.cpp +++ b/src/common/hex_conversion.cpp @@ -3,38 +3,44 @@ #include namespace lightstep { +//-------------------------------------------------------------------------------------------------- +// Nil +//-------------------------------------------------------------------------------------------------- +static const unsigned char Nil = std::numeric_limits::max(); + +//-------------------------------------------------------------------------------------------------- +// HexTable +//-------------------------------------------------------------------------------------------------- +static const std::array 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}}; //-------------------------------------------------------------------------------------------------- // HexToUintImpl //-------------------------------------------------------------------------------------------------- template -static opentracing::expected HexToUintImpl( - const char* i, const char* last) noexcept { - static const unsigned char nil = std::numeric_limits::max(); - static const std::array 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}}; +static opentracing::expected HexToUintImpl(const char* i, + const char* last) noexcept { T result = 0; for (; i != last; ++i) { - auto value = hextable[*i]; - if (value == nil) { + auto value = HexTable[*i]; + if (value == Nil) { return opentracing::make_unexpected( std::make_error_code(std::errc::invalid_argument)); } @@ -189,7 +195,8 @@ opentracing::expected HexToUint128(opentracing::string_view s, //-------------------------------------------------------------------------------------------------- // NormalizedHexToUint8 //-------------------------------------------------------------------------------------------------- -opentracing::expected NormalizedHexToUint8(opentracing::string_view s) noexcept { +opentracing::expected NormalizedHexToUint8( + opentracing::string_view s) noexcept { assert(s.size() == Num8BitHexDigits); return HexToUintImpl(s.data(), s.data() + s.size()); } diff --git a/src/common/hex_conversion.h b/src/common/hex_conversion.h index 8d1af56f..9d6625e1 100644 --- a/src/common/hex_conversion.h +++ b/src/common/hex_conversion.h @@ -72,9 +72,11 @@ opentracing::expected HexToUint128(opentracing::string_view s, uint64_t& x_high, uint64_t& x_low) noexcept; -opentracing::expected NormalizedHexToUint8(opentracing::string_view s) noexcept; +opentracing::expected NormalizedHexToUint8( + opentracing::string_view s) noexcept; -opentracing::expected NormalizedHexToUint64(opentracing::string_view s) noexcept; +opentracing::expected NormalizedHexToUint64( + opentracing::string_view s) noexcept; opentracing::expected NormalizedHexToUint128(opentracing::string_view s, uint64_t& x_high, diff --git a/src/tracer/lightstep_span_context.h b/src/tracer/lightstep_span_context.h index 101128c8..6733a892 100644 --- a/src/tracer/lightstep_span_context.h +++ b/src/tracer/lightstep_span_context.h @@ -19,9 +19,7 @@ class LightStepSpanContext : public opentracing::SpanContext { virtual bool sampled() const noexcept = 0; - virtual opentracing::string_view trace_state() const noexcept { - return {}; - } + virtual opentracing::string_view trace_state() const noexcept { return {}; } virtual opentracing::expected Inject( const PropagationOptions& propagation_options, diff --git a/src/tracer/propagation/trace_context.cpp b/src/tracer/propagation/trace_context.cpp index 8466a19c..efc1d41d 100644 --- a/src/tracer/propagation/trace_context.cpp +++ b/src/tracer/propagation/trace_context.cpp @@ -64,10 +64,6 @@ opentracing::expected ParseTraceContext( return opentracing::make_unexpected(trace_flags_maybe.error()); } trace_context.trace_flags = *trace_flags_maybe; - offset += Num8BitHexDigits; - if (s[offset] != '-') { - return opentracing::make_unexpected(parent_id_maybe.error()); - } return {}; } diff --git a/src/tracer/propagation/trace_context_propagator.h b/src/tracer/propagation/trace_context_propagator.h index 5a9c11e5..09a8c297 100644 --- a/src/tracer/propagation/trace_context_propagator.h +++ b/src/tracer/propagation/trace_context_propagator.h @@ -20,6 +20,7 @@ class TraceContextPropagator final : Propagator { const opentracing::TextMapReader& carrier, bool case_sensitive, uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, bool& sampled, BaggageProtobufMap& baggage) const override; + private: }; -} // namespace lightstep +} // namespace lightstep diff --git a/test/tracer/propagation/trace_context_test.cpp b/test/tracer/propagation/trace_context_test.cpp index 37b77900..70875411 100644 --- a/test/tracer/propagation/trace_context_test.cpp +++ b/test/tracer/propagation/trace_context_test.cpp @@ -2,3 +2,18 @@ #include "3rd_party/catch2/catch.hpp" using namespace lightstep; + +TEST_CASE("We can parse a trace-context") { + TraceContext trace_context; + + SECTION("Parse an example trace context") { + REQUIRE(ParseTraceContext( + "00-0af7651916cd43dd8448eb211c80319c-b9c7c989f97918e1-01", + trace_context)); + REQUIRE(trace_context.version == 0); + REQUIRE(trace_context.trace_id_high == 0x0af7651916cd43dd); + REQUIRE(trace_context.trace_id_low == 0x8448eb211c80319c); + REQUIRE(trace_context.parent_id == 0xb9c7c989f97918e1); + REQUIRE(trace_context.trace_flags == 1); + } +} From 0410560fda0202c5aa6665fd88033bd6c0b9e27a Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 12 Dec 2019 12:40:47 -0800 Subject: [PATCH 07/42] Add additional test coverage. --- src/tracer/propagation/trace_context.cpp | 3 +- .../tracer/propagation/trace_context_test.cpp | 32 ++++++++++++++++--- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/tracer/propagation/trace_context.cpp b/src/tracer/propagation/trace_context.cpp index efc1d41d..7b01744a 100644 --- a/src/tracer/propagation/trace_context.cpp +++ b/src/tracer/propagation/trace_context.cpp @@ -53,7 +53,8 @@ opentracing::expected ParseTraceContext( trace_context.parent_id = *parent_id_maybe; offset += Num64BitHexDigits; if (s[offset] != '-') { - return opentracing::make_unexpected(parent_id_maybe.error()); + return opentracing::make_unexpected( + std::make_error_code(std::errc::invalid_argument)); } ++offset; diff --git a/test/tracer/propagation/trace_context_test.cpp b/test/tracer/propagation/trace_context_test.cpp index 70875411..8f0e9dc7 100644 --- a/test/tracer/propagation/trace_context_test.cpp +++ b/test/tracer/propagation/trace_context_test.cpp @@ -6,14 +6,38 @@ using namespace lightstep; TEST_CASE("We can parse a trace-context") { TraceContext trace_context; - SECTION("Parse an example trace context") { - REQUIRE(ParseTraceContext( - "00-0af7651916cd43dd8448eb211c80319c-b9c7c989f97918e1-01", - trace_context)); + opentracing::string_view valid_trace_context = + "00-0af7651916cd43dd8448eb211c80319c-b9c7c989f97918e1-01"; + + SECTION("We can parse a valid trace context") { + REQUIRE(ParseTraceContext(valid_trace_context, trace_context)); REQUIRE(trace_context.version == 0); REQUIRE(trace_context.trace_id_high == 0x0af7651916cd43dd); REQUIRE(trace_context.trace_id_low == 0x8448eb211c80319c); REQUIRE(trace_context.parent_id == 0xb9c7c989f97918e1); REQUIRE(trace_context.trace_flags == 1); } + + SECTION("We fail if we encounter an invalid character") { + for (int i = 0; i < static_cast(valid_trace_context.size()); ++i) { + std::string s = valid_trace_context; + s[i] = 'X'; + REQUIRE(!ParseTraceContext(s, trace_context)); + } + } + + SECTION( + "We fail when trace context has fewer than the minimum required " + "characters") { + std::string s = valid_trace_context; + s = s.substr(1); + REQUIRE(!ParseTraceContext(s, trace_context)); + } + + SECTION( + "We can parse a trace-context with more than the required characters") { + std::string s = valid_trace_context; + s += ' '; + REQUIRE(ParseTraceContext(s, trace_context)); + } } From 00b2dbf559af00a9d3393b8e01b5afc6ab7c013d Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 13 Dec 2019 11:30:10 -0800 Subject: [PATCH 08/42] Support serializing trace contexts. --- src/tracer/propagation/trace_context.cpp | 33 +++++++++++++++++-- src/tracer/propagation/trace_context.h | 4 +++ .../tracer/propagation/trace_context_test.cpp | 11 ++++++- 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/tracer/propagation/trace_context.cpp b/src/tracer/propagation/trace_context.cpp index 7b01744a..0a80d73d 100644 --- a/src/tracer/propagation/trace_context.cpp +++ b/src/tracer/propagation/trace_context.cpp @@ -2,8 +2,6 @@ #include "common/hex_conversion.h" -const size_t TraceContextMinLength = 55; - namespace lightstep { //-------------------------------------------------------------------------------------------------- // ParseTraceContext @@ -68,4 +66,35 @@ opentracing::expected ParseTraceContext( return {}; } + +//-------------------------------------------------------------------------------------------------- +// SerializeTraceContext +//-------------------------------------------------------------------------------------------------- +void SerializeTraceContext(const TraceContext& trace_context, + char* s) noexcept { + size_t offset = 0; + + // version + Uint8ToHex(trace_context.version, s); + offset += Num8BitHexDigits; + *(s + offset) = '-'; + ++offset; + + // trace-id + Uint64ToHex(trace_context.trace_id_high, s + offset); + offset += Num64BitHexDigits; + Uint64ToHex(trace_context.trace_id_low, s + offset); + offset += Num64BitHexDigits; + *(s + offset) = '-'; + ++offset; + + // parent-id + Uint64ToHex(trace_context.parent_id, s + offset); + offset += Num64BitHexDigits; + *(s + offset) = '-'; + ++offset; + + // trace-flags + Uint8ToHex(trace_context.trace_flags, s + offset); +} } // namespace lightstep diff --git a/src/tracer/propagation/trace_context.h b/src/tracer/propagation/trace_context.h index 7c4a8bf1..89b357ce 100644 --- a/src/tracer/propagation/trace_context.h +++ b/src/tracer/propagation/trace_context.h @@ -6,6 +6,8 @@ #include namespace lightstep { +const size_t TraceContextMinLength = 55; + struct TraceContext { uint64_t trace_id_high; uint64_t trace_id_low; @@ -16,4 +18,6 @@ struct TraceContext { opentracing::expected ParseTraceContext( opentracing::string_view s, TraceContext& trace_context) noexcept; + +void SerializeTraceContext(const TraceContext& trace_context, char* s) noexcept; } // namespace lightstep diff --git a/test/tracer/propagation/trace_context_test.cpp b/test/tracer/propagation/trace_context_test.cpp index 8f0e9dc7..1d66b294 100644 --- a/test/tracer/propagation/trace_context_test.cpp +++ b/test/tracer/propagation/trace_context_test.cpp @@ -3,7 +3,7 @@ #include "3rd_party/catch2/catch.hpp" using namespace lightstep; -TEST_CASE("We can parse a trace-context") { +TEST_CASE("We can serialize and deserialize a trace-context") { TraceContext trace_context; opentracing::string_view valid_trace_context = @@ -40,4 +40,13 @@ TEST_CASE("We can parse a trace-context") { s += ' '; REQUIRE(ParseTraceContext(s, trace_context)); } + + SECTION( + "If we deserialize a trace-context and serialize again, we'll get the " + "same value") { + REQUIRE(ParseTraceContext(valid_trace_context, trace_context)); + std::string s(TraceContextMinLength, ' '); + SerializeTraceContext(trace_context, &s[0]); + REQUIRE(s == valid_trace_context); + } } From c4643667a4d23e027aee5750cbb74b38b2bddcd3 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 17 Dec 2019 14:26:05 -0800 Subject: [PATCH 09/42] Change ImmutableSpanContext to use TraceContext internally. --- src/tracer/BUILD | 1 + src/tracer/immutable_span_context.cpp | 18 +++++++++--------- src/tracer/immutable_span_context.h | 23 +++++++++++++---------- src/tracer/propagation/trace_context.h | 16 ++++++++++++++++ 4 files changed, 39 insertions(+), 19 deletions(-) diff --git a/src/tracer/BUILD b/src/tracer/BUILD index e4d023ad..a7bd1d2b 100644 --- a/src/tracer/BUILD +++ b/src/tracer/BUILD @@ -77,6 +77,7 @@ lightstep_cc_library( ], deps = [ "//src/tracer:lightstep_span_context_interface", + "//src/tracer/propagation:trace_context_lib", ], ) diff --git a/src/tracer/immutable_span_context.cpp b/src/tracer/immutable_span_context.cpp index d9ec21b7..24a0b738 100644 --- a/src/tracer/immutable_span_context.cpp +++ b/src/tracer/immutable_span_context.cpp @@ -7,10 +7,8 @@ namespace lightstep { ImmutableSpanContext::ImmutableSpanContext( uint64_t trace_id_high, uint64_t trace_id_low, uint64_t span_id, bool sampled, const std::unordered_map& baggage) - : trace_id_high_{trace_id_high}, - trace_id_low_{trace_id_low}, - span_id_{span_id}, - sampled_{sampled} { + : ImmutableSpanContext{trace_id_high, trace_id_low, span_id, sampled, + BaggageProtobufMap{}} { for (auto& baggage_item : baggage) { baggage_.insert(BaggageProtobufMap::value_type(baggage_item.first, baggage_item.second)); @@ -25,11 +23,13 @@ ImmutableSpanContext::ImmutableSpanContext( ImmutableSpanContext::ImmutableSpanContext( uint64_t trace_id_high, uint64_t trace_id_low, uint64_t span_id, bool sampled, BaggageProtobufMap&& baggage) noexcept - : trace_id_high_{trace_id_high}, - trace_id_low_{trace_id_low}, - span_id_{span_id}, - sampled_{sampled}, - baggage_{std::move(baggage)} {} + : baggage_{std::move(baggage)} { + trace_context_.trace_id_high = trace_id_high; + trace_context_.trace_id_low = trace_id_low; + trace_context_.parent_id = span_id; + trace_context_.trace_flags = SetTraceFlag(0, sampled); + trace_context_.version = TraceContextVersion; +} //------------------------------------------------------------------------------ // ForeachBaggageItem diff --git a/src/tracer/immutable_span_context.h b/src/tracer/immutable_span_context.h index d09769d5..dd52b726 100644 --- a/src/tracer/immutable_span_context.h +++ b/src/tracer/immutable_span_context.h @@ -1,6 +1,7 @@ #pragma once #include "tracer/lightstep_span_context.h" +#include "tracer/propagation/trace_context.h" namespace lightstep { /** @@ -21,13 +22,17 @@ class ImmutableSpanContext final : public LightStepSpanContext { BaggageProtobufMap&& baggage) noexcept; // LightStepSpanContext - uint64_t trace_id_high() const noexcept override { return trace_id_high_; } + uint64_t trace_id_high() const noexcept override { return trace_context_.trace_id_high; } - uint64_t trace_id_low() const noexcept override { return trace_id_low_; } + uint64_t trace_id_low() const noexcept override { return trace_context_.trace_id_low; } - uint64_t span_id() const noexcept override { return span_id_; } + uint64_t span_id() const noexcept override { + return trace_context_.parent_id; + } - bool sampled() const noexcept override { return sampled_; } + bool sampled() const noexcept override { + return IsTraceFlagSet(trace_context_.trace_flags); + } void ForeachBaggageItem( std::function f) @@ -52,18 +57,16 @@ class ImmutableSpanContext final : public LightStepSpanContext { } private: - uint64_t trace_id_high_; - uint64_t trace_id_low_; - uint64_t span_id_; - bool sampled_; + TraceContext trace_context_; BaggageProtobufMap baggage_; std::string trace_state_; template opentracing::expected InjectImpl( const PropagationOptions& propagation_options, Carrier& writer) const { - return InjectSpanContext(propagation_options, writer, trace_id_high_, - trace_id_low_, span_id_, sampled_, baggage_); + return InjectSpanContext(propagation_options, writer, this->trace_id_high(), + this->trace_id_low(), this->span_id(), + this->sampled(), baggage_); } }; } // namespace lightstep diff --git a/src/tracer/propagation/trace_context.h b/src/tracer/propagation/trace_context.h index 89b357ce..a9182de7 100644 --- a/src/tracer/propagation/trace_context.h +++ b/src/tracer/propagation/trace_context.h @@ -7,6 +7,8 @@ namespace lightstep { const size_t TraceContextMinLength = 55; +const uint8_t SampledFlagMask = 1; +const uint8_t TraceContextVersion = 0; struct TraceContext { uint64_t trace_id_high; @@ -20,4 +22,18 @@ opentracing::expected ParseTraceContext( opentracing::string_view s, TraceContext& trace_context) noexcept; void SerializeTraceContext(const TraceContext& trace_context, char* s) noexcept; + +template +inline bool IsTraceFlagSet(uint8_t flags) noexcept { + return static_cast(flags & Mask); +} + +template +inline uint8_t SetTraceFlag(uint8_t flags, bool value) noexcept { + if (value) { + return flags | Mask; + } else { + return flags & ~Mask; + } +} } // namespace lightstep From 68553380e4b38bd41503713da4d5d977ca5f9f1e Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 2 Jan 2020 11:11:06 -0800 Subject: [PATCH 10/42] Refactor ImmutableSpanContext --- src/tracer/immutable_span_context.cpp | 12 +++++------- src/tracer/immutable_span_context.h | 15 ++++++++------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/tracer/immutable_span_context.cpp b/src/tracer/immutable_span_context.cpp index 24a0b738..f6740d90 100644 --- a/src/tracer/immutable_span_context.cpp +++ b/src/tracer/immutable_span_context.cpp @@ -23,13 +23,11 @@ ImmutableSpanContext::ImmutableSpanContext( ImmutableSpanContext::ImmutableSpanContext( uint64_t trace_id_high, uint64_t trace_id_low, uint64_t span_id, bool sampled, BaggageProtobufMap&& baggage) noexcept - : baggage_{std::move(baggage)} { - trace_context_.trace_id_high = trace_id_high; - trace_context_.trace_id_low = trace_id_low; - trace_context_.parent_id = span_id; - trace_context_.trace_flags = SetTraceFlag(0, sampled); - trace_context_.version = TraceContextVersion; -} + : trace_id_high_{trace_id_high}, + trace_id_low_{trace_id_low}, + span_id_{span_id}, + trace_flags_{SetTraceFlag(0, sampled)}, + baggage_{std::move(baggage)} {} //------------------------------------------------------------------------------ // ForeachBaggageItem diff --git a/src/tracer/immutable_span_context.h b/src/tracer/immutable_span_context.h index dd52b726..02ee8893 100644 --- a/src/tracer/immutable_span_context.h +++ b/src/tracer/immutable_span_context.h @@ -22,16 +22,14 @@ class ImmutableSpanContext final : public LightStepSpanContext { BaggageProtobufMap&& baggage) noexcept; // LightStepSpanContext - uint64_t trace_id_high() const noexcept override { return trace_context_.trace_id_high; } + uint64_t trace_id_high() const noexcept override { return trace_id_high_; } - uint64_t trace_id_low() const noexcept override { return trace_context_.trace_id_low; } + uint64_t trace_id_low() const noexcept override { return trace_id_low_; } - uint64_t span_id() const noexcept override { - return trace_context_.parent_id; - } + uint64_t span_id() const noexcept override { return span_id_; } bool sampled() const noexcept override { - return IsTraceFlagSet(trace_context_.trace_flags); + return IsTraceFlagSet(trace_flags_); } void ForeachBaggageItem( @@ -57,7 +55,10 @@ class ImmutableSpanContext final : public LightStepSpanContext { } private: - TraceContext trace_context_; + uint64_t trace_id_high_; + uint64_t trace_id_low_; + uint64_t span_id_; + uint8_t trace_flags_; BaggageProtobufMap baggage_; std::string trace_state_; From a2c23d7ddce5379d4728e43703ee03f2102d1110 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 2 Jan 2020 13:33:54 -0800 Subject: [PATCH 11/42] Add functions to inject TraceContext --- src/tracer/immutable_span_context.h | 10 ++++++--- src/tracer/propagation/BUILD | 2 ++ src/tracer/propagation/propagation.cpp | 28 ++++++++++++++++++++++++++ src/tracer/propagation/propagation.h | 17 ++++++++++++++++ src/tracer/propagation/propagator.h | 23 +++++++++++++++++++++ src/tracer/propagation/trace_context.h | 10 ++++----- 6 files changed, 82 insertions(+), 8 deletions(-) diff --git a/src/tracer/immutable_span_context.h b/src/tracer/immutable_span_context.h index 02ee8893..66657c8b 100644 --- a/src/tracer/immutable_span_context.h +++ b/src/tracer/immutable_span_context.h @@ -65,9 +65,13 @@ class ImmutableSpanContext final : public LightStepSpanContext { template opentracing::expected InjectImpl( const PropagationOptions& propagation_options, Carrier& writer) const { - return InjectSpanContext(propagation_options, writer, this->trace_id_high(), - this->trace_id_low(), this->span_id(), - this->sampled(), baggage_); + TraceContext trace_context; + trace_context.trace_id_high = trace_id_high_; + trace_context.trace_id_low = trace_id_low_; + trace_context.parent_id = span_id_; + trace_context.trace_flags = trace_flags_; + return InjectSpanContext(propagation_options, writer, trace_context, + trace_state_, baggage_); } }; } // namespace lightstep diff --git a/src/tracer/propagation/BUILD b/src/tracer/propagation/BUILD index 03b9f2b4..e157bf90 100644 --- a/src/tracer/propagation/BUILD +++ b/src/tracer/propagation/BUILD @@ -26,6 +26,7 @@ lightstep_cc_library( ], deps = [ "//src/tracer:baggage_flat_map_lib", + ":trace_context_lib", ], external_deps = [ "@com_google_protobuf//:protobuf", @@ -179,5 +180,6 @@ lightstep_cc_library( deps = [ "//src/common:in_memory_stream_lib", ":propagation_options_lib", + ":trace_context_lib", ], ) diff --git a/src/tracer/propagation/propagation.cpp b/src/tracer/propagation/propagation.cpp index a1f140d6..7626663f 100644 --- a/src/tracer/propagation/propagation.cpp +++ b/src/tracer/propagation/propagation.cpp @@ -44,6 +44,34 @@ template opentracing::expected InjectSpanContext( uint64_t trace_id_low, uint64_t span_id, bool sampled, const BaggageFlatMap& baggage); +template +opentracing::expected InjectSpanContext( + const PropagationOptions& propagation_options, + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view trace_state, + const BaggageMap& baggage) { + for (auto& propagator : propagation_options.inject_propagators) { + auto was_successful = propagator->InjectSpanContext(carrier, trace_context, + trace_state, baggage); + if (!was_successful) { + return was_successful; + } + } + return {}; +} + +template opentracing::expected InjectSpanContext( + const PropagationOptions& propagation_options, + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view trace_state, + const BaggageProtobufMap& baggage); + +template opentracing::expected InjectSpanContext( + const PropagationOptions& propagation_options, + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view trace_state, + const BaggageFlatMap& baggage); + //------------------------------------------------------------------------------ // ExtractSpanContext //------------------------------------------------------------------------------ diff --git a/src/tracer/propagation/propagation.h b/src/tracer/propagation/propagation.h index 33cb6c4f..136d4c4e 100644 --- a/src/tracer/propagation/propagation.h +++ b/src/tracer/propagation/propagation.h @@ -20,6 +20,16 @@ opentracing::expected InjectSpanContext( return InjectSpanContext(carrier, trace_id, span_id, sampled, baggage); } +template +opentracing::expected InjectSpanContext( + const PropagationOptions& /*propagation_options*/, std::ostream& carrier, + const TraceContext& trace_context, opentracing::string_view /*trace_state*/, + const BaggageMap& baggage) { + return InjectSpanContext( + carrier, trace_context.trace_id_low, trace_context.parent_id, + IsTraceFlagSet(trace_context.trace_flags), baggage); +} + template opentracing::expected InjectSpanContext( const PropagationOptions& propagation_options, @@ -27,6 +37,13 @@ opentracing::expected InjectSpanContext( uint64_t trace_id_low, uint64_t span_id, bool sampled, const BaggageMap& baggage); +template +opentracing::expected InjectSpanContext( + const PropagationOptions& propagation_options, + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view trace_state, + const BaggageMap& baggage); + inline opentracing::expected ExtractSpanContext( const PropagationOptions& /*propagation_options*/, std::istream& carrier, uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, diff --git a/src/tracer/propagation/propagator.h b/src/tracer/propagation/propagator.h index 346af207..8895e359 100644 --- a/src/tracer/propagation/propagator.h +++ b/src/tracer/propagation/propagator.h @@ -1,6 +1,7 @@ #pragma once #include "tracer/baggage_flat_map.h" +#include "tracer/propagation/trace_context.h" #include @@ -23,6 +24,28 @@ class Propagator { uint64_t trace_id_low, uint64_t span_id, bool sampled, const BaggageFlatMap& baggage) const = 0; + virtual opentracing::expected InjectSpanContext( + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view trace_state, + const BaggageProtobufMap& baggage) const { + (void)trace_state; + return InjectSpanContext( + carrier, trace_context.trace_id_high, trace_context.trace_id_low, + trace_context.parent_id, + IsTraceFlagSet(trace_context.trace_flags), baggage); + } + + virtual opentracing::expected InjectSpanContext( + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view trace_state, + const BaggageFlatMap& baggage) const { + (void)trace_state; + return InjectSpanContext( + carrier, trace_context.trace_id_high, trace_context.trace_id_low, + trace_context.parent_id, + IsTraceFlagSet(trace_context.trace_flags), baggage); + } + virtual opentracing::expected ExtractSpanContext( const opentracing::TextMapReader& carrier, bool case_sensitive, uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, diff --git a/src/tracer/propagation/trace_context.h b/src/tracer/propagation/trace_context.h index a9182de7..298bdcbd 100644 --- a/src/tracer/propagation/trace_context.h +++ b/src/tracer/propagation/trace_context.h @@ -11,11 +11,11 @@ const uint8_t SampledFlagMask = 1; const uint8_t TraceContextVersion = 0; struct TraceContext { - uint64_t trace_id_high; - uint64_t trace_id_low; - uint64_t parent_id; - uint8_t trace_flags; - uint8_t version; + uint64_t trace_id_high{0}; + uint64_t trace_id_low{0}; + uint64_t parent_id{0}; + uint8_t trace_flags{0}; + uint8_t version{TraceContextVersion}; }; opentracing::expected ParseTraceContext( From 8297e8d5a45c50cd8c893dd319d4db28df883241 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 2 Jan 2020 18:26:56 -0800 Subject: [PATCH 12/42] Convert Span to use trace-context storage --- src/tracer/span.cpp | 17 ++++++++++------- src/tracer/span.h | 14 +++++++++++--- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/tracer/span.cpp b/src/tracer/span.cpp index a3010ef2..d6d9902a 100644 --- a/src/tracer/span.cpp +++ b/src/tracer/span.cpp @@ -35,7 +35,7 @@ Span::Span(std::shared_ptr&& tracer, WriteStartTimestamp(stream_, start_timestamp); // Set any span references. - sampled_ = false; + trace_flags_ = 0; int reference_count = 0; for (auto& reference : options.references) { uint64_t trace_id_high; @@ -51,7 +51,7 @@ Span::Span(std::shared_ptr&& tracer, // If there are any span references, sampled should be true if any of the // references are sampled; with no refences, we set sampled to true. if (reference_count == 0) { - sampled_ = true; + trace_flags_ = SetTraceFlag(trace_flags_, true); auto ids = GenerateIds<2>(); trace_id_high_ = 0; trace_id_ = ids[0]; @@ -67,7 +67,7 @@ Span::Span(std::shared_ptr&& tracer, // If sampling_priority is set, it overrides whatever sampling decision was // derived from the referenced spans. if (tag.first == SamplingPriorityKey) { - sampled_ = is_sampled(tag.second); + trace_flags_ = SetTraceFlag(trace_flags_, is_sampled(tag.second)); } } } @@ -115,7 +115,8 @@ void Span::SetTag(opentracing::string_view key, } WriteTag(stream_, key, value); if (key == SamplingPriorityKey) { - sampled_ = is_sampled(value); + trace_flags_ = + SetTraceFlag(trace_flags_, is_sampled(value)); } } catch (const std::exception& e) { tracer_->logger().Error("SetTag failed: ", e.what()); @@ -189,7 +190,7 @@ void Span::ForeachBaggageItem( //------------------------------------------------------------------------------ bool Span::sampled() const noexcept { SpinLockGuard lock_guard{mutex_}; - return sampled_; + return IsTraceFlagSet(trace_flags_); } //-------------------------------------------------------------------------------------------------- @@ -213,7 +214,9 @@ bool Span::SetSpanReference( trace_id = referenced_context->trace_id_low(); WriteSpanReference(stream_, reference.first, trace_id, referenced_context->span_id()); - sampled_ = sampled_ || referenced_context->sampled(); + if (referenced_context->sampled()) { + trace_flags_ = SetTraceFlag(trace_flags_, true); + } referenced_context->ForeachBaggageItem( [this](const std::string& key, const std::string& value) { this->baggage_.insert_or_assign(std::string{key}, std::string{value}); @@ -232,7 +235,7 @@ void Span::FinishImpl( return; } - if (!sampled_) { + if(!IsTraceFlagSet(trace_flags_)) { return; } diff --git a/src/tracer/span.h b/src/tracer/span.h index 4419607d..7856b8a9 100644 --- a/src/tracer/span.h +++ b/src/tracer/span.h @@ -104,15 +104,23 @@ class Span final : public opentracing::Span, public LightStepSpanContext { uint64_t trace_id_high_{0}; uint64_t trace_id_; uint64_t span_id_; + uint8_t trace_flags_; BaggageFlatMap baggage_; - bool sampled_; + std::string trace_state_; template opentracing::expected InjectImpl( const PropagationOptions& propagation_options, Carrier& writer) const { SpinLockGuard lock_guard{mutex_}; - return InjectSpanContext(propagation_options, writer, trace_id_high_, - trace_id_, span_id_, sampled_, baggage_); + TraceContext trace_context; + trace_context.trace_id_high = trace_id_high_; + trace_context.trace_id_low = trace_id_; + trace_context.parent_id = span_id_; + trace_context.trace_flags = trace_flags_; + return InjectSpanContext(propagation_options, writer, trace_context, + trace_state_, baggage_); + /* return InjectSpanContext(propagation_options, writer, trace_id_high_, */ + /* trace_id_, span_id_, sampled_, baggage_); */ } bool SetSpanReference( From 565a4ca51dda3b4a6860b743cf267d96a4a34090 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 2 Jan 2020 20:47:22 -0800 Subject: [PATCH 13/42] Convert LegacySpan to use TraceContext --- src/tracer/legacy/legacy_span.cpp | 23 ++++++++++++++--------- src/tracer/legacy/legacy_span.h | 15 ++++++++++----- src/tracer/span.h | 2 -- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/tracer/legacy/legacy_span.cpp b/src/tracer/legacy/legacy_span.cpp index 175fc95e..397f94e6 100644 --- a/src/tracer/legacy/legacy_span.cpp +++ b/src/tracer/legacy/legacy_span.cpp @@ -31,7 +31,7 @@ LegacySpan::LegacySpan(std::shared_ptr&& tracer, *span_.mutable_start_timestamp() = ToTimestamp(start_timestamp); // Set any span references. - sampled_ = false; + trace_flags_ = 0; collector::Reference collector_reference; auto& references = *span_.mutable_references(); references.Reserve(static_cast(options.references.size())); @@ -39,7 +39,7 @@ LegacySpan::LegacySpan(std::shared_ptr&& tracer, uint64_t trace_id_high; uint64_t trace_id; if (!SetSpanReference(reference, baggage, collector_reference, - trace_id_high, trace_id, sampled_)) { + trace_id_high, trace_id)) { continue; } *references.Add() = collector_reference; @@ -52,7 +52,7 @@ LegacySpan::LegacySpan(std::shared_ptr&& tracer, // If there are any span references, sampled should be true if any of the // references are sampled; with no refences, we set sampled to true. if (references.empty()) { - sampled_ = true; + trace_flags_ = SetTraceFlag(trace_flags_, true); } // Set tags. @@ -64,7 +64,8 @@ LegacySpan::LegacySpan(std::shared_ptr&& tracer, // If sampling_priority is set, it overrides whatever sampling decision was // derived from the referenced spans. if (tag.first == SamplingPriorityKey) { - sampled_ = is_sampled(tag.second); + trace_flags_ = + SetTraceFlag(trace_flags_, is_sampled(tag.second)); } } @@ -96,7 +97,7 @@ void LegacySpan::FinishWithOptions( } std::lock_guard lock_guard{mutex_}; - if (!sampled_) { + if(!IsTraceFlagSet(trace_flags_)) { return; } @@ -147,7 +148,8 @@ void LegacySpan::SetTag(opentracing::string_view key, *span_.mutable_tags()->Add() = ToKeyValue(key, value); if (key == SamplingPriorityKey) { - sampled_ = is_sampled(value); + trace_flags_ = + SetTraceFlag(trace_flags_, is_sampled(value)); } } catch (const std::exception& e) { logger_.Error("SetTag failed: ", e.what()); @@ -218,7 +220,7 @@ void LegacySpan::ForeachBaggageItem( //------------------------------------------------------------------------------ bool LegacySpan::sampled() const noexcept { std::lock_guard lock_guard{mutex_}; - return sampled_; + return IsTraceFlagSet(trace_flags_); } //-------------------------------------------------------------------------------------------------- @@ -228,7 +230,7 @@ bool LegacySpan::SetSpanReference( const std::pair& reference, BaggageProtobufMap& baggage, collector::Reference& collector_reference, - uint64_t& trace_id_high, uint64_t& trace_id, bool& sampled) { + uint64_t& trace_id_high, uint64_t& trace_id) { collector_reference.Clear(); switch (reference.first) { case opentracing::SpanReferenceType::ChildOfRef: @@ -253,7 +255,10 @@ bool LegacySpan::SetSpanReference( collector_reference.mutable_span_context()->set_trace_id(trace_id); collector_reference.mutable_span_context()->set_span_id( referenced_context->span_id()); - sampled = sampled || referenced_context->sampled(); + + if (referenced_context->sampled()) { + trace_flags_ = SetTraceFlag(trace_flags_, true); + } referenced_context->ForeachBaggageItem( [&baggage](const std::string& key, const std::string& value) { diff --git a/src/tracer/legacy/legacy_span.h b/src/tracer/legacy/legacy_span.h index 3bcd42b9..b0dd689b 100644 --- a/src/tracer/legacy/legacy_span.h +++ b/src/tracer/legacy/legacy_span.h @@ -88,9 +88,10 @@ class LegacySpan final : public opentracing::Span, public LightStepSpanContext { private: // Fields set in StartSpan() are not protected by a mutex. uint64_t trace_id_high_{0}; + uint8_t trace_flags_; collector::Span span_; + std::string trace_state_; std::chrono::steady_clock::time_point start_steady_; - bool sampled_; mutable std::mutex mutex_; std::shared_ptr tracer_; Logger& logger_; @@ -103,15 +104,19 @@ class LegacySpan final : public opentracing::Span, public LightStepSpanContext { const PropagationOptions& propagation_options, Carrier& writer) const { std::lock_guard lock_guard{mutex_}; auto& span_context = span_.span_context(); - return InjectSpanContext(propagation_options, writer, trace_id_high_, - span_context.trace_id(), span_context.span_id(), - sampled_, span_context.baggage()); + TraceContext trace_context; + trace_context.trace_id_high = trace_id_high_; + trace_context.trace_id_low = span_context.trace_id(); + trace_context.parent_id = span_context.span_id(); + trace_context.trace_flags = trace_flags_; + return InjectSpanContext(propagation_options, writer, trace_context, + trace_state_, span_context.baggage()); } bool SetSpanReference( const std::pair& reference, BaggageProtobufMap& baggage, collector::Reference& collector_reference, - uint64_t& trace_id_high, uint64_t& trace_id, bool& sampled); + uint64_t& trace_id_high, uint64_t& trace_id); }; } // namespace lightstep diff --git a/src/tracer/span.h b/src/tracer/span.h index 7856b8a9..d8b20d5f 100644 --- a/src/tracer/span.h +++ b/src/tracer/span.h @@ -119,8 +119,6 @@ class Span final : public opentracing::Span, public LightStepSpanContext { trace_context.trace_flags = trace_flags_; return InjectSpanContext(propagation_options, writer, trace_context, trace_state_, baggage_); - /* return InjectSpanContext(propagation_options, writer, trace_id_high_, */ - /* trace_id_, span_id_, sampled_, baggage_); */ } bool SetSpanReference( From 112fdcb4d83f7771a301817bec0e6ca4a45e6df3 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 2 Jan 2020 21:15:18 -0800 Subject: [PATCH 14/42] Add flag accessor as part of span context interface --- src/tracer/immutable_span_context.h | 2 ++ src/tracer/legacy/legacy_span.cpp | 8 ++++++++ src/tracer/legacy/legacy_span.h | 2 ++ src/tracer/lightstep_span_context.h | 2 ++ src/tracer/span.cpp | 8 ++++++++ src/tracer/span.h | 2 ++ 6 files changed, 24 insertions(+) diff --git a/src/tracer/immutable_span_context.h b/src/tracer/immutable_span_context.h index 66657c8b..6abc3c99 100644 --- a/src/tracer/immutable_span_context.h +++ b/src/tracer/immutable_span_context.h @@ -32,6 +32,8 @@ class ImmutableSpanContext final : public LightStepSpanContext { return IsTraceFlagSet(trace_flags_); } + uint8_t trace_flags() const noexcept override { return trace_flags_; } + void ForeachBaggageItem( std::function f) const override; diff --git a/src/tracer/legacy/legacy_span.cpp b/src/tracer/legacy/legacy_span.cpp index 397f94e6..20f8410b 100644 --- a/src/tracer/legacy/legacy_span.cpp +++ b/src/tracer/legacy/legacy_span.cpp @@ -223,6 +223,14 @@ bool LegacySpan::sampled() const noexcept { return IsTraceFlagSet(trace_flags_); } +//------------------------------------------------------------------------------ +// trace_flags +//------------------------------------------------------------------------------ +uint8_t LegacySpan::trace_flags() const noexcept { + std::lock_guard lock_guard{mutex_}; + return trace_flags_; +} + //-------------------------------------------------------------------------------------------------- // SetSpanReference //-------------------------------------------------------------------------------------------------- diff --git a/src/tracer/legacy/legacy_span.h b/src/tracer/legacy/legacy_span.h index b0dd689b..a2d8f857 100644 --- a/src/tracer/legacy/legacy_span.h +++ b/src/tracer/legacy/legacy_span.h @@ -67,6 +67,8 @@ class LegacySpan final : public opentracing::Span, public LightStepSpanContext { bool sampled() const noexcept override; + uint8_t trace_flags() const noexcept override; + opentracing::expected Inject( const PropagationOptions& propagation_options, std::ostream& writer) const override { diff --git a/src/tracer/lightstep_span_context.h b/src/tracer/lightstep_span_context.h index 6733a892..9aff3f7c 100644 --- a/src/tracer/lightstep_span_context.h +++ b/src/tracer/lightstep_span_context.h @@ -19,6 +19,8 @@ class LightStepSpanContext : public opentracing::SpanContext { virtual bool sampled() const noexcept = 0; + virtual uint8_t trace_flags() const noexcept = 0; + virtual opentracing::string_view trace_state() const noexcept { return {}; } virtual opentracing::expected Inject( diff --git a/src/tracer/span.cpp b/src/tracer/span.cpp index d6d9902a..f94f810a 100644 --- a/src/tracer/span.cpp +++ b/src/tracer/span.cpp @@ -193,6 +193,14 @@ bool Span::sampled() const noexcept { return IsTraceFlagSet(trace_flags_); } +//------------------------------------------------------------------------------ +// trace_flags +//------------------------------------------------------------------------------ +uint8_t Span::trace_flags() const noexcept { + SpinLockGuard lock_guard{mutex_}; + return trace_flags_; +} + //-------------------------------------------------------------------------------------------------- // SetSpanReference //-------------------------------------------------------------------------------------------------- diff --git a/src/tracer/span.h b/src/tracer/span.h index d8b20d5f..6bca6b9e 100644 --- a/src/tracer/span.h +++ b/src/tracer/span.h @@ -85,6 +85,8 @@ class Span final : public opentracing::Span, public LightStepSpanContext { bool sampled() const noexcept override; + uint8_t trace_flags() const noexcept override; + private: // Profiling shows that even with no contention, locking and unlocking a // standard mutex represents a significant portion of the cost of From 8e94e3614b9e8215e92f9cc256caa71e522a421e Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 2 Jan 2020 21:30:17 -0800 Subject: [PATCH 15/42] Refactor sampled code --- src/tracer/immutable_span_context.h | 4 ---- src/tracer/legacy/legacy_span.cpp | 12 +----------- src/tracer/legacy/legacy_span.h | 2 -- src/tracer/lightstep_span_context.cpp | 2 +- src/tracer/lightstep_span_context.h | 4 +++- src/tracer/span.cpp | 12 +----------- src/tracer/span.h | 2 -- 7 files changed, 6 insertions(+), 32 deletions(-) diff --git a/src/tracer/immutable_span_context.h b/src/tracer/immutable_span_context.h index 6abc3c99..0ceb3f81 100644 --- a/src/tracer/immutable_span_context.h +++ b/src/tracer/immutable_span_context.h @@ -28,10 +28,6 @@ class ImmutableSpanContext final : public LightStepSpanContext { uint64_t span_id() const noexcept override { return span_id_; } - bool sampled() const noexcept override { - return IsTraceFlagSet(trace_flags_); - } - uint8_t trace_flags() const noexcept override { return trace_flags_; } void ForeachBaggageItem( diff --git a/src/tracer/legacy/legacy_span.cpp b/src/tracer/legacy/legacy_span.cpp index 20f8410b..c09f40b6 100644 --- a/src/tracer/legacy/legacy_span.cpp +++ b/src/tracer/legacy/legacy_span.cpp @@ -215,14 +215,6 @@ void LegacySpan::ForeachBaggageItem( } } -//------------------------------------------------------------------------------ -// sampled -//------------------------------------------------------------------------------ -bool LegacySpan::sampled() const noexcept { - std::lock_guard lock_guard{mutex_}; - return IsTraceFlagSet(trace_flags_); -} - //------------------------------------------------------------------------------ // trace_flags //------------------------------------------------------------------------------ @@ -264,9 +256,7 @@ bool LegacySpan::SetSpanReference( collector_reference.mutable_span_context()->set_span_id( referenced_context->span_id()); - if (referenced_context->sampled()) { - trace_flags_ = SetTraceFlag(trace_flags_, true); - } + trace_flags_ |= referenced_context->trace_flags(); referenced_context->ForeachBaggageItem( [&baggage](const std::string& key, const std::string& value) { diff --git a/src/tracer/legacy/legacy_span.h b/src/tracer/legacy/legacy_span.h index a2d8f857..a305377b 100644 --- a/src/tracer/legacy/legacy_span.h +++ b/src/tracer/legacy/legacy_span.h @@ -65,8 +65,6 @@ class LegacySpan final : public opentracing::Span, public LightStepSpanContext { return span_.span_context().span_id(); } - bool sampled() const noexcept override; - uint8_t trace_flags() const noexcept override; opentracing::expected Inject( diff --git a/src/tracer/lightstep_span_context.cpp b/src/tracer/lightstep_span_context.cpp index f24a9fd8..b04e55bd 100644 --- a/src/tracer/lightstep_span_context.cpp +++ b/src/tracer/lightstep_span_context.cpp @@ -18,7 +18,7 @@ bool operator==(const LightStepSpanContext& lhs, return lhs.trace_id_high() == rhs.trace_id_high() && lhs.trace_id_low() == rhs.trace_id_low() && - lhs.span_id() == rhs.span_id() && lhs.sampled() == rhs.sampled() && + lhs.span_id() == rhs.span_id() && lhs.trace_flags() == rhs.trace_flags() && extract_baggage(lhs) == extract_baggage(rhs); } } // namespace lightstep diff --git a/src/tracer/lightstep_span_context.h b/src/tracer/lightstep_span_context.h index 9aff3f7c..6a09a500 100644 --- a/src/tracer/lightstep_span_context.h +++ b/src/tracer/lightstep_span_context.h @@ -17,7 +17,9 @@ class LightStepSpanContext : public opentracing::SpanContext { virtual uint64_t span_id() const noexcept = 0; - virtual bool sampled() const noexcept = 0; + bool sampled() const noexcept { + return IsTraceFlagSet(this->trace_flags()); + } virtual uint8_t trace_flags() const noexcept = 0; diff --git a/src/tracer/span.cpp b/src/tracer/span.cpp index f94f810a..f2132f9b 100644 --- a/src/tracer/span.cpp +++ b/src/tracer/span.cpp @@ -185,14 +185,6 @@ void Span::ForeachBaggageItem( } } -//------------------------------------------------------------------------------ -// sampled -//------------------------------------------------------------------------------ -bool Span::sampled() const noexcept { - SpinLockGuard lock_guard{mutex_}; - return IsTraceFlagSet(trace_flags_); -} - //------------------------------------------------------------------------------ // trace_flags //------------------------------------------------------------------------------ @@ -222,9 +214,7 @@ bool Span::SetSpanReference( trace_id = referenced_context->trace_id_low(); WriteSpanReference(stream_, reference.first, trace_id, referenced_context->span_id()); - if (referenced_context->sampled()) { - trace_flags_ = SetTraceFlag(trace_flags_, true); - } + trace_flags_ |= referenced_context->trace_flags(); referenced_context->ForeachBaggageItem( [this](const std::string& key, const std::string& value) { this->baggage_.insert_or_assign(std::string{key}, std::string{value}); diff --git a/src/tracer/span.h b/src/tracer/span.h index 6bca6b9e..9e3dc163 100644 --- a/src/tracer/span.h +++ b/src/tracer/span.h @@ -83,8 +83,6 @@ class Span final : public opentracing::Span, public LightStepSpanContext { uint64_t span_id() const noexcept override { return span_id_; } - bool sampled() const noexcept override; - uint8_t trace_flags() const noexcept override; private: From 5fa82dfc0bf4f0bd6a3bfb74422069a2279f8826 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 2 Jan 2020 22:44:40 -0800 Subject: [PATCH 16/42] Join trace-states from referenced spans --- CMakeLists.txt | 1 + src/tracer/BUILD | 3 +++ src/tracer/legacy/legacy_span.cpp | 1 + src/tracer/span.cpp | 1 + src/tracer/utility.h | 8 ++++++++ test/tracer/BUILD | 10 ++++++++++ 6 files changed, 24 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index ed4fda8d..500f6746 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -231,6 +231,7 @@ set(LIGHTSTEP_SRCS src/common/utility.cpp src/tracer/tracer_impl.cpp src/tracer/tracer.cpp src/tracer/tag.cpp + src/tracer/utility.cpp ) if (WIN32) diff --git a/src/tracer/BUILD b/src/tracer/BUILD index a7bd1d2b..8db48d8f 100644 --- a/src/tracer/BUILD +++ b/src/tracer/BUILD @@ -36,6 +36,9 @@ lightstep_cc_library( private_hdrs = [ "utility.h", ], + srcs = [ + "utility.cpp", + ], deps = [ "//src/recorder:recorder_interface", ], diff --git a/src/tracer/legacy/legacy_span.cpp b/src/tracer/legacy/legacy_span.cpp index c09f40b6..7fbd2d32 100644 --- a/src/tracer/legacy/legacy_span.cpp +++ b/src/tracer/legacy/legacy_span.cpp @@ -257,6 +257,7 @@ bool LegacySpan::SetSpanReference( referenced_context->span_id()); trace_flags_ |= referenced_context->trace_flags(); + AppendTraceState(trace_state_, referenced_context->trace_state()); referenced_context->ForeachBaggageItem( [&baggage](const std::string& key, const std::string& value) { diff --git a/src/tracer/span.cpp b/src/tracer/span.cpp index f2132f9b..acb8028a 100644 --- a/src/tracer/span.cpp +++ b/src/tracer/span.cpp @@ -215,6 +215,7 @@ bool Span::SetSpanReference( WriteSpanReference(stream_, reference.first, trace_id, referenced_context->span_id()); trace_flags_ |= referenced_context->trace_flags(); + AppendTraceState(trace_state_, referenced_context->trace_state()); referenced_context->ForeachBaggageItem( [this](const std::string& key, const std::string& value) { this->baggage_.insert_or_assign(std::string{key}, std::string{value}); diff --git a/src/tracer/utility.h b/src/tracer/utility.h index e6fe804f..824d7c2a 100644 --- a/src/tracer/utility.h +++ b/src/tracer/utility.h @@ -6,6 +6,7 @@ #include "recorder/recorder.h" #include +#include namespace lightstep { using SystemClock = std::chrono::system_clock; @@ -56,4 +57,11 @@ inline std::tuple ComputeStartTimestamps( return std::tuple{start_system_timestamp, start_steady_timestamp}; } + +/** + * Appends trace-states to have the union of the two key-value lists. + * @param trace_state the trace_state to append to + * @param key_values the key-values of another trace-state + */ +void AppendTraceState(std::string& trace_state, opentracing::string_view key_values); } // namespace lightstep diff --git a/test/tracer/BUILD b/test/tracer/BUILD index c5e5a512..1042d772 100644 --- a/test/tracer/BUILD +++ b/test/tracer/BUILD @@ -21,6 +21,16 @@ lightstep_catch_test( ], ) +lightstep_catch_test( + name = "utility_test", + srcs = [ + "utility_test.cpp", + ], + deps = [ + "//src/tracer:utility_lib", + ], +) + lightstep_catch_test( name = "dynamic_load_test", srcs = [ From bbf5c68d1a14ff0ad039a5fafc0803bdd7397303 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 2 Jan 2020 23:04:11 -0800 Subject: [PATCH 17/42] Join trace-states from referenced spans --- src/tracer/utility.cpp | 14 ++++++++++++++ test/tracer/utility_test.cpp | 19 +++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 src/tracer/utility.cpp create mode 100644 test/tracer/utility_test.cpp diff --git a/src/tracer/utility.cpp b/src/tracer/utility.cpp new file mode 100644 index 00000000..c21e95a7 --- /dev/null +++ b/src/tracer/utility.cpp @@ -0,0 +1,14 @@ +#include "tracer/utility.h" + +namespace lightstep { +void AppendTraceState(std::string& trace_state, + opentracing::string_view key_values) { + if (trace_state.empty()) { + trace_state.assign(key_values.data(), key_values.size()); + return; + } + trace_state.reserve(trace_state.size() + 1 + key_values.size()); + trace_state.append(","); + trace_state.append(key_values.data(), key_values.size()); +} +} // namespace lightstep diff --git a/test/tracer/utility_test.cpp b/test/tracer/utility_test.cpp new file mode 100644 index 00000000..0fe1834d --- /dev/null +++ b/test/tracer/utility_test.cpp @@ -0,0 +1,19 @@ +#include "tracer/utility.h" + +#include "3rd_party/catch2/catch.hpp" +using namespace lightstep; + +TEST_CASE("We can append the values of w3 trace-state values") { + std::string trace_state; + + SECTION("We can append to an empty trace-state") { + AppendTraceState(trace_state, "abc=123"); + REQUIRE(trace_state == "abc=123"); + } + + SECTION("We can append to a non-empty trace-state") { + trace_state = "abc=123"; + AppendTraceState(trace_state, "xyz=456"); + REQUIRE(trace_state == "abc=123,xyz=456"); + } +} From d235b4ea7c578266bc3e490be0044cf6edac3815 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 2 Jan 2020 23:09:48 -0800 Subject: [PATCH 18/42] Add trace-state support --- src/tracer/immutable_span_context.h | 4 ++++ src/tracer/legacy/legacy_span.cpp | 2 +- src/tracer/legacy/legacy_span.h | 4 ++++ src/tracer/lightstep_span_context.cpp | 3 ++- src/tracer/lightstep_span_context.h | 2 +- src/tracer/span.cpp | 5 +++-- src/tracer/span.h | 4 ++++ src/tracer/utility.cpp | 2 +- src/tracer/utility.h | 5 +++-- 9 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/tracer/immutable_span_context.h b/src/tracer/immutable_span_context.h index 0ceb3f81..834ea4f2 100644 --- a/src/tracer/immutable_span_context.h +++ b/src/tracer/immutable_span_context.h @@ -30,6 +30,10 @@ class ImmutableSpanContext final : public LightStepSpanContext { uint8_t trace_flags() const noexcept override { return trace_flags_; } + opentracing::string_view trace_state() const noexcept override { + return trace_state_; + } + void ForeachBaggageItem( std::function f) const override; diff --git a/src/tracer/legacy/legacy_span.cpp b/src/tracer/legacy/legacy_span.cpp index 7fbd2d32..4f07c327 100644 --- a/src/tracer/legacy/legacy_span.cpp +++ b/src/tracer/legacy/legacy_span.cpp @@ -97,7 +97,7 @@ void LegacySpan::FinishWithOptions( } std::lock_guard lock_guard{mutex_}; - if(!IsTraceFlagSet(trace_flags_)) { + if (!IsTraceFlagSet(trace_flags_)) { return; } diff --git a/src/tracer/legacy/legacy_span.h b/src/tracer/legacy/legacy_span.h index a305377b..fc4c8f53 100644 --- a/src/tracer/legacy/legacy_span.h +++ b/src/tracer/legacy/legacy_span.h @@ -67,6 +67,10 @@ class LegacySpan final : public opentracing::Span, public LightStepSpanContext { uint8_t trace_flags() const noexcept override; + opentracing::string_view trace_state() const noexcept override { + return trace_state_; + } + opentracing::expected Inject( const PropagationOptions& propagation_options, std::ostream& writer) const override { diff --git a/src/tracer/lightstep_span_context.cpp b/src/tracer/lightstep_span_context.cpp index b04e55bd..40927616 100644 --- a/src/tracer/lightstep_span_context.cpp +++ b/src/tracer/lightstep_span_context.cpp @@ -18,7 +18,8 @@ bool operator==(const LightStepSpanContext& lhs, return lhs.trace_id_high() == rhs.trace_id_high() && lhs.trace_id_low() == rhs.trace_id_low() && - lhs.span_id() == rhs.span_id() && lhs.trace_flags() == rhs.trace_flags() && + lhs.span_id() == rhs.span_id() && + lhs.trace_flags() == rhs.trace_flags() && extract_baggage(lhs) == extract_baggage(rhs); } } // namespace lightstep diff --git a/src/tracer/lightstep_span_context.h b/src/tracer/lightstep_span_context.h index 6a09a500..b63aae1d 100644 --- a/src/tracer/lightstep_span_context.h +++ b/src/tracer/lightstep_span_context.h @@ -23,7 +23,7 @@ class LightStepSpanContext : public opentracing::SpanContext { virtual uint8_t trace_flags() const noexcept = 0; - virtual opentracing::string_view trace_state() const noexcept { return {}; } + virtual opentracing::string_view trace_state() const noexcept = 0; virtual opentracing::expected Inject( const PropagationOptions& propagation_options, diff --git a/src/tracer/span.cpp b/src/tracer/span.cpp index acb8028a..2cda12e3 100644 --- a/src/tracer/span.cpp +++ b/src/tracer/span.cpp @@ -67,7 +67,8 @@ Span::Span(std::shared_ptr&& tracer, // If sampling_priority is set, it overrides whatever sampling decision was // derived from the referenced spans. if (tag.first == SamplingPriorityKey) { - trace_flags_ = SetTraceFlag(trace_flags_, is_sampled(tag.second)); + trace_flags_ = + SetTraceFlag(trace_flags_, is_sampled(tag.second)); } } } @@ -234,7 +235,7 @@ void Span::FinishImpl( return; } - if(!IsTraceFlagSet(trace_flags_)) { + if (!IsTraceFlagSet(trace_flags_)) { return; } diff --git a/src/tracer/span.h b/src/tracer/span.h index 9e3dc163..275584d0 100644 --- a/src/tracer/span.h +++ b/src/tracer/span.h @@ -85,6 +85,10 @@ class Span final : public opentracing::Span, public LightStepSpanContext { uint8_t trace_flags() const noexcept override; + opentracing::string_view trace_state() const noexcept override { + return trace_state_; + } + private: // Profiling shows that even with no contention, locking and unlocking a // standard mutex represents a significant portion of the cost of diff --git a/src/tracer/utility.cpp b/src/tracer/utility.cpp index c21e95a7..b16b9665 100644 --- a/src/tracer/utility.cpp +++ b/src/tracer/utility.cpp @@ -11,4 +11,4 @@ void AppendTraceState(std::string& trace_state, trace_state.append(","); trace_state.append(key_values.data(), key_values.size()); } -} // namespace lightstep +} // namespace lightstep diff --git a/src/tracer/utility.h b/src/tracer/utility.h index 824d7c2a..51272ef5 100644 --- a/src/tracer/utility.h +++ b/src/tracer/utility.h @@ -5,8 +5,8 @@ #include "recorder/recorder.h" -#include #include +#include namespace lightstep { using SystemClock = std::chrono::system_clock; @@ -63,5 +63,6 @@ inline std::tuple ComputeStartTimestamps( * @param trace_state the trace_state to append to * @param key_values the key-values of another trace-state */ -void AppendTraceState(std::string& trace_state, opentracing::string_view key_values); +void AppendTraceState(std::string& trace_state, + opentracing::string_view key_values); } // namespace lightstep From 3044e4c4bbb635ae76ec9ecc2d74391c555176b4 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 3 Jan 2020 13:44:48 -0800 Subject: [PATCH 19/42] Fill out TraceContextPropagator's injection method --- src/tracer/propagation/trace_context.cpp | 2 +- src/tracer/propagation/trace_context.h | 2 +- .../propagation/trace_context_propagator.cpp | 39 +++++++++++++++++++ .../propagation/trace_context_propagator.h | 12 ++++++ .../tracer/propagation/trace_context_test.cpp | 2 +- 5 files changed, 54 insertions(+), 3 deletions(-) diff --git a/src/tracer/propagation/trace_context.cpp b/src/tracer/propagation/trace_context.cpp index 0a80d73d..e6255675 100644 --- a/src/tracer/propagation/trace_context.cpp +++ b/src/tracer/propagation/trace_context.cpp @@ -8,7 +8,7 @@ namespace lightstep { //-------------------------------------------------------------------------------------------------- opentracing::expected ParseTraceContext( opentracing::string_view s, TraceContext& trace_context) noexcept { - if (s.size() < TraceContextMinLength) { + if (s.size() < TraceContextLength) { return opentracing::make_unexpected( std::make_error_code(std::errc::invalid_argument)); } diff --git a/src/tracer/propagation/trace_context.h b/src/tracer/propagation/trace_context.h index 298bdcbd..cb15f409 100644 --- a/src/tracer/propagation/trace_context.h +++ b/src/tracer/propagation/trace_context.h @@ -6,7 +6,7 @@ #include namespace lightstep { -const size_t TraceContextMinLength = 55; +const size_t TraceContextLength = 55; const uint8_t SampledFlagMask = 1; const uint8_t TraceContextVersion = 0; diff --git a/src/tracer/propagation/trace_context_propagator.cpp b/src/tracer/propagation/trace_context_propagator.cpp index 81565d2c..29cb46c5 100644 --- a/src/tracer/propagation/trace_context_propagator.cpp +++ b/src/tracer/propagation/trace_context_propagator.cpp @@ -1,6 +1,31 @@ #include "tracer/propagation/trace_context_propagator.h" +#include + +const opentracing::string_view TraceParentHeaderKey = "traceparent"; +const opentracing::string_view TraceStateHeaderKey = "tracestate"; + namespace lightstep { +//-------------------------------------------------------------------------------------------------- +// InjectSpanContextImpl +//-------------------------------------------------------------------------------------------------- +static opentracing::expected InjectSpanContextImpl( + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view trace_state) { + std::array buffer; + SerializeTraceContext(trace_context, buffer.data()); + auto result = carrier.Set( + TraceParentHeaderKey, + opentracing::string_view{buffer.data(), buffer.size()}); + if (!result) { + return result; + } + if (trace_state.empty()) { + return {}; + } + return carrier.Set(TraceStateHeaderKey, trace_state); +} + //-------------------------------------------------------------------------------------------------- // InjectSpanContext //-------------------------------------------------------------------------------------------------- @@ -30,6 +55,20 @@ opentracing::expected TraceContextPropagator::InjectSpanContext( return {}; } +opentracing::expected TraceContextPropagator::InjectSpanContext( + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view trace_state, + const BaggageProtobufMap& /*baggage*/) const { + return InjectSpanContextImpl(carrier, trace_context, trace_state); +} + +opentracing::expected TraceContextPropagator::InjectSpanContext( + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view trace_state, + const BaggageFlatMap& /*baggage*/) const { + return InjectSpanContextImpl(carrier, trace_context, trace_state); +} + //-------------------------------------------------------------------------------------------------- // ExtractSpanContext //-------------------------------------------------------------------------------------------------- diff --git a/src/tracer/propagation/trace_context_propagator.h b/src/tracer/propagation/trace_context_propagator.h index 09a8c297..5e30779c 100644 --- a/src/tracer/propagation/trace_context_propagator.h +++ b/src/tracer/propagation/trace_context_propagator.h @@ -16,6 +16,18 @@ class TraceContextPropagator final : Propagator { uint64_t trace_id_low, uint64_t span_id, bool sampled, const BaggageFlatMap& baggage) const override; + opentracing::expected InjectSpanContext( + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, + opentracing::string_view trace_state, + const BaggageProtobufMap& baggage) const override; + + opentracing::expected InjectSpanContext( + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, + opentracing::string_view trace_state, + const BaggageFlatMap& baggage) const override; + opentracing::expected ExtractSpanContext( const opentracing::TextMapReader& carrier, bool case_sensitive, uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, diff --git a/test/tracer/propagation/trace_context_test.cpp b/test/tracer/propagation/trace_context_test.cpp index 1d66b294..bcc5b104 100644 --- a/test/tracer/propagation/trace_context_test.cpp +++ b/test/tracer/propagation/trace_context_test.cpp @@ -45,7 +45,7 @@ TEST_CASE("We can serialize and deserialize a trace-context") { "If we deserialize a trace-context and serialize again, we'll get the " "same value") { REQUIRE(ParseTraceContext(valid_trace_context, trace_context)); - std::string s(TraceContextMinLength, ' '); + std::string s(TraceContextLength, ' '); SerializeTraceContext(trace_context, &s[0]); REQUIRE(s == valid_trace_context); } From 6de5320a9c9e0ed4770ee35a3ae9662c7a5969b4 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 3 Jan 2020 13:59:05 -0800 Subject: [PATCH 20/42] Migrate to new injectors --- src/tracer/propagation/propagation.cpp | 28 -------------- src/tracer/propagation/propagator.h | 37 ++++++++++++++----- .../propagation/trace_context_propagator.cpp | 26 ------------- .../propagation/trace_context_propagator.h | 10 ----- 4 files changed, 27 insertions(+), 74 deletions(-) diff --git a/src/tracer/propagation/propagation.cpp b/src/tracer/propagation/propagation.cpp index 7626663f..41124dcf 100644 --- a/src/tracer/propagation/propagation.cpp +++ b/src/tracer/propagation/propagation.cpp @@ -16,34 +16,6 @@ namespace lightstep { //------------------------------------------------------------------------------ // InjectSpanContext //------------------------------------------------------------------------------ -template -opentracing::expected InjectSpanContext( - const PropagationOptions& propagation_options, - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, - const BaggageMap& baggage) { - for (auto& propagator : propagation_options.inject_propagators) { - auto was_successful = propagator->InjectSpanContext( - carrier, trace_id_high, trace_id_low, span_id, sampled, baggage); - if (!was_successful) { - return was_successful; - } - } - return {}; -} - -template opentracing::expected InjectSpanContext( - const PropagationOptions& propagation_options, - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, - const BaggageProtobufMap& baggage); - -template opentracing::expected InjectSpanContext( - const PropagationOptions& propagation_options, - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, - const BaggageFlatMap& baggage); - template opentracing::expected InjectSpanContext( const PropagationOptions& propagation_options, diff --git a/src/tracer/propagation/propagator.h b/src/tracer/propagation/propagator.h index 8895e359..bced747d 100644 --- a/src/tracer/propagation/propagator.h +++ b/src/tracer/propagation/propagator.h @@ -14,16 +14,6 @@ class Propagator { public: virtual ~Propagator() noexcept = default; - virtual opentracing::expected InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, - const BaggageProtobufMap& baggage) const = 0; - - virtual opentracing::expected InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, - const BaggageFlatMap& baggage) const = 0; - virtual opentracing::expected InjectSpanContext( const opentracing::TextMapWriter& carrier, const TraceContext& trace_context, opentracing::string_view trace_state, @@ -50,5 +40,32 @@ class Propagator { const opentracing::TextMapReader& carrier, bool case_sensitive, uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, bool& sampled, BaggageProtobufMap& baggage) const = 0; + private: + virtual opentracing::expected InjectSpanContext( + const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, + uint64_t trace_id_low, uint64_t span_id, bool sampled, + const BaggageProtobufMap& baggage) const { + (void)carrier; + (void)trace_id_high; + (void)trace_id_low; + (void)span_id; + (void)sampled; + (void)baggage; + std::terminate(); + } + + virtual opentracing::expected InjectSpanContext( + const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, + uint64_t trace_id_low, uint64_t span_id, bool sampled, + const BaggageFlatMap& baggage) const { + (void)carrier; + (void)trace_id_high; + (void)trace_id_low; + (void)span_id; + (void)sampled; + (void)baggage; + std::terminate(); + } + }; } // namespace lightstep diff --git a/src/tracer/propagation/trace_context_propagator.cpp b/src/tracer/propagation/trace_context_propagator.cpp index 29cb46c5..7887746a 100644 --- a/src/tracer/propagation/trace_context_propagator.cpp +++ b/src/tracer/propagation/trace_context_propagator.cpp @@ -29,32 +29,6 @@ static opentracing::expected InjectSpanContextImpl( //-------------------------------------------------------------------------------------------------- // InjectSpanContext //-------------------------------------------------------------------------------------------------- -opentracing::expected TraceContextPropagator::InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, - const BaggageProtobufMap& baggage) const { - (void)carrier; - (void)trace_id_high; - (void)trace_id_low; - (void)span_id; - (void)sampled; - (void)baggage; - return {}; -} - -opentracing::expected TraceContextPropagator::InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, - const BaggageFlatMap& baggage) const { - (void)carrier; - (void)trace_id_high; - (void)trace_id_low; - (void)span_id; - (void)sampled; - (void)baggage; - return {}; -} - opentracing::expected TraceContextPropagator::InjectSpanContext( const opentracing::TextMapWriter& carrier, const TraceContext& trace_context, opentracing::string_view trace_state, diff --git a/src/tracer/propagation/trace_context_propagator.h b/src/tracer/propagation/trace_context_propagator.h index 5e30779c..3dc9e171 100644 --- a/src/tracer/propagation/trace_context_propagator.h +++ b/src/tracer/propagation/trace_context_propagator.h @@ -6,16 +6,6 @@ namespace lightstep { class TraceContextPropagator final : Propagator { public: // Propagator - opentracing::expected InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, - const BaggageProtobufMap& baggage) const override; - - opentracing::expected InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, - const BaggageFlatMap& baggage) const override; - opentracing::expected InjectSpanContext( const opentracing::TextMapWriter& carrier, const TraceContext& trace_context, From 3a9cddc34a8881c974b677ae678ecaf185f67c93 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 3 Jan 2020 15:40:09 -0800 Subject: [PATCH 21/42] Convert MultiheaderPropagator to use new Injector --- .../propagation/multiheader_propagator.cpp | 31 +++++++++---------- .../propagation/multiheader_propagator.h | 12 +++---- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/tracer/propagation/multiheader_propagator.cpp b/src/tracer/propagation/multiheader_propagator.cpp index 434e1a14..69debe6d 100644 --- a/src/tracer/propagation/multiheader_propagator.cpp +++ b/src/tracer/propagation/multiheader_propagator.cpp @@ -26,19 +26,17 @@ MultiheaderPropagator::MultiheaderPropagator( // InjectSpanContext //-------------------------------------------------------------------------------------------------- opentracing::expected MultiheaderPropagator::InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view /*trace_state*/, const BaggageProtobufMap& /*baggage*/) const { - return this->InjectSpanContextImpl(carrier, trace_id_high, trace_id_low, - span_id, sampled); + return this->InjectSpanContextImpl(carrier, trace_context); } opentracing::expected MultiheaderPropagator::InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view /*trace_state*/, const BaggageFlatMap& /*baggage*/) const { - return this->InjectSpanContextImpl(carrier, trace_id_high, trace_id_low, - span_id, sampled); + return this->InjectSpanContextImpl(carrier, trace_context); } //-------------------------------------------------------------------------------------------------- @@ -69,25 +67,26 @@ opentracing::expected MultiheaderPropagator::ExtractSpanContext( // InjectSpanContextImpl //-------------------------------------------------------------------------------------------------- opentracing::expected MultiheaderPropagator::InjectSpanContextImpl( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled) const { + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context) const { HexSerializer hex_serializer; opentracing::expected result; if (supports_128bit_) { - result = carrier.Set(trace_id_key_, hex_serializer.Uint128ToHex( - trace_id_high, trace_id_low)); + result = carrier.Set( + trace_id_key_, hex_serializer.Uint128ToHex(trace_context.trace_id_high, + trace_context.trace_id_low)); } else { - result = - carrier.Set(trace_id_key_, hex_serializer.Uint64ToHex(trace_id_low)); + result = carrier.Set( + trace_id_key_, hex_serializer.Uint64ToHex(trace_context.trace_id_low)); } if (!result) { return result; } - result = carrier.Set(span_id_key_, hex_serializer.Uint64ToHex(span_id)); + result = carrier.Set(span_id_key_, hex_serializer.Uint64ToHex(trace_context.parent_id)); if (!result) { return result; } - if (sampled) { + if (IsTraceFlagSet(trace_context.trace_flags)) { result = carrier.Set(sampled_key_, OneStr); } else { result = carrier.Set(sampled_key_, ZeroStr); diff --git a/src/tracer/propagation/multiheader_propagator.h b/src/tracer/propagation/multiheader_propagator.h index 345f6062..f1331838 100644 --- a/src/tracer/propagation/multiheader_propagator.h +++ b/src/tracer/propagation/multiheader_propagator.h @@ -16,13 +16,13 @@ class MultiheaderPropagator final : public Propagator { // Propagator opentracing::expected InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view trace_state, const BaggageProtobufMap& baggage) const override; opentracing::expected InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view trace_state, const BaggageFlatMap& baggage) const override; opentracing::expected ExtractSpanContext( @@ -38,8 +38,8 @@ class MultiheaderPropagator final : public Propagator { bool supports_128bit_; opentracing::expected InjectSpanContextImpl( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled) const; + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context) const; template opentracing::expected ExtractSpanContextImpl( From 41679a723e86bb4614929fbbe86d9c3a63336f6a Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 3 Jan 2020 16:17:26 -0800 Subject: [PATCH 22/42] Convert BaggagePropagator to use new injection --- src/tracer/propagation/baggage_propagator.cpp | 8 ++++---- src/tracer/propagation/baggage_propagator.h | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/tracer/propagation/baggage_propagator.cpp b/src/tracer/propagation/baggage_propagator.cpp index 8b948210..7cb05122 100644 --- a/src/tracer/propagation/baggage_propagator.cpp +++ b/src/tracer/propagation/baggage_propagator.cpp @@ -44,15 +44,15 @@ BaggagePropagator::BaggagePropagator( // InjectSpanContext //-------------------------------------------------------------------------------------------------- opentracing::expected BaggagePropagator::InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t /*trace_id_high*/, - uint64_t /*trace_id_low*/, uint64_t /*span_id*/, bool /*sampled*/, + const opentracing::TextMapWriter& carrier, + const TraceContext& /*trace_context*/, opentracing::string_view /*trace_state*/, const BaggageProtobufMap& baggage) const { return this->InjectSpanContextImpl(carrier, baggage); } opentracing::expected BaggagePropagator::InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t /*trace_id_high*/, - uint64_t /*trace_id_low*/, uint64_t /*span_id*/, bool /*sampled*/, + const opentracing::TextMapWriter& carrier, + const TraceContext& /*trace_context*/, opentracing::string_view /*trace_state*/, const BaggageFlatMap& baggage) const { return this->InjectSpanContextImpl(carrier, baggage); } diff --git a/src/tracer/propagation/baggage_propagator.h b/src/tracer/propagation/baggage_propagator.h index c356a73d..75d52fe5 100644 --- a/src/tracer/propagation/baggage_propagator.h +++ b/src/tracer/propagation/baggage_propagator.h @@ -9,13 +9,13 @@ class BaggagePropagator final : public Propagator { // Propagator opentracing::expected InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view trace_state, const BaggageProtobufMap& baggage) const override; opentracing::expected InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view trace_state, const BaggageFlatMap& baggage) const override; opentracing::expected ExtractSpanContext( From 258c417fe6a2812e88ce9558edbf4aef13f8516c Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 3 Jan 2020 17:11:12 -0800 Subject: [PATCH 23/42] Convert EnvoyPropagator to use new inject --- src/tracer/propagation/envoy_propagator.cpp | 24 ++++++++++----------- src/tracer/propagation/envoy_propagator.h | 13 ++++++----- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/tracer/propagation/envoy_propagator.cpp b/src/tracer/propagation/envoy_propagator.cpp index 166ec557..e980f3fa 100644 --- a/src/tracer/propagation/envoy_propagator.cpp +++ b/src/tracer/propagation/envoy_propagator.cpp @@ -13,19 +13,17 @@ namespace lightstep { // InjectSpanContext //-------------------------------------------------------------------------------------------------- opentracing::expected EnvoyPropagator::InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view /*trace_state*/, const BaggageProtobufMap& baggage) const { - return this->InjectSpanContextImpl(carrier, trace_id_high, trace_id_low, - span_id, sampled, baggage); + return this->InjectSpanContextImpl(carrier, trace_context, baggage); } opentracing::expected EnvoyPropagator::InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view /*trace_state*/, const BaggageFlatMap& baggage) const { - return this->InjectSpanContextImpl(carrier, trace_id_high, trace_id_low, - span_id, sampled, baggage); + return this->InjectSpanContextImpl(carrier, trace_context, baggage); } //-------------------------------------------------------------------------------------------------- @@ -57,12 +55,12 @@ opentracing::expected EnvoyPropagator::ExtractSpanContext( //-------------------------------------------------------------------------------------------------- template opentracing::expected EnvoyPropagator::InjectSpanContextImpl( - const opentracing::TextMapWriter& carrier, uint64_t /*trace_id_high*/, - uint64_t trace_id_low, uint64_t span_id, bool sampled, - const BaggageMap& baggage) const { + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, const BaggageMap& baggage) const { std::ostringstream ostream; - auto result = lightstep::InjectSpanContext(ostream, trace_id_low, span_id, - sampled, baggage); + auto result = lightstep::InjectSpanContext( + ostream, trace_context.trace_id_low, trace_context.parent_id, + IsTraceFlagSet(trace_context.trace_flags), baggage); if (!result) { return result; } diff --git a/src/tracer/propagation/envoy_propagator.h b/src/tracer/propagation/envoy_propagator.h index 7a0e597a..ea8efb32 100644 --- a/src/tracer/propagation/envoy_propagator.h +++ b/src/tracer/propagation/envoy_propagator.h @@ -10,13 +10,13 @@ class EnvoyPropagator final : public Propagator { public: // Propagator opentracing::expected InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view trace_state, const BaggageProtobufMap& baggage) const override; opentracing::expected InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view trace_state, const BaggageFlatMap& baggage) const override; opentracing::expected ExtractSpanContext( @@ -27,9 +27,8 @@ class EnvoyPropagator final : public Propagator { private: template opentracing::expected InjectSpanContextImpl( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, - const BaggageMap& baggage) const; + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, const BaggageMap& baggage) const; template opentracing::expected ExtractSpanContextImpl( From 2c439231582fc0fd580f36761c5da607bd50ddf6 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 3 Jan 2020 17:15:08 -0800 Subject: [PATCH 24/42] Remove old-style inject --- src/tracer/propagation/propagator.h | 43 ++--------------------------- 1 file changed, 2 insertions(+), 41 deletions(-) diff --git a/src/tracer/propagation/propagator.h b/src/tracer/propagation/propagator.h index bced747d..ab164122 100644 --- a/src/tracer/propagation/propagator.h +++ b/src/tracer/propagation/propagator.h @@ -17,55 +17,16 @@ class Propagator { virtual opentracing::expected InjectSpanContext( const opentracing::TextMapWriter& carrier, const TraceContext& trace_context, opentracing::string_view trace_state, - const BaggageProtobufMap& baggage) const { - (void)trace_state; - return InjectSpanContext( - carrier, trace_context.trace_id_high, trace_context.trace_id_low, - trace_context.parent_id, - IsTraceFlagSet(trace_context.trace_flags), baggage); - } + const BaggageProtobufMap& baggage) const = 0; virtual opentracing::expected InjectSpanContext( const opentracing::TextMapWriter& carrier, const TraceContext& trace_context, opentracing::string_view trace_state, - const BaggageFlatMap& baggage) const { - (void)trace_state; - return InjectSpanContext( - carrier, trace_context.trace_id_high, trace_context.trace_id_low, - trace_context.parent_id, - IsTraceFlagSet(trace_context.trace_flags), baggage); - } + const BaggageFlatMap& baggage) const = 0; virtual opentracing::expected ExtractSpanContext( const opentracing::TextMapReader& carrier, bool case_sensitive, uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, bool& sampled, BaggageProtobufMap& baggage) const = 0; - private: - virtual opentracing::expected InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, - const BaggageProtobufMap& baggage) const { - (void)carrier; - (void)trace_id_high; - (void)trace_id_low; - (void)span_id; - (void)sampled; - (void)baggage; - std::terminate(); - } - - virtual opentracing::expected InjectSpanContext( - const opentracing::TextMapWriter& carrier, uint64_t trace_id_high, - uint64_t trace_id_low, uint64_t span_id, bool sampled, - const BaggageFlatMap& baggage) const { - (void)carrier; - (void)trace_id_high; - (void)trace_id_low; - (void)span_id; - (void)sampled; - (void)baggage; - std::terminate(); - } - }; } // namespace lightstep From 23da0d31ba153bc0af87b7f177aa81fb5f6ca779 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 3 Jan 2020 23:35:27 -0800 Subject: [PATCH 25/42] Update extract methods --- src/tracer/immutable_span_context.cpp | 10 +++ src/tracer/immutable_span_context.h | 6 +- src/tracer/lightstep_span_context.cpp | 1 + src/tracer/propagation/propagation.cpp | 87 +++++++++++++++++++------- src/tracer/propagation/propagation.h | 10 +++ src/tracer/propagation/propagator.h | 20 ++++++ src/tracer/tracer_impl.cpp | 28 ++++++++- 7 files changed, 137 insertions(+), 25 deletions(-) diff --git a/src/tracer/immutable_span_context.cpp b/src/tracer/immutable_span_context.cpp index f6740d90..ddea0eb4 100644 --- a/src/tracer/immutable_span_context.cpp +++ b/src/tracer/immutable_span_context.cpp @@ -29,6 +29,16 @@ ImmutableSpanContext::ImmutableSpanContext( trace_flags_{SetTraceFlag(0, sampled)}, baggage_{std::move(baggage)} {} +ImmutableSpanContext::ImmutableSpanContext( + const TraceContext& trace_context, std::string&& trace_state, + BaggageProtobufMap&& baggage) noexcept + : trace_id_high_{trace_context.trace_id_high}, + trace_id_low_{trace_context.trace_id_low}, + span_id_{trace_context.parent_id}, + trace_flags_{trace_context.trace_flags}, + trace_state_{std::move(trace_state)}, + baggage_{std::move(baggage)} {} + //------------------------------------------------------------------------------ // ForeachBaggageItem //------------------------------------------------------------------------------ diff --git a/src/tracer/immutable_span_context.h b/src/tracer/immutable_span_context.h index 834ea4f2..1033697e 100644 --- a/src/tracer/immutable_span_context.h +++ b/src/tracer/immutable_span_context.h @@ -21,6 +21,10 @@ class ImmutableSpanContext final : public LightStepSpanContext { uint64_t span_id, bool sampled, BaggageProtobufMap&& baggage) noexcept; + ImmutableSpanContext(const TraceContext& trace_context, + std::string&& trace_state, + BaggageProtobufMap&& baggage) noexcept; + // LightStepSpanContext uint64_t trace_id_high() const noexcept override { return trace_id_high_; } @@ -61,8 +65,8 @@ class ImmutableSpanContext final : public LightStepSpanContext { uint64_t trace_id_low_; uint64_t span_id_; uint8_t trace_flags_; - BaggageProtobufMap baggage_; std::string trace_state_; + BaggageProtobufMap baggage_; template opentracing::expected InjectImpl( diff --git a/src/tracer/lightstep_span_context.cpp b/src/tracer/lightstep_span_context.cpp index 40927616..e25a406c 100644 --- a/src/tracer/lightstep_span_context.cpp +++ b/src/tracer/lightstep_span_context.cpp @@ -19,6 +19,7 @@ bool operator==(const LightStepSpanContext& lhs, return lhs.trace_id_high() == rhs.trace_id_high() && lhs.trace_id_low() == rhs.trace_id_low() && lhs.span_id() == rhs.span_id() && + lhs.trace_state() == rhs.trace_state() && lhs.trace_flags() == rhs.trace_flags() && extract_baggage(lhs) == extract_baggage(rhs); } diff --git a/src/tracer/propagation/propagation.cpp b/src/tracer/propagation/propagation.cpp index 41124dcf..41eabbbc 100644 --- a/src/tracer/propagation/propagation.cpp +++ b/src/tracer/propagation/propagation.cpp @@ -13,6 +13,54 @@ #include "common/in_memory_stream.h" namespace lightstep { +//-------------------------------------------------------------------------------------------------- +// ExtractSpanContextImpl +//-------------------------------------------------------------------------------------------------- +static opentracing::expected ExtractSpanContextImpl( + const PropagationOptions& propagation_options, + const opentracing::TextMapReader& carrier, bool case_sensitive, + uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, + bool& sampled, BaggageProtobufMap& baggage) { + for (auto& propagator : propagation_options.extract_propagators) { + baggage.clear(); + auto result = + propagator->ExtractSpanContext(carrier, case_sensitive, trace_id_high, + trace_id_low, span_id, sampled, baggage); + if (!result) { + // One of the injected span contexts is corrupt, return immediately + // without trying the other extractors. + return result; + } + if (*result) { + // An extractor succeeded, return without trying other extractors + return result; + } + } + return false; +} + +static opentracing::expected ExtractSpanContextImpl( + const PropagationOptions& propagation_options, + const opentracing::TextMapReader& carrier, bool case_sensitive, + TraceContext& trace_context, std::string& trace_state, + BaggageProtobufMap& baggage) { + for (auto& propagator : propagation_options.extract_propagators) { + baggage.clear(); + auto result = propagator->ExtractSpanContext( + carrier, case_sensitive, trace_context, trace_state, baggage); + if (!result) { + // One of the injected span contexts is corrupt, return immediately + // without trying the other extractors. + return result; + } + if (*result) { + // An extractor succeeded, return without trying other extractors + return result; + } + } + return false; +} + //------------------------------------------------------------------------------ // InjectSpanContext //------------------------------------------------------------------------------ @@ -47,29 +95,6 @@ template opentracing::expected InjectSpanContext( //------------------------------------------------------------------------------ // ExtractSpanContext //------------------------------------------------------------------------------ -static opentracing::expected ExtractSpanContextImpl( - const PropagationOptions& propagation_options, - const opentracing::TextMapReader& carrier, bool case_sensitive, - uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, - bool& sampled, BaggageProtobufMap& baggage) { - for (auto& propagator : propagation_options.extract_propagators) { - baggage.clear(); - auto result = - propagator->ExtractSpanContext(carrier, case_sensitive, trace_id_high, - trace_id_low, span_id, sampled, baggage); - if (!result) { - // One of the injected span contexts is corrupt, return immediately - // without trying the other extractors. - return result; - } - if (*result) { - // An extractor succeeded, return without trying other extractors - return result; - } - } - return false; -} - opentracing::expected ExtractSpanContext( const PropagationOptions& propagation_options, const opentracing::TextMapReader& carrier, uint64_t& trace_id_high, @@ -89,4 +114,20 @@ opentracing::expected ExtractSpanContext( trace_id_high, trace_id_low, span_id, sampled, baggage); } + +opentracing::expected ExtractSpanContext( + const PropagationOptions& propagation_options, + const opentracing::TextMapReader& carrier, TraceContext& trace_context, + std::string& trace_state, BaggageProtobufMap& baggage) { + return ExtractSpanContextImpl(propagation_options, carrier, true, + trace_context, trace_state, baggage); +} + +opentracing::expected ExtractSpanContext( + const PropagationOptions& propagation_options, + const opentracing::HTTPHeadersReader& carrier, TraceContext& trace_context, + std::string& trace_state, BaggageProtobufMap& baggage) { + return ExtractSpanContextImpl(propagation_options, carrier, false, + trace_context, trace_state, baggage); +} } // namespace lightstep diff --git a/src/tracer/propagation/propagation.h b/src/tracer/propagation/propagation.h index 136d4c4e..ed5728b9 100644 --- a/src/tracer/propagation/propagation.h +++ b/src/tracer/propagation/propagation.h @@ -63,4 +63,14 @@ opentracing::expected ExtractSpanContext( const opentracing::HTTPHeadersReader& carrier, uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, bool& sampled, BaggageProtobufMap& baggage); + +opentracing::expected ExtractSpanContext( + const PropagationOptions& propagation_options, + const opentracing::TextMapReader& carrier, TraceContext& trace_context, + std::string& trace_state, BaggageProtobufMap& baggage); + +opentracing::expected ExtractSpanContext( + const PropagationOptions& propagation_options, + const opentracing::HTTPHeadersReader& carrier, TraceContext& trace_context, + std::string& trace_state, BaggageProtobufMap& baggage); } // namespace lightstep diff --git a/src/tracer/propagation/propagator.h b/src/tracer/propagation/propagator.h index ab164122..29aac592 100644 --- a/src/tracer/propagation/propagator.h +++ b/src/tracer/propagation/propagator.h @@ -28,5 +28,25 @@ class Propagator { const opentracing::TextMapReader& carrier, bool case_sensitive, uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, bool& sampled, BaggageProtobufMap& baggage) const = 0; + + virtual opentracing::expected ExtractSpanContext( + const opentracing::TextMapReader& carrier, bool case_sensitive, + TraceContext& trace_context, std::string& trace_state, + BaggageProtobufMap& baggage) const { + (void)trace_state; + bool sampled; + auto result = ExtractSpanContext( + carrier, case_sensitive, trace_context.trace_id_high, + trace_context.trace_id_low, trace_context.parent_id, sampled, baggage); + if (!result || !*result) { + return result; + } + trace_context.trace_flags = IsTraceFlagSet(0); + if (trace_context.trace_id_high == 0 && trace_context.trace_id_low == 0) { + return opentracing::make_unexpected( + opentracing::invalid_span_context_error); + } + return true; + } }; } // namespace lightstep diff --git a/src/tracer/tracer_impl.cpp b/src/tracer/tracer_impl.cpp index 0ea327d1..d57ab1ce 100644 --- a/src/tracer/tracer_impl.cpp +++ b/src/tracer/tracer_impl.cpp @@ -26,7 +26,7 @@ static opentracing::expected InjectImpl( // ExtractImpl //------------------------------------------------------------------------------ template -opentracing::expected> ExtractImpl( +static opentracing::expected> ExtractImpl( const PropagationOptions& propagation_options, Carrier& reader) try { uint64_t trace_id_high, trace_id_low, span_id; bool sampled; @@ -49,6 +49,32 @@ opentracing::expected> ExtractImpl( make_error_code(std::errc::not_enough_memory)); } +#if 0 +template +static opentracing::expected> ExtractImpl( + const PropagationOptions& propagation_options, Carrier& reader) try { + uint64_t trace_id_high, trace_id_low, span_id; + bool sampled; + BaggageProtobufMap baggage; + auto extract_maybe = + ExtractSpanContext(propagation_options, reader, trace_id_high, + trace_id_low, span_id, sampled, baggage); + + if (!extract_maybe) { + return opentracing::make_unexpected(extract_maybe.error()); + } + if (!*extract_maybe) { + return std::unique_ptr{nullptr}; + } + std::unique_ptr result{new ImmutableSpanContext{ + trace_id_high, trace_id_low, span_id, sampled, std::move(baggage)}}; + return std::move(result); +} catch (const std::bad_alloc&) { + return opentracing::make_unexpected( + make_error_code(std::errc::not_enough_memory)); +} +#endif + //-------------------------------------------------------------------------------------------------- // constructor //-------------------------------------------------------------------------------------------------- From d67c54b4e4ea54b81defbd9175fe1b0bb157c834 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 6 Jan 2020 13:45:07 -0800 Subject: [PATCH 26/42] Fix trace-context propagation --- src/tracer/propagation/propagator.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tracer/propagation/propagator.h b/src/tracer/propagation/propagator.h index 29aac592..f2ee5f72 100644 --- a/src/tracer/propagation/propagator.h +++ b/src/tracer/propagation/propagator.h @@ -41,7 +41,7 @@ class Propagator { if (!result || !*result) { return result; } - trace_context.trace_flags = IsTraceFlagSet(0); + trace_context.trace_flags = SetTraceFlag(0, sampled); if (trace_context.trace_id_high == 0 && trace_context.trace_id_low == 0) { return opentracing::make_unexpected( opentracing::invalid_span_context_error); From 32e017d20856839ceaf0db52e8c56343ed87433e Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 6 Jan 2020 13:58:14 -0800 Subject: [PATCH 27/42] Change Extract calls --- src/tracer/propagation/propagation.h | 14 +++++++++++ src/tracer/tracer_impl.cpp | 36 ++++------------------------ 2 files changed, 19 insertions(+), 31 deletions(-) diff --git a/src/tracer/propagation/propagation.h b/src/tracer/propagation/propagation.h index ed5728b9..0b35515a 100644 --- a/src/tracer/propagation/propagation.h +++ b/src/tracer/propagation/propagation.h @@ -52,6 +52,20 @@ inline opentracing::expected ExtractSpanContext( return ExtractSpanContext(carrier, trace_id_low, span_id, sampled, baggage); } +inline opentracing::expected ExtractSpanContext( + const PropagationOptions& /*propagation_options*/, std::istream& carrier, + TraceContext& trace_context, std::string& /*trace_state*/, BaggageProtobufMap& baggage) { + trace_context.trace_id_high = 0; + bool sampled; + auto result = ExtractSpanContext(carrier, trace_context.trace_id_low, + trace_context.parent_id, sampled, baggage); + if (!result || !*result) { + return result; + } + trace_context.trace_flags = SetTraceFlag(0, sampled); + return result; +} + opentracing::expected ExtractSpanContext( const PropagationOptions& propagation_options, const opentracing::TextMapReader& carrier, uint64_t& trace_id_high, diff --git a/src/tracer/tracer_impl.cpp b/src/tracer/tracer_impl.cpp index d57ab1ce..497a4f82 100644 --- a/src/tracer/tracer_impl.cpp +++ b/src/tracer/tracer_impl.cpp @@ -28,37 +28,12 @@ static opentracing::expected InjectImpl( template static opentracing::expected> ExtractImpl( const PropagationOptions& propagation_options, Carrier& reader) try { - uint64_t trace_id_high, trace_id_low, span_id; - bool sampled; + TraceContext trace_context; + std::string trace_state; BaggageProtobufMap baggage; - auto extract_maybe = - ExtractSpanContext(propagation_options, reader, trace_id_high, - trace_id_low, span_id, sampled, baggage); - if (!extract_maybe) { - return opentracing::make_unexpected(extract_maybe.error()); - } - if (!*extract_maybe) { - return std::unique_ptr{nullptr}; - } - std::unique_ptr result{new ImmutableSpanContext{ - trace_id_high, trace_id_low, span_id, sampled, std::move(baggage)}}; - return std::move(result); -} catch (const std::bad_alloc&) { - return opentracing::make_unexpected( - make_error_code(std::errc::not_enough_memory)); -} - -#if 0 -template -static opentracing::expected> ExtractImpl( - const PropagationOptions& propagation_options, Carrier& reader) try { - uint64_t trace_id_high, trace_id_low, span_id; - bool sampled; - BaggageProtobufMap baggage; - auto extract_maybe = - ExtractSpanContext(propagation_options, reader, trace_id_high, - trace_id_low, span_id, sampled, baggage); + auto extract_maybe = ExtractSpanContext(propagation_options, reader, + trace_context, trace_state, baggage); if (!extract_maybe) { return opentracing::make_unexpected(extract_maybe.error()); @@ -67,13 +42,12 @@ static opentracing::expected> ExtractI return std::unique_ptr{nullptr}; } std::unique_ptr result{new ImmutableSpanContext{ - trace_id_high, trace_id_low, span_id, sampled, std::move(baggage)}}; + trace_context, std::move(trace_state), std::move(baggage)}}; return std::move(result); } catch (const std::bad_alloc&) { return opentracing::make_unexpected( make_error_code(std::errc::not_enough_memory)); } -#endif //-------------------------------------------------------------------------------------------------- // constructor From cd4774818b4a01f656f26c849482ae34fef244dc Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 6 Jan 2020 21:02:24 -0800 Subject: [PATCH 28/42] Fill out trace-context propagator --- .../propagation/trace_context_propagator.cpp | 33 +++++++++++++++++-- .../propagation/trace_context_propagator.h | 5 +++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/tracer/propagation/trace_context_propagator.cpp b/src/tracer/propagation/trace_context_propagator.cpp index 7887746a..91d98d2d 100644 --- a/src/tracer/propagation/trace_context_propagator.cpp +++ b/src/tracer/propagation/trace_context_propagator.cpp @@ -14,9 +14,9 @@ static opentracing::expected InjectSpanContextImpl( const TraceContext& trace_context, opentracing::string_view trace_state) { std::array buffer; SerializeTraceContext(trace_context, buffer.data()); - auto result = carrier.Set( - TraceParentHeaderKey, - opentracing::string_view{buffer.data(), buffer.size()}); + auto result = + carrier.Set(TraceParentHeaderKey, + opentracing::string_view{buffer.data(), buffer.size()}); if (!result) { return result; } @@ -59,4 +59,31 @@ opentracing::expected TraceContextPropagator::ExtractSpanContext( (void)baggage; return true; } + +opentracing::expected TraceContextPropagator::ExtractSpanContext( + const opentracing::TextMapReader& carrier, bool case_sensitive, + TraceContext& trace_context, std::string& trace_state, + BaggageProtobufMap& /*baggage*/) const { + (void)case_sensitive; + bool parent_header_found = false; + auto result = + carrier.ForeachKey([&](opentracing::string_view key, + opentracing::string_view + value) noexcept->opentracing::expected { + if (key == TraceParentHeaderKey) { + auto was_successful = ParseTraceContext(value, trace_context); + if (!was_successful) { + return opentracing::make_unexpected(was_successful.error()); + } + parent_header_found = true; + } else if (key == TraceStateHeaderKey) { + trace_state.assign(value.data(), value.size()); + } + return {}; + }); + if (!result) { + return opentracing::make_unexpected(result.error()); + } + return parent_header_found; +} } // namespace lightstep diff --git a/src/tracer/propagation/trace_context_propagator.h b/src/tracer/propagation/trace_context_propagator.h index 3dc9e171..69a1f0a5 100644 --- a/src/tracer/propagation/trace_context_propagator.h +++ b/src/tracer/propagation/trace_context_propagator.h @@ -23,6 +23,11 @@ class TraceContextPropagator final : Propagator { uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, bool& sampled, BaggageProtobufMap& baggage) const override; + opentracing::expected ExtractSpanContext( + const opentracing::TextMapReader& carrier, bool case_sensitive, + TraceContext& trace_context, std::string& trace_state, + BaggageProtobufMap& baggage) const override; + private: }; } // namespace lightstep From c4153b31886487eac5e437103b56a5c363817d3d Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 6 Jan 2020 21:49:32 -0800 Subject: [PATCH 29/42] Fill out trace-context propagator --- .../propagation/trace_context_propagator.cpp | 61 +++++++++++++------ 1 file changed, 41 insertions(+), 20 deletions(-) diff --git a/src/tracer/propagation/trace_context_propagator.cpp b/src/tracer/propagation/trace_context_propagator.cpp index 91d98d2d..84a5d234 100644 --- a/src/tracer/propagation/trace_context_propagator.cpp +++ b/src/tracer/propagation/trace_context_propagator.cpp @@ -26,6 +26,35 @@ static opentracing::expected InjectSpanContextImpl( return carrier.Set(TraceStateHeaderKey, trace_state); } +//-------------------------------------------------------------------------------------------------- +// ExtractSpanContextImpl +//-------------------------------------------------------------------------------------------------- +template +static opentracing::expected ExtractSpanContextImpl( + const opentracing::TextMapReader& carrier, TraceContext& trace_context, + std::string& trace_state, const KeyCompare& key_compare) { + bool parent_header_found = false; + auto result = + carrier.ForeachKey([&](opentracing::string_view key, + opentracing::string_view + value) noexcept->opentracing::expected { + if (key_compare(key, TraceParentHeaderKey)) { + auto was_successful = ParseTraceContext(value, trace_context); + if (!was_successful) { + return opentracing::make_unexpected(was_successful.error()); + } + parent_header_found = true; + } else if (key_compare(key, TraceStateHeaderKey)) { + trace_state.assign(value.data(), value.size()); + } + return {}; + }); + if (!result) { + return opentracing::make_unexpected(result.error()); + } + return parent_header_found; +} + //-------------------------------------------------------------------------------------------------- // InjectSpanContext //-------------------------------------------------------------------------------------------------- @@ -64,26 +93,18 @@ opentracing::expected TraceContextPropagator::ExtractSpanContext( const opentracing::TextMapReader& carrier, bool case_sensitive, TraceContext& trace_context, std::string& trace_state, BaggageProtobufMap& /*baggage*/) const { - (void)case_sensitive; - bool parent_header_found = false; - auto result = - carrier.ForeachKey([&](opentracing::string_view key, - opentracing::string_view - value) noexcept->opentracing::expected { - if (key == TraceParentHeaderKey) { - auto was_successful = ParseTraceContext(value, trace_context); - if (!was_successful) { - return opentracing::make_unexpected(was_successful.error()); - } - parent_header_found = true; - } else if (key == TraceStateHeaderKey) { - trace_state.assign(value.data(), value.size()); - } - return {}; - }); - if (!result) { - return opentracing::make_unexpected(result.error()); + auto iequals = + [](opentracing::string_view lhs, opentracing::string_view rhs) noexcept { + return lhs.length() == rhs.length() && + std::equal(std::begin(lhs), std::end(lhs), std::begin(rhs), + [](char a, char b) { + return std::tolower(a) == std::tolower(b); + }); + }; + if (case_sensitive) { + return ExtractSpanContextImpl(carrier, trace_context, trace_state, + std::equal_to{}); } - return parent_header_found; + return ExtractSpanContextImpl(carrier, trace_context, trace_state, iequals); } } // namespace lightstep From b8bec761921416df58942efd22ecb069ffaa6db6 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 7 Jan 2020 14:22:20 -0800 Subject: [PATCH 30/42] Refactoring Extract interface --- src/tracer/propagation/baggage_propagator.h | 5 ++--- src/tracer/propagation/propagator.h | 15 ++++++++++++++- .../propagation/trace_context_propagator.cpp | 14 -------------- src/tracer/propagation/trace_context_propagator.h | 5 ----- 4 files changed, 16 insertions(+), 23 deletions(-) diff --git a/src/tracer/propagation/baggage_propagator.h b/src/tracer/propagation/baggage_propagator.h index 75d52fe5..128128bd 100644 --- a/src/tracer/propagation/baggage_propagator.h +++ b/src/tracer/propagation/baggage_propagator.h @@ -20,11 +20,10 @@ class BaggagePropagator final : public Propagator { opentracing::expected ExtractSpanContext( const opentracing::TextMapReader& /*carrier*/, bool /*case_sensitive*/, - uint64_t& /*trace_id_high*/, uint64_t& /*trace_id_low*/, - uint64_t& /*span_id*/, bool& /*sampled*/, + TraceContext& /*trace_context*/, std::string& /*trace_state*/, BaggageProtobufMap& /*baggage*/) const override { // Do nothing: baggage is extracted in the other propagators - return true; + return false; } private: diff --git a/src/tracer/propagation/propagator.h b/src/tracer/propagation/propagator.h index f2ee5f72..70a3a670 100644 --- a/src/tracer/propagation/propagator.h +++ b/src/tracer/propagation/propagator.h @@ -27,7 +27,20 @@ class Propagator { virtual opentracing::expected ExtractSpanContext( const opentracing::TextMapReader& carrier, bool case_sensitive, uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, - bool& sampled, BaggageProtobufMap& baggage) const = 0; + bool& sampled, BaggageProtobufMap& baggage) const { + std::string trace_state; + TraceContext trace_context; + auto result = this->ExtractSpanContext(carrier, case_sensitive, + trace_context, trace_state, baggage); + if (!result || !*result) { + return result; + } + trace_id_high = trace_context.trace_id_high; + trace_id_low = trace_context.trace_id_low; + span_id = trace_context.parent_id; + sampled = IsTraceFlagSet(trace_context.trace_flags); + return result; + } virtual opentracing::expected ExtractSpanContext( const opentracing::TextMapReader& carrier, bool case_sensitive, diff --git a/src/tracer/propagation/trace_context_propagator.cpp b/src/tracer/propagation/trace_context_propagator.cpp index 84a5d234..3eda44c7 100644 --- a/src/tracer/propagation/trace_context_propagator.cpp +++ b/src/tracer/propagation/trace_context_propagator.cpp @@ -75,20 +75,6 @@ opentracing::expected TraceContextPropagator::InjectSpanContext( //-------------------------------------------------------------------------------------------------- // ExtractSpanContext //-------------------------------------------------------------------------------------------------- -opentracing::expected TraceContextPropagator::ExtractSpanContext( - const opentracing::TextMapReader& carrier, bool case_sensitive, - uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, - bool& sampled, BaggageProtobufMap& baggage) const { - (void)carrier; - (void)case_sensitive; - (void)trace_id_high; - (void)trace_id_low; - (void)span_id; - (void)sampled; - (void)baggage; - return true; -} - opentracing::expected TraceContextPropagator::ExtractSpanContext( const opentracing::TextMapReader& carrier, bool case_sensitive, TraceContext& trace_context, std::string& trace_state, diff --git a/src/tracer/propagation/trace_context_propagator.h b/src/tracer/propagation/trace_context_propagator.h index 69a1f0a5..9877441d 100644 --- a/src/tracer/propagation/trace_context_propagator.h +++ b/src/tracer/propagation/trace_context_propagator.h @@ -18,11 +18,6 @@ class TraceContextPropagator final : Propagator { opentracing::string_view trace_state, const BaggageFlatMap& baggage) const override; - opentracing::expected ExtractSpanContext( - const opentracing::TextMapReader& carrier, bool case_sensitive, - uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, - bool& sampled, BaggageProtobufMap& baggage) const override; - opentracing::expected ExtractSpanContext( const opentracing::TextMapReader& carrier, bool case_sensitive, TraceContext& trace_context, std::string& trace_state, From 85b7fdb06b29dcc7056af52361f48313e17547d6 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 7 Jan 2020 14:33:35 -0800 Subject: [PATCH 31/42] Refactor MultiheaderPropagator --- .../propagation/multiheader_propagator.cpp | 24 +++++++++++++------ .../propagation/multiheader_propagator.h | 4 ++-- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/tracer/propagation/multiheader_propagator.cpp b/src/tracer/propagation/multiheader_propagator.cpp index 69debe6d..ebbf9ba9 100644 --- a/src/tracer/propagation/multiheader_propagator.cpp +++ b/src/tracer/propagation/multiheader_propagator.cpp @@ -44,8 +44,8 @@ opentracing::expected MultiheaderPropagator::InjectSpanContext( //-------------------------------------------------------------------------------------------------- opentracing::expected MultiheaderPropagator::ExtractSpanContext( const opentracing::TextMapReader& carrier, bool case_sensitive, - uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, - bool& sampled, BaggageProtobufMap& baggage) const { + TraceContext& trace_context, std::string& /*trace_state*/, + BaggageProtobufMap& baggage) const { auto iequals = [](opentracing::string_view lhs, opentracing::string_view rhs) noexcept { return lhs.length() == rhs.length() && @@ -54,13 +54,23 @@ opentracing::expected MultiheaderPropagator::ExtractSpanContext( return std::tolower(a) == std::tolower(b); }); }; + bool sampled; + opentracing::expected result; if (case_sensitive) { - return ExtractSpanContextImpl(carrier, trace_id_high, trace_id_low, span_id, - sampled, baggage, - std::equal_to{}); + result = ExtractSpanContextImpl(carrier, trace_context.trace_id_high, + trace_context.trace_id_low, + trace_context.parent_id, sampled, baggage, + std::equal_to{}); + } else { + result = result = ExtractSpanContextImpl( + carrier, trace_context.trace_id_high, trace_context.trace_id_low, + trace_context.parent_id, sampled, baggage, iequals); + } + if (!result || !*result) { + return result; } - return ExtractSpanContextImpl(carrier, trace_id_high, trace_id_low, span_id, - sampled, baggage, iequals); + trace_context.trace_flags = SetTraceFlag(0, sampled); + return result; } //-------------------------------------------------------------------------------------------------- diff --git a/src/tracer/propagation/multiheader_propagator.h b/src/tracer/propagation/multiheader_propagator.h index f1331838..e0c3b504 100644 --- a/src/tracer/propagation/multiheader_propagator.h +++ b/src/tracer/propagation/multiheader_propagator.h @@ -27,8 +27,8 @@ class MultiheaderPropagator final : public Propagator { opentracing::expected ExtractSpanContext( const opentracing::TextMapReader& carrier, bool case_sensitive, - uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, - bool& sampled, BaggageProtobufMap& baggage) const override; + TraceContext& trace_context, std::string& trace_state, + BaggageProtobufMap& baggage) const override; private: opentracing::string_view trace_id_key_; From 3fd68f1ac18caecfc8c5b178c80c838a86e0e7c3 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 7 Jan 2020 14:40:31 -0800 Subject: [PATCH 32/42] Update envoy propagator --- src/tracer/propagation/envoy_propagator.cpp | 24 +++++++++++++++------ src/tracer/propagation/envoy_propagator.h | 4 ++-- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/tracer/propagation/envoy_propagator.cpp b/src/tracer/propagation/envoy_propagator.cpp index e980f3fa..2e32b73a 100644 --- a/src/tracer/propagation/envoy_propagator.cpp +++ b/src/tracer/propagation/envoy_propagator.cpp @@ -31,8 +31,8 @@ opentracing::expected EnvoyPropagator::InjectSpanContext( //-------------------------------------------------------------------------------------------------- opentracing::expected EnvoyPropagator::ExtractSpanContext( const opentracing::TextMapReader& carrier, bool case_sensitive, - uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, - bool& sampled, BaggageProtobufMap& baggage) const { + TraceContext& trace_context, std::string& /*trace_state*/, + BaggageProtobufMap& baggage) const { auto iequals = [](opentracing::string_view lhs, opentracing::string_view rhs) noexcept { return lhs.length() == rhs.length() && @@ -41,13 +41,23 @@ opentracing::expected EnvoyPropagator::ExtractSpanContext( return std::tolower(a) == std::tolower(b); }); }; + bool sampled; + opentracing::expected result; if (case_sensitive) { - return ExtractSpanContextImpl(carrier, trace_id_high, trace_id_low, span_id, - sampled, baggage, - std::equal_to{}); + result = ExtractSpanContextImpl(carrier, trace_context.trace_id_high, + trace_context.trace_id_low, + trace_context.parent_id, sampled, baggage, + std::equal_to{}); + } else { + result = result = ExtractSpanContextImpl( + carrier, trace_context.trace_id_high, trace_context.trace_id_low, + trace_context.parent_id, sampled, baggage, iequals); } - return ExtractSpanContextImpl(carrier, trace_id_high, trace_id_low, span_id, - sampled, baggage, iequals); + if (!result || !*result) { + return result; + } + trace_context.trace_flags = SetTraceFlag(0, sampled); + return result; } //-------------------------------------------------------------------------------------------------- diff --git a/src/tracer/propagation/envoy_propagator.h b/src/tracer/propagation/envoy_propagator.h index ea8efb32..8ce2883a 100644 --- a/src/tracer/propagation/envoy_propagator.h +++ b/src/tracer/propagation/envoy_propagator.h @@ -21,8 +21,8 @@ class EnvoyPropagator final : public Propagator { opentracing::expected ExtractSpanContext( const opentracing::TextMapReader& carrier, bool case_sensitive, - uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, - bool& sampled, BaggageProtobufMap& baggage) const override; + TraceContext& trace_context, std::string& trace_state, + BaggageProtobufMap& baggage) const override; private: template From 82106499b73b7a52c0d3b2af745c0c03a243e2d7 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 7 Jan 2020 14:52:47 -0800 Subject: [PATCH 33/42] Update LegacyTracer code --- src/tracer/legacy/legacy_tracer_impl.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tracer/legacy/legacy_tracer_impl.cpp b/src/tracer/legacy/legacy_tracer_impl.cpp index 2b685198..2ac398f8 100644 --- a/src/tracer/legacy/legacy_tracer_impl.cpp +++ b/src/tracer/legacy/legacy_tracer_impl.cpp @@ -27,12 +27,12 @@ static opentracing::expected InjectImpl( template opentracing::expected> ExtractImpl( const PropagationOptions& propagation_options, Carrier& reader) try { - uint64_t trace_id_high, trace_id_low, span_id; - bool sampled; + TraceContext trace_context; + std::string trace_state; BaggageProtobufMap baggage; - auto extract_maybe = - ExtractSpanContext(propagation_options, reader, trace_id_high, - trace_id_low, span_id, sampled, baggage); + + auto extract_maybe = ExtractSpanContext(propagation_options, reader, + trace_context, trace_state, baggage); if (!extract_maybe) { return opentracing::make_unexpected(extract_maybe.error()); @@ -41,7 +41,7 @@ opentracing::expected> ExtractImpl( return std::unique_ptr{nullptr}; } std::unique_ptr result{new ImmutableSpanContext{ - trace_id_high, trace_id_low, span_id, sampled, std::move(baggage)}}; + trace_context, std::move(trace_state), std::move(baggage)}}; return std::move(result); } catch (const std::bad_alloc&) { return opentracing::make_unexpected( From d0d662a629fa8d5b9472c761148346e886806d9e Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 7 Jan 2020 14:56:34 -0800 Subject: [PATCH 34/42] Drop old extractor interface --- src/tracer/propagation/propagation.cpp | 43 -------------------------- src/tracer/propagation/propagation.h | 20 ------------ src/tracer/propagation/propagator.h | 35 +-------------------- 3 files changed, 1 insertion(+), 97 deletions(-) diff --git a/src/tracer/propagation/propagation.cpp b/src/tracer/propagation/propagation.cpp index 41eabbbc..57fa67ff 100644 --- a/src/tracer/propagation/propagation.cpp +++ b/src/tracer/propagation/propagation.cpp @@ -16,29 +16,6 @@ namespace lightstep { //-------------------------------------------------------------------------------------------------- // ExtractSpanContextImpl //-------------------------------------------------------------------------------------------------- -static opentracing::expected ExtractSpanContextImpl( - const PropagationOptions& propagation_options, - const opentracing::TextMapReader& carrier, bool case_sensitive, - uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, - bool& sampled, BaggageProtobufMap& baggage) { - for (auto& propagator : propagation_options.extract_propagators) { - baggage.clear(); - auto result = - propagator->ExtractSpanContext(carrier, case_sensitive, trace_id_high, - trace_id_low, span_id, sampled, baggage); - if (!result) { - // One of the injected span contexts is corrupt, return immediately - // without trying the other extractors. - return result; - } - if (*result) { - // An extractor succeeded, return without trying other extractors - return result; - } - } - return false; -} - static opentracing::expected ExtractSpanContextImpl( const PropagationOptions& propagation_options, const opentracing::TextMapReader& carrier, bool case_sensitive, @@ -95,26 +72,6 @@ template opentracing::expected InjectSpanContext( //------------------------------------------------------------------------------ // ExtractSpanContext //------------------------------------------------------------------------------ -opentracing::expected ExtractSpanContext( - const PropagationOptions& propagation_options, - const opentracing::TextMapReader& carrier, uint64_t& trace_id_high, - uint64_t& trace_id_low, uint64_t& span_id, bool& sampled, - BaggageProtobufMap& baggage) { - return ExtractSpanContextImpl(propagation_options, carrier, true, - trace_id_high, trace_id_low, span_id, sampled, - baggage); -} - -opentracing::expected ExtractSpanContext( - const PropagationOptions& propagation_options, - const opentracing::HTTPHeadersReader& carrier, uint64_t& trace_id_high, - uint64_t& trace_id_low, uint64_t& span_id, bool& sampled, - BaggageProtobufMap& baggage) { - return ExtractSpanContextImpl(propagation_options, carrier, false, - trace_id_high, trace_id_low, span_id, sampled, - baggage); -} - opentracing::expected ExtractSpanContext( const PropagationOptions& propagation_options, const opentracing::TextMapReader& carrier, TraceContext& trace_context, diff --git a/src/tracer/propagation/propagation.h b/src/tracer/propagation/propagation.h index 0b35515a..8020ad1f 100644 --- a/src/tracer/propagation/propagation.h +++ b/src/tracer/propagation/propagation.h @@ -44,14 +44,6 @@ opentracing::expected InjectSpanContext( const TraceContext& trace_context, opentracing::string_view trace_state, const BaggageMap& baggage); -inline opentracing::expected ExtractSpanContext( - const PropagationOptions& /*propagation_options*/, std::istream& carrier, - uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, - bool& sampled, BaggageProtobufMap& baggage) { - trace_id_high = 0; - return ExtractSpanContext(carrier, trace_id_low, span_id, sampled, baggage); -} - inline opentracing::expected ExtractSpanContext( const PropagationOptions& /*propagation_options*/, std::istream& carrier, TraceContext& trace_context, std::string& /*trace_state*/, BaggageProtobufMap& baggage) { @@ -66,18 +58,6 @@ inline opentracing::expected ExtractSpanContext( return result; } -opentracing::expected ExtractSpanContext( - const PropagationOptions& propagation_options, - const opentracing::TextMapReader& carrier, uint64_t& trace_id_high, - uint64_t& trace_id_low, uint64_t& span_id, bool& sampled, - BaggageProtobufMap& baggage); - -opentracing::expected ExtractSpanContext( - const PropagationOptions& propagation_options, - const opentracing::HTTPHeadersReader& carrier, uint64_t& trace_id_high, - uint64_t& trace_id_low, uint64_t& span_id, bool& sampled, - BaggageProtobufMap& baggage); - opentracing::expected ExtractSpanContext( const PropagationOptions& propagation_options, const opentracing::TextMapReader& carrier, TraceContext& trace_context, diff --git a/src/tracer/propagation/propagator.h b/src/tracer/propagation/propagator.h index 70a3a670..39c7c1f4 100644 --- a/src/tracer/propagation/propagator.h +++ b/src/tracer/propagation/propagator.h @@ -24,42 +24,9 @@ class Propagator { const TraceContext& trace_context, opentracing::string_view trace_state, const BaggageFlatMap& baggage) const = 0; - virtual opentracing::expected ExtractSpanContext( - const opentracing::TextMapReader& carrier, bool case_sensitive, - uint64_t& trace_id_high, uint64_t& trace_id_low, uint64_t& span_id, - bool& sampled, BaggageProtobufMap& baggage) const { - std::string trace_state; - TraceContext trace_context; - auto result = this->ExtractSpanContext(carrier, case_sensitive, - trace_context, trace_state, baggage); - if (!result || !*result) { - return result; - } - trace_id_high = trace_context.trace_id_high; - trace_id_low = trace_context.trace_id_low; - span_id = trace_context.parent_id; - sampled = IsTraceFlagSet(trace_context.trace_flags); - return result; - } - virtual opentracing::expected ExtractSpanContext( const opentracing::TextMapReader& carrier, bool case_sensitive, TraceContext& trace_context, std::string& trace_state, - BaggageProtobufMap& baggage) const { - (void)trace_state; - bool sampled; - auto result = ExtractSpanContext( - carrier, case_sensitive, trace_context.trace_id_high, - trace_context.trace_id_low, trace_context.parent_id, sampled, baggage); - if (!result || !*result) { - return result; - } - trace_context.trace_flags = SetTraceFlag(0, sampled); - if (trace_context.trace_id_high == 0 && trace_context.trace_id_low == 0) { - return opentracing::make_unexpected( - opentracing::invalid_span_context_error); - } - return true; - } + BaggageProtobufMap& baggage) const = 0; }; } // namespace lightstep From e4de96157dc3ce3f95b2881db9d82548d0e2c8d8 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 7 Jan 2020 14:56:49 -0800 Subject: [PATCH 35/42] Run clang-format --- src/tracer/propagation/baggage_propagator.cpp | 6 ++++-- src/tracer/propagation/multiheader_propagator.cpp | 3 ++- src/tracer/propagation/multiheader_propagator.h | 2 +- src/tracer/propagation/propagation.h | 3 ++- src/tracer/propagation/trace_context_propagator.h | 10 ++++------ src/tracer/tracer_impl.cpp | 5 +++-- 6 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/tracer/propagation/baggage_propagator.cpp b/src/tracer/propagation/baggage_propagator.cpp index 7cb05122..61e3032a 100644 --- a/src/tracer/propagation/baggage_propagator.cpp +++ b/src/tracer/propagation/baggage_propagator.cpp @@ -45,14 +45,16 @@ BaggagePropagator::BaggagePropagator( //-------------------------------------------------------------------------------------------------- opentracing::expected BaggagePropagator::InjectSpanContext( const opentracing::TextMapWriter& carrier, - const TraceContext& /*trace_context*/, opentracing::string_view /*trace_state*/, + const TraceContext& /*trace_context*/, + opentracing::string_view /*trace_state*/, const BaggageProtobufMap& baggage) const { return this->InjectSpanContextImpl(carrier, baggage); } opentracing::expected BaggagePropagator::InjectSpanContext( const opentracing::TextMapWriter& carrier, - const TraceContext& /*trace_context*/, opentracing::string_view /*trace_state*/, + const TraceContext& /*trace_context*/, + opentracing::string_view /*trace_state*/, const BaggageFlatMap& baggage) const { return this->InjectSpanContextImpl(carrier, baggage); } diff --git a/src/tracer/propagation/multiheader_propagator.cpp b/src/tracer/propagation/multiheader_propagator.cpp index ebbf9ba9..12632e18 100644 --- a/src/tracer/propagation/multiheader_propagator.cpp +++ b/src/tracer/propagation/multiheader_propagator.cpp @@ -92,7 +92,8 @@ opentracing::expected MultiheaderPropagator::InjectSpanContextImpl( if (!result) { return result; } - result = carrier.Set(span_id_key_, hex_serializer.Uint64ToHex(trace_context.parent_id)); + result = carrier.Set(span_id_key_, + hex_serializer.Uint64ToHex(trace_context.parent_id)); if (!result) { return result; } diff --git a/src/tracer/propagation/multiheader_propagator.h b/src/tracer/propagation/multiheader_propagator.h index e0c3b504..7c2823ab 100644 --- a/src/tracer/propagation/multiheader_propagator.h +++ b/src/tracer/propagation/multiheader_propagator.h @@ -38,7 +38,7 @@ class MultiheaderPropagator final : public Propagator { bool supports_128bit_; opentracing::expected InjectSpanContextImpl( - const opentracing::TextMapWriter& carrier, + const opentracing::TextMapWriter& carrier, const TraceContext& trace_context) const; template diff --git a/src/tracer/propagation/propagation.h b/src/tracer/propagation/propagation.h index 8020ad1f..22daa967 100644 --- a/src/tracer/propagation/propagation.h +++ b/src/tracer/propagation/propagation.h @@ -46,7 +46,8 @@ opentracing::expected InjectSpanContext( inline opentracing::expected ExtractSpanContext( const PropagationOptions& /*propagation_options*/, std::istream& carrier, - TraceContext& trace_context, std::string& /*trace_state*/, BaggageProtobufMap& baggage) { + TraceContext& trace_context, std::string& /*trace_state*/, + BaggageProtobufMap& baggage) { trace_context.trace_id_high = 0; bool sampled; auto result = ExtractSpanContext(carrier, trace_context.trace_id_low, diff --git a/src/tracer/propagation/trace_context_propagator.h b/src/tracer/propagation/trace_context_propagator.h index 9877441d..8d9e5187 100644 --- a/src/tracer/propagation/trace_context_propagator.h +++ b/src/tracer/propagation/trace_context_propagator.h @@ -7,15 +7,13 @@ class TraceContextPropagator final : Propagator { public: // Propagator opentracing::expected InjectSpanContext( - const opentracing::TextMapWriter& carrier, - const TraceContext& trace_context, - opentracing::string_view trace_state, + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view trace_state, const BaggageProtobufMap& baggage) const override; opentracing::expected InjectSpanContext( - const opentracing::TextMapWriter& carrier, - const TraceContext& trace_context, - opentracing::string_view trace_state, + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view trace_state, const BaggageFlatMap& baggage) const override; opentracing::expected ExtractSpanContext( diff --git a/src/tracer/tracer_impl.cpp b/src/tracer/tracer_impl.cpp index 497a4f82..0a9fa9a6 100644 --- a/src/tracer/tracer_impl.cpp +++ b/src/tracer/tracer_impl.cpp @@ -26,8 +26,9 @@ static opentracing::expected InjectImpl( // ExtractImpl //------------------------------------------------------------------------------ template -static opentracing::expected> ExtractImpl( - const PropagationOptions& propagation_options, Carrier& reader) try { +static opentracing::expected> +ExtractImpl(const PropagationOptions& propagation_options, + Carrier& reader) try { TraceContext trace_context; std::string trace_state; BaggageProtobufMap baggage; From 290d8544c0ba98d25c15f54297ccb5db35d53d85 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 7 Jan 2020 15:10:51 -0800 Subject: [PATCH 36/42] Fix clang-tidy error --- src/tracer/propagation/trace_context.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tracer/propagation/trace_context.h b/src/tracer/propagation/trace_context.h index cb15f409..718ccf83 100644 --- a/src/tracer/propagation/trace_context.h +++ b/src/tracer/propagation/trace_context.h @@ -32,8 +32,7 @@ template inline uint8_t SetTraceFlag(uint8_t flags, bool value) noexcept { if (value) { return flags | Mask; - } else { - return flags & ~Mask; } + return flags & ~Mask; } } // namespace lightstep From 245d98adf3e2bbbe1f4cc4cfc09b349500df9678 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 7 Jan 2020 17:33:17 -0800 Subject: [PATCH 37/42] Expose trace-context as an option --- include/lightstep/tracer.h | 7 ++++++- src/tracer/propagation/BUILD | 1 + src/tracer/propagation/propagation_options.cpp | 4 ++++ src/tracer/propagation/trace_context_propagator.h | 2 +- 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/include/lightstep/tracer.h b/include/lightstep/tracer.h index f926bee0..e7ecc708 100644 --- a/include/lightstep/tracer.h +++ b/include/lightstep/tracer.h @@ -22,7 +22,12 @@ const std::string& CollectorMethodName(); enum class LogLevel { debug = 1, info = 2, warn = 3, error = 4, off = 6 }; // Denotes different span context propagation formats. -enum class PropagationMode { lightstep = 1, b3 = 2, envoy = 3 }; +enum class PropagationMode { + lightstep = 1, + b3 = 2, + envoy = 3, + trace_context = 4 +}; // DynamicConfigurationValue is used for configuration values that can // be either fixed or changed at runtime. To specify a fixed value, just assign diff --git a/src/tracer/propagation/BUILD b/src/tracer/propagation/BUILD index e157bf90..6675deb5 100644 --- a/src/tracer/propagation/BUILD +++ b/src/tracer/propagation/BUILD @@ -165,6 +165,7 @@ lightstep_cc_library( ":lightstep_propagator_lib", ":b3_propagator_lib", ":envoy_propagator_lib", + ":trace_context_propagator_lib", ":baggage_propagator_lib", ], ) diff --git a/src/tracer/propagation/propagation_options.cpp b/src/tracer/propagation/propagation_options.cpp index 128d581d..0dc4eba3 100644 --- a/src/tracer/propagation/propagation_options.cpp +++ b/src/tracer/propagation/propagation_options.cpp @@ -6,6 +6,7 @@ #include "tracer/propagation/baggage_propagator.h" #include "tracer/propagation/envoy_propagator.h" #include "tracer/propagation/lightstep_propagator.h" +#include "tracer/propagation/trace_context_propagator.h" namespace lightstep { //-------------------------------------------------------------------------------------------------- @@ -72,6 +73,9 @@ static std::vector> MakePropagators( case PropagationMode::envoy: result.emplace_back(new EnvoyPropagator{}); break; + case PropagationMode::trace_context: + result.emplace_back(new TraceContextPropagator{}); + break; } } return result; diff --git a/src/tracer/propagation/trace_context_propagator.h b/src/tracer/propagation/trace_context_propagator.h index 8d9e5187..0bdb3756 100644 --- a/src/tracer/propagation/trace_context_propagator.h +++ b/src/tracer/propagation/trace_context_propagator.h @@ -3,7 +3,7 @@ #include "tracer/propagation/propagator.h" namespace lightstep { -class TraceContextPropagator final : Propagator { +class TraceContextPropagator final : public Propagator { public: // Propagator opentracing::expected InjectSpanContext( From c8c3698a8fd380c881fd6d1818b15bad69f68cf9 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 7 Jan 2020 21:39:42 -0800 Subject: [PATCH 38/42] Fix baggage handling with TraceContextPropagator --- .../propagation/trace_context_propagator.cpp | 23 +++++++++--- .../propagation/trace_context_propagator.h | 2 -- test/tracer/propagation/BUILD | 14 ++++++++ .../trace_context_propagation_test.cpp | 36 +++++++++++++++++++ 4 files changed, 69 insertions(+), 6 deletions(-) create mode 100644 test/tracer/propagation/trace_context_propagation_test.cpp diff --git a/src/tracer/propagation/trace_context_propagator.cpp b/src/tracer/propagation/trace_context_propagator.cpp index 3eda44c7..e64512ce 100644 --- a/src/tracer/propagation/trace_context_propagator.cpp +++ b/src/tracer/propagation/trace_context_propagator.cpp @@ -2,8 +2,11 @@ #include +#include "common/utility.h" + const opentracing::string_view TraceParentHeaderKey = "traceparent"; const opentracing::string_view TraceStateHeaderKey = "tracestate"; +const opentracing::string_view PrefixBaggage = "ot-baggage-"; namespace lightstep { //-------------------------------------------------------------------------------------------------- @@ -32,7 +35,8 @@ static opentracing::expected InjectSpanContextImpl( template static opentracing::expected ExtractSpanContextImpl( const opentracing::TextMapReader& carrier, TraceContext& trace_context, - std::string& trace_state, const KeyCompare& key_compare) { + std::string& trace_state, const KeyCompare& key_compare, + BaggageProtobufMap& baggage) { bool parent_header_found = false; auto result = carrier.ForeachKey([&](opentracing::string_view key, @@ -46,6 +50,15 @@ static opentracing::expected ExtractSpanContextImpl( parent_header_found = true; } else if (key_compare(key, TraceStateHeaderKey)) { trace_state.assign(value.data(), value.size()); + } else if (key.length() > PrefixBaggage.size() && + key_compare(opentracing::string_view{key.data(), + PrefixBaggage.size()}, + PrefixBaggage)) { + baggage.insert(BaggageProtobufMap::value_type( + ToLower( + opentracing::string_view{key.data() + PrefixBaggage.size(), + key.size() - PrefixBaggage.size()}), + value)); } return {}; }); @@ -78,7 +91,7 @@ opentracing::expected TraceContextPropagator::InjectSpanContext( opentracing::expected TraceContextPropagator::ExtractSpanContext( const opentracing::TextMapReader& carrier, bool case_sensitive, TraceContext& trace_context, std::string& trace_state, - BaggageProtobufMap& /*baggage*/) const { + BaggageProtobufMap& baggage) const { auto iequals = [](opentracing::string_view lhs, opentracing::string_view rhs) noexcept { return lhs.length() == rhs.length() && @@ -89,8 +102,10 @@ opentracing::expected TraceContextPropagator::ExtractSpanContext( }; if (case_sensitive) { return ExtractSpanContextImpl(carrier, trace_context, trace_state, - std::equal_to{}); + std::equal_to{}, + baggage); } - return ExtractSpanContextImpl(carrier, trace_context, trace_state, iequals); + return ExtractSpanContextImpl(carrier, trace_context, trace_state, iequals, + baggage); } } // namespace lightstep diff --git a/src/tracer/propagation/trace_context_propagator.h b/src/tracer/propagation/trace_context_propagator.h index 0bdb3756..66d701cf 100644 --- a/src/tracer/propagation/trace_context_propagator.h +++ b/src/tracer/propagation/trace_context_propagator.h @@ -20,7 +20,5 @@ class TraceContextPropagator final : public Propagator { const opentracing::TextMapReader& carrier, bool case_sensitive, TraceContext& trace_context, std::string& trace_state, BaggageProtobufMap& baggage) const override; - - private: }; } // namespace lightstep diff --git a/test/tracer/propagation/BUILD b/test/tracer/propagation/BUILD index bd6eba64..435cb54c 100644 --- a/test/tracer/propagation/BUILD +++ b/test/tracer/propagation/BUILD @@ -118,6 +118,20 @@ lightstep_catch_test( ], ) +lightstep_catch_test( + name = "trace_context_propagation_test", + srcs = [ + "trace_context_propagation_test.cpp", + ], + deps = [ + "//:manual_tracer_lib", + "//test/recorder:in_memory_recorder_lib", + ":text_map_carrier_lib", + ":http_headers_carrier_lib", + ":utility_lib", + ], +) + lightstep_catch_test( name = "propagation_options_test", srcs = [ diff --git a/test/tracer/propagation/trace_context_propagation_test.cpp b/test/tracer/propagation/trace_context_propagation_test.cpp new file mode 100644 index 00000000..b20ef67e --- /dev/null +++ b/test/tracer/propagation/trace_context_propagation_test.cpp @@ -0,0 +1,36 @@ +#include "test/recorder/in_memory_recorder.h" +#include "test/tracer/propagation/http_headers_carrier.h" +#include "test/tracer/propagation/text_map_carrier.h" +#include "test/tracer/propagation/utility.h" +#include "tracer/legacy/legacy_tracer_impl.h" +#include "tracer/tracer_impl.h" + +#include "3rd_party/catch2/catch.hpp" +using namespace lightstep; + +TEST_CASE("trace-context propagation") { + LightStepTracerOptions tracer_options; + tracer_options.propagation_modes = {PropagationMode::trace_context}; + auto recorder = new InMemoryRecorder{}; + auto tracer = std::shared_ptr{ + new TracerImpl{MakePropagationOptions(tracer_options), + std::unique_ptr{recorder}}}; + std::unordered_map text_map; + TextMapCarrier text_map_carrier{text_map}; + HTTPHeadersCarrier http_headers_carrier{text_map}; + + SECTION("Inject, extract yields the same span context.") { + auto test_span_contexts = MakeTestSpanContexts(true); + for (auto& span_context : test_span_contexts) { + // text map carrier + CHECK_NOTHROW( + VerifyInjectExtract(*tracer, *span_context, text_map_carrier)); + text_map.clear(); + + // http headers carrier + CHECK_NOTHROW( + VerifyInjectExtract(*tracer, *span_context, http_headers_carrier)); + text_map.clear(); + } + } +} From 6634f144ca197a7d2e4d5c525cd48ba3e3280917 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 7 Jan 2020 22:13:15 -0800 Subject: [PATCH 39/42] Add test coverage for trace-context --- .../trace_context_propagation_test.cpp | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/test/tracer/propagation/trace_context_propagation_test.cpp b/test/tracer/propagation/trace_context_propagation_test.cpp index b20ef67e..9428f0de 100644 --- a/test/tracer/propagation/trace_context_propagation_test.cpp +++ b/test/tracer/propagation/trace_context_propagation_test.cpp @@ -2,6 +2,7 @@ #include "test/tracer/propagation/http_headers_carrier.h" #include "test/tracer/propagation/text_map_carrier.h" #include "test/tracer/propagation/utility.h" +#include "tracer/immutable_span_context.h" #include "tracer/legacy/legacy_tracer_impl.h" #include "tracer/tracer_impl.h" @@ -33,4 +34,63 @@ TEST_CASE("trace-context propagation") { text_map.clear(); } } + + SECTION("trace_flags is copied over to children") { + TraceContext trace_context; + trace_context.trace_id_high = 123; + trace_context.trace_id_low = 456; + trace_context.parent_id = 789; + trace_context.trace_flags = 2; + ImmutableSpanContext span_context{trace_context, "", BaggageProtobufMap{}}; + auto span = tracer->StartSpan("abc", {opentracing::ChildOf(&span_context)}); + REQUIRE(dynamic_cast(span->context()) + .trace_flags() == 2); + } + + SECTION("trace_flags are ORed from multiple parents") { + TraceContext trace_context; + trace_context.trace_id_high = 123; + trace_context.trace_id_low = 456; + trace_context.parent_id = 789; + trace_context.trace_flags = 2; + ImmutableSpanContext span_context1{trace_context, "", BaggageProtobufMap{}}; + trace_context.trace_flags = 1; + ImmutableSpanContext span_context2{trace_context, "", BaggageProtobufMap{}}; + auto span = + tracer->StartSpan("abc", {opentracing::ChildOf(&span_context1), + opentracing::ChildOf(&span_context2)}); + REQUIRE(dynamic_cast(span->context()) + .trace_flags() == 3); + } + + SECTION("trace_state is copied over to children") { + TraceContext trace_context; + trace_context.trace_id_high = 123; + trace_context.trace_id_low = 456; + trace_context.parent_id = 789; + trace_context.trace_flags = 2; + ImmutableSpanContext span_context{trace_context, "abc=123", + BaggageProtobufMap{}}; + auto span = tracer->StartSpan("abc", {opentracing::ChildOf(&span_context)}); + REQUIRE(dynamic_cast(span->context()) + .trace_state() == "abc=123"); + } + + SECTION("trace_states are joined from multiple parents") { + TraceContext trace_context; + trace_context.trace_id_high = 123; + trace_context.trace_id_low = 456; + trace_context.parent_id = 789; + trace_context.trace_flags = 2; + ImmutableSpanContext span_context1{trace_context, "abc=123", + BaggageProtobufMap{}}; + trace_context.trace_flags = 1; + ImmutableSpanContext span_context2{trace_context, "xyz=456", + BaggageProtobufMap{}}; + auto span = + tracer->StartSpan("abc", {opentracing::ChildOf(&span_context1), + opentracing::ChildOf(&span_context2)}); + REQUIRE(dynamic_cast(span->context()) + .trace_state() == "abc=123,xyz=456"); + } } From e1afc0bf559441894e820df891047043af78e592 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 7 Jan 2020 22:18:41 -0800 Subject: [PATCH 40/42] Fix cmake build --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 500f6746..8eec4cba 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -217,6 +217,7 @@ set(LIGHTSTEP_SRCS src/common/utility.cpp src/tracer/propagation/baggage_propagator.cpp src/tracer/propagation/binary_propagation.cpp src/tracer/propagation/envoy_propagator.cpp + src/tracer/propagation/trace_context_propagator.cpp src/tracer/propagation/lightstep_propagator.cpp src/tracer/propagation/multiheader_propagator.cpp src/tracer/propagation/propagation.cpp From 158155d8d92ba4f37aa09e7b9c55d8ffe2206dca Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 7 Jan 2020 23:19:39 -0800 Subject: [PATCH 41/42] Add missing cmake file --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8eec4cba..decec1cd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -217,6 +217,7 @@ set(LIGHTSTEP_SRCS src/common/utility.cpp src/tracer/propagation/baggage_propagator.cpp src/tracer/propagation/binary_propagation.cpp src/tracer/propagation/envoy_propagator.cpp + src/tracer/propagation/trace_context.cpp src/tracer/propagation/trace_context_propagator.cpp src/tracer/propagation/lightstep_propagator.cpp src/tracer/propagation/multiheader_propagator.cpp From 5dd8dea307f88d330f8b444cc719ddfd3f0fa50f Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 4 Feb 2020 18:02:09 -0800 Subject: [PATCH 42/42] Support trace_context via json options --- .../tracer_configuration.schema.json | 2 +- src/tracer/json_options.cpp | 3 +++ src/tracer/utility.cpp | 3 +++ test/tracer/json_options_test.cpp | 6 +++--- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/lightstep-tracer-configuration/tracer_configuration.schema.json b/lightstep-tracer-configuration/tracer_configuration.schema.json index 43d53085..a1210bf2 100644 --- a/lightstep-tracer-configuration/tracer_configuration.schema.json +++ b/lightstep-tracer-configuration/tracer_configuration.schema.json @@ -79,7 +79,7 @@ "items": { "type": "string" }, - "description": "A list of propagation modes to use for injection/extraction.\nValid values are \"lightstep\", \"b3\", and \"envoy\"." + "description": "A list of propagation modes to use for injection/extraction.\nValid values are \"lightstep\", \"b3\", \"envoy\", and \"trace_context\"." } }, "definitions": { diff --git a/src/tracer/json_options.cpp b/src/tracer/json_options.cpp index afbe4f42..9d2d21f6 100644 --- a/src/tracer/json_options.cpp +++ b/src/tracer/json_options.cpp @@ -44,6 +44,9 @@ static PropagationMode GetPropagationMode(opentracing::string_view s) { if (s == "envoy") { return PropagationMode::envoy; } + if (s == "trace_context") { + return PropagationMode::trace_context; + } std::ostringstream oss; oss << "invalid propagation mode " << s; throw std::runtime_error{oss.str()}; diff --git a/src/tracer/utility.cpp b/src/tracer/utility.cpp index b16b9665..f0021d77 100644 --- a/src/tracer/utility.cpp +++ b/src/tracer/utility.cpp @@ -1,6 +1,9 @@ #include "tracer/utility.h" namespace lightstep { +//-------------------------------------------------------------------------------------------------- +// AppendTraceState +//-------------------------------------------------------------------------------------------------- void AppendTraceState(std::string& trace_state, opentracing::string_view key_values) { if (trace_state.empty()) { diff --git a/test/tracer/json_options_test.cpp b/test/tracer/json_options_test.cpp index 4bdba2f6..4990d2db 100644 --- a/test/tracer/json_options_test.cpp +++ b/test/tracer/json_options_test.cpp @@ -9,13 +9,13 @@ TEST_CASE("LightStepTracerFactory") { SECTION("We can specify propagation modes via a tracers json configuration") { const char* config = R"({ "component_name": "t", - "propagation_modes": ["lightstep", "b3", "envoy"] + "propagation_modes": ["lightstep", "b3", "envoy", "trace_context"] })"; auto options_maybe = MakeTracerOptions(config, error_message); REQUIRE(options_maybe); std::vector expected_propagation_modes = { - PropagationMode::lightstep, PropagationMode::b3, - PropagationMode::envoy}; + PropagationMode::lightstep, PropagationMode::b3, PropagationMode::envoy, + PropagationMode::trace_context}; REQUIRE(options_maybe->propagation_modes == expected_propagation_modes); }