From 20545c7d7eb3f11278a9a8049b359e7c46c34a96 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Thu, 30 Jul 2020 18:16:47 +0100 Subject: [PATCH 01/31] Add support for x-cloud-trace-context --- CMakeLists.txt | 5 + include/lightstep/tracer.h | 3 +- src/tracer/json_options.cpp | 3 + src/tracer/propagation/BUILD | 16 +++ .../propagation/cloud_trace_propagator.cpp | 126 ++++++++++++++++++ .../propagation/cloud_trace_propagator.h | 36 +++++ .../propagation/propagation_options.cpp | 4 + test/tracer/propagation/BUILD | 14 ++ .../cloud_trace_propagation_test.cpp | 86 ++++++++++++ 9 files changed, 292 insertions(+), 1 deletion(-) create mode 100644 src/tracer/propagation/cloud_trace_propagator.cpp create mode 100644 src/tracer/propagation/cloud_trace_propagator.h create mode 100644 test/tracer/propagation/cloud_trace_propagation_test.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 7e7f7995..8a87b00e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -131,6 +131,11 @@ endif() include_directories(SYSTEM ${OPENTRACING_INCLUDE_DIR}) list(APPEND LIGHTSTEP_LINK_LIBRARIES ${OPENTRACING_LIBRARY}) +set(CMAKE_THREAD_PREFER_PTHREAD TRUE) +set(THREADS_PREFER_PTHREAD_FLAG TRUE) +find_package(Threads REQUIRED) +list(APPEND LIGHTSTEP_LINK_LIBRARIES Threads::Threads) + if (WITH_GRPC) find_package(gRPC CONFIG) # First attempt to set up gRPC via cmake; but if cmake config files aren't diff --git a/include/lightstep/tracer.h b/include/lightstep/tracer.h index a78481bf..f9f51aac 100644 --- a/include/lightstep/tracer.h +++ b/include/lightstep/tracer.h @@ -26,7 +26,8 @@ enum class PropagationMode { lightstep = 1, b3 = 2, envoy = 3, - trace_context = 4 + trace_context = 4, + cloud_trace = 5 }; // DynamicConfigurationValue is used for configuration values that can diff --git a/src/tracer/json_options.cpp b/src/tracer/json_options.cpp index 88cf7a2b..1702dd71 100644 --- a/src/tracer/json_options.cpp +++ b/src/tracer/json_options.cpp @@ -47,6 +47,9 @@ static PropagationMode GetPropagationMode(opentracing::string_view s) { if (s == "trace_context") { return PropagationMode::trace_context; } + if (s == "cloud_trace") { + return PropagationMode::cloud_trace; + } std::ostringstream oss; oss << "invalid propagation mode " << s; throw std::runtime_error{oss.str()}; diff --git a/src/tracer/propagation/BUILD b/src/tracer/propagation/BUILD index 6675deb5..92c6dd98 100644 --- a/src/tracer/propagation/BUILD +++ b/src/tracer/propagation/BUILD @@ -167,6 +167,7 @@ lightstep_cc_library( ":envoy_propagator_lib", ":trace_context_propagator_lib", ":baggage_propagator_lib", + ":cloud_trace_propagator_lib", ], ) @@ -184,3 +185,18 @@ lightstep_cc_library( ":trace_context_lib", ], ) + +lightstep_cc_library( + name = "cloud_trace_propagator_lib", + private_hdrs = [ + "cloud_trace_propagator.h", + ], + srcs = [ + "cloud_trace_propagator.cpp", + ], + deps = [ + "//3rd_party/base64:base64_lib", + ":binary_propagation_lib", + ":utility_lib", + ], +) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp new file mode 100644 index 00000000..2b9daff5 --- /dev/null +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -0,0 +1,126 @@ +#include "tracer/propagation/cloud_trace_propagator.h" + +#include + +#include "common/in_memory_stream.h" +#include "tracer/propagation/binary_propagation.h" +#include "tracer/propagation/utility.h" + +const opentracing::string_view PropagationSingleKey = "x-cloud-trace-context"; + +namespace lightstep { +//-------------------------------------------------------------------------------------------------- +// InjectSpanContext +//-------------------------------------------------------------------------------------------------- +opentracing::expected CloudTracePropagator::InjectSpanContext( + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view /*trace_state*/, + const BaggageProtobufMap& baggage) const { + return this->InjectSpanContextImpl(carrier, trace_context, baggage); +} + +opentracing::expected CloudTracePropagator::InjectSpanContext( + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, opentracing::string_view /*trace_state*/, + const BaggageFlatMap& baggage) const { + return this->InjectSpanContextImpl(carrier, trace_context, baggage); +} + +//-------------------------------------------------------------------------------------------------- +// ExtractSpanContext +//-------------------------------------------------------------------------------------------------- +opentracing::expected CloudTracePropagator::ExtractSpanContext( + const opentracing::TextMapReader& carrier, bool case_sensitive, + 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() && + std::equal(std::begin(lhs), std::end(lhs), std::begin(rhs), + [](char a, char b) { + return std::tolower(a) == std::tolower(b); + }); + }; + bool sampled; + opentracing::expected result; + if (case_sensitive) { + 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; + } + trace_context.trace_flags = SetTraceFlag(0, sampled); + return result; +} + +//-------------------------------------------------------------------------------------------------- +// InjectSpanContextImpl +//-------------------------------------------------------------------------------------------------- +template +opentracing::expected CloudTracePropagator::InjectSpanContextImpl( + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, const BaggageMap& baggage) const { + std::ostringstream ostream; + auto result = lightstep::InjectSpanContext( + ostream, trace_context.trace_id_low, trace_context.parent_id, + IsTraceFlagSet(trace_context.trace_flags), baggage); + if (!result) { + return result; + } + std::string context_value; + try { + auto binary_encoding = ostream.str(); + context_value = + Base64::encode(binary_encoding.data(), binary_encoding.size()); + } catch (const std::bad_alloc&) { + return opentracing::make_unexpected( + std::make_error_code(std::errc::not_enough_memory)); + } + + result = carrier.Set(PropagationSingleKey, context_value); + if (!result) { + return result; + } + return {}; +} + +//-------------------------------------------------------------------------------------------------- +// ExtractSpanContextImpl +//-------------------------------------------------------------------------------------------------- +template +opentracing::expected CloudTracePropagator::ExtractSpanContextImpl( + const opentracing::TextMapReader& carrier, uint64_t& trace_id_high, + uint64_t& trace_id_low, uint64_t& span_id, bool& sampled, + BaggageProtobufMap& baggage, const KeyCompare& key_compare) const { + trace_id_high = 0; + auto value_maybe = LookupKey(carrier, PropagationSingleKey, key_compare); + if (!value_maybe) { + if (AreErrorsEqual(value_maybe.error(), opentracing::key_not_found_error)) { + return false; + } + return opentracing::make_unexpected(value_maybe.error()); + } + auto value = *value_maybe; + std::string base64_decoding; + try { + base64_decoding = Base64::decode(value.data(), value.size()); + } catch (const std::bad_alloc&) { + return opentracing::make_unexpected( + std::make_error_code(std::errc::not_enough_memory)); + } + if (base64_decoding.empty()) { + return opentracing::make_unexpected( + opentracing::span_context_corrupted_error); + } + in_memory_stream istream{base64_decoding.data(), base64_decoding.size()}; + return lightstep::ExtractSpanContext(istream, trace_id_low, span_id, sampled, + baggage); +} +} // namespace lightstep diff --git a/src/tracer/propagation/cloud_trace_propagator.h b/src/tracer/propagation/cloud_trace_propagator.h new file mode 100644 index 00000000..c896d3ee --- /dev/null +++ b/src/tracer/propagation/cloud_trace_propagator.h @@ -0,0 +1,36 @@ +#pragma once + +#include "tracer/propagation/propagator.h" + +namespace lightstep { +class CloudTracePropagator final : public Propagator { + public: + // Propagator + 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, + TraceContext& trace_context, std::string& trace_state, + BaggageProtobufMap& baggage) const override; + + private: + template + opentracing::expected InjectSpanContextImpl( + const opentracing::TextMapWriter& carrier, + const TraceContext& trace_context, const BaggageMap& baggage) const; + + template + opentracing::expected ExtractSpanContextImpl( + const opentracing::TextMapReader& carrier, uint64_t& trace_id_high, + uint64_t& trace_id_low, uint64_t& span_id, bool& sampled, + BaggageProtobufMap& baggage, const KeyCompare& key_compare) const; +}; +} // namespace lightstep diff --git a/src/tracer/propagation/propagation_options.cpp b/src/tracer/propagation/propagation_options.cpp index 0dc4eba3..f36073dc 100644 --- a/src/tracer/propagation/propagation_options.cpp +++ b/src/tracer/propagation/propagation_options.cpp @@ -7,6 +7,7 @@ #include "tracer/propagation/envoy_propagator.h" #include "tracer/propagation/lightstep_propagator.h" #include "tracer/propagation/trace_context_propagator.h" +#include "tracer/propagation/cloud_trace_propagator.h" namespace lightstep { //-------------------------------------------------------------------------------------------------- @@ -76,6 +77,9 @@ static std::vector> MakePropagators( case PropagationMode::trace_context: result.emplace_back(new TraceContextPropagator{}); break; + case PropagationMode::cloud_trace: + result.emplace_back(new CloudTracePropagator{}); + break; } } return result; diff --git a/test/tracer/propagation/BUILD b/test/tracer/propagation/BUILD index 435cb54c..afbdd54b 100644 --- a/test/tracer/propagation/BUILD +++ b/test/tracer/propagation/BUILD @@ -141,3 +141,17 @@ lightstep_catch_test( "//src/tracer/propagation:propagation_lib", ], ) + +lightstep_catch_test( + name = "cloud_trace_propagation_test", + srcs = [ + "cloud_trace_propagation_test.cpp", + ], + deps = [ + "//:manual_tracer_lib", + "//test/recorder:in_memory_recorder_lib", + ":text_map_carrier_lib", + ":http_headers_carrier_lib", + ":utility_lib", + ], +) diff --git a/test/tracer/propagation/cloud_trace_propagation_test.cpp b/test/tracer/propagation/cloud_trace_propagation_test.cpp new file mode 100644 index 00000000..7c742ed9 --- /dev/null +++ b/test/tracer/propagation/cloud_trace_propagation_test.cpp @@ -0,0 +1,86 @@ +#include + +#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("cloud_trace propagation") { + LightStepTracerOptions tracer_options; + tracer_options.propagation_modes = {PropagationMode::cloud_trace}; + 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.") { + for (auto use_128bit_trace_ids : {true, false}) { + auto test_span_contexts = MakeTestSpanContexts(use_128bit_trace_ids); + 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(); + } + } + } + + SECTION("Verify extraction against a 128-bit trace id") { + text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/aef5705a09004083;o=1"}}; + auto span_context_maybe = tracer->Extract(http_headers_carrier); + REQUIRE(span_context_maybe); + auto span_context = + dynamic_cast(span_context_maybe->get()); + REQUIRE(span_context->trace_id_high() == 0xaef5705a09004083ul); + REQUIRE(span_context->trace_id_low() == 0x8f1359ebafa5c0c6ul); + REQUIRE(span_context->span_id() == 0xaef5705a09004083ul); + REQUIRE(span_context->sampled() == true); + } + + SECTION("Verify extraction with a 128 bit trace id when sampled is false") { + text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/aef5705a09004083;o=0"}}; + auto span_context_maybe = tracer->Extract(http_headers_carrier); + REQUIRE(span_context_maybe); + auto span_context = + dynamic_cast(span_context_maybe->get()); + REQUIRE(span_context->trace_id_high() == 0xaef5705a09004083ul); + REQUIRE(span_context->trace_id_low() == 0x8f1359ebafa5c0c6ul); + REQUIRE(span_context->span_id() == 0xaef5705a09004083ul); + REQUIRE(span_context->sampled() == false); + } + + SECTION("A child keeps the same trace id as its parent") { + text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/aef5705a09004083;o=0"}}; + auto span_context_maybe = tracer->Extract(http_headers_carrier); + REQUIRE(span_context_maybe); + auto span = tracer->StartSpan( + "abc", {opentracing::ChildOf(span_context_maybe->get())}); + auto span_context1 = + dynamic_cast(span_context_maybe->get()); + auto span_context2 = + dynamic_cast(&span->context()); + REQUIRE(span_context1->trace_id_high() == span_context2->trace_id_high()); + REQUIRE(span_context1->trace_id_low() == span_context2->trace_id_low()); + } + + SECTION("The low part of 128-bit trace ids are sent to satellites") { + text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/aef5705a09004083;o=1"}}; + auto span_context_maybe = tracer->Extract(http_headers_carrier); + REQUIRE(span_context_maybe); + tracer->StartSpan("abc", {opentracing::ChildOf(span_context_maybe->get())}); + REQUIRE(recorder->top().span_context().trace_id() == 0x8f1359ebafa5c0c6ul); + } +} From d311a7aea1d7d975938ffbc34a3b286a3324bd29 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Thu, 30 Jul 2020 21:58:41 +0100 Subject: [PATCH 02/31] needs debugging --- .../propagation/cloud_trace_propagator.cpp | 186 ++++++++++++------ .../propagation/cloud_trace_propagator.h | 14 +- 2 files changed, 136 insertions(+), 64 deletions(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index 2b9daff5..a0b067d4 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -1,12 +1,12 @@ #include "tracer/propagation/cloud_trace_propagator.h" -#include +#include "common/hex_conversion.h" -#include "common/in_memory_stream.h" #include "tracer/propagation/binary_propagation.h" #include "tracer/propagation/utility.h" const opentracing::string_view PropagationSingleKey = "x-cloud-trace-context"; +const opentracing::string_view PrefixBaggage = "ot-baggage-"; namespace lightstep { //-------------------------------------------------------------------------------------------------- @@ -15,17 +15,16 @@ namespace lightstep { opentracing::expected CloudTracePropagator::InjectSpanContext( const opentracing::TextMapWriter& carrier, const TraceContext& trace_context, opentracing::string_view /*trace_state*/, - const BaggageProtobufMap& baggage) const { - return this->InjectSpanContextImpl(carrier, trace_context, baggage); + const BaggageProtobufMap& /*baggage*/) const { + return this->InjectSpanContextImpl(carrier, trace_context); } opentracing::expected CloudTracePropagator::InjectSpanContext( const opentracing::TextMapWriter& carrier, const TraceContext& trace_context, opentracing::string_view /*trace_state*/, - const BaggageFlatMap& baggage) const { - return this->InjectSpanContextImpl(carrier, trace_context, baggage); + const BaggageFlatMap& /*baggage*/) const { + return this->InjectSpanContextImpl(carrier, trace_context); } - //-------------------------------------------------------------------------------------------------- // ExtractSpanContext //-------------------------------------------------------------------------------------------------- @@ -44,14 +43,10 @@ opentracing::expected CloudTracePropagator::ExtractSpanContext( bool sampled; opentracing::expected result; if (case_sensitive) { - result = ExtractSpanContextImpl(carrier, trace_context.trace_id_high, - trace_context.trace_id_low, - trace_context.parent_id, sampled, baggage, - std::equal_to{}); + result = this->ExtractSpanContextImpl(carrier, trace_context, + std::equal_to{}, baggage); } else { - result = result = ExtractSpanContextImpl( - carrier, trace_context.trace_id_high, trace_context.trace_id_low, - trace_context.parent_id, sampled, baggage, iequals); + result = this->ExtractSpanContextImpl(carrier, trace_context, iequals, baggage); } if (!result || !*result) { return result; @@ -63,32 +58,13 @@ opentracing::expected CloudTracePropagator::ExtractSpanContext( //-------------------------------------------------------------------------------------------------- // InjectSpanContextImpl //-------------------------------------------------------------------------------------------------- -template opentracing::expected CloudTracePropagator::InjectSpanContextImpl( const opentracing::TextMapWriter& carrier, - const TraceContext& trace_context, const BaggageMap& baggage) const { - std::ostringstream ostream; - auto result = lightstep::InjectSpanContext( - ostream, trace_context.trace_id_low, trace_context.parent_id, - IsTraceFlagSet(trace_context.trace_flags), baggage); - if (!result) { - return result; - } - std::string context_value; - try { - auto binary_encoding = ostream.str(); - context_value = - Base64::encode(binary_encoding.data(), binary_encoding.size()); - } catch (const std::bad_alloc&) { - return opentracing::make_unexpected( - std::make_error_code(std::errc::not_enough_memory)); - } - - result = carrier.Set(PropagationSingleKey, context_value); - if (!result) { - return result; - } - return {}; + const TraceContext& trace_context) const { + std::array buffer; + this->SerializeCloudTrace(trace_context, buffer.data()); + return carrier.Set(PropagationSingleKey, + opentracing::string_view{buffer.data(), buffer.size()}); } //-------------------------------------------------------------------------------------------------- @@ -96,31 +72,123 @@ opentracing::expected CloudTracePropagator::InjectSpanContextImpl( //-------------------------------------------------------------------------------------------------- template opentracing::expected CloudTracePropagator::ExtractSpanContextImpl( - const opentracing::TextMapReader& carrier, uint64_t& trace_id_high, - uint64_t& trace_id_low, uint64_t& span_id, bool& sampled, - BaggageProtobufMap& baggage, const KeyCompare& key_compare) const { - trace_id_high = 0; - auto value_maybe = LookupKey(carrier, PropagationSingleKey, key_compare); - if (!value_maybe) { - if (AreErrorsEqual(value_maybe.error(), opentracing::key_not_found_error)) { - return false; - } - return opentracing::make_unexpected(value_maybe.error()); + const opentracing::TextMapReader& carrier, TraceContext& trace_context, const KeyCompare& key_compare, + BaggageProtobufMap& baggage) const { + bool parent_header_found = false; + auto result = + carrier.ForeachKey([&](opentracing::string_view key, + opentracing::string_view + value) noexcept->opentracing::expected { + if (key_compare(key, PropagationSingleKey)) { + auto was_successful = this->ParseCloudTrace(value, trace_context); + if (!was_successful) { + return opentracing::make_unexpected(was_successful.error()); + } + parent_header_found = true; + } 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 {}; + }); + if (!result) { + return opentracing::make_unexpected(result.error()); + } + return parent_header_found; +} + +opentracing::expected CloudTracePropagator::ParseCloudTrace( + opentracing::string_view s, TraceContext& trace_context) const noexcept { + if (s.size() < TraceContextLength) { + return opentracing::make_unexpected( + 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()); } - auto value = *value_maybe; - std::string base64_decoding; - try { - base64_decoding = Base64::decode(value.data(), value.size()); - } catch (const std::bad_alloc&) { + trace_context.version = *version_maybe; + offset += Num8BitHexDigits; + if (s[offset] != '-') { return opentracing::make_unexpected( - std::make_error_code(std::errc::not_enough_memory)); + std::make_error_code(std::errc::invalid_argument)); } - if (base64_decoding.empty()) { + ++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 (!error_maybe) { + return error_maybe; + } + offset += Num128BitHexDigits; + if (s[offset] != '-') { + return opentracing::make_unexpected( + 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 opentracing::make_unexpected( - opentracing::span_context_corrupted_error); + 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()); } - in_memory_stream istream{base64_decoding.data(), base64_decoding.size()}; - return lightstep::ExtractSpanContext(istream, trace_id_low, span_id, sampled, - baggage); + trace_context.trace_flags = *trace_flags_maybe; + + return {}; +} + +void CloudTracePropagator::SerializeCloudTrace(const TraceContext& trace_context, + char* s) const 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/cloud_trace_propagator.h b/src/tracer/propagation/cloud_trace_propagator.h index c896d3ee..030797e5 100644 --- a/src/tracer/propagation/cloud_trace_propagator.h +++ b/src/tracer/propagation/cloud_trace_propagator.h @@ -22,15 +22,19 @@ class CloudTracePropagator final : public Propagator { BaggageProtobufMap& baggage) const override; private: - template opentracing::expected InjectSpanContextImpl( const opentracing::TextMapWriter& carrier, - const TraceContext& trace_context, const BaggageMap& baggage) const; + const TraceContext& trace_context) const; template opentracing::expected ExtractSpanContextImpl( - const opentracing::TextMapReader& carrier, uint64_t& trace_id_high, - uint64_t& trace_id_low, uint64_t& span_id, bool& sampled, - BaggageProtobufMap& baggage, const KeyCompare& key_compare) const; + const opentracing::TextMapReader& carrier, TraceContext& trace_context, + const KeyCompare& key_compare, BaggageProtobufMap& baggage) const; + + opentracing::expected ParseCloudTrace( + opentracing::string_view s, lightstep::TraceContext& trace_context) const noexcept; + + void SerializeCloudTrace(const TraceContext& trace_context, + char* s) const noexcept; }; } // namespace lightstep From c5b1149c20631a8b5f70ab97da395bf4b93a5d19 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Fri, 31 Jul 2020 13:06:24 +0100 Subject: [PATCH 03/31] Fixed one test, just need to implement the parsing and serialising of the x-cloud-trace-context header --- src/tracer/propagation/cloud_trace_propagator.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index a0b067d4..b76b6e55 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -40,7 +40,6 @@ opentracing::expected CloudTracePropagator::ExtractSpanContext( return std::tolower(a) == std::tolower(b); }); }; - bool sampled; opentracing::expected result; if (case_sensitive) { result = this->ExtractSpanContextImpl(carrier, trace_context, @@ -51,7 +50,7 @@ opentracing::expected CloudTracePropagator::ExtractSpanContext( if (!result || !*result) { return result; } - trace_context.trace_flags = SetTraceFlag(0, sampled); + return result; } From dbbcff601a9b127ad49cfcc7e06727af88ef6a06 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Fri, 31 Jul 2020 15:58:06 +0100 Subject: [PATCH 04/31] Now supports long format trace_id/span_id;o=[0-1] - TODO support the trace_id & trace_id/span_id formats --- .../propagation/cloud_trace_propagator.cpp | 63 +++++++++---------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index b76b6e55..f2bc2890 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -104,26 +104,12 @@ opentracing::expected CloudTracePropagator::ExtractSpanContextImpl( opentracing::expected CloudTracePropagator::ParseCloudTrace( opentracing::string_view s, TraceContext& trace_context) const noexcept { - if (s.size() < TraceContextLength) { + if (s.size() < 32) { return opentracing::make_unexpected( 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 opentracing::make_unexpected( - std::make_error_code(std::errc::invalid_argument)); - } - ++offset; - // trace-id auto error_maybe = NormalizedHexToUint128( opentracing::string_view{s.data() + offset, Num128BitHexDigits}, @@ -131,8 +117,14 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( if (!error_maybe) { return error_maybe; } + offset += Num128BitHexDigits; - if (s[offset] != '-') { + if (offset >= s.size()) { + // only a trace ID has been given + return {}; + } + + if (s[offset] != '/') { return opentracing::make_unexpected( std::make_error_code(std::errc::invalid_argument)); } @@ -146,19 +138,24 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( } trace_context.parent_id = *parent_id_maybe; offset += Num64BitHexDigits; - if (s[offset] != '-') { + + char subbuff[4]; + memcpy( subbuff, &s[offset], 3 ); + subbuff[3] = '\0'; + + if (strcmp(subbuff, ";o=") != 0) { return opentracing::make_unexpected( std::make_error_code(std::errc::invalid_argument)); } - ++offset; + offset += 3; // 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()); + if (s[offset] == '1') { + // sampled + trace_context.trace_flags = SetTraceFlag(trace_context.trace_flags, true); + } else { + trace_context.trace_flags = SetTraceFlag(trace_context.trace_flags, false); } - trace_context.trace_flags = *trace_flags_maybe; return {}; } @@ -167,27 +164,29 @@ void CloudTracePropagator::SerializeCloudTrace(const TraceContext& trace_context char* s) const 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) = '-'; + *(s + offset) = '/'; ++offset; // parent-id Uint64ToHex(trace_context.parent_id, s + offset); offset += Num64BitHexDigits; - *(s + offset) = '-'; + *(s + offset) = ';'; + ++offset; + *(s + offset) = 'o'; + ++offset; + *(s + offset) = '='; ++offset; // trace-flags - Uint8ToHex(trace_context.trace_flags, s + offset); + if(IsTraceFlagSet(trace_context.trace_flags)) { + s[offset] = '1'; + } else { + s[offset] = '0'; + } } } // namespace lightstep From d6d90b9c643e13a2266a59b377abdef608d9364c Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Fri, 31 Jul 2020 16:40:25 +0100 Subject: [PATCH 05/31] Support medium and short form cloud-trace headers (TODO verify what we should do when no span ID is provided) --- .../propagation/cloud_trace_propagator.cpp | 22 +++++++++---- .../cloud_trace_propagation_test.cpp | 32 +++++++++++++++++-- 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index f2bc2890..1545a3e1 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -104,12 +104,17 @@ opentracing::expected CloudTracePropagator::ExtractSpanContextImpl( opentracing::expected CloudTracePropagator::ParseCloudTrace( opentracing::string_view s, TraceContext& trace_context) const noexcept { - if (s.size() < 32) { + if (s.size() < Num128BitHexDigits) { return opentracing::make_unexpected( std::make_error_code(std::errc::invalid_argument)); } size_t offset = 0; + // default sampled to on (this comes from the ;o=1 part of + // x-cloud-trace-context; if it is not set we will default to sampling + // this request) + trace_context.trace_flags = SetTraceFlag(trace_context.trace_flags, true); + // trace-id auto error_maybe = NormalizedHexToUint128( opentracing::string_view{s.data() + offset, Num128BitHexDigits}, @@ -119,8 +124,8 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( } offset += Num128BitHexDigits; - if (offset >= s.size()) { - // only a trace ID has been given + if (s.size() < Num128BitHexDigits + 1 + Num64BitHexDigits) { + // only a short form trace ID has been given (not a "trace id/span id") return {}; } @@ -139,6 +144,11 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( trace_context.parent_id = *parent_id_maybe; offset += Num64BitHexDigits; + if (s.size() < Num128BitHexDigits + 1 + Num64BitHexDigits + 4) { + // only a "trace ID/span ID" has been given (not a "trace id/span id;o=[0-1]") + return {}; + } + char subbuff[4]; memcpy( subbuff, &s[offset], 3 ); subbuff[3] = '\0'; @@ -150,10 +160,8 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( offset += 3; // trace-flags - if (s[offset] == '1') { - // sampled - trace_context.trace_flags = SetTraceFlag(trace_context.trace_flags, true); - } else { + if (s[offset] == '0') { + // don't sample trace_context.trace_flags = SetTraceFlag(trace_context.trace_flags, false); } diff --git a/test/tracer/propagation/cloud_trace_propagation_test.cpp b/test/tracer/propagation/cloud_trace_propagation_test.cpp index 7c742ed9..9cbd3a1b 100644 --- a/test/tracer/propagation/cloud_trace_propagation_test.cpp +++ b/test/tracer/propagation/cloud_trace_propagation_test.cpp @@ -38,7 +38,7 @@ TEST_CASE("cloud_trace propagation") { } } - SECTION("Verify extraction against a 128-bit trace id") { + SECTION("Verify extraction against a long-form header with sampled set to true") { text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/aef5705a09004083;o=1"}}; auto span_context_maybe = tracer->Extract(http_headers_carrier); REQUIRE(span_context_maybe); @@ -50,7 +50,7 @@ TEST_CASE("cloud_trace propagation") { REQUIRE(span_context->sampled() == true); } - SECTION("Verify extraction with a 128 bit trace id when sampled is false") { + SECTION("Verify extraction against a long-form header when sampled is false") { text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/aef5705a09004083;o=0"}}; auto span_context_maybe = tracer->Extract(http_headers_carrier); REQUIRE(span_context_maybe); @@ -62,6 +62,34 @@ TEST_CASE("cloud_trace propagation") { REQUIRE(span_context->sampled() == false); } + SECTION("Verify extraction against a medium-form header (trace id/span id)") { + text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/aef5705a09004083"}}; + auto span_context_maybe = tracer->Extract(http_headers_carrier); + REQUIRE(span_context_maybe); + auto span_context = + dynamic_cast(span_context_maybe->get()); + REQUIRE(span_context->trace_id_high() == 0xaef5705a09004083ul); + REQUIRE(span_context->trace_id_low() == 0x8f1359ebafa5c0c6ul); + REQUIRE(span_context->span_id() == 0xaef5705a09004083ul); + REQUIRE(span_context->sampled() == true); + } + + SECTION("Verify extraction against a short-form header (trace id)") { + text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6"}}; + auto span_context_maybe = tracer->Extract(http_headers_carrier); + REQUIRE(span_context_maybe); + auto span_context = + dynamic_cast(span_context_maybe->get()); + REQUIRE(span_context->trace_id_high() == 0xaef5705a09004083ul); + REQUIRE(span_context->trace_id_low() == 0x8f1359ebafa5c0c6ul); + + // TODO - this will fail - I'm not sure what the behaviour should be if a + // span ID isn't given + REQUIRE(span_context->span_id() == 0xaef5705a09004083ul); + + REQUIRE(span_context->sampled() == true); + } + SECTION("A child keeps the same trace id as its parent") { text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/aef5705a09004083;o=0"}}; auto span_context_maybe = tracer->Extract(http_headers_carrier); From 252358aad8a234a92028c39a37b6dbe7ba2d0930 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Mon, 3 Aug 2020 09:23:01 +0100 Subject: [PATCH 06/31] Revert CMakeLists.txt change --- CMakeLists.txt | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8a87b00e..32415438 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,7 +30,7 @@ set(LIGHTSTEP_VERSION_STRING # ============================================================================== # Set up cpack -set(CPACK_PACKAGE_DESCRIPTION_SUMMARY +set(CPACK_PACKAGE_DESCRIPTION_SUMMARY "A LightStep implementation of the C++ OpenTracing API") set(CPACK_PACKAGE_VENDOR "lightstep.com") set(CPACK_PACKAGE_DESCRIPTION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/README.md") @@ -56,13 +56,13 @@ option(WITH_CARES "Build with support for dns resolution using c-ares." OFF) option(WITH_DYNAMIC_LOAD "Build support for dynamic loading." ON) option(HEADERS_ONLY "Only generate version.h." OFF) -# Allow a user to specify an optional default roots.pem file to embed into the -# library. +# Allow a user to specify an optional default roots.pem file to embed into the +# library. # # This is useful if the library is distributed to an environment where gRPC # hasn't been installed. # -# To use, invoke cmake with +# To use, invoke cmake with # cmake -DDEFAULT_SSL_ROOTS_PEM:STRING=/path/to/roots.pem ... # # See also discussion on https://github.com/grpc/grpc/issues/4834 @@ -87,7 +87,7 @@ endif() configure_file(version.h.in include/lightstep/version.h) include_directories(${CMAKE_CURRENT_BINARY_DIR}/include) -install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/include/lightstep +install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/include/lightstep DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) if(HEADERS_ONLY) @@ -131,12 +131,7 @@ endif() include_directories(SYSTEM ${OPENTRACING_INCLUDE_DIR}) list(APPEND LIGHTSTEP_LINK_LIBRARIES ${OPENTRACING_LIBRARY}) -set(CMAKE_THREAD_PREFER_PTHREAD TRUE) -set(THREADS_PREFER_PTHREAD_FLAG TRUE) -find_package(Threads REQUIRED) -list(APPEND LIGHTSTEP_LINK_LIBRARIES Threads::Threads) - -if (WITH_GRPC) +if (WITH_GRPC) find_package(gRPC CONFIG) # First attempt to set up gRPC via cmake; but if cmake config files aren't # available, fallback to pkg-config. @@ -154,7 +149,7 @@ if (WITH_GRPC) find_package(PkgConfig REQUIRED) pkg_search_module(GRPC REQUIRED grpc) pkg_search_module(GRPCPP REQUIRED grpc++) - list(APPEND LIGHTSTEP_LINK_LIBRARIES ${GRPCPP_LDFLAGS} ${GRPC_LDFLAGS}) + list(APPEND LIGHTSTEP_LINK_LIBRARIES ${GRPCPP_LDFLAGS} ${GRPC_LDFLAGS}) include_directories(SYSTEM ${GRPC_INCLUDE_DIRS} ${GRPCPP_INCLUDE_DIRS}) endif() endif() @@ -180,7 +175,7 @@ set(THREADS_PREFER_PTHREAD_FLAG TRUE) find_package(Threads REQUIRED) list(APPEND LIGHTSTEP_LINK_LIBRARIES Threads::Threads) -if (WIN32) +if (WIN32) list(APPEND LIGHTSTEP_LINK_LIBRARIES "wsock32" "ws2_32") endif() @@ -320,7 +315,7 @@ else() list(APPEND LIGHTSTEP_SRCS ${EMBED_SSL_ROOTS_PEM_CPP_FILE}) endif() -if (BUILD_SHARED_LIBS) +if (BUILD_SHARED_LIBS) add_library(lightstep_tracer SHARED $ $ $ @@ -332,7 +327,7 @@ if (BUILD_SHARED_LIBS) LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}) endif() -if (BUILD_STATIC_LIBS) +if (BUILD_STATIC_LIBS) add_library(lightstep_tracer-static STATIC $ $ $ From 6181730c3616ecfefc7dcc2a3fc32a125bd00b77 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Mon, 3 Aug 2020 09:57:28 +0100 Subject: [PATCH 07/31] Revert CMakeLists.txt --- CMakeLists.txt | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 32415438..7e7f7995 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,7 +30,7 @@ set(LIGHTSTEP_VERSION_STRING # ============================================================================== # Set up cpack -set(CPACK_PACKAGE_DESCRIPTION_SUMMARY +set(CPACK_PACKAGE_DESCRIPTION_SUMMARY "A LightStep implementation of the C++ OpenTracing API") set(CPACK_PACKAGE_VENDOR "lightstep.com") set(CPACK_PACKAGE_DESCRIPTION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/README.md") @@ -56,13 +56,13 @@ option(WITH_CARES "Build with support for dns resolution using c-ares." OFF) option(WITH_DYNAMIC_LOAD "Build support for dynamic loading." ON) option(HEADERS_ONLY "Only generate version.h." OFF) -# Allow a user to specify an optional default roots.pem file to embed into the -# library. +# Allow a user to specify an optional default roots.pem file to embed into the +# library. # # This is useful if the library is distributed to an environment where gRPC # hasn't been installed. # -# To use, invoke cmake with +# To use, invoke cmake with # cmake -DDEFAULT_SSL_ROOTS_PEM:STRING=/path/to/roots.pem ... # # See also discussion on https://github.com/grpc/grpc/issues/4834 @@ -87,7 +87,7 @@ endif() configure_file(version.h.in include/lightstep/version.h) include_directories(${CMAKE_CURRENT_BINARY_DIR}/include) -install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/include/lightstep +install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/include/lightstep DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) if(HEADERS_ONLY) @@ -131,7 +131,7 @@ endif() include_directories(SYSTEM ${OPENTRACING_INCLUDE_DIR}) list(APPEND LIGHTSTEP_LINK_LIBRARIES ${OPENTRACING_LIBRARY}) -if (WITH_GRPC) +if (WITH_GRPC) find_package(gRPC CONFIG) # First attempt to set up gRPC via cmake; but if cmake config files aren't # available, fallback to pkg-config. @@ -149,7 +149,7 @@ if (WITH_GRPC) find_package(PkgConfig REQUIRED) pkg_search_module(GRPC REQUIRED grpc) pkg_search_module(GRPCPP REQUIRED grpc++) - list(APPEND LIGHTSTEP_LINK_LIBRARIES ${GRPCPP_LDFLAGS} ${GRPC_LDFLAGS}) + list(APPEND LIGHTSTEP_LINK_LIBRARIES ${GRPCPP_LDFLAGS} ${GRPC_LDFLAGS}) include_directories(SYSTEM ${GRPC_INCLUDE_DIRS} ${GRPCPP_INCLUDE_DIRS}) endif() endif() @@ -175,7 +175,7 @@ set(THREADS_PREFER_PTHREAD_FLAG TRUE) find_package(Threads REQUIRED) list(APPEND LIGHTSTEP_LINK_LIBRARIES Threads::Threads) -if (WIN32) +if (WIN32) list(APPEND LIGHTSTEP_LINK_LIBRARIES "wsock32" "ws2_32") endif() @@ -315,7 +315,7 @@ else() list(APPEND LIGHTSTEP_SRCS ${EMBED_SSL_ROOTS_PEM_CPP_FILE}) endif() -if (BUILD_SHARED_LIBS) +if (BUILD_SHARED_LIBS) add_library(lightstep_tracer SHARED $ $ $ @@ -327,7 +327,7 @@ if (BUILD_SHARED_LIBS) LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}) endif() -if (BUILD_STATIC_LIBS) +if (BUILD_STATIC_LIBS) add_library(lightstep_tracer-static STATIC $ $ $ From be25e16a18d9be6ff6cf0b2548d6e3897d28fc49 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Mon, 3 Aug 2020 11:09:24 +0100 Subject: [PATCH 08/31] Expect span ID to be 0 when none is given (need to confirm this is the correct behaviour) --- test/tracer/propagation/cloud_trace_propagation_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/tracer/propagation/cloud_trace_propagation_test.cpp b/test/tracer/propagation/cloud_trace_propagation_test.cpp index 9cbd3a1b..10521af0 100644 --- a/test/tracer/propagation/cloud_trace_propagation_test.cpp +++ b/test/tracer/propagation/cloud_trace_propagation_test.cpp @@ -85,7 +85,7 @@ TEST_CASE("cloud_trace propagation") { // TODO - this will fail - I'm not sure what the behaviour should be if a // span ID isn't given - REQUIRE(span_context->span_id() == 0xaef5705a09004083ul); + REQUIRE(span_context->span_id() == 0x0); REQUIRE(span_context->sampled() == true); } From b1d0b38179fdc5d915e16f9766f52ec28852b3c1 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Mon, 3 Aug 2020 11:59:03 +0100 Subject: [PATCH 09/31] Add cloud_trace propagator to CMakeLists.txt --- CMakeLists.txt | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7e7f7995..8ad3caf1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,7 +30,7 @@ set(LIGHTSTEP_VERSION_STRING # ============================================================================== # Set up cpack -set(CPACK_PACKAGE_DESCRIPTION_SUMMARY +set(CPACK_PACKAGE_DESCRIPTION_SUMMARY "A LightStep implementation of the C++ OpenTracing API") set(CPACK_PACKAGE_VENDOR "lightstep.com") set(CPACK_PACKAGE_DESCRIPTION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/README.md") @@ -56,13 +56,13 @@ option(WITH_CARES "Build with support for dns resolution using c-ares." OFF) option(WITH_DYNAMIC_LOAD "Build support for dynamic loading." ON) option(HEADERS_ONLY "Only generate version.h." OFF) -# Allow a user to specify an optional default roots.pem file to embed into the -# library. +# Allow a user to specify an optional default roots.pem file to embed into the +# library. # # This is useful if the library is distributed to an environment where gRPC # hasn't been installed. # -# To use, invoke cmake with +# To use, invoke cmake with # cmake -DDEFAULT_SSL_ROOTS_PEM:STRING=/path/to/roots.pem ... # # See also discussion on https://github.com/grpc/grpc/issues/4834 @@ -87,7 +87,7 @@ endif() configure_file(version.h.in include/lightstep/version.h) include_directories(${CMAKE_CURRENT_BINARY_DIR}/include) -install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/include/lightstep +install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/include/lightstep DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) if(HEADERS_ONLY) @@ -131,7 +131,7 @@ endif() include_directories(SYSTEM ${OPENTRACING_INCLUDE_DIR}) list(APPEND LIGHTSTEP_LINK_LIBRARIES ${OPENTRACING_LIBRARY}) -if (WITH_GRPC) +if (WITH_GRPC) find_package(gRPC CONFIG) # First attempt to set up gRPC via cmake; but if cmake config files aren't # available, fallback to pkg-config. @@ -149,7 +149,7 @@ if (WITH_GRPC) find_package(PkgConfig REQUIRED) pkg_search_module(GRPC REQUIRED grpc) pkg_search_module(GRPCPP REQUIRED grpc++) - list(APPEND LIGHTSTEP_LINK_LIBRARIES ${GRPCPP_LDFLAGS} ${GRPC_LDFLAGS}) + list(APPEND LIGHTSTEP_LINK_LIBRARIES ${GRPCPP_LDFLAGS} ${GRPC_LDFLAGS}) include_directories(SYSTEM ${GRPC_INCLUDE_DIRS} ${GRPCPP_INCLUDE_DIRS}) endif() endif() @@ -175,7 +175,7 @@ set(THREADS_PREFER_PTHREAD_FLAG TRUE) find_package(Threads REQUIRED) list(APPEND LIGHTSTEP_LINK_LIBRARIES Threads::Threads) -if (WIN32) +if (WIN32) list(APPEND LIGHTSTEP_LINK_LIBRARIES "wsock32" "ws2_32") endif() @@ -228,6 +228,7 @@ set(LIGHTSTEP_SRCS src/common/utility.cpp src/tracer/propagation/envoy_propagator.cpp src/tracer/propagation/trace_context.cpp src/tracer/propagation/trace_context_propagator.cpp + src/tracer/propagation/cloud_trace_propagator.cpp src/tracer/propagation/lightstep_propagator.cpp src/tracer/propagation/multiheader_propagator.cpp src/tracer/propagation/propagation.cpp @@ -315,7 +316,7 @@ else() list(APPEND LIGHTSTEP_SRCS ${EMBED_SSL_ROOTS_PEM_CPP_FILE}) endif() -if (BUILD_SHARED_LIBS) +if (BUILD_SHARED_LIBS) add_library(lightstep_tracer SHARED $ $ $ @@ -327,7 +328,7 @@ if (BUILD_SHARED_LIBS) LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}) endif() -if (BUILD_STATIC_LIBS) +if (BUILD_STATIC_LIBS) add_library(lightstep_tracer-static STATIC $ $ $ From 4ae8982e3eb320fac6734a3fcbe58de94fd029d0 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Mon, 3 Aug 2020 13:53:41 +0100 Subject: [PATCH 10/31] span ID is an unsigned long, not a hex value --- .../propagation/cloud_trace_propagator.cpp | 35 +++++++++++++------ .../cloud_trace_propagation_test.cpp | 10 +++--- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index 1545a3e1..1073d2fb 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -124,7 +124,7 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( } offset += Num128BitHexDigits; - if (s.size() < Num128BitHexDigits + 1 + Num64BitHexDigits) { + if (s.size() < Num128BitHexDigits + 2) { // only a short form trace ID has been given (not a "trace id/span id") return {}; } @@ -135,16 +135,22 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( } ++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()); + char parent_id [21]; + int span_id_length; + for(span_id_length = 0; span_id_length<20; span_id_length++) { + if(s[offset]==';' || s[offset]=='\0') { + parent_id[span_id_length] = '\0'; + break; + } + + parent_id[span_id_length] = s[offset]; + ++offset; } - trace_context.parent_id = *parent_id_maybe; - offset += Num64BitHexDigits; - if (s.size() < Num128BitHexDigits + 1 + Num64BitHexDigits + 4) { + // parent-id + trace_context.parent_id = std::strtoull(parent_id, NULL, 10); + + if (s.size() < Num128BitHexDigits + 1 + span_id_length + 4) { // only a "trace ID/span ID" has been given (not a "trace id/span id;o=[0-1]") return {}; } @@ -180,9 +186,12 @@ void CloudTracePropagator::SerializeCloudTrace(const TraceContext& trace_context *(s + offset) = '/'; ++offset; + *(s + offset) = '\0'; // parent-id - Uint64ToHex(trace_context.parent_id, s + offset); - offset += Num64BitHexDigits; + auto parent_id = std::to_string(trace_context.parent_id); + std::strcat(s, parent_id.c_str()); + + offset += parent_id.length(); *(s + offset) = ';'; ++offset; *(s + offset) = 'o'; @@ -196,5 +205,9 @@ void CloudTracePropagator::SerializeCloudTrace(const TraceContext& trace_context } else { s[offset] = '0'; } + + // terminate the string + ++offset; + s[offset] = '\0'; } } // namespace lightstep diff --git a/test/tracer/propagation/cloud_trace_propagation_test.cpp b/test/tracer/propagation/cloud_trace_propagation_test.cpp index 10521af0..1b80703f 100644 --- a/test/tracer/propagation/cloud_trace_propagation_test.cpp +++ b/test/tracer/propagation/cloud_trace_propagation_test.cpp @@ -39,7 +39,7 @@ TEST_CASE("cloud_trace propagation") { } SECTION("Verify extraction against a long-form header with sampled set to true") { - text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/aef5705a09004083;o=1"}}; + text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/12607106263893950595;o=1"}}; auto span_context_maybe = tracer->Extract(http_headers_carrier); REQUIRE(span_context_maybe); auto span_context = @@ -51,7 +51,7 @@ TEST_CASE("cloud_trace propagation") { } SECTION("Verify extraction against a long-form header when sampled is false") { - text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/aef5705a09004083;o=0"}}; + text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/12607106263893950595;o=0"}}; auto span_context_maybe = tracer->Extract(http_headers_carrier); REQUIRE(span_context_maybe); auto span_context = @@ -63,7 +63,7 @@ TEST_CASE("cloud_trace propagation") { } SECTION("Verify extraction against a medium-form header (trace id/span id)") { - text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/aef5705a09004083"}}; + text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/12607106263893950595"}}; auto span_context_maybe = tracer->Extract(http_headers_carrier); REQUIRE(span_context_maybe); auto span_context = @@ -91,7 +91,7 @@ TEST_CASE("cloud_trace propagation") { } SECTION("A child keeps the same trace id as its parent") { - text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/aef5705a09004083;o=0"}}; + text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/12607106263893950595;o=0"}}; auto span_context_maybe = tracer->Extract(http_headers_carrier); REQUIRE(span_context_maybe); auto span = tracer->StartSpan( @@ -105,7 +105,7 @@ TEST_CASE("cloud_trace propagation") { } SECTION("The low part of 128-bit trace ids are sent to satellites") { - text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/aef5705a09004083;o=1"}}; + text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/12607106263893950595;o=1"}}; auto span_context_maybe = tracer->Extract(http_headers_carrier); REQUIRE(span_context_maybe); tracer->StartSpan("abc", {opentracing::ChildOf(span_context_maybe->get())}); From 46306357d345071b6792e62fd6efb82d090cbf17 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Mon, 3 Aug 2020 14:58:58 +0100 Subject: [PATCH 11/31] Fix the max length of the x-cloud-trace-context header --- src/tracer/propagation/cloud_trace_propagator.cpp | 2 +- src/tracer/propagation/cloud_trace_propagator.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index 1073d2fb..96ccf768 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -60,7 +60,7 @@ opentracing::expected CloudTracePropagator::ExtractSpanContext( opentracing::expected CloudTracePropagator::InjectSpanContextImpl( const opentracing::TextMapWriter& carrier, const TraceContext& trace_context) const { - std::array buffer; + std::array buffer; this->SerializeCloudTrace(trace_context, buffer.data()); return carrier.Set(PropagationSingleKey, opentracing::string_view{buffer.data(), buffer.size()}); diff --git a/src/tracer/propagation/cloud_trace_propagator.h b/src/tracer/propagation/cloud_trace_propagator.h index 030797e5..99bdf306 100644 --- a/src/tracer/propagation/cloud_trace_propagator.h +++ b/src/tracer/propagation/cloud_trace_propagator.h @@ -3,6 +3,9 @@ #include "tracer/propagation/propagator.h" namespace lightstep { + +const size_t CloudContextLength = 58; // max x-cloud-trace-context header + class CloudTracePropagator final : public Propagator { public: // Propagator From 022b2e2c172fdbb12d9a3c781ba2aeb634a08094 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Mon, 3 Aug 2020 15:19:03 +0100 Subject: [PATCH 12/31] incorrect char array assignment --- src/tracer/propagation/cloud_trace_propagator.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index 96ccf768..65f1c564 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -201,13 +201,13 @@ void CloudTracePropagator::SerializeCloudTrace(const TraceContext& trace_context // trace-flags if(IsTraceFlagSet(trace_context.trace_flags)) { - s[offset] = '1'; + *(s + offset) = '1'; } else { - s[offset] = '0'; + *(s + offset) = '0'; } // terminate the string ++offset; - s[offset] = '\0'; + *(s + offset) = '\0'; } } // namespace lightstep From 8176a65ee23c18220125b40d76bad4234878fc6f Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Mon, 3 Aug 2020 15:41:48 +0100 Subject: [PATCH 13/31] set the header length properly --- src/tracer/propagation/cloud_trace_propagator.cpp | 10 ++++------ src/tracer/propagation/cloud_trace_propagator.h | 4 ++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index 65f1c564..d0fe6768 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -61,9 +61,9 @@ opentracing::expected CloudTracePropagator::InjectSpanContextImpl( const opentracing::TextMapWriter& carrier, const TraceContext& trace_context) const { std::array buffer; - this->SerializeCloudTrace(trace_context, buffer.data()); + auto data_length = this->SerializeCloudTrace(trace_context, buffer.data()); return carrier.Set(PropagationSingleKey, - opentracing::string_view{buffer.data(), buffer.size()}); + opentracing::string_view{buffer.data(), data_length}); } //-------------------------------------------------------------------------------------------------- @@ -174,7 +174,7 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( return {}; } -void CloudTracePropagator::SerializeCloudTrace(const TraceContext& trace_context, +size_t CloudTracePropagator::SerializeCloudTrace(const TraceContext& trace_context, char* s) const noexcept { size_t offset = 0; @@ -206,8 +206,6 @@ void CloudTracePropagator::SerializeCloudTrace(const TraceContext& trace_context *(s + offset) = '0'; } - // terminate the string - ++offset; - *(s + offset) = '\0'; + return ++offset; } } // namespace lightstep diff --git a/src/tracer/propagation/cloud_trace_propagator.h b/src/tracer/propagation/cloud_trace_propagator.h index 99bdf306..5fdc610c 100644 --- a/src/tracer/propagation/cloud_trace_propagator.h +++ b/src/tracer/propagation/cloud_trace_propagator.h @@ -4,7 +4,7 @@ namespace lightstep { -const size_t CloudContextLength = 58; // max x-cloud-trace-context header +const size_t CloudContextLength = 57; // max x-cloud-trace-context header class CloudTracePropagator final : public Propagator { public: @@ -37,7 +37,7 @@ class CloudTracePropagator final : public Propagator { opentracing::expected ParseCloudTrace( opentracing::string_view s, lightstep::TraceContext& trace_context) const noexcept; - void SerializeCloudTrace(const TraceContext& trace_context, + size_t SerializeCloudTrace(const TraceContext& trace_context, char* s) const noexcept; }; } // namespace lightstep From 1fbdd022fbd83ff984fa195ee73448a2e17ad2ba Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Tue, 4 Aug 2020 10:14:18 +0100 Subject: [PATCH 14/31] Revert CMakeLists.txt --- CMakeLists.txt | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8ad3caf1..7e7f7995 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,7 +30,7 @@ set(LIGHTSTEP_VERSION_STRING # ============================================================================== # Set up cpack -set(CPACK_PACKAGE_DESCRIPTION_SUMMARY +set(CPACK_PACKAGE_DESCRIPTION_SUMMARY "A LightStep implementation of the C++ OpenTracing API") set(CPACK_PACKAGE_VENDOR "lightstep.com") set(CPACK_PACKAGE_DESCRIPTION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/README.md") @@ -56,13 +56,13 @@ option(WITH_CARES "Build with support for dns resolution using c-ares." OFF) option(WITH_DYNAMIC_LOAD "Build support for dynamic loading." ON) option(HEADERS_ONLY "Only generate version.h." OFF) -# Allow a user to specify an optional default roots.pem file to embed into the -# library. +# Allow a user to specify an optional default roots.pem file to embed into the +# library. # # This is useful if the library is distributed to an environment where gRPC # hasn't been installed. # -# To use, invoke cmake with +# To use, invoke cmake with # cmake -DDEFAULT_SSL_ROOTS_PEM:STRING=/path/to/roots.pem ... # # See also discussion on https://github.com/grpc/grpc/issues/4834 @@ -87,7 +87,7 @@ endif() configure_file(version.h.in include/lightstep/version.h) include_directories(${CMAKE_CURRENT_BINARY_DIR}/include) -install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/include/lightstep +install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/include/lightstep DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) if(HEADERS_ONLY) @@ -131,7 +131,7 @@ endif() include_directories(SYSTEM ${OPENTRACING_INCLUDE_DIR}) list(APPEND LIGHTSTEP_LINK_LIBRARIES ${OPENTRACING_LIBRARY}) -if (WITH_GRPC) +if (WITH_GRPC) find_package(gRPC CONFIG) # First attempt to set up gRPC via cmake; but if cmake config files aren't # available, fallback to pkg-config. @@ -149,7 +149,7 @@ if (WITH_GRPC) find_package(PkgConfig REQUIRED) pkg_search_module(GRPC REQUIRED grpc) pkg_search_module(GRPCPP REQUIRED grpc++) - list(APPEND LIGHTSTEP_LINK_LIBRARIES ${GRPCPP_LDFLAGS} ${GRPC_LDFLAGS}) + list(APPEND LIGHTSTEP_LINK_LIBRARIES ${GRPCPP_LDFLAGS} ${GRPC_LDFLAGS}) include_directories(SYSTEM ${GRPC_INCLUDE_DIRS} ${GRPCPP_INCLUDE_DIRS}) endif() endif() @@ -175,7 +175,7 @@ set(THREADS_PREFER_PTHREAD_FLAG TRUE) find_package(Threads REQUIRED) list(APPEND LIGHTSTEP_LINK_LIBRARIES Threads::Threads) -if (WIN32) +if (WIN32) list(APPEND LIGHTSTEP_LINK_LIBRARIES "wsock32" "ws2_32") endif() @@ -228,7 +228,6 @@ set(LIGHTSTEP_SRCS src/common/utility.cpp src/tracer/propagation/envoy_propagator.cpp src/tracer/propagation/trace_context.cpp src/tracer/propagation/trace_context_propagator.cpp - src/tracer/propagation/cloud_trace_propagator.cpp src/tracer/propagation/lightstep_propagator.cpp src/tracer/propagation/multiheader_propagator.cpp src/tracer/propagation/propagation.cpp @@ -316,7 +315,7 @@ else() list(APPEND LIGHTSTEP_SRCS ${EMBED_SSL_ROOTS_PEM_CPP_FILE}) endif() -if (BUILD_SHARED_LIBS) +if (BUILD_SHARED_LIBS) add_library(lightstep_tracer SHARED $ $ $ @@ -328,7 +327,7 @@ if (BUILD_SHARED_LIBS) LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}) endif() -if (BUILD_STATIC_LIBS) +if (BUILD_STATIC_LIBS) add_library(lightstep_tracer-static STATIC $ $ $ From 13e0e724381fbdcd1b87caea5a06cf2cecfc08cb Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Tue, 4 Aug 2020 10:19:30 +0100 Subject: [PATCH 15/31] fix style error --- test/tracer/propagation/cloud_trace_propagation_test.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/tracer/propagation/cloud_trace_propagation_test.cpp b/test/tracer/propagation/cloud_trace_propagation_test.cpp index 1b80703f..5b652c8a 100644 --- a/test/tracer/propagation/cloud_trace_propagation_test.cpp +++ b/test/tracer/propagation/cloud_trace_propagation_test.cpp @@ -82,11 +82,7 @@ TEST_CASE("cloud_trace propagation") { dynamic_cast(span_context_maybe->get()); REQUIRE(span_context->trace_id_high() == 0xaef5705a09004083ul); REQUIRE(span_context->trace_id_low() == 0x8f1359ebafa5c0c6ul); - - // TODO - this will fail - I'm not sure what the behaviour should be if a - // span ID isn't given REQUIRE(span_context->span_id() == 0x0); - REQUIRE(span_context->sampled() == true); } From cf4eb9752ea00a4e4f991cb226e8e533365221de Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Tue, 4 Aug 2020 16:25:42 +0100 Subject: [PATCH 16/31] fix linting issues --- .../propagation/cloud_trace_propagator.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index d0fe6768..9c761ded 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -4,6 +4,7 @@ #include "tracer/propagation/binary_propagation.h" #include "tracer/propagation/utility.h" +#include const opentracing::string_view PropagationSingleKey = "x-cloud-trace-context"; const opentracing::string_view PrefixBaggage = "ot-baggage-"; @@ -135,11 +136,10 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( } ++offset; - char parent_id [21]; + char parent_id [20]; int span_id_length; for(span_id_length = 0; span_id_length<20; span_id_length++) { if(s[offset]==';' || s[offset]=='\0') { - parent_id[span_id_length] = '\0'; break; } @@ -148,9 +148,10 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( } // parent-id - trace_context.parent_id = std::strtoull(parent_id, NULL, 10); + char * pEnd = parent_id; + trace_context.parent_id = std::strtoull(pEnd, &pEnd, 10); - if (s.size() < Num128BitHexDigits + 1 + span_id_length + 4) { + if (s.size() - offset < 4) { // only a "trace ID/span ID" has been given (not a "trace id/span id;o=[0-1]") return {}; } @@ -186,12 +187,14 @@ size_t CloudTracePropagator::SerializeCloudTrace(const TraceContext& trace_conte *(s + offset) = '/'; ++offset; - *(s + offset) = '\0'; // parent-id auto parent_id = std::to_string(trace_context.parent_id); - std::strcat(s, parent_id.c_str()); + auto parent_id_char = parent_id.c_str(); + for(uint x=0; x < parent_id.length(); x++) { + *(s + offset) = parent_id_char[x]; + ++offset; + } - offset += parent_id.length(); *(s + offset) = ';'; ++offset; *(s + offset) = 'o'; From 5f7a33f66b07efdd0c1582d22bedd82a97984e76 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Tue, 4 Aug 2020 16:41:43 +0100 Subject: [PATCH 17/31] unused import --- src/tracer/propagation/cloud_trace_propagator.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index 9c761ded..c09c7971 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -4,7 +4,6 @@ #include "tracer/propagation/binary_propagation.h" #include "tracer/propagation/utility.h" -#include const opentracing::string_view PropagationSingleKey = "x-cloud-trace-context"; const opentracing::string_view PrefixBaggage = "ot-baggage-"; From 395af64d31eb4a40f631eadc9d59c73af4eaed3f Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Tue, 4 Aug 2020 17:07:24 +0100 Subject: [PATCH 18/31] Add cloud_trace_propagator.cpp to build list --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7e7f7995..b0e78e85 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -230,6 +230,7 @@ set(LIGHTSTEP_SRCS src/common/utility.cpp src/tracer/propagation/trace_context_propagator.cpp src/tracer/propagation/lightstep_propagator.cpp src/tracer/propagation/multiheader_propagator.cpp + src/tracer/propagation/cloud_trace_propagator.cpp src/tracer/propagation/propagation.cpp src/tracer/propagation/propagation_options.cpp src/tracer/immutable_span_context.cpp From 7a230b2ddde92a2a42d5c982575803e03147ae86 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Tue, 4 Aug 2020 17:31:52 +0100 Subject: [PATCH 19/31] Fix portability error --- src/tracer/propagation/cloud_trace_propagator.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index c09c7971..c86dfd92 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -189,7 +189,7 @@ size_t CloudTracePropagator::SerializeCloudTrace(const TraceContext& trace_conte // parent-id auto parent_id = std::to_string(trace_context.parent_id); auto parent_id_char = parent_id.c_str(); - for(uint x=0; x < parent_id.length(); x++) { + for(uint16_t x=0; x < parent_id.length(); x++) { *(s + offset) = parent_id_char[x]; ++offset; } From 2c9c678917492a6a569905d783f3fddd49cc98a3 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Thu, 6 Aug 2020 14:03:05 +0100 Subject: [PATCH 20/31] use string_view and nullptr --- src/tracer/propagation/cloud_trace_propagator.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index c86dfd92..1869d665 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -148,21 +148,18 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( // parent-id char * pEnd = parent_id; - trace_context.parent_id = std::strtoull(pEnd, &pEnd, 10); + trace_context.parent_id = std::strtoull(pEnd, nullptr, 10); if (s.size() - offset < 4) { // only a "trace ID/span ID" has been given (not a "trace id/span id;o=[0-1]") return {}; } - char subbuff[4]; - memcpy( subbuff, &s[offset], 3 ); - subbuff[3] = '\0'; - - if (strcmp(subbuff, ";o=") != 0) { + if(opentracing::string_view(s.begin() + offset, 3) != opentracing::string_view(";o=")) { return opentracing::make_unexpected( std::make_error_code(std::errc::invalid_argument)); } + offset += 3; // trace-flags From 0c0f66a84e3f2ecdcb1303ca46831d2b420b8be0 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Thu, 6 Aug 2020 16:00:33 +0100 Subject: [PATCH 21/31] Fix span_id short parsing --- src/tracer/propagation/cloud_trace_propagator.cpp | 5 +++-- .../propagation/cloud_trace_propagation_test.cpp | 12 ++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index 1869d665..ee1d2d76 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -135,9 +135,9 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( } ++offset; - char parent_id [20]; + char parent_id [21]; int span_id_length; - for(span_id_length = 0; span_id_length<20; span_id_length++) { + for(span_id_length = 0; span_id_length<20 && offset CloudTracePropagator::ParseCloudTrace( parent_id[span_id_length] = s[offset]; ++offset; } + parent_id[span_id_length] = '\0'; // parent-id char * pEnd = parent_id; diff --git a/test/tracer/propagation/cloud_trace_propagation_test.cpp b/test/tracer/propagation/cloud_trace_propagation_test.cpp index 5b652c8a..ed101e06 100644 --- a/test/tracer/propagation/cloud_trace_propagation_test.cpp +++ b/test/tracer/propagation/cloud_trace_propagation_test.cpp @@ -74,6 +74,18 @@ TEST_CASE("cloud_trace propagation") { REQUIRE(span_context->sampled() == true); } + SECTION("Verify extraction against a medium-form header with a short span id(trace id/span id)") { + text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/123"}}; + auto span_context_maybe = tracer->Extract(http_headers_carrier); + REQUIRE(span_context_maybe); + auto span_context = + dynamic_cast(span_context_maybe->get()); + REQUIRE(span_context->trace_id_high() == 0xaef5705a09004083ul); + REQUIRE(span_context->trace_id_low() == 0x8f1359ebafa5c0c6ul); + REQUIRE(span_context->span_id() == 0x7b); + REQUIRE(span_context->sampled() == true); + } + SECTION("Verify extraction against a short-form header (trace id)") { text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6"}}; auto span_context_maybe = tracer->Extract(http_headers_carrier); From 19d62e04ae53d67b2b9465e4021030612e39954d Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Thu, 6 Aug 2020 16:31:32 +0100 Subject: [PATCH 22/31] Simplify serialize method with snprintf --- .../propagation/cloud_trace_propagator.cpp | 34 +++---------------- 1 file changed, 5 insertions(+), 29 deletions(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index ee1d2d76..cc0ef2a6 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -174,37 +174,13 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( size_t CloudTracePropagator::SerializeCloudTrace(const TraceContext& trace_context, char* s) const noexcept { - size_t offset = 0; - // 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 - auto parent_id = std::to_string(trace_context.parent_id); - auto parent_id_char = parent_id.c_str(); - for(uint16_t x=0; x < parent_id.length(); x++) { - *(s + offset) = parent_id_char[x]; - ++offset; - } + char trace_id[Num64BitHexDigits + Num64BitHexDigits + 1]; + Uint64ToHex(trace_context.trace_id_high, trace_id); + Uint64ToHex(trace_context.trace_id_low, trace_id + Num64BitHexDigits); + *(trace_id + Num64BitHexDigits + Num64BitHexDigits) = '\0'; - *(s + offset) = ';'; - ++offset; - *(s + offset) = 'o'; - ++offset; - *(s + offset) = '='; - ++offset; - - // trace-flags - if(IsTraceFlagSet(trace_context.trace_flags)) { - *(s + offset) = '1'; - } else { - *(s + offset) = '0'; - } + size_t offset = snprintf (s, CloudContextLength, "%s/%lu;o=%d", trace_id, trace_context.parent_id, IsTraceFlagSet(trace_context.trace_flags) ? 1 : 0); return ++offset; } From 54323d6ae754b8632b84789bbe746748a413d788 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Thu, 6 Aug 2020 17:20:29 +0100 Subject: [PATCH 23/31] cover error handling with additional test cases --- .../cloud_trace_propagation_test.cpp | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/tracer/propagation/cloud_trace_propagation_test.cpp b/test/tracer/propagation/cloud_trace_propagation_test.cpp index ed101e06..42d7187e 100644 --- a/test/tracer/propagation/cloud_trace_propagation_test.cpp +++ b/test/tracer/propagation/cloud_trace_propagation_test.cpp @@ -86,6 +86,14 @@ TEST_CASE("cloud_trace propagation") { REQUIRE(span_context->sampled() == true); } + SECTION("Verify error handling against a medium-form header with a bad separator (trace id/span id)") { + text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6_1"}}; + auto span_context_maybe = tracer->Extract(http_headers_carrier); + CHECK(!span_context_maybe); + CHECK(span_context_maybe.error() == + std::errc::invalid_argument); + } + SECTION("Verify extraction against a short-form header (trace id)") { text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6"}}; auto span_context_maybe = tracer->Extract(http_headers_carrier); @@ -98,6 +106,22 @@ TEST_CASE("cloud_trace propagation") { REQUIRE(span_context->sampled() == true); } + SECTION("Verify error handling on too short a trace id") { + text_map = {{"x-cloud-trace-context", "AABB0011"}}; + auto span_context_maybe = tracer->Extract(http_headers_carrier); + CHECK(!span_context_maybe); + CHECK(span_context_maybe.error() == + std::errc::invalid_argument); + } + + SECTION("Verify error handling on invalid hex in trace id") { + text_map = {{"x-cloud-trace-context", "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"}}; + auto span_context_maybe = tracer->Extract(http_headers_carrier); + CHECK(!span_context_maybe); + CHECK(span_context_maybe.error() == + std::errc::invalid_argument); + } + SECTION("A child keeps the same trace id as its parent") { text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/12607106263893950595;o=0"}}; auto span_context_maybe = tracer->Extract(http_headers_carrier); From c9bd7abbed427e5d1f68833eb44926631fabe4bc Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Thu, 6 Aug 2020 18:18:55 +0100 Subject: [PATCH 24/31] Test potential maximum ID values for propagators --- .../propagation/cloud_trace_propagator.h | 2 +- test/tracer/propagation/utility.cpp | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.h b/src/tracer/propagation/cloud_trace_propagator.h index 5fdc610c..cde1410a 100644 --- a/src/tracer/propagation/cloud_trace_propagator.h +++ b/src/tracer/propagation/cloud_trace_propagator.h @@ -4,7 +4,7 @@ namespace lightstep { -const size_t CloudContextLength = 57; // max x-cloud-trace-context header +const size_t CloudContextLength = 58; // max x-cloud-trace-context header class CloudTracePropagator final : public Propagator { public: diff --git a/test/tracer/propagation/utility.cpp b/test/tracer/propagation/utility.cpp index 86c3d6a1..202f426b 100644 --- a/test/tracer/propagation/utility.cpp +++ b/test/tracer/propagation/utility.cpp @@ -17,6 +17,18 @@ static std::tuple GenerateTraceID(bool use_128bit_ids) { return {0, random_number_generator()}; } +//------------------------------------------------------------------------------ +// GenerateMaxTraceID +//------------------------------------------------------------------------------ +// Generates the maximum value a trace ID can possibly be +static std::tuple GenerateMaxTraceID(bool use_128bit_ids) { + if (use_128bit_ids) { + return {std::numeric_limits::max(), std::numeric_limits::max()}; + } + return {0, std::numeric_limits::max()}; +} + + //------------------------------------------------------------------------------ // MakeTestSpanContexts //------------------------------------------------------------------------------ @@ -54,6 +66,13 @@ std::vector> MakeTestSpanContexts( trace_id_high, trace_id_low, 456, false, std::unordered_map{}}}); + // max values span context + std::tie(trace_id_high, trace_id_low) = GenerateMaxTraceID(use_128bit_trace_ids); + result.push_back( + std::unique_ptr{new ImmutableSpanContext{ + trace_id_high, trace_id_low, std::numeric_limits::max(), true, + std::unordered_map{}}}); + return result; } } // namespace lightstep From 6c18e332e8bb81039837d50d2a24ca9e755162c3 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Thu, 6 Aug 2020 18:57:48 +0100 Subject: [PATCH 25/31] Cover the bad trace flag case --- test/tracer/propagation/cloud_trace_propagation_test.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/tracer/propagation/cloud_trace_propagation_test.cpp b/test/tracer/propagation/cloud_trace_propagation_test.cpp index 42d7187e..05ca1be0 100644 --- a/test/tracer/propagation/cloud_trace_propagation_test.cpp +++ b/test/tracer/propagation/cloud_trace_propagation_test.cpp @@ -62,6 +62,14 @@ TEST_CASE("cloud_trace propagation") { REQUIRE(span_context->sampled() == false); } + SECTION("Verify error handling against a long-form header when sampled flag is invalid") { + text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/12607106263893950595;__0"}}; + auto span_context_maybe = tracer->Extract(http_headers_carrier); + CHECK(!span_context_maybe); + CHECK(span_context_maybe.error() == + std::errc::invalid_argument); + } + SECTION("Verify extraction against a medium-form header (trace id/span id)") { text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/12607106263893950595"}}; auto span_context_maybe = tracer->Extract(http_headers_carrier); From 4e3411d72ea1e9310498d060c6df72f2b13ee2a5 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Fri, 7 Aug 2020 11:59:35 +0100 Subject: [PATCH 26/31] Was adding a space at the end of the header --- src/tracer/propagation/cloud_trace_propagator.cpp | 2 +- src/tracer/propagation/cloud_trace_propagator.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index cc0ef2a6..57431c7f 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -182,6 +182,6 @@ size_t CloudTracePropagator::SerializeCloudTrace(const TraceContext& trace_conte size_t offset = snprintf (s, CloudContextLength, "%s/%lu;o=%d", trace_id, trace_context.parent_id, IsTraceFlagSet(trace_context.trace_flags) ? 1 : 0); - return ++offset; + return offset; } } // namespace lightstep diff --git a/src/tracer/propagation/cloud_trace_propagator.h b/src/tracer/propagation/cloud_trace_propagator.h index cde1410a..5fdc610c 100644 --- a/src/tracer/propagation/cloud_trace_propagator.h +++ b/src/tracer/propagation/cloud_trace_propagator.h @@ -4,7 +4,7 @@ namespace lightstep { -const size_t CloudContextLength = 58; // max x-cloud-trace-context header +const size_t CloudContextLength = 57; // max x-cloud-trace-context header class CloudTracePropagator final : public Propagator { public: From b214ed8a83bced9a7448a118aaccaf1421e3e402 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Fri, 7 Aug 2020 13:46:13 +0100 Subject: [PATCH 27/31] Extend header length (it looks like it does need it for the possible null terminator) --- src/tracer/propagation/cloud_trace_propagator.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.h b/src/tracer/propagation/cloud_trace_propagator.h index 5fdc610c..cde1410a 100644 --- a/src/tracer/propagation/cloud_trace_propagator.h +++ b/src/tracer/propagation/cloud_trace_propagator.h @@ -4,7 +4,7 @@ namespace lightstep { -const size_t CloudContextLength = 57; // max x-cloud-trace-context header +const size_t CloudContextLength = 58; // max x-cloud-trace-context header class CloudTracePropagator final : public Propagator { public: From 146f9b9c24ee375666f076db66782c85f0b3da49 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Mon, 10 Aug 2020 10:43:32 +0100 Subject: [PATCH 28/31] Review comments --- .../propagation/cloud_trace_propagator.cpp | 18 ++++++++++++------ .../cloud_trace_propagation_test.cpp | 8 ++++++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index 57431c7f..190d151e 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -138,7 +138,7 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( char parent_id [21]; int span_id_length; for(span_id_length = 0; span_id_length<20 && offset CloudTracePropagator::ParseCloudTrace( // parent-id char * pEnd = parent_id; + errno = 0; trace_context.parent_id = std::strtoull(pEnd, nullptr, 10); + if (errno == ERANGE) { + return opentracing::make_unexpected( + std::make_error_code(std::errc::result_out_of_range)); + } if (s.size() - offset < 4) { // only a "trace ID/span ID" has been given (not a "trace id/span id;o=[0-1]") @@ -174,13 +179,14 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( size_t CloudTracePropagator::SerializeCloudTrace(const TraceContext& trace_context, char* s) const noexcept { + size_t offset = 0; // trace-id - char trace_id[Num64BitHexDigits + Num64BitHexDigits + 1]; - Uint64ToHex(trace_context.trace_id_high, trace_id); - Uint64ToHex(trace_context.trace_id_low, trace_id + Num64BitHexDigits); - *(trace_id + Num64BitHexDigits + Num64BitHexDigits) = '\0'; + Uint64ToHex(trace_context.trace_id_high, s); + offset += Num64BitHexDigits; + Uint64ToHex(trace_context.trace_id_low, s + offset); + offset += Num64BitHexDigits; - size_t offset = snprintf (s, CloudContextLength, "%s/%lu;o=%d", trace_id, trace_context.parent_id, IsTraceFlagSet(trace_context.trace_flags) ? 1 : 0); + offset += snprintf(s + offset, CloudContextLength, "/%lu;o=%d", trace_context.parent_id, IsTraceFlagSet(trace_context.trace_flags) ? 1 : 0); return offset; } diff --git a/test/tracer/propagation/cloud_trace_propagation_test.cpp b/test/tracer/propagation/cloud_trace_propagation_test.cpp index 05ca1be0..fbbce19e 100644 --- a/test/tracer/propagation/cloud_trace_propagation_test.cpp +++ b/test/tracer/propagation/cloud_trace_propagation_test.cpp @@ -102,6 +102,14 @@ TEST_CASE("cloud_trace propagation") { std::errc::invalid_argument); } + SECTION("Verify error handling against a medium-form header with a span ID that is out of range of a 64bit unsigned integer") { + text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6/99999999999999999999"}}; + auto span_context_maybe = tracer->Extract(http_headers_carrier); + CHECK(!span_context_maybe); + CHECK(span_context_maybe.error() == + std::errc::result_out_of_range); + } + SECTION("Verify extraction against a short-form header (trace id)") { text_map = {{"x-cloud-trace-context", "aef5705a090040838f1359ebafa5c0c6"}}; auto span_context_maybe = tracer->Extract(http_headers_carrier); From baa3603d99cbdbb507b1ae552ca59d3f3064b547 Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Mon, 10 Aug 2020 15:13:48 +0100 Subject: [PATCH 29/31] Fix clang-tidy error --- src/tracer/propagation/cloud_trace_propagator.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index 190d151e..b1d41724 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -138,7 +138,7 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( char parent_id [21]; int span_id_length; for(span_id_length = 0; span_id_length<20 && offset Date: Wed, 12 Aug 2020 15:29:21 +0100 Subject: [PATCH 30/31] Review comments --- .../propagation/cloud_trace_propagator.cpp | 19 ++++++++----------- .../propagation/cloud_trace_propagator.h | 2 ++ 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index b1d41724..5f48e0f5 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -124,7 +124,7 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( } offset += Num128BitHexDigits; - if (s.size() < Num128BitHexDigits + 2) { + if (s.size() - offset < 2) { // only a short form trace ID has been given (not a "trace id/span id") return {}; } @@ -135,22 +135,19 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( } ++offset; - char parent_id [21]; - int span_id_length; - for(span_id_length = 0; span_id_length<20 && offset parent_id; + for (size_t i=0; i < Num64BitDecimalDigits; ++i) { + if (offset == s.length() || std::isdigit(s[offset]) == 0) { break; } - - parent_id[span_id_length] = s[offset]; + parent_id[i] = s[offset]; ++offset; } - parent_id[span_id_length] = '\0'; + parent_id.back() = '\0'; // parent-id - char * pEnd = parent_id; errno = 0; - trace_context.parent_id = std::strtoull(pEnd, nullptr, 10); + trace_context.parent_id = std::strtoull(parent_id.data(), nullptr, 10); if (errno == ERANGE) { return opentracing::make_unexpected( std::make_error_code(std::errc::result_out_of_range)); @@ -186,7 +183,7 @@ size_t CloudTracePropagator::SerializeCloudTrace(const TraceContext& trace_conte Uint64ToHex(trace_context.trace_id_low, s + offset); offset += Num64BitHexDigits; - offset += snprintf(s + offset, CloudContextLength, "/%lu;o=%d", trace_context.parent_id, IsTraceFlagSet(trace_context.trace_flags) ? 1 : 0); + offset += snprintf(s + offset, CloudContextLength - offset, "/%lu;o=%d", trace_context.parent_id, IsTraceFlagSet(trace_context.trace_flags) ? 1 : 0); return offset; } diff --git a/src/tracer/propagation/cloud_trace_propagator.h b/src/tracer/propagation/cloud_trace_propagator.h index cde1410a..6d520e4f 100644 --- a/src/tracer/propagation/cloud_trace_propagator.h +++ b/src/tracer/propagation/cloud_trace_propagator.h @@ -6,6 +6,8 @@ namespace lightstep { const size_t CloudContextLength = 58; // max x-cloud-trace-context header +const size_t Num64BitDecimalDigits = 20; + class CloudTracePropagator final : public Propagator { public: // Propagator From b7bd88d79e1042f1bbc3fda54ccefef3902b077b Mon Sep 17 00:00:00 2001 From: Rob Graham Date: Thu, 13 Aug 2020 10:19:11 +0100 Subject: [PATCH 31/31] Fix failing test --- src/tracer/propagation/cloud_trace_propagator.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tracer/propagation/cloud_trace_propagator.cpp b/src/tracer/propagation/cloud_trace_propagator.cpp index 5f48e0f5..5219bceb 100644 --- a/src/tracer/propagation/cloud_trace_propagator.cpp +++ b/src/tracer/propagation/cloud_trace_propagator.cpp @@ -136,14 +136,15 @@ opentracing::expected CloudTracePropagator::ParseCloudTrace( ++offset; std::array parent_id; - for (size_t i=0; i < Num64BitDecimalDigits; ++i) { + size_t i; + for (i=0; i < Num64BitDecimalDigits; ++i) { if (offset == s.length() || std::isdigit(s[offset]) == 0) { break; } parent_id[i] = s[offset]; ++offset; } - parent_id.back() = '\0'; + parent_id[i] = '\0'; // parent-id errno = 0;