diff --git a/.circleci/config.yml b/.circleci/config.yml index 46a9d1f..ee250fb 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -7,13 +7,20 @@ jobs: - checkout - run: ./ci/install_dependencies.sh - run: ./ci/do_ci.sh cmake.asan + - store_artifacts: + path: /build/Testing/Temporary/LastTest.log + destination: Test.log tsan: docker: - image: ubuntu:17.10 steps: - checkout + - run: mkdir -p /build - run: ./ci/install_dependencies.sh - - run: ./ci/do_ci.sh cmake.tsan + - run: BUILD_DIR=/build ./ci/do_ci.sh cmake.tsan + - store_artifacts: + path: /build/Testing/Temporary/LastTest.log + destination: Test.log bazel: docker: - image: ubuntu:17.10 diff --git a/.gitignore b/.gitignore index 6fc863d..87b0eb5 100644 --- a/.gitignore +++ b/.gitignore @@ -32,3 +32,5 @@ *.app bazel-* + +.build/ diff --git a/zipkin/include/zipkin/span_context.h b/zipkin/include/zipkin/span_context.h index 04816e0..48c08f5 100644 --- a/zipkin/include/zipkin/span_context.h +++ b/zipkin/include/zipkin/span_context.h @@ -58,6 +58,8 @@ class SpanContext { : trace_id_{trace_id}, id_{id}, parent_id_{parent_id}, flags_{flags}, is_initialized_{true} {} + bool isSampled() const { return flags_ & zipkin::sampled_flag; } + /** * @return the span id as an integer */ diff --git a/zipkin/include/zipkin/zipkin_core_types.h b/zipkin/include/zipkin/zipkin_core_types.h index 04b275f..510a0d9 100644 --- a/zipkin/include/zipkin/zipkin_core_types.h +++ b/zipkin/include/zipkin/zipkin_core_types.h @@ -332,6 +332,9 @@ class Span : public ZipkinBase { : trace_id_(0), name_(), id_(0), debug_(false), monotonic_start_time_(0), tracer_(nullptr) {} + void setSampled(const bool val) { sampled_ = val; } + bool isSampled() const { return sampled_; } + /** * Sets the span's trace id attribute. */ @@ -566,6 +569,7 @@ class Span : public ZipkinBase { std::string name_; uint64_t id_; Optional parent_id_; + bool sampled_{true}; bool debug_; std::vector annotations_; std::vector binary_annotations_; diff --git a/zipkin/src/span_context.cc b/zipkin/src/span_context.cc index b5954e6..28c148c 100644 --- a/zipkin/src/span_context.cc +++ b/zipkin/src/span_context.cc @@ -8,6 +8,11 @@ SpanContext::SpanContext(const Span &span) { trace_id_ = span.traceId(); id_ = span.id(); parent_id_ = span.isSetParentId() ? span.parentId() : 0; + flags_ = 0; + + if (span.isSampled()) { + flags_ |= static_cast(zipkin::sampled_flag); + } for (const Annotation &annotation : span.annotations()) { if (annotation.value() == ZipkinCoreConstants::get().CLIENT_RECV) { diff --git a/zipkin/src/zipkin_core_types.cc b/zipkin/src/zipkin_core_types.cc index 94a8221..b7ae704 100644 --- a/zipkin/src/zipkin_core_types.cc +++ b/zipkin/src/zipkin_core_types.cc @@ -185,7 +185,9 @@ const std::string Span::toJson() { void Span::finish() { if (auto t = tracer()) { - t->reportSpan(std::move(*this)); + if (this->isSampled()) { + t->reportSpan(std::move(*this)); + } } } diff --git a/zipkin_opentracing/CMakeLists.txt b/zipkin_opentracing/CMakeLists.txt index 93b582c..723ea5d 100644 --- a/zipkin_opentracing/CMakeLists.txt +++ b/zipkin_opentracing/CMakeLists.txt @@ -8,7 +8,8 @@ set(ZIPKIN_OPENTRACING_SRCS src/utility.cc src/propagation.cc src/dynamic_load.cc src/tracer_factory.cc - src/opentracing.cc) + src/opentracing.cc + src/sampling.cc) if (BUILD_SHARED_LIBS) add_library(zipkin_opentracing SHARED ${ZIPKIN_OPENTRACING_SRCS}) diff --git a/zipkin_opentracing/example/text_map_carrier.h b/zipkin_opentracing/example/text_map_carrier.h index 1ff45b3..fa0a5d9 100644 --- a/zipkin_opentracing/example/text_map_carrier.h +++ b/zipkin_opentracing/example/text_map_carrier.h @@ -5,10 +5,10 @@ #include #include -using opentracing::TextMapReader; -using opentracing::TextMapWriter; using opentracing::expected; using opentracing::string_view; +using opentracing::TextMapReader; +using opentracing::TextMapWriter; class TextMapCarrier : public TextMapReader, public TextMapWriter { public: diff --git a/zipkin_opentracing/include/zipkin/opentracing.h b/zipkin_opentracing/include/zipkin/opentracing.h index 8c869dd..e4f012a 100644 --- a/zipkin_opentracing/include/zipkin/opentracing.h +++ b/zipkin_opentracing/include/zipkin/opentracing.h @@ -8,6 +8,7 @@ struct ZipkinOtTracerOptions { uint32_t collector_port = 9411; SteadyClock::duration reporting_period = DEFAULT_REPORTING_PERIOD; size_t max_buffered_spans = DEFAULT_SPAN_BUFFER_SIZE; + double sample_rate = 1.0; std::string service_name; IpAddress service_address; @@ -19,4 +20,5 @@ makeZipkinOtTracer(const ZipkinOtTracerOptions &options); std::shared_ptr makeZipkinOtTracer(const ZipkinOtTracerOptions &options, std::unique_ptr &&reporter); + } // namespace zipkin diff --git a/zipkin_opentracing/src/opentracing.cc b/zipkin_opentracing/src/opentracing.cc index 771c331..ac4e05d 100644 --- a/zipkin_opentracing/src/opentracing.cc +++ b/zipkin_opentracing/src/opentracing.cc @@ -1,19 +1,22 @@ +#include #include #include "propagation.h" +#include "sampling.h" #include "utility.h" #include #include #include +#include #include #include #include #include -using opentracing::Value; using opentracing::expected; using opentracing::make_unexpected; using opentracing::string_view; +using opentracing::Value; namespace ot = opentracing; @@ -99,6 +102,12 @@ class OtSpanContext : public ot::SpanContext { return injectSpanContext(writer, span_context_, baggage_); } + bool isSampled() const { return span_context_.isSampled(); } + + bool isValid() const { + return span_context_.id() != 0 && !span_context_.trace_id().empty(); + } + private: zipkin::SpanContext span_context_; mutable std::mutex baggage_mutex_; @@ -271,17 +280,29 @@ class OtSpan : public ot::Span { class OtTracer : public ot::Tracer, public std::enable_shared_from_this { public: - explicit OtTracer(TracerPtr &&tracer) : tracer_{std::move(tracer)} {} + explicit OtTracer(TracerPtr &&tracer) + : tracer_{std::move(tracer)}, sampler_{new ProbabilisticSampler(1.0)} {} + explicit OtTracer(TracerPtr &&tracer, SamplerPtr &&sampler) + : tracer_{std::move(tracer)}, sampler_{std::move(sampler)} {} std::unique_ptr StartSpanWithOptions(string_view operation_name, const ot::StartSpanOptions &options) const noexcept override { + // Create the core zipkin span. SpanPtr span{new zipkin::Span{}}; span->setName(operation_name); span->setTracer(tracer_.get()); + auto parent = findSpanContext(options.references); + + if (parent && parent->isValid()) { + span->setSampled(parent->isSampled()); + } else { + span->setSampled(sampler_->ShouldSample()); + } + Endpoint endpoint{tracer_->serviceName(), tracer_->address()}; // Add a binary annotation for the serviceName. @@ -329,6 +350,7 @@ class OtTracer : public ot::Tracer, private: TracerPtr tracer_; + SamplerPtr sampler_; template expected InjectImpl(const ot::SpanContext &sc, Carrier &writer) const @@ -368,7 +390,8 @@ makeZipkinOtTracer(const ZipkinOtTracerOptions &options, std::unique_ptr &&reporter) { TracerPtr tracer{new Tracer{options.service_name, options.service_address}}; tracer->setReporter(std::move(reporter)); - return std::shared_ptr{new OtTracer{std::move(tracer)}}; + SamplerPtr sampler{new ProbabilisticSampler{options.sample_rate}}; + return std::make_shared(std::move(tracer), std::move(sampler)); } std::shared_ptr diff --git a/zipkin_opentracing/src/propagation.cc b/zipkin_opentracing/src/propagation.cc index 1eb5aae..29fb734 100644 --- a/zipkin_opentracing/src/propagation.cc +++ b/zipkin_opentracing/src/propagation.cc @@ -64,7 +64,7 @@ injectSpanContext(const opentracing::TextMapWriter &carrier, if (!result) { return result; } - result = carrier.Set(zipkin_sampled, "1"); + result = carrier.Set(zipkin_sampled, span_context.isSampled() ? "1" : "0"); if (!result) { return result; } @@ -135,7 +135,9 @@ extractSpanContext(const opentracing::TextMapReader &carrier, if (!parseBool(value, sampled)) { return ot::make_unexpected(ot::span_context_corrupted_error); } - flags |= sampled_flag; + if (sampled) { + flags |= sampled_flag; + } } else if (keyCompare(key, zipkin_parent_span_id)) { parent_id = Hex::hexToTraceId(value); if (!parent_id.valid()) { diff --git a/zipkin_opentracing/src/sampling.cc b/zipkin_opentracing/src/sampling.cc new file mode 100644 index 0000000..309e964 --- /dev/null +++ b/zipkin_opentracing/src/sampling.cc @@ -0,0 +1,10 @@ +#include "sampling.h" +#include + +namespace zipkin { +bool ProbabilisticSampler::ShouldSample() { + static thread_local std::mt19937 rng(std::random_device{}()); + std::bernoulli_distribution dist(sample_rate_); + return dist(rng); +} +} // namespace zipkin \ No newline at end of file diff --git a/zipkin_opentracing/src/sampling.h b/zipkin_opentracing/src/sampling.h new file mode 100644 index 0000000..770cc40 --- /dev/null +++ b/zipkin_opentracing/src/sampling.h @@ -0,0 +1,23 @@ +#pragma once +#include +#include + +namespace zipkin { +class Sampler { +public: + virtual ~Sampler() = default; + virtual bool ShouldSample() = 0; +}; + +class ProbabilisticSampler : public Sampler { +public: + explicit ProbabilisticSampler(double sample_rate) + : sample_rate_(std::max(0.0, std::min(sample_rate, 1.0))){}; + bool ShouldSample() override; + +private: + double sample_rate_; +}; + +typedef std::unique_ptr SamplerPtr; +} // namespace zipkin \ No newline at end of file diff --git a/zipkin_opentracing/src/tracer_factory.cc b/zipkin_opentracing/src/tracer_factory.cc index 34cb39f..d1f2a02 100644 --- a/zipkin_opentracing/src/tracer_factory.cc +++ b/zipkin_opentracing/src/tracer_factory.cc @@ -37,6 +37,11 @@ const char *const configuration_schema = R"( "description": "The maximum number of spans to buffer before sending them to the collector", "minimum": 1 + }, + "sample_rate": { + "type": "float", + "minimum": 0.0, + "maxiumum": 1.0 } } } @@ -91,6 +96,9 @@ OtTracerFactory::MakeTracer(const char *configuration, if (document.HasMember("max_buffered_spans")) { options.max_buffered_spans = document["max_buffered_spans"].GetInt(); } + if (document.HasMember("sample_rate")) { + options.sample_rate = document["sample_rate"].GetDouble(); + } return makeZipkinOtTracer(options); } catch (const std::bad_alloc &) { return opentracing::make_unexpected( diff --git a/zipkin_opentracing/test/ot_tracer_test.cc b/zipkin_opentracing/test/ot_tracer_test.cc index bfddf78..4a0868f 100644 --- a/zipkin_opentracing/test/ot_tracer_test.cc +++ b/zipkin_opentracing/test/ot_tracer_test.cc @@ -1,3 +1,4 @@ +#include "../src/sampling.h" #include "../src/utility.h" #include "in_memory_reporter.h" #include @@ -55,6 +56,45 @@ TEST_CASE("ot_tracer") { CHECK(hasTag(span, "xyz", true)); } + SECTION("Uses sampling rate to determine whether to sample a span.") { + auto r = new InMemoryReporter(); + + ZipkinOtTracerOptions options; + options.sample_rate = 0.0; + auto t = makeZipkinOtTracer(options, std::unique_ptr(r)); + + auto span_a = t->StartSpan("a"); + CHECK(span_a); + span_a->Finish(); + + CHECK(r->spans().empty()); + } + + SECTION("Propagates sampling decision to child span") { + ZipkinOtTracerOptions no_sampling; + no_sampling.sample_rate = 0.0; + auto r1 = new InMemoryReporter(); + auto no_sampling_tracer = + makeZipkinOtTracer(no_sampling, std::unique_ptr(r1)); + + ZipkinOtTracerOptions always_sample; + always_sample.sample_rate = 1.0; + auto r2 = new InMemoryReporter(); + auto sampling_tracer = + makeZipkinOtTracer(always_sample, std::unique_ptr(r2)); + + auto span_a = no_sampling_tracer->StartSpan("a"); + CHECK(span_a); + span_a->Finish(); + auto span_b = + sampling_tracer->StartSpan("b", {ChildOf(&span_a->context())}); + CHECK(span_b); + span_b->Finish(); + + CHECK(r1->spans().empty()); + CHECK(r2->spans().empty()); + } + SECTION("You can set a single child-of reference when starting a span.") { auto span_a = tracer->StartSpan("a"); CHECK(span_a);