From 5f82a06d23418453b091785a0a05ce40e6f277e9 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Fri, 5 Mar 2021 22:09:41 +0200 Subject: [PATCH 01/13] Add jaeger propagator --- api/include/opentelemetry/context/context.h | 13 +- .../trace/propagation/b3_propagator.h | 15 +- .../trace/propagation/detail/hex.h | 74 +++++++++ .../trace/propagation/detail/string.h | 54 +++++++ .../trace/propagation/http_trace_context.h | 13 +- .../opentelemetry/trace/propagation/jaeger.h | 129 +++++++++++++++ api/test/trace/propagation/BUILD | 11 ++ api/test/trace/propagation/CMakeLists.txt | 5 +- .../propagation/jaeger_propagation_test.cc | 150 ++++++++++++++++++ api/test/trace/propagation/util.h | 11 ++ 10 files changed, 448 insertions(+), 27 deletions(-) create mode 100644 api/include/opentelemetry/trace/propagation/detail/hex.h create mode 100644 api/include/opentelemetry/trace/propagation/detail/string.h create mode 100644 api/include/opentelemetry/trace/propagation/jaeger.h create mode 100644 api/test/trace/propagation/jaeger_propagation_test.cc create mode 100644 api/test/trace/propagation/util.h diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index 804155d381..5519537418 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -60,7 +60,7 @@ class Context } // Returns the value associated with the passed in key. - context::ContextValue GetValue(const nostd::string_view key) noexcept + context::ContextValue GetValue(const nostd::string_view key) const noexcept { for (DataList *data = head_.get(); data != nullptr; data = data->next_.get()) { @@ -93,6 +93,17 @@ class Context bool operator==(const Context &other) const noexcept { return (head_ == other.head_); } + trace::SpanContext GetCurrentSpan() const + { + ContextValue span = GetValue(trace::kSpanKey); + if (nostd::holds_alternative>(span)) + { + return nostd::get>(span).get()->GetContext(); + } + + return trace::SpanContext::GetInvalid(); + } + private: // A linked list to contain the keys and values of this context node class DataList diff --git a/api/include/opentelemetry/trace/propagation/b3_propagator.h b/api/include/opentelemetry/trace/propagation/b3_propagator.h index d769528d83..318f541733 100644 --- a/api/include/opentelemetry/trace/propagation/b3_propagator.h +++ b/api/include/opentelemetry/trace/propagation/b3_propagator.h @@ -62,17 +62,6 @@ class B3PropagatorExtractor : public TextMapPropagator return context.SetValue(kSpanKey, sp); } - static SpanContext GetCurrentSpan(const context::Context &context) - { - context::Context ctx(context); - context::ContextValue span = ctx.GetValue(kSpanKey); - if (nostd::holds_alternative>(span)) - { - return nostd::get>(span).get()->GetContext(); - } - return SpanContext::GetInvalid(); - } - static TraceId GenerateTraceIdFromString(nostd::string_view trace_id) { uint8_t buf[kTraceIdHexStrLength / 2]; @@ -213,7 +202,7 @@ class B3Propagator : public B3PropagatorExtractor // Sets the context for a HTTP header carrier with self defined rules. void Inject(Setter setter, T &carrier, const context::Context &context) noexcept override { - SpanContext span_context = B3PropagatorExtractor::GetCurrentSpan(context); + SpanContext span_context = context.GetCurrentSpan(); if (!span_context.IsValid()) { return; @@ -251,7 +240,7 @@ class B3PropagatorMultiHeader : public B3PropagatorExtractor nostd::string_view trace_description); void Inject(Setter setter, T &carrier, const context::Context &context) noexcept override { - SpanContext span_context = B3PropagatorExtractor::GetCurrentSpan(context); + SpanContext span_context = context.GetCurrentSpan(); if (!span_context.IsValid()) { return; diff --git a/api/include/opentelemetry/trace/propagation/detail/hex.h b/api/include/opentelemetry/trace/propagation/detail/hex.h new file mode 100644 index 0000000000..06ced34d83 --- /dev/null +++ b/api/include/opentelemetry/trace/propagation/detail/hex.h @@ -0,0 +1,74 @@ +#pragma once + +#include "opentelemetry/nostd/string_view.h" + +#include +#include + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace trace +{ +namespace propagation +{ +namespace detail +{ + +constexpr int8_t kHexDigits[256] = { + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, -1, -1, -1, -1, -1, -1, -1, 10, 11, 12, 13, 14, 15, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, 10, 11, 12, 13, 14, 15, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, +}; + +inline int8_t HexToInt(char c) +{ + return kHexDigits[uint8_t(c)]; +} + +bool IsValidHex(nostd::string_view s) +{ + return std::all_of(s.begin(), s.end(), [](char c) { return HexToInt(c) != -1; }); +} + +/** + * Converts a hexadecimal to binary format if the hex string will fit the buffer. + * Smaller hex strings are left padded with zeroes. + */ +bool HexToBinary(nostd::string_view hex, uint8_t *buffer, size_t buffer_size) +{ + if (hex.size() > buffer_size * 2) + { + return false; + } + + std::memset(buffer, 0, buffer_size); + + int64_t hex_size = int64_t(hex.size()); + int64_t buffer_pos = int64_t(buffer_size) - (hex_size + 1) / 2; + int64_t last_hex_pos = hex_size - 1; + + int64_t i = 0; + for (; i < last_hex_pos; i += 2) + { + buffer[buffer_pos++] = (HexToInt(hex[i]) << 4) | HexToInt(hex[i + 1]); + } + + if (i == last_hex_pos) + { + buffer[buffer_pos] = HexToInt(hex[i]); + } + + return true; +} + +} // namespace detail +} // namespace propagation +} // namespace trace +OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/trace/propagation/detail/string.h b/api/include/opentelemetry/trace/propagation/detail/string.h new file mode 100644 index 0000000000..c9ebcbf52c --- /dev/null +++ b/api/include/opentelemetry/trace/propagation/detail/string.h @@ -0,0 +1,54 @@ +#pragma once + +#include "opentelemetry/nostd/string_view.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace trace +{ +namespace propagation +{ +namespace detail +{ + +/** + * Splits a string by separator, up to given buffer count words. + * Returns the amount of words the input was split into. + */ +size_t SplitString(nostd::string_view s, char separator, nostd::string_view *results, size_t count) +{ + if (count == 0) + { + return count; + } + + size_t filled = 0; + size_t token_start = 0; + for (size_t i = 0; i < s.size(); i++) + { + if (s[i] != separator) + { + continue; + } + + results[filled++] = s.substr(token_start, i - token_start); + + if (filled == count) + { + return count; + } + + token_start = i + 1; + } + + if (filled < count) + { + results[filled++] = s.substr(token_start); + } + + return filled; +} + +} // namespace detail +} // namespace propagation +} // namespace trace +OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/trace/propagation/http_trace_context.h b/api/include/opentelemetry/trace/propagation/http_trace_context.h index 6dc8e248d3..d24dd9f05a 100644 --- a/api/include/opentelemetry/trace/propagation/http_trace_context.h +++ b/api/include/opentelemetry/trace/propagation/http_trace_context.h @@ -65,7 +65,7 @@ class HttpTraceContext : public TextMapPropagator void Inject(Setter setter, T &carrier, const context::Context &context) noexcept override { - SpanContext span_context = GetCurrentSpan(context); + SpanContext span_context = context.GetCurrentSpan(); if (!span_context.IsValid()) { return; @@ -82,17 +82,6 @@ class HttpTraceContext : public TextMapPropagator return context.SetValue(kSpanKey, sp); } - static SpanContext GetCurrentSpan(const context::Context &context) - { - context::Context ctx(context); - context::ContextValue span = ctx.GetValue(kSpanKey); - if (nostd::holds_alternative>(span)) - { - return nostd::get>(span).get()->GetContext(); - } - return SpanContext::GetInvalid(); - } - static TraceId GenerateTraceIdFromString(nostd::string_view trace_id) { int trace_id_len = kHeaderElementLengths[1]; diff --git a/api/include/opentelemetry/trace/propagation/jaeger.h b/api/include/opentelemetry/trace/propagation/jaeger.h new file mode 100644 index 0000000000..7b0c84a5d7 --- /dev/null +++ b/api/include/opentelemetry/trace/propagation/jaeger.h @@ -0,0 +1,129 @@ +#pragma once + +// Copyright 2021, OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "detail/hex.h" +#include "detail/string.h" +#include "opentelemetry/trace/default_span.h" +#include "opentelemetry/trace/propagation/text_map_propagator.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace trace +{ +namespace propagation +{ + +static const nostd::string_view kTraceHeader = "uber-trace-id"; + +template +class JaegerPropagator : public TextMapPropagator +{ +public: + using Getter = nostd::string_view (*)(const T &carrier, nostd::string_view trace_type); + + using Setter = void (*)(T &carrier, + nostd::string_view trace_type, + nostd::string_view trace_description); + + void Inject(Setter setter, T &carrier, const context::Context &context) noexcept override + { + SpanContext span_context = context.GetCurrentSpan(); + if (!span_context.IsValid()) + { + return; + } + + const size_t trace_id_length = 32; + const size_t span_id_length = 16; + + // trace-id(32):span-id(16):0:debug(2) + char trace_state[trace_id_length + span_id_length + 6]; + span_context.trace_id().ToLowerBase16( + nostd::span(&trace_state[0], trace_id_length)); + trace_state[trace_id_length] = ':'; + span_context.span_id().ToLowerBase16( + nostd::span(&trace_state[trace_id_length + 1], span_id_length)); + trace_state[trace_id_length + span_id_length + 1] = ':'; + trace_state[trace_id_length + span_id_length + 2] = '0'; + trace_state[trace_id_length + span_id_length + 3] = ':'; + trace_state[trace_id_length + span_id_length + 4] = '0'; + trace_state[trace_id_length + span_id_length + 5] = span_context.IsSampled() ? '1' : '0'; + + setter(carrier, kTraceHeader, trace_state); + } + + context::Context Extract(Getter getter, + const T &carrier, + context::Context &context) noexcept override + { + SpanContext span_context = ExtractImpl(getter, carrier); + nostd::shared_ptr sp{new DefaultSpan(span_context)}; + return context.SetValue(kSpanKey, sp); + } + +private: + static TraceFlags GetTraceFlags(uint8_t jaeger_flags) + { + uint8_t sampled = jaeger_flags & 0x01; + return TraceFlags(sampled); + } + + static SpanContext ExtractImpl(Getter getter, const T &carrier) + { + nostd::string_view trace_state = getter(carrier, kTraceHeader); + + const size_t trace_field_count = 4; + nostd::string_view trace_fields[trace_field_count]; + + if (detail::SplitString(trace_state, ':', trace_fields, trace_field_count) != trace_field_count) + { + return SpanContext::GetInvalid(); + } + + nostd::string_view trace_id_hex = trace_fields[0]; + nostd::string_view span_id_hex = trace_fields[1]; + nostd::string_view flags_hex = trace_fields[3]; + + if (!detail::IsValidHex(trace_id_hex) || !detail::IsValidHex(span_id_hex) || + !detail::IsValidHex(flags_hex)) + { + return SpanContext::GetInvalid(); + } + + uint8_t trace_id[16]; + if (!detail::HexToBinary(trace_id_hex, trace_id, sizeof(trace_id))) + { + return SpanContext::GetInvalid(); + } + + uint8_t span_id[8]; + if (!detail::HexToBinary(span_id_hex, span_id, sizeof(span_id))) + { + return SpanContext::GetInvalid(); + } + + uint8_t flags; + if (!detail::HexToBinary(flags_hex, &flags, sizeof(flags))) + { + return SpanContext::GetInvalid(); + } + + return SpanContext(TraceId(trace_id), SpanId(span_id), GetTraceFlags(flags), true); + } +}; + +} // namespace propagation +} // namespace trace +OPENTELEMETRY_END_NAMESPACE diff --git a/api/test/trace/propagation/BUILD b/api/test/trace/propagation/BUILD index 974c7666d7..ed3792dc90 100644 --- a/api/test/trace/propagation/BUILD +++ b/api/test/trace/propagation/BUILD @@ -10,3 +10,14 @@ cc_test( "@com_google_googletest//:gtest_main", ], ) + +cc_test( + name = "jaeger_propagation_test", + srcs = [ + "jaeger_propagation_test.cc", + ], + deps = [ + "//api", + "@com_google_googletest//:gtest_main", + ], +) diff --git a/api/test/trace/propagation/CMakeLists.txt b/api/test/trace/propagation/CMakeLists.txt index 99de61f1c4..a3abfd7712 100644 --- a/api/test/trace/propagation/CMakeLists.txt +++ b/api/test/trace/propagation/CMakeLists.txt @@ -1,4 +1,7 @@ -foreach(testname http_text_format_test b3_propagation_test) +foreach(testname + http_text_format_test + b3_propagation_test + jaeger_propagation_test) add_executable(${testname} "${testname}.cc") target_link_libraries( ${testname} ${GTEST_BOTH_LIBRARIES} ${CORE_RUNTIME_LIBS} diff --git a/api/test/trace/propagation/jaeger_propagation_test.cc b/api/test/trace/propagation/jaeger_propagation_test.cc new file mode 100644 index 0000000000..d577bf70bf --- /dev/null +++ b/api/test/trace/propagation/jaeger_propagation_test.cc @@ -0,0 +1,150 @@ +#include "opentelemetry/trace/propagation/jaeger.h" +#include "opentelemetry/trace/scope.h" +#include "util.h" + +#include + +#include + +using namespace opentelemetry; + +static nostd::string_view Getter(const std::map &carrier, + nostd::string_view trace_type = "uber-trace-id") +{ + auto it = carrier.find(std::string(trace_type)); + if (it != carrier.end()) + { + return nostd::string_view(it->second); + } + return ""; +} + +static void Setter(std::map &carrier, + nostd::string_view trace_type = "uber-trace-id", + nostd::string_view trace_description = "") +{ + carrier[std::string(trace_type)] = std::string(trace_description); +} + +using Propagator = trace::propagation::JaegerPropagator>; + +static Propagator format = Propagator(); + +TEST(JaegerPropagatorTest, ExtractValidSpans) +{ + struct TestTrace + { + std::string trace_state; + std::string expected_trace_id; + std::string expected_span_id; + bool sampled; + }; + + std::vector traces = { + { + "4bf92f3577b34da6a3ce929d0e0e4736:0102030405060708:0:00", + "4bf92f3577b34da6a3ce929d0e0e4736", + "0102030405060708", + false, + }, + { + "4bf92f3577b34da6a3ce929d0e0e4736:0102030405060708:0:ff", + "4bf92f3577b34da6a3ce929d0e0e4736", + "0102030405060708", + true, + }, + { + "4bf92f3577b34da6a3ce929d0e0e4736:0102030405060708:0:f", + "4bf92f3577b34da6a3ce929d0e0e4736", + "0102030405060708", + true, + }, + { + "a3ce929d0e0e4736:0102030405060708:0:00", + "0000000000000000a3ce929d0e0e4736", + "0102030405060708", + false, + }, + { + "A3CE929D0E0E4736:ABCDEFABCDEF1234:0:01", + "0000000000000000a3ce929d0e0e4736", + "abcdefabcdef1234", + true, + }, + { + "ff:ABCDEFABCDEF1234:0:0", + "000000000000000000000000000000ff", + "abcdefabcdef1234", + false, + }, + { + "4bf92f3577b34da6a3ce929d0e0e4736:0102030405060708:0102030405060708:00", + "4bf92f3577b34da6a3ce929d0e0e4736", + "0102030405060708", + false, + }, + + }; + + for (TestTrace &test_trace : traces) + { + const std::map carrier = {{"uber-trace-id", test_trace.trace_state}}; + context::Context ctx1 = context::Context{}; + context::Context ctx2 = format.Extract(Getter, carrier, ctx1); + + auto span = ctx2.GetCurrentSpan(); + EXPECT_TRUE(span.IsValid()); + + EXPECT_EQ(Hex(span.trace_id()), test_trace.expected_trace_id); + EXPECT_EQ(Hex(span.span_id()), test_trace.expected_span_id); + EXPECT_EQ(span.IsSampled(), test_trace.sampled); + EXPECT_EQ(span.IsRemote(), true); + } +} + +TEST(JaegerPropagatorTest, ExctractInvalidSpans) +{ + std::vector traces = { + "4bf92f3577b34da6a3ce929d0e0e47344:0102030405060708:0:00", // too long trace id + "4bf92f3577b34da6a3ce929d0e0e4734:01020304050607089:0:00", // too long span id + "4bf92f3577b34da6a3ce929d0e0e4734::0:00", + "", + "::::", + "0:0:0:0", + ":abcdef12:0:0", + }; + + for (auto &trace : traces) + { + const std::map carrier = {{"uber-trace-id", trace}}; + context::Context ctx1 = context::Context{}; + context::Context ctx2 = format.Extract(Getter, carrier, ctx1); + + auto span = ctx2.GetCurrentSpan(); + EXPECT_FALSE(span.IsValid()); + } +} + +TEST(JaegerPropagatorTest, InjectsContext) +{ + constexpr uint8_t buf_span[] = {1, 2, 3, 4, 5, 6, 7, 8}; + constexpr uint8_t buf_trace[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; + trace::SpanContext span_context{trace::TraceId{buf_trace}, trace::SpanId{buf_span}, + trace::TraceFlags{true}, false}; + nostd::shared_ptr sp{new trace::DefaultSpan{span_context}}; + trace::Scope scoped_span{sp}; + + std::map headers = {}; + format.Inject(Setter, headers, context::RuntimeContext::GetCurrent()); + EXPECT_EQ(headers["uber-trace-id"], "0102030405060708090a0b0c0d0e0f10:0102030405060708:0:01"); +} + +TEST(JaegerPropagatorTest, DoNotInjectInvalidContext) +{ + std::map carrier = {}; + context::Context ctx{ + "current-span", + nostd::shared_ptr(new trace::DefaultSpan(trace::SpanContext::GetInvalid()))}; + format.Inject(Setter, carrier, ctx); + EXPECT_TRUE(carrier.count("uber-trace-id") == 0); +} diff --git a/api/test/trace/propagation/util.h b/api/test/trace/propagation/util.h new file mode 100644 index 0000000000..1f6a8f8451 --- /dev/null +++ b/api/test/trace/propagation/util.h @@ -0,0 +1,11 @@ +#pragma once + +#include + +template +static std::string Hex(const T &id_item) +{ + char buf[T::kSize * 2]; + id_item.ToLowerBase16(buf); + return std::string(buf, sizeof(buf)); +} From 37370b8740344c66409b3a33804618491458d025 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Fri, 5 Mar 2021 22:19:46 +0200 Subject: [PATCH 02/13] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54b6d0df48..c9600b549a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Increment the: ## [Unreleased] * [EXPORTER] Added Zipkin Exporter. ([#471](https://github.com/open-telemetry/opentelemetry-cpp/pull/471)) +* [API] Added Jaeger propagator. ([#599](https://github.com/open-telemetry/opentelemetry-cpp/pull/599)) ## [0.2.0] 2021-03-02 From 3f22fe53b417bf0e54267189f536508ac542c1c2 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Fri, 5 Mar 2021 22:23:17 +0200 Subject: [PATCH 03/13] CMake format --- api/test/trace/propagation/CMakeLists.txt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/api/test/trace/propagation/CMakeLists.txt b/api/test/trace/propagation/CMakeLists.txt index a3abfd7712..8e581fd262 100644 --- a/api/test/trace/propagation/CMakeLists.txt +++ b/api/test/trace/propagation/CMakeLists.txt @@ -1,7 +1,5 @@ -foreach(testname - http_text_format_test - b3_propagation_test - jaeger_propagation_test) +foreach(testname http_text_format_test b3_propagation_test + jaeger_propagation_test) add_executable(${testname} "${testname}.cc") target_link_libraries( ${testname} ${GTEST_BOTH_LIBRARIES} ${CORE_RUNTIME_LIBS} From 1d023727e36158463f648439df4107930df97955 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Fri, 5 Mar 2021 22:31:53 +0200 Subject: [PATCH 04/13] Attempt at bazel build fix --- api/test/trace/propagation/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/api/test/trace/propagation/BUILD b/api/test/trace/propagation/BUILD index ed3792dc90..dfa4e23868 100644 --- a/api/test/trace/propagation/BUILD +++ b/api/test/trace/propagation/BUILD @@ -16,6 +16,7 @@ cc_test( srcs = [ "jaeger_propagation_test.cc", ], + hdrs = ["util.h"], deps = [ "//api", "@com_google_googletest//:gtest_main", From 32c85599f309d81a1a40e0d2897fb698eb5a85d4 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Fri, 5 Mar 2021 22:36:10 +0200 Subject: [PATCH 05/13] Bazel include --- api/test/trace/propagation/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/test/trace/propagation/BUILD b/api/test/trace/propagation/BUILD index dfa4e23868..781be8a2c3 100644 --- a/api/test/trace/propagation/BUILD +++ b/api/test/trace/propagation/BUILD @@ -15,8 +15,8 @@ cc_test( name = "jaeger_propagation_test", srcs = [ "jaeger_propagation_test.cc", + "util.h" ], - hdrs = ["util.h"], deps = [ "//api", "@com_google_googletest//:gtest_main", From 37aab69fbba57f486c79c1111b2fc92eb25fd8b4 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Fri, 5 Mar 2021 22:39:32 +0200 Subject: [PATCH 06/13] Bazel script format ... --- api/test/trace/propagation/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/test/trace/propagation/BUILD b/api/test/trace/propagation/BUILD index 781be8a2c3..4de46a7dae 100644 --- a/api/test/trace/propagation/BUILD +++ b/api/test/trace/propagation/BUILD @@ -15,7 +15,7 @@ cc_test( name = "jaeger_propagation_test", srcs = [ "jaeger_propagation_test.cc", - "util.h" + "util.h", ], deps = [ "//api", From 822b823c813a5fbf33e9e3fa949f0e887260c320 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Fri, 5 Mar 2021 23:21:39 +0200 Subject: [PATCH 07/13] Explicit string_view construct --- api/include/opentelemetry/trace/propagation/jaeger.h | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/api/include/opentelemetry/trace/propagation/jaeger.h b/api/include/opentelemetry/trace/propagation/jaeger.h index 7b0c84a5d7..c5b98666d7 100644 --- a/api/include/opentelemetry/trace/propagation/jaeger.h +++ b/api/include/opentelemetry/trace/propagation/jaeger.h @@ -50,18 +50,16 @@ class JaegerPropagator : public TextMapPropagator // trace-id(32):span-id(16):0:debug(2) char trace_state[trace_id_length + span_id_length + 6]; - span_context.trace_id().ToLowerBase16( - nostd::span(&trace_state[0], trace_id_length)); + span_context.trace_id().ToLowerBase16({&trace_state[0], trace_id_length}); trace_state[trace_id_length] = ':'; - span_context.span_id().ToLowerBase16( - nostd::span(&trace_state[trace_id_length + 1], span_id_length)); + span_context.span_id().ToLowerBase16({&trace_state[trace_id_length + 1], span_id_length}); trace_state[trace_id_length + span_id_length + 1] = ':'; trace_state[trace_id_length + span_id_length + 2] = '0'; trace_state[trace_id_length + span_id_length + 3] = ':'; trace_state[trace_id_length + span_id_length + 4] = '0'; trace_state[trace_id_length + span_id_length + 5] = span_context.IsSampled() ? '1' : '0'; - setter(carrier, kTraceHeader, trace_state); + setter(carrier, kTraceHeader, nostd::string_view(trace_state, sizeof(trace_state))); } context::Context Extract(Getter getter, From c1db8323fc4103c6847acc195b0a7ec57f28a01b Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Wed, 10 Mar 2021 11:42:14 +0200 Subject: [PATCH 08/13] jaeger propagator: rename trace_state to trace_identity --- .../opentelemetry/trace/propagation/jaeger.h | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/api/include/opentelemetry/trace/propagation/jaeger.h b/api/include/opentelemetry/trace/propagation/jaeger.h index c5b98666d7..73aa7ae194 100644 --- a/api/include/opentelemetry/trace/propagation/jaeger.h +++ b/api/include/opentelemetry/trace/propagation/jaeger.h @@ -49,17 +49,17 @@ class JaegerPropagator : public TextMapPropagator const size_t span_id_length = 16; // trace-id(32):span-id(16):0:debug(2) - char trace_state[trace_id_length + span_id_length + 6]; - span_context.trace_id().ToLowerBase16({&trace_state[0], trace_id_length}); - trace_state[trace_id_length] = ':'; - span_context.span_id().ToLowerBase16({&trace_state[trace_id_length + 1], span_id_length}); - trace_state[trace_id_length + span_id_length + 1] = ':'; - trace_state[trace_id_length + span_id_length + 2] = '0'; - trace_state[trace_id_length + span_id_length + 3] = ':'; - trace_state[trace_id_length + span_id_length + 4] = '0'; - trace_state[trace_id_length + span_id_length + 5] = span_context.IsSampled() ? '1' : '0'; - - setter(carrier, kTraceHeader, nostd::string_view(trace_state, sizeof(trace_state))); + char trace_identity[trace_id_length + span_id_length + 6]; + span_context.trace_id().ToLowerBase16({&trace_identity[0], trace_id_length}); + trace_identity[trace_id_length] = ':'; + span_context.span_id().ToLowerBase16({&trace_identity[trace_id_length + 1], span_id_length}); + trace_identity[trace_id_length + span_id_length + 1] = ':'; + trace_identity[trace_id_length + span_id_length + 2] = '0'; + trace_identity[trace_id_length + span_id_length + 3] = ':'; + trace_identity[trace_id_length + span_id_length + 4] = '0'; + trace_identity[trace_id_length + span_id_length + 5] = span_context.IsSampled() ? '1' : '0'; + + setter(carrier, kTraceHeader, nostd::string_view(trace_identity, sizeof(trace_identity))); } context::Context Extract(Getter getter, @@ -80,12 +80,12 @@ class JaegerPropagator : public TextMapPropagator static SpanContext ExtractImpl(Getter getter, const T &carrier) { - nostd::string_view trace_state = getter(carrier, kTraceHeader); + nostd::string_view trace_identity = getter(carrier, kTraceHeader); const size_t trace_field_count = 4; nostd::string_view trace_fields[trace_field_count]; - if (detail::SplitString(trace_state, ':', trace_fields, trace_field_count) != trace_field_count) + if (detail::SplitString(trace_identity, ':', trace_fields, trace_field_count) != trace_field_count) { return SpanContext::GetInvalid(); } From d1cbbd878c428b61cc39279c01feaac5ceaf304e Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Wed, 10 Mar 2021 12:06:31 +0200 Subject: [PATCH 09/13] Move GetCurrentSpan to propagation detail --- api/include/opentelemetry/context/context.h | 11 -------- .../trace/propagation/b3_propagator.h | 5 ++-- .../trace/propagation/detail/context.h | 26 +++++++++++++++++++ .../trace/propagation/http_trace_context.h | 3 ++- .../opentelemetry/trace/propagation/jaeger.h | 3 ++- .../propagation/jaeger_propagation_test.cc | 4 +-- 6 files changed, 35 insertions(+), 17 deletions(-) create mode 100644 api/include/opentelemetry/trace/propagation/detail/context.h diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index 5519537418..3f7455e55a 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -93,17 +93,6 @@ class Context bool operator==(const Context &other) const noexcept { return (head_ == other.head_); } - trace::SpanContext GetCurrentSpan() const - { - ContextValue span = GetValue(trace::kSpanKey); - if (nostd::holds_alternative>(span)) - { - return nostd::get>(span).get()->GetContext(); - } - - return trace::SpanContext::GetInvalid(); - } - private: // A linked list to contain the keys and values of this context node class DataList diff --git a/api/include/opentelemetry/trace/propagation/b3_propagator.h b/api/include/opentelemetry/trace/propagation/b3_propagator.h index 318f541733..fed1a19c14 100644 --- a/api/include/opentelemetry/trace/propagation/b3_propagator.h +++ b/api/include/opentelemetry/trace/propagation/b3_propagator.h @@ -4,6 +4,7 @@ #include #include #include +#include "detail/context.h" #include "opentelemetry/common/key_value_iterable.h" #include "opentelemetry/context/context.h" #include "opentelemetry/nostd/shared_ptr.h" @@ -202,7 +203,7 @@ class B3Propagator : public B3PropagatorExtractor // Sets the context for a HTTP header carrier with self defined rules. void Inject(Setter setter, T &carrier, const context::Context &context) noexcept override { - SpanContext span_context = context.GetCurrentSpan(); + SpanContext span_context = detail::GetCurrentSpan(context); if (!span_context.IsValid()) { return; @@ -240,7 +241,7 @@ class B3PropagatorMultiHeader : public B3PropagatorExtractor nostd::string_view trace_description); void Inject(Setter setter, T &carrier, const context::Context &context) noexcept override { - SpanContext span_context = context.GetCurrentSpan(); + SpanContext span_context = detail::GetCurrentSpan(context); if (!span_context.IsValid()) { return; diff --git a/api/include/opentelemetry/trace/propagation/detail/context.h b/api/include/opentelemetry/trace/propagation/detail/context.h new file mode 100644 index 0000000000..dd0424f493 --- /dev/null +++ b/api/include/opentelemetry/trace/propagation/detail/context.h @@ -0,0 +1,26 @@ +#pragma once +#include "opentelemetry/context/context.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace trace +{ +namespace propagation +{ +namespace detail +{ + +trace::SpanContext GetCurrentSpan(const context::Context& context) +{ + context::ContextValue span = context.GetValue(trace::kSpanKey); + if (nostd::holds_alternative>(span)) + { + return nostd::get>(span).get()->GetContext(); + } + + return trace::SpanContext::GetInvalid(); +} + +} // namespace detail +} // namespace propagation +} // namespace trace +OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/trace/propagation/http_trace_context.h b/api/include/opentelemetry/trace/propagation/http_trace_context.h index 850cd9e6dc..3127c2f2d4 100644 --- a/api/include/opentelemetry/trace/propagation/http_trace_context.h +++ b/api/include/opentelemetry/trace/propagation/http_trace_context.h @@ -16,6 +16,7 @@ #include #include #include +#include "detail/context.h" #include "opentelemetry/common/key_value_iterable.h" #include "opentelemetry/context/context.h" #include "opentelemetry/nostd/shared_ptr.h" @@ -65,7 +66,7 @@ class HttpTraceContext : public TextMapPropagator void Inject(Setter setter, T &carrier, const context::Context &context) noexcept override { - SpanContext span_context = context.GetCurrentSpan(); + SpanContext span_context = detail::GetCurrentSpan(context); if (!span_context.IsValid()) { return; diff --git a/api/include/opentelemetry/trace/propagation/jaeger.h b/api/include/opentelemetry/trace/propagation/jaeger.h index 73aa7ae194..5e6d394c0f 100644 --- a/api/include/opentelemetry/trace/propagation/jaeger.h +++ b/api/include/opentelemetry/trace/propagation/jaeger.h @@ -16,6 +16,7 @@ #include "detail/hex.h" #include "detail/string.h" +#include "detail/context.h" #include "opentelemetry/trace/default_span.h" #include "opentelemetry/trace/propagation/text_map_propagator.h" @@ -39,7 +40,7 @@ class JaegerPropagator : public TextMapPropagator void Inject(Setter setter, T &carrier, const context::Context &context) noexcept override { - SpanContext span_context = context.GetCurrentSpan(); + SpanContext span_context = detail::GetCurrentSpan(context); if (!span_context.IsValid()) { return; diff --git a/api/test/trace/propagation/jaeger_propagation_test.cc b/api/test/trace/propagation/jaeger_propagation_test.cc index d577bf70bf..ef526a5023 100644 --- a/api/test/trace/propagation/jaeger_propagation_test.cc +++ b/api/test/trace/propagation/jaeger_propagation_test.cc @@ -92,7 +92,7 @@ TEST(JaegerPropagatorTest, ExtractValidSpans) context::Context ctx1 = context::Context{}; context::Context ctx2 = format.Extract(Getter, carrier, ctx1); - auto span = ctx2.GetCurrentSpan(); + auto span = trace::propagation::detail::GetCurrentSpan(ctx2); EXPECT_TRUE(span.IsValid()); EXPECT_EQ(Hex(span.trace_id()), test_trace.expected_trace_id); @@ -120,7 +120,7 @@ TEST(JaegerPropagatorTest, ExctractInvalidSpans) context::Context ctx1 = context::Context{}; context::Context ctx2 = format.Extract(Getter, carrier, ctx1); - auto span = ctx2.GetCurrentSpan(); + auto span = trace::propagation::detail::GetCurrentSpan(ctx2); EXPECT_FALSE(span.IsValid()); } } From 96795eaebd8f716284132df57978a2783eceeaa8 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Wed, 10 Mar 2021 13:30:32 +0200 Subject: [PATCH 10/13] Formatting --- api/include/opentelemetry/trace/propagation/jaeger.h | 5 +++-- api/test/trace/propagation/CMakeLists.txt | 3 +-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/include/opentelemetry/trace/propagation/jaeger.h b/api/include/opentelemetry/trace/propagation/jaeger.h index 5e6d394c0f..a97be134c5 100644 --- a/api/include/opentelemetry/trace/propagation/jaeger.h +++ b/api/include/opentelemetry/trace/propagation/jaeger.h @@ -14,9 +14,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include "detail/context.h" #include "detail/hex.h" #include "detail/string.h" -#include "detail/context.h" #include "opentelemetry/trace/default_span.h" #include "opentelemetry/trace/propagation/text_map_propagator.h" @@ -86,7 +86,8 @@ class JaegerPropagator : public TextMapPropagator const size_t trace_field_count = 4; nostd::string_view trace_fields[trace_field_count]; - if (detail::SplitString(trace_identity, ':', trace_fields, trace_field_count) != trace_field_count) + if (detail::SplitString(trace_identity, ':', trace_fields, trace_field_count) != + trace_field_count) { return SpanContext::GetInvalid(); } diff --git a/api/test/trace/propagation/CMakeLists.txt b/api/test/trace/propagation/CMakeLists.txt index 105156f656..fd3129408c 100644 --- a/api/test/trace/propagation/CMakeLists.txt +++ b/api/test/trace/propagation/CMakeLists.txt @@ -1,6 +1,5 @@ foreach(testname http_text_format_test b3_propagation_test - jaeger_propagation_test - composite_propagator_test) + jaeger_propagation_test composite_propagator_test) add_executable(${testname} "${testname}.cc") target_link_libraries( ${testname} ${GTEST_BOTH_LIBRARIES} ${CORE_RUNTIME_LIBS} From 35493ee87bff062361400309833efab2986004b4 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Wed, 10 Mar 2021 13:36:46 +0200 Subject: [PATCH 11/13] Formatting --- api/include/opentelemetry/trace/propagation/detail/context.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/include/opentelemetry/trace/propagation/detail/context.h b/api/include/opentelemetry/trace/propagation/detail/context.h index dd0424f493..9a1e30db29 100644 --- a/api/include/opentelemetry/trace/propagation/detail/context.h +++ b/api/include/opentelemetry/trace/propagation/detail/context.h @@ -9,7 +9,7 @@ namespace propagation namespace detail { -trace::SpanContext GetCurrentSpan(const context::Context& context) +trace::SpanContext GetCurrentSpan(const context::Context &context) { context::ContextValue span = context.GetValue(trace::kSpanKey); if (nostd::holds_alternative>(span)) From 1dd50c57e54c8685c4a43b4e8639d322e1eed703 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Wed, 10 Mar 2021 14:02:09 +0200 Subject: [PATCH 12/13] More test cases --- api/test/trace/propagation/jaeger_propagation_test.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/test/trace/propagation/jaeger_propagation_test.cc b/api/test/trace/propagation/jaeger_propagation_test.cc index ef526a5023..c7125a84f1 100644 --- a/api/test/trace/propagation/jaeger_propagation_test.cc +++ b/api/test/trace/propagation/jaeger_propagation_test.cc @@ -107,6 +107,8 @@ TEST(JaegerPropagatorTest, ExctractInvalidSpans) std::vector traces = { "4bf92f3577b34da6a3ce929d0e0e47344:0102030405060708:0:00", // too long trace id "4bf92f3577b34da6a3ce929d0e0e4734:01020304050607089:0:00", // too long span id + "4bf92f3577b34da6x3ce929d0y0e4734:01020304050607089:0:00", // invalid trace id character + "4bf92f3577b34da6a3ce929d0e0e4734:01020304g50607089:0:00", // invalid span id character "4bf92f3577b34da6a3ce929d0e0e4734::0:00", "", "::::", From 6ed17d49ec5400a37800b208887e3a7d562b0342 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Fri, 12 Mar 2021 12:08:59 +0200 Subject: [PATCH 13/13] jaeger propagator: add kIsSampled flag --- api/include/opentelemetry/trace/propagation/jaeger.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/include/opentelemetry/trace/propagation/jaeger.h b/api/include/opentelemetry/trace/propagation/jaeger.h index a97be134c5..6d242accf8 100644 --- a/api/include/opentelemetry/trace/propagation/jaeger.h +++ b/api/include/opentelemetry/trace/propagation/jaeger.h @@ -73,9 +73,11 @@ class JaegerPropagator : public TextMapPropagator } private: + static constexpr uint8_t kIsSampled = 0x01; + static TraceFlags GetTraceFlags(uint8_t jaeger_flags) { - uint8_t sampled = jaeger_flags & 0x01; + uint8_t sampled = jaeger_flags & kIsSampled; return TraceFlags(sampled); }