From 40906690e0595a96e1b99f89044d2b19b546f97b Mon Sep 17 00:00:00 2001 From: Oliver Zhang Date: Mon, 29 Jun 2020 17:26:32 +0000 Subject: [PATCH 01/10] add header --- .../sdk/trace/samplers/parent_or_else.h | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h diff --git a/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h b/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h new file mode 100644 index 0000000000..0547068280 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h @@ -0,0 +1,41 @@ +#pragma once + +#include "opentelemetry/sdk/trace/sampler.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace trace +{ +namespace trace_api = opentelemetry::trace; +/** + * The always off sampler always returns NOT_RECORD, effectively disabling + * tracing functionality. + */ +class AlwaysOffSampler : public Sampler +{ +public: + /** + * @return Returns NOT_RECORD always + */ + SamplingResult ShouldSample( + const SpanContext * /*parent_context*/, + trace_api::TraceId /*trace_id*/, + nostd::string_view /*name*/, + trace_api::SpanKind /*span_kind*/, + const trace_api::KeyValueIterable & /*attributes*/) noexcept override + { + return { Decision::NOT_RECORD, nullptr }; + } + + /** + * @return Description MUST be AlwaysOffSampler + */ + std::string GetDescription() const noexcept override + { + return "AlwaysOffSampler"; + } +}; +} // namespace trace +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE From 7843d59780af8e48067856578750aed279812c06 Mon Sep 17 00:00:00 2001 From: Oliver Zhang Date: Mon, 29 Jun 2020 19:33:24 +0000 Subject: [PATCH 02/10] add test case --- .../sdk/trace/samplers/parent_or_else.h | 48 +++++++++++------- sdk/src/trace/samplers/parent_or_else.cc | 50 +++++++++++++++++++ sdk/test/trace/parent_or_else_sampler_test.cc | 30 +++++++++++ 3 files changed, 109 insertions(+), 19 deletions(-) create mode 100644 sdk/src/trace/samplers/parent_or_else.cc create mode 100644 sdk/test/trace/parent_or_else_sampler_test.cc diff --git a/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h b/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h index 0547068280..f3cbab78f0 100644 --- a/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h +++ b/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h @@ -1,5 +1,6 @@ #pragma once +#include "opentelemetry/sdk/common/atomic_shared_ptr.h" #include "opentelemetry/sdk/trace/sampler.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -8,33 +9,42 @@ namespace sdk namespace trace { namespace trace_api = opentelemetry::trace; + +// a placeholder class +class Sampler::SpanContext +{ +public: + inline explicit SpanContext(bool is_recording, bool sampled_flag) + : is_recording(is_recording), sampled_flag(sampled_flag) {} + + bool is_recording; + bool sampled_flag; +}; /** - * The always off sampler always returns NOT_RECORD, effectively disabling - * tracing functionality. + * The parent or else sampler is a composite sampler. ParentOrElse(delegateSampler) either respects + * the parent span's sampling decision or delegates to delegateSampler for root spans. */ -class AlwaysOffSampler : public Sampler +class ParentOrElseSampler : public Sampler { public: - /** + explicit ParentOrElseSampler(std::shared_ptr delegate_sampler) noexcept; + /** The decision either respects the parent span's sampling decision or delegates to + * delegateSampler for root spans * @return Returns NOT_RECORD always */ - SamplingResult ShouldSample( - const SpanContext * /*parent_context*/, - trace_api::TraceId /*trace_id*/, - nostd::string_view /*name*/, - trace_api::SpanKind /*span_kind*/, - const trace_api::KeyValueIterable & /*attributes*/) noexcept override - { - return { Decision::NOT_RECORD, nullptr }; - } - + SamplingResult ShouldSample(const SpanContext * /*parent_context*/, + trace_api::TraceId /*trace_id*/, + nostd::string_view /*name*/, + trace_api::SpanKind /*span_kind*/, + const trace_api::KeyValueIterable & /*attributes*/) noexcept override; + /** - * @return Description MUST be AlwaysOffSampler + * @return Description MUST be ParentOrElse{delegate_sampler_.getDescription()} */ - std::string GetDescription() const noexcept override - { - return "AlwaysOffSampler"; - } + std::string GetDescription() const noexcept override; + +private: + std::shared_ptr delegate_sampler_; }; } // namespace trace } // namespace sdk diff --git a/sdk/src/trace/samplers/parent_or_else.cc b/sdk/src/trace/samplers/parent_or_else.cc new file mode 100644 index 0000000000..2ab64b633c --- /dev/null +++ b/sdk/src/trace/samplers/parent_or_else.cc @@ -0,0 +1,50 @@ +#pragma once + +#include "opentelemetry/sdk/trace/samplers/parent_or_else.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace trace +{ +ParentOrElseSampler::ParentOrElseSampler(std::shared_ptr delegate_sampler) noexcept + : delegate_sampler_(delegate_sampler) +{} + +SamplingResult ParentOrElseSampler::ShouldSample( + const SpanContext *parent_context, + trace_api::TraceId trace_id, + nostd::string_view name, + trace_api::SpanKind span_kind, + const trace_api::KeyValueIterable &attributes) noexcept +{ + if (parent_context == nullptr) + { + // If no parent (root span) exists returns the result of the delegateSampler + return delegate_sampler_->ShouldSample(parent_context, trace_id, name, span_kind, attributes); + } + else + { + // If parent exists: + if (parent_context->sampled_flag) + { + return { Decision::RECORD_AND_SAMPLED, nullptr } + } + else + { + return { Decision::NOT_RECORD, nullptr } + } + } +} + +std::string ParentOrElseSampler::GetDescription() const noexcept +{ + // use += to concatenate string for speed + std::string description = "ParentOrElse{"; + description += delegate_sampler_->GetDescription(); + description += "}"; + return description; +} +} // namespace trace +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE \ No newline at end of file diff --git a/sdk/test/trace/parent_or_else_sampler_test.cc b/sdk/test/trace/parent_or_else_sampler_test.cc new file mode 100644 index 0000000000..f59a11fe1c --- /dev/null +++ b/sdk/test/trace/parent_or_else_sampler_test.cc @@ -0,0 +1,30 @@ +#include "opentelemetry/sdk/trace/samplers/parent_or_else.h" + +#include + +using opentelemetry::sdk::trace::ParentOrElseSampler; +using opentelemetry::sdk::trace::Decision; + +TEST(ParentOrElseSampler, ShouldSample) +{ + ParentOrElseSampler sampler; + + opentelemetry::trace::TraceId trace_id; + opentelemetry::trace::SpanKind span_kind = opentelemetry::trace::SpanKind::kInternal; + + using M = std::map; + M m1 = {{}}; + opentelemetry::trace::KeyValueIterableView view{m1}; + + auto sampling_result = sampler.ShouldSample(nullptr, trace_id, "", span_kind, view); + + ASSERT_EQ(Decision::NOT_RECORD, sampling_result.decision); + ASSERT_EQ(nullptr, sampling_result.attributes); +} + +TEST(ParentOrElseSampler, GetDescription) +{ + ParentOrElseSampler sampler; + + ASSERT_EQ("AlwaysOffSampler", sampler.GetDescription()); +} From d2cdef3fbc75b58902a79cf1f6afb7fbe9fedfac Mon Sep 17 00:00:00 2001 From: Oliver Zhang Date: Tue, 30 Jun 2020 00:29:45 +0000 Subject: [PATCH 03/10] add testcase --- sdk/src/trace/samplers/parent_or_else.cc | 6 ++-- sdk/test/trace/BUILD | 15 +++++++-- sdk/test/trace/parent_or_else_sampler_test.cc | 32 +++++++++++++++---- 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/sdk/src/trace/samplers/parent_or_else.cc b/sdk/src/trace/samplers/parent_or_else.cc index 2ab64b633c..4586d1510c 100644 --- a/sdk/src/trace/samplers/parent_or_else.cc +++ b/sdk/src/trace/samplers/parent_or_else.cc @@ -1,5 +1,3 @@ -#pragma once - #include "opentelemetry/sdk/trace/samplers/parent_or_else.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -28,11 +26,11 @@ SamplingResult ParentOrElseSampler::ShouldSample( // If parent exists: if (parent_context->sampled_flag) { - return { Decision::RECORD_AND_SAMPLED, nullptr } + return { Decision::RECORD_AND_SAMPLE, nullptr }; } else { - return { Decision::NOT_RECORD, nullptr } + return { Decision::NOT_RECORD, nullptr }; } } } diff --git a/sdk/test/trace/BUILD b/sdk/test/trace/BUILD index 37018d7eeb..a3c3c77f91 100644 --- a/sdk/test/trace/BUILD +++ b/sdk/test/trace/BUILD @@ -53,8 +53,6 @@ cc_test( ], ) - - cc_test( name = "always_off_sampler_test", srcs = [ @@ -64,4 +62,15 @@ cc_test( "//sdk/src/trace", "@com_google_googletest//:gtest_main", ], -) \ No newline at end of file +) + +cc_test( + name = "parent_or_else_sampler_test", + srcs = [ + "parent_or_else_sampler_test.cc", + ], + deps = [ + "//sdk/src/trace", + "@com_google_googletest//:gtest_main", + ], +) diff --git a/sdk/test/trace/parent_or_else_sampler_test.cc b/sdk/test/trace/parent_or_else_sampler_test.cc index f59a11fe1c..86157f3891 100644 --- a/sdk/test/trace/parent_or_else_sampler_test.cc +++ b/sdk/test/trace/parent_or_else_sampler_test.cc @@ -1,30 +1,48 @@ #include "opentelemetry/sdk/trace/samplers/parent_or_else.h" - +#include "opentelemetry/sdk/trace/samplers/always_on.h" +#include "opentelemetry/sdk/trace/samplers/always_off.h" #include +#include using opentelemetry::sdk::trace::ParentOrElseSampler; +using opentelemetry::sdk::trace::AlwaysOnSampler; +using opentelemetry::sdk::trace::AlwaysOffSampler; using opentelemetry::sdk::trace::Decision; TEST(ParentOrElseSampler, ShouldSample) { - ParentOrElseSampler sampler; + // Initialize a sampler with a AlwaysOffSampler as its delegateSampler + ParentOrElseSampler sampler_off(std::make_shared()); + // Initialize a sampler with a AlwaysOnSampler as its delegateSampler + ParentOrElseSampler sampler_on(std::make_shared()); + // Set up parameters opentelemetry::trace::TraceId trace_id; opentelemetry::trace::SpanKind span_kind = opentelemetry::trace::SpanKind::kInternal; - using M = std::map; M m1 = {{}}; opentelemetry::trace::KeyValueIterableView view{m1}; + opentelemetry::sdk::trace::Sampler::SpanContext parent_context_sampled(true, true); + opentelemetry::sdk::trace::Sampler::SpanContext parent_context_nonsampled(true, false); - auto sampling_result = sampler.ShouldSample(nullptr, trace_id, "", span_kind, view); + // Case 1: Parent doesn't exist. Return result of delegateSampler() + auto sampling_result = sampler_off.ShouldSample(nullptr, trace_id, "", span_kind, view); ASSERT_EQ(Decision::NOT_RECORD, sampling_result.decision); - ASSERT_EQ(nullptr, sampling_result.attributes); + + // Case 2: Parent exists and SampledFlag is true + auto sampling_result2 = sampler_off.ShouldSample(&parent_context_sampled, trace_id, "", span_kind, view); + ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result2.decision); + + // Case 3: Parent exists and SampledFlag is false + + auto sampling_result3 = sampler_on.ShouldSample(&parent_context_nonsampled, trace_id, "", span_kind, view); + ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result2.decision); } TEST(ParentOrElseSampler, GetDescription) { - ParentOrElseSampler sampler; + ParentOrElseSampler sampler(std::make_shared()); - ASSERT_EQ("AlwaysOffSampler", sampler.GetDescription()); + ASSERT_EQ("ParentOrElse{AlwaysOffSampler}", sampler.GetDescription()); } From be589b98008a574aa9f016c61d75eaddb2e60761 Mon Sep 17 00:00:00 2001 From: Oliver Zhang Date: Tue, 30 Jun 2020 21:49:44 +0000 Subject: [PATCH 04/10] add test case to cmakelists --- sdk/test/trace/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/test/trace/CMakeLists.txt b/sdk/test/trace/CMakeLists.txt index 75bfd4f97a..b4524da0f2 100644 --- a/sdk/test/trace/CMakeLists.txt +++ b/sdk/test/trace/CMakeLists.txt @@ -1,5 +1,6 @@ foreach(testname tracer_provider_test span_data_test simple_processor_test - tracer_test always_off_sampler_test always_on_sampler_test) + tracer_test always_off_sampler_test always_on_sampler_test + parent_or_else_sampler_test) add_executable(${testname} "${testname}.cc") target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} opentelemetry_trace) From a25152331adff849902e069abf546e5837c7b50e Mon Sep 17 00:00:00 2001 From: Oliver Zhang Date: Tue, 30 Jun 2020 21:58:48 +0000 Subject: [PATCH 05/10] add test case to cmakelists --- sdk/src/trace/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/trace/CMakeLists.txt b/sdk/src/trace/CMakeLists.txt index 455665be68..98f949997e 100644 --- a/sdk/src/trace/CMakeLists.txt +++ b/sdk/src/trace/CMakeLists.txt @@ -1 +1 @@ -add_library(opentelemetry_trace tracer_provider.cc tracer.cc span.cc) +add_library(opentelemetry_trace tracer_provider.cc tracer.cc span.cc samplers/parent_or_else.cc) From da9c643a2e3c019c6b9fb56921111dec88a2d29b Mon Sep 17 00:00:00 2001 From: Oliver Zhang Date: Wed, 1 Jul 2020 22:52:57 +0000 Subject: [PATCH 06/10] add comments --- .../sdk/trace/samplers/parent_or_else.h | 9 ++- sdk/src/trace/samplers/parent_or_else.cc | 13 ++-- sdk/test/trace/parent_or_else_sampler_test.cc | 69 ++++++++++--------- 3 files changed, 47 insertions(+), 44 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h b/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h index f3cbab78f0..4994fd1545 100644 --- a/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h +++ b/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h @@ -10,12 +10,16 @@ namespace trace { namespace trace_api = opentelemetry::trace; -// a placeholder class +/** + * A placeholder class that stores the states of a span. + * Will be replaced by the real SpanContext class once it is implemented. + */ class Sampler::SpanContext { public: inline explicit SpanContext(bool is_recording, bool sampled_flag) - : is_recording(is_recording), sampled_flag(sampled_flag) {} + : is_recording(is_recording), sampled_flag(sampled_flag) + {} bool is_recording; bool sampled_flag; @@ -45,6 +49,7 @@ class ParentOrElseSampler : public Sampler private: std::shared_ptr delegate_sampler_; + std::string description_; }; } // namespace trace } // namespace sdk diff --git a/sdk/src/trace/samplers/parent_or_else.cc b/sdk/src/trace/samplers/parent_or_else.cc index 4586d1510c..24c1016fda 100644 --- a/sdk/src/trace/samplers/parent_or_else.cc +++ b/sdk/src/trace/samplers/parent_or_else.cc @@ -6,7 +6,8 @@ namespace sdk namespace trace { ParentOrElseSampler::ParentOrElseSampler(std::shared_ptr delegate_sampler) noexcept - : delegate_sampler_(delegate_sampler) + : delegate_sampler_(delegate_sampler), + description_("ParentOrElse{" + delegate_sampler->GetDescription() + "}") {} SamplingResult ParentOrElseSampler::ShouldSample( @@ -26,22 +27,18 @@ SamplingResult ParentOrElseSampler::ShouldSample( // If parent exists: if (parent_context->sampled_flag) { - return { Decision::RECORD_AND_SAMPLE, nullptr }; + return {Decision::RECORD_AND_SAMPLE, nullptr}; } else { - return { Decision::NOT_RECORD, nullptr }; + return {Decision::NOT_RECORD, nullptr}; } } } std::string ParentOrElseSampler::GetDescription() const noexcept { - // use += to concatenate string for speed - std::string description = "ParentOrElse{"; - description += delegate_sampler_->GetDescription(); - description += "}"; - return description; + return description_; } } // namespace trace } // namespace sdk diff --git a/sdk/test/trace/parent_or_else_sampler_test.cc b/sdk/test/trace/parent_or_else_sampler_test.cc index 86157f3891..42c84e6744 100644 --- a/sdk/test/trace/parent_or_else_sampler_test.cc +++ b/sdk/test/trace/parent_or_else_sampler_test.cc @@ -1,48 +1,49 @@ -#include "opentelemetry/sdk/trace/samplers/parent_or_else.h" -#include "opentelemetry/sdk/trace/samplers/always_on.h" -#include "opentelemetry/sdk/trace/samplers/always_off.h" #include #include +#include "opentelemetry/sdk/trace/samplers/always_off.h" +#include "opentelemetry/sdk/trace/samplers/always_on.h" +#include "opentelemetry/sdk/trace/samplers/parent_or_else.h" -using opentelemetry::sdk::trace::ParentOrElseSampler; -using opentelemetry::sdk::trace::AlwaysOnSampler; using opentelemetry::sdk::trace::AlwaysOffSampler; +using opentelemetry::sdk::trace::AlwaysOnSampler; using opentelemetry::sdk::trace::Decision; +using opentelemetry::sdk::trace::ParentOrElseSampler; TEST(ParentOrElseSampler, ShouldSample) { - // Initialize a sampler with a AlwaysOffSampler as its delegateSampler - ParentOrElseSampler sampler_off(std::make_shared()); - // Initialize a sampler with a AlwaysOnSampler as its delegateSampler - ParentOrElseSampler sampler_on(std::make_shared()); - - // Set up parameters - opentelemetry::trace::TraceId trace_id; - opentelemetry::trace::SpanKind span_kind = opentelemetry::trace::SpanKind::kInternal; - using M = std::map; - M m1 = {{}}; - opentelemetry::trace::KeyValueIterableView view{m1}; - opentelemetry::sdk::trace::Sampler::SpanContext parent_context_sampled(true, true); - opentelemetry::sdk::trace::Sampler::SpanContext parent_context_nonsampled(true, false); - - // Case 1: Parent doesn't exist. Return result of delegateSampler() - auto sampling_result = sampler_off.ShouldSample(nullptr, trace_id, "", span_kind, view); - - ASSERT_EQ(Decision::NOT_RECORD, sampling_result.decision); - - // Case 2: Parent exists and SampledFlag is true - auto sampling_result2 = sampler_off.ShouldSample(&parent_context_sampled, trace_id, "", span_kind, view); - ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result2.decision); - - // Case 3: Parent exists and SampledFlag is false - - auto sampling_result3 = sampler_on.ShouldSample(&parent_context_nonsampled, trace_id, "", span_kind, view); - ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result2.decision); + // Initialize a sampler with a AlwaysOffSampler as its delegateSampler + ParentOrElseSampler sampler_off(std::make_shared()); + // Initialize a sampler with a AlwaysOnSampler as its delegateSampler + ParentOrElseSampler sampler_on(std::make_shared()); + + // Set up parameters + opentelemetry::trace::TraceId trace_id; + opentelemetry::trace::SpanKind span_kind = opentelemetry::trace::SpanKind::kInternal; + using M = std::map; + M m1 = {{}}; + opentelemetry::trace::KeyValueIterableView view{m1}; + opentelemetry::sdk::trace::Sampler::SpanContext parent_context_sampled(true, true); + opentelemetry::sdk::trace::Sampler::SpanContext parent_context_nonsampled(true, false); + + // Case 1: Parent doesn't exist. Return result of delegateSampler() + auto sampling_result = sampler_off.ShouldSample(nullptr, trace_id, "", span_kind, view); + + ASSERT_EQ(Decision::NOT_RECORD, sampling_result.decision); + + // Case 2: Parent exists and SampledFlag is true + auto sampling_result2 = + sampler_off.ShouldSample(&parent_context_sampled, trace_id, "", span_kind, view); + ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result2.decision); + + // Case 3: Parent exists and SampledFlag is false + auto sampling_result3 = + sampler_on.ShouldSample(&parent_context_nonsampled, trace_id, "", span_kind, view); + ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result2.decision); } TEST(ParentOrElseSampler, GetDescription) { - ParentOrElseSampler sampler(std::make_shared()); + ParentOrElseSampler sampler(std::make_shared()); - ASSERT_EQ("ParentOrElse{AlwaysOffSampler}", sampler.GetDescription()); + ASSERT_EQ("ParentOrElse{AlwaysOffSampler}", sampler.GetDescription()); } From a91663cd8b3fdd445ee1be0fda2528ebf778cba5 Mon Sep 17 00:00:00 2001 From: Oliver Zhang Date: Mon, 6 Jul 2020 18:46:54 +0000 Subject: [PATCH 07/10] modify test case --- .../sdk/trace/samplers/parent_or_else.h | 10 +++++----- sdk/test/trace/parent_or_else_sampler_test.cc | 15 ++++++++------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h b/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h index 4994fd1545..07d4b9a818 100644 --- a/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h +++ b/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h @@ -36,11 +36,11 @@ class ParentOrElseSampler : public Sampler * delegateSampler for root spans * @return Returns NOT_RECORD always */ - SamplingResult ShouldSample(const SpanContext * /*parent_context*/, - trace_api::TraceId /*trace_id*/, - nostd::string_view /*name*/, - trace_api::SpanKind /*span_kind*/, - const trace_api::KeyValueIterable & /*attributes*/) noexcept override; + SamplingResult ShouldSample(const SpanContext * parent_context, + trace_api::TraceId trace_id, + nostd::string_view name, + trace_api::SpanKind span_kind, + const trace_api::KeyValueIterable & attributes) noexcept override; /** * @return Description MUST be ParentOrElse{delegate_sampler_.getDescription()} diff --git a/sdk/test/trace/parent_or_else_sampler_test.cc b/sdk/test/trace/parent_or_else_sampler_test.cc index 42c84e6744..152121f4c3 100644 --- a/sdk/test/trace/parent_or_else_sampler_test.cc +++ b/sdk/test/trace/parent_or_else_sampler_test.cc @@ -11,9 +11,7 @@ using opentelemetry::sdk::trace::ParentOrElseSampler; TEST(ParentOrElseSampler, ShouldSample) { - // Initialize a sampler with a AlwaysOffSampler as its delegateSampler ParentOrElseSampler sampler_off(std::make_shared()); - // Initialize a sampler with a AlwaysOnSampler as its delegateSampler ParentOrElseSampler sampler_on(std::make_shared()); // Set up parameters @@ -27,23 +25,26 @@ TEST(ParentOrElseSampler, ShouldSample) // Case 1: Parent doesn't exist. Return result of delegateSampler() auto sampling_result = sampler_off.ShouldSample(nullptr, trace_id, "", span_kind, view); + auto sampling_result2 = sampler_on.ShouldSample(nullptr, trace_id, "", span_kind, view); ASSERT_EQ(Decision::NOT_RECORD, sampling_result.decision); + ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result2.decision); // Case 2: Parent exists and SampledFlag is true - auto sampling_result2 = + auto sampling_result3 = sampler_off.ShouldSample(&parent_context_sampled, trace_id, "", span_kind, view); - ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result2.decision); + ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result3.decision); // Case 3: Parent exists and SampledFlag is false - auto sampling_result3 = + auto sampling_result4 = sampler_on.ShouldSample(&parent_context_nonsampled, trace_id, "", span_kind, view); - ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result2.decision); + ASSERT_EQ(Decision::NOT_RECORD, sampling_result4.decision); } TEST(ParentOrElseSampler, GetDescription) { ParentOrElseSampler sampler(std::make_shared()); - ASSERT_EQ("ParentOrElse{AlwaysOffSampler}", sampler.GetDescription()); + ParentOrElseSampler sampler2(std::make_shared()); + ASSERT_EQ("ParentOrElse{AlwaysOnSampler}", sampler2.GetDescription()); } From 822144ad59da39b9b76da7bf4ae6f0969eaf90bf Mon Sep 17 00:00:00 2001 From: Oliver Zhang Date: Thu, 9 Jul 2020 19:38:45 +0000 Subject: [PATCH 08/10] minor change --- .../sdk/trace/samplers/parent_or_else.h | 4 ++-- sdk/src/trace/samplers/parent_or_else.cc | 16 ++++++---------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h b/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h index 07d4b9a818..044cd9816a 100644 --- a/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h +++ b/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h @@ -48,8 +48,8 @@ class ParentOrElseSampler : public Sampler std::string GetDescription() const noexcept override; private: - std::shared_ptr delegate_sampler_; - std::string description_; + const std::shared_ptr delegate_sampler_; + const std::string description_; }; } // namespace trace } // namespace sdk diff --git a/sdk/src/trace/samplers/parent_or_else.cc b/sdk/src/trace/samplers/parent_or_else.cc index 24c1016fda..411de491a3 100644 --- a/sdk/src/trace/samplers/parent_or_else.cc +++ b/sdk/src/trace/samplers/parent_or_else.cc @@ -22,18 +22,14 @@ SamplingResult ParentOrElseSampler::ShouldSample( // If no parent (root span) exists returns the result of the delegateSampler return delegate_sampler_->ShouldSample(parent_context, trace_id, name, span_kind, attributes); } - else + + // If parent exists: + if (parent_context->sampled_flag) { - // If parent exists: - if (parent_context->sampled_flag) - { - return {Decision::RECORD_AND_SAMPLE, nullptr}; - } - else - { - return {Decision::NOT_RECORD, nullptr}; - } + return {Decision::RECORD_AND_SAMPLE, nullptr}; } + + return {Decision::NOT_RECORD, nullptr}; } std::string ParentOrElseSampler::GetDescription() const noexcept From 1eef1980dbec0356322dd9b8cbf84fd9ff6c03a8 Mon Sep 17 00:00:00 2001 From: Oliver Zhang Date: Thu, 9 Jul 2020 21:21:31 +0000 Subject: [PATCH 09/10] add eof --- sdk/src/trace/samplers/parent_or_else.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/trace/samplers/parent_or_else.cc b/sdk/src/trace/samplers/parent_or_else.cc index 411de491a3..fd0d909e84 100644 --- a/sdk/src/trace/samplers/parent_or_else.cc +++ b/sdk/src/trace/samplers/parent_or_else.cc @@ -38,4 +38,4 @@ std::string ParentOrElseSampler::GetDescription() const noexcept } } // namespace trace } // namespace sdk -OPENTELEMETRY_END_NAMESPACE \ No newline at end of file +OPENTELEMETRY_END_NAMESPACE From fc6947781a040d2f3b56d3a3c22c72ae212a82da Mon Sep 17 00:00:00 2001 From: Oliver Zhang Date: Thu, 9 Jul 2020 21:22:42 +0000 Subject: [PATCH 10/10] add newline --- sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h b/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h index 044cd9816a..c562277957 100644 --- a/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h +++ b/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h @@ -24,6 +24,7 @@ class Sampler::SpanContext bool is_recording; bool sampled_flag; }; + /** * The parent or else sampler is a composite sampler. ParentOrElse(delegateSampler) either respects * the parent span's sampling decision or delegates to delegateSampler for root spans.