From f19577dba1a3487c5e3b4e85c434d4ccedbba9d9 Mon Sep 17 00:00:00 2001 From: Sergei Turchanov Date: Fri, 20 Dec 2019 16:59:29 +1000 Subject: [PATCH 1/2] Opentracing 1.6 API support Implemented Clone(), ToTraceID(), ToSpanID(), extra variants of Log() methods for SpanContext. Test cases for Clone(), ToTraceID(), and ToSpanID(). --- ci/WORKSPACE | 2 +- ci/build_plugin.sh | 2 +- ci/install_dependencies.sh | 2 +- zipkin/include/zipkin/span_context.h | 10 ++++ zipkin_opentracing/src/opentracing.cc | 25 +++++++++ zipkin_opentracing/test/ot_tracer_test.cc | 68 +++++++++++++++++++++++ 6 files changed, 106 insertions(+), 3 deletions(-) diff --git a/ci/WORKSPACE b/ci/WORKSPACE index bfcb171..0d8327b 100644 --- a/ci/WORKSPACE +++ b/ci/WORKSPACE @@ -5,7 +5,7 @@ load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository") git_repository( name = "io_opentracing_cpp", remote = "https://github.com/opentracing/opentracing-cpp", - commit = "ac50154a7713877f877981c33c3375003b6ebfe1", + commit = "597b0fa9507c854877b3efa9f4f20293a9e1ed30", ) new_local_repository( diff --git a/ci/build_plugin.sh b/ci/build_plugin.sh index 7efd378..b9ae864 100755 --- a/ci/build_plugin.sh +++ b/ci/build_plugin.sh @@ -9,7 +9,7 @@ apt-get install --no-install-recommends --no-install-suggests -y \ git \ ca-certificates -export OPENTRACING_VERSION=1.5.0 +export OPENTRACING_VERSION=1.6.0 # Compile for a portable cpu architecture export CFLAGS="-march=x86-64" diff --git a/ci/install_dependencies.sh b/ci/install_dependencies.sh index 2722e10..180aeb2 100755 --- a/ci/install_dependencies.sh +++ b/ci/install_dependencies.sh @@ -11,7 +11,7 @@ apt-get install --no-install-recommends --no-install-suggests -y \ # Build OpenTracing cd / -export OPENTRACING_VERSION=1.5.0 +export OPENTRACING_VERSION=1.6.0 git clone -b v$OPENTRACING_VERSION https://github.com/opentracing/opentracing-cpp.git cd opentracing-cpp mkdir .build && cd .build diff --git a/zipkin/include/zipkin/span_context.h b/zipkin/include/zipkin/span_context.h index 48c08f5..ca5da1b 100644 --- a/zipkin/include/zipkin/span_context.h +++ b/zipkin/include/zipkin/span_context.h @@ -24,6 +24,7 @@ namespace zipkin { */ struct AnnotationSet { AnnotationSet() : cs_(false), cr_(false), ss_(false), sr_(false) {} + AnnotationSet(const AnnotationSet &set) : cs_{set.cs_}, cr_{set.cr_}, ss_{set.ss_}, sr_{set.sr_} {} bool cs_ : 1; bool cr_ : 1; bool ss_ : 1; @@ -58,6 +59,15 @@ class SpanContext { : trace_id_{trace_id}, id_{id}, parent_id_{parent_id}, flags_{flags}, is_initialized_{true} {} + /** + * Copy constructor that creates a context object from the given SpanContext + * instance. + */ + SpanContext(const SpanContext &span) + : trace_id_{span.trace_id_}, id_{span.id_}, parent_id_{span.parent_id_}, + annotation_values_{span.annotation_values_}, flags_{span.flags_}, + is_initialized_{span.is_initialized_} {} + bool isSampled() const { return flags_ & zipkin::sampled_flag; } /** diff --git a/zipkin_opentracing/src/opentracing.cc b/zipkin_opentracing/src/opentracing.cc index 302edc2..a55ce9c 100644 --- a/zipkin_opentracing/src/opentracing.cc +++ b/zipkin_opentracing/src/opentracing.cc @@ -50,6 +50,9 @@ class OtSpanContext : public ot::SpanContext { public: OtSpanContext() = default; + OtSpanContext(const OtSpanContext &span) + : span_context_{span.span_context_}, baggage_{span.baggage_} {} + explicit OtSpanContext(zipkin::SpanContext &&span_context) : span_context_{std::move(span_context)} {} @@ -69,6 +72,20 @@ class OtSpanContext : public ot::SpanContext { return *this; } + std::unique_ptr Clone() const noexcept override try { + return std::unique_ptr (new OtSpanContext(*this)); + } catch (...) { + return nullptr; + } + + std::string ToTraceID() const noexcept override { + return span_context_.traceIdAsHexString(); + } + + std::string ToSpanID() const noexcept override { + return span_context_.idAsHexString(); + } + void ForeachBaggageItem( std::function f) const override { @@ -258,6 +275,14 @@ class OtSpan : public ot::Span { void Log(std::initializer_list> fields) noexcept override {} + void Log( + SystemTime timestamp, + std::initializer_list> fields) noexcept override {} + + void Log( + SystemTime timestamp, + const std::vector>& fields) noexcept override {} + const ot::SpanContext &context() const noexcept override { return span_context_; } diff --git a/zipkin_opentracing/test/ot_tracer_test.cc b/zipkin_opentracing/test/ot_tracer_test.cc index 4a0868f..de46394 100644 --- a/zipkin_opentracing/test/ot_tracer_test.cc +++ b/zipkin_opentracing/test/ot_tracer_test.cc @@ -37,6 +37,20 @@ static bool IsChildOf(const zipkin::Span &a, const zipkin::Span &b) { a.traceId() == b.traceId(); } +class TextMapCarrier : public ot::TextMapWriter { +public: + TextMapCarrier(std::unordered_map &text_map) + : text_map_(text_map) {} + + ot::expected Set(ot::string_view key, ot::string_view value) const override { + text_map_[key] = value; + return {}; + } + +private: + std::unordered_map &text_map_; +}; + TEST_CASE("ot_tracer") { auto reporter = new InMemoryReporter(); ZipkinOtTracerOptions options; @@ -167,4 +181,58 @@ TEST_CASE("ot_tracer") { span->Finish(); CHECK(hasTag(reporter->top(), "abc", 123)); } + + SECTION("Get SpanContext and check that it is valid after Finish") { + auto span = tracer->StartSpan("a"); + CHECK(span); + span->SetTag("abc", 123); + auto &ctx = span->context(); + span->Finish(); + auto trace_id = ctx.ToTraceID(); + CHECK(trace_id != ""); + auto span_id = ctx.ToSpanID(); + CHECK(span_id != ""); + } + + SECTION("Check SpanContext.Clone() preserves attributes") { + auto span = tracer->StartSpan("a"); + CHECK(span); + span->SetTag("abc", 123); + span->SetBaggageItem("a", "1"); + auto &ctx = span->context(); + span->Finish(); + auto ctx2 = ctx.Clone(); + + CHECK(ctx.ToTraceID() == ctx2->ToTraceID()); + CHECK(ctx.ToSpanID() == ctx2->ToSpanID()); + + std::unordered_map items; + ctx.ForeachBaggageItem( + [&items] (const std::string& key, const std::string& value) { + items[key] = value; + return true; + } + ); + + std::unordered_map items2; + ctx2->ForeachBaggageItem( + [&items2] (const std::string& key, const std::string& value) { + items2[key] = value; + return true; + } + ); + CHECK(items == items2); + + // Cross-check: serialize span context to a carrier + // and compare low-level representation + items.clear(); + items2.clear(); + + TextMapCarrier carrier{items}; + tracer->Inject(ctx, carrier); + TextMapCarrier carrier2{items2}; + tracer->Inject(*ctx2, carrier2); + + CHECK(items == items2); + } } From b33618014b5da68b1f30fe2d1a6cd5d2fa30cad3 Mon Sep 17 00:00:00 2001 From: Wilhansen Li Date: Thu, 12 Nov 2020 23:36:27 +0800 Subject: [PATCH 2/2] Generate 128-bit trace IDs --- zipkin/include/zipkin/trace_id.h | 2 +- zipkin/src/span_context.cc | 2 +- zipkin/src/tracer.cc | 6 +++--- zipkin_opentracing/src/opentracing.cc | 4 ++-- zipkin_opentracing/test/ot_tracer_test.cc | 3 ++- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/zipkin/include/zipkin/trace_id.h b/zipkin/include/zipkin/trace_id.h index c4b35e1..80c0c16 100644 --- a/zipkin/include/zipkin/trace_id.h +++ b/zipkin/include/zipkin/trace_id.h @@ -9,7 +9,7 @@ class TraceId { TraceId(uint64_t high, uint64_t low) : high_{high}, low_{low} {} - TraceId(uint64_t low) : TraceId{0, low} {} + explicit TraceId(uint64_t low) : TraceId{0, low} {} uint64_t high() const { return high_; } diff --git a/zipkin/src/span_context.cc b/zipkin/src/span_context.cc index 28c148c..8ab356c 100644 --- a/zipkin/src/span_context.cc +++ b/zipkin/src/span_context.cc @@ -7,7 +7,7 @@ namespace zipkin { SpanContext::SpanContext(const Span &span) { trace_id_ = span.traceId(); id_ = span.id(); - parent_id_ = span.isSetParentId() ? span.parentId() : 0; + parent_id_ = span.isSetParentId() ? span.parentId() : TraceId(0); flags_ = 0; if (span.isSampled()) { diff --git a/zipkin/src/tracer.cc b/zipkin/src/tracer.cc index 04f7796..d33633e 100644 --- a/zipkin/src/tracer.cc +++ b/zipkin/src/tracer.cc @@ -13,8 +13,8 @@ SpanPtr Tracer::startSpan(const std::string &span_name, SystemTime timestamp) { // Create an all-new span, with no parent id SpanPtr span_ptr(new Span()); span_ptr->setName(span_name); - uint64_t random_number = RandomUtil::generateId(); - span_ptr->setId(random_number); + TraceId random_number(RandomUtil::generateId(), RandomUtil::generateId()); + span_ptr->setId(random_number.low()); span_ptr->setTraceId(random_number); int64_t start_time_micro = std::chrono::duration_cast( @@ -56,7 +56,7 @@ SpanPtr Tracer::startSpan(const std::string &span_name, SystemTime timestamp, span_ptr->setName(span_name); // Set the parent id to the id of the previous span - span_ptr->setParentId(previous_context.id()); + span_ptr->setParentId(TraceId(previous_context.id())); // Set the CS annotation value annotation.setValue(ZipkinCoreConstants::get().CLIENT_SEND); diff --git a/zipkin_opentracing/src/opentracing.cc b/zipkin_opentracing/src/opentracing.cc index a55ce9c..d674fe8 100644 --- a/zipkin_opentracing/src/opentracing.cc +++ b/zipkin_opentracing/src/opentracing.cc @@ -157,9 +157,9 @@ class OtSpan : public ot::Span { span_->setId(RandomUtil::generateId()); if (parent_span_context) { span_->setTraceId(parent_span_context->span_context_.trace_id()); - span_->setParentId(parent_span_context->span_context_.id()); + span_->setParentId(TraceId(parent_span_context->span_context_.id())); } else { - span_->setTraceId(RandomUtil::generateId()); + span_->setTraceId(TraceId(RandomUtil::generateId(), RandomUtil::generateId())); } // Set timestamp. diff --git a/zipkin_opentracing/test/ot_tracer_test.cc b/zipkin_opentracing/test/ot_tracer_test.cc index de46394..2b1f76b 100644 --- a/zipkin_opentracing/test/ot_tracer_test.cc +++ b/zipkin_opentracing/test/ot_tracer_test.cc @@ -29,11 +29,12 @@ static bool hasTag(const Span &span, ot::string_view key, ot::Value value) { case DOUBLE: return tag_annotation.valueDouble() == annotation.valueDouble(); } + return false; }); } static bool IsChildOf(const zipkin::Span &a, const zipkin::Span &b) { - return a.isSetParentId() && a.parentId() == b.id() && + return a.isSetParentId() && a.parentId().low() == b.id() && a.traceId() == b.traceId(); }