From 812e5895f1f1485d0927b95bab56e6b2daa3f6ce Mon Sep 17 00:00:00 2001 From: Nick Holbrook Date: Fri, 10 Jul 2020 09:39:41 -0600 Subject: [PATCH 01/10] Move SpanContext and references --- sdk/include/opentelemetry/sdk/trace/sampler.h | 6 +- .../sdk/trace/samplers/always_off.h | 4 +- .../sdk/trace/samplers/always_on.h | 4 +- .../sdk/trace/samplers/parent_or_else.h | 18 +- .../sdk/trace/samplers/probability.h | 52 ++++ .../opentelemetry/sdk/trace/span_context.h | 26 ++ sdk/src/trace/samplers/parent_or_else.cc | 4 +- sdk/src/trace/samplers/probability.cc | 114 +++++++++ sdk/test/trace/parent_or_else_sampler_test.cc | 5 +- sdk/test/trace/probability_sampler_test.cc | 224 ++++++++++++++++++ 10 files changed, 432 insertions(+), 25 deletions(-) create mode 100644 sdk/include/opentelemetry/sdk/trace/samplers/probability.h create mode 100644 sdk/include/opentelemetry/sdk/trace/span_context.h create mode 100644 sdk/src/trace/samplers/probability.cc create mode 100644 sdk/test/trace/probability_sampler_test.cc diff --git a/sdk/include/opentelemetry/sdk/trace/sampler.h b/sdk/include/opentelemetry/sdk/trace/sampler.h index 27071b458c..8214173895 100644 --- a/sdk/include/opentelemetry/sdk/trace/sampler.h +++ b/sdk/include/opentelemetry/sdk/trace/sampler.h @@ -4,6 +4,7 @@ #include "opentelemetry/trace/span.h" #include "opentelemetry/trace/trace_id.h" #include "opentelemetry/version.h" +#include "opentelemetry/sdk/trace/span_context.h" #include #include @@ -15,6 +16,7 @@ namespace sdk namespace trace { namespace trace_api = opentelemetry::trace; +namespace trace_sdk = opentelemetry::sdk::trace; /** * A sampling Decision for a Span to be created. @@ -48,8 +50,6 @@ struct SamplingResult class Sampler { public: - // TODO: Remove this placeholder with real class - class SpanContext; virtual ~Sampler() = default; /** * Called during Span creation to make a sampling decision. @@ -66,7 +66,7 @@ class Sampler * @since 0.1.0 */ - virtual SamplingResult ShouldSample(const SpanContext *parent_context, + virtual SamplingResult ShouldSample(const trace_sdk::SpanContext *parent_context, trace_api::TraceId trace_id, nostd::string_view name, trace_api::SpanKind span_kind, diff --git a/sdk/include/opentelemetry/sdk/trace/samplers/always_off.h b/sdk/include/opentelemetry/sdk/trace/samplers/always_off.h index 0547068280..4437db9d88 100644 --- a/sdk/include/opentelemetry/sdk/trace/samplers/always_off.h +++ b/sdk/include/opentelemetry/sdk/trace/samplers/always_off.h @@ -8,6 +8,8 @@ namespace sdk namespace trace { namespace trace_api = opentelemetry::trace; +namespace trace_sdk = opentelemetry::sdk::trace; + /** * The always off sampler always returns NOT_RECORD, effectively disabling * tracing functionality. @@ -19,7 +21,7 @@ class AlwaysOffSampler : public Sampler * @return Returns NOT_RECORD always */ SamplingResult ShouldSample( - const SpanContext * /*parent_context*/, + const trace_sdk::SpanContext * /*parent_context*/, trace_api::TraceId /*trace_id*/, nostd::string_view /*name*/, trace_api::SpanKind /*span_kind*/, diff --git a/sdk/include/opentelemetry/sdk/trace/samplers/always_on.h b/sdk/include/opentelemetry/sdk/trace/samplers/always_on.h index e50d15829f..5d227eef13 100644 --- a/sdk/include/opentelemetry/sdk/trace/samplers/always_on.h +++ b/sdk/include/opentelemetry/sdk/trace/samplers/always_on.h @@ -8,6 +8,8 @@ namespace sdk namespace trace { namespace trace_api = opentelemetry::trace; +namespace trace_sdk = opentelemetry::sdk::trace; + /** * The always on sampler is a default sampler which always return Decision::RECORD_AND_SAMPLE */ @@ -17,7 +19,7 @@ class AlwaysOnSampler : public Sampler /** * @return Always return Decision RECORD_AND_SAMPLE */ - inline SamplingResult ShouldSample(const SpanContext * /*parent_context*/, + inline SamplingResult ShouldSample(const trace_sdk::SpanContext * /*parent_context*/, trace_api::TraceId /*trace_id*/, nostd::string_view /*name*/, trace_api::SpanKind /*span_kind*/, 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 c562277957..3d63f256ce 100644 --- a/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h +++ b/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h @@ -9,21 +9,7 @@ namespace sdk namespace trace { namespace trace_api = opentelemetry::trace; - -/** - * 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) - {} - - bool is_recording; - bool sampled_flag; -}; +namespace trace_sdk = opentelemetry::sdk::trace; /** * The parent or else sampler is a composite sampler. ParentOrElse(delegateSampler) either respects @@ -37,7 +23,7 @@ class ParentOrElseSampler : public Sampler * delegateSampler for root spans * @return Returns NOT_RECORD always */ - SamplingResult ShouldSample(const SpanContext * parent_context, + SamplingResult ShouldSample(const trace_sdk::SpanContext * parent_context, trace_api::TraceId trace_id, nostd::string_view name, trace_api::SpanKind span_kind, diff --git a/sdk/include/opentelemetry/sdk/trace/samplers/probability.h b/sdk/include/opentelemetry/sdk/trace/samplers/probability.h new file mode 100644 index 0000000000..966880eab6 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/trace/samplers/probability.h @@ -0,0 +1,52 @@ +#pragma once + +#include "opentelemetry/sdk/trace/sampler.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace trace +{ +namespace trace_api = opentelemetry::trace; +namespace trace_sdk = opentelemetry::sdk::trace; + +/** + * The probability sampler, based on it's configuration, should either defer the + * decision to sample to it's parent, or compute and return a decision based on + * the provided trace_id and probability. + */ +class ProbabilitySampler : public Sampler +{ +public: + /** + * @param probability a required value, 1.0 >= probability >= 0.0, that given any + * random trace_id, ShouldSample will return RECORD_AND_SAMPLE + * @throws invalid_argument if probability is out of bounds [0.0, 1.0] + */ + explicit ProbabilitySampler(double probability); + + /** + * @return Returns either RECORD_AND_SAMPLE or NOT_RECORD based on current + * sampler configuration and provided parent_context / tracer_id. tracer_id + * is used as a pseudorandom value in conjunction with the predefined + * threshold to determine whether this trace should be sampled + */ + SamplingResult ShouldSample( + const trace_sdk::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 ProbabilitySampler{0.000100} + */ + std::string GetDescription() const noexcept override; + +private: + std::string sampler_description_; + const uint64_t threshold_; +}; +} // namespace trace +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/trace/span_context.h b/sdk/include/opentelemetry/sdk/trace/span_context.h new file mode 100644 index 0000000000..7f79111263 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/trace/span_context.h @@ -0,0 +1,26 @@ +#pragma once + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace trace +{ +namespace trace_api = opentelemetry::trace; + +// TODO: Remove this placeholder with real class +class SpanContext +{ +public: +explicit SpanContext(bool sampled_flag, bool is_remote) + : traceFlags(sampled_flag), is_remote(is_remote) {} + +uint8_t traceFlags; +bool is_remote; + +const uint8_t FLAG_SAMPLED = 1; // 00000001 + +bool sampled = (traceFlags & FLAG_SAMPLED) == FLAG_SAMPLED; +}; +} // namespace trace +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE \ No newline at end of file diff --git a/sdk/src/trace/samplers/parent_or_else.cc b/sdk/src/trace/samplers/parent_or_else.cc index fd0d909e84..e1bdc51e69 100644 --- a/sdk/src/trace/samplers/parent_or_else.cc +++ b/sdk/src/trace/samplers/parent_or_else.cc @@ -11,7 +11,7 @@ ParentOrElseSampler::ParentOrElseSampler(std::shared_ptr delegate_sampl {} SamplingResult ParentOrElseSampler::ShouldSample( - const SpanContext *parent_context, + const trace_sdk::SpanContext *parent_context, trace_api::TraceId trace_id, nostd::string_view name, trace_api::SpanKind span_kind, @@ -24,7 +24,7 @@ SamplingResult ParentOrElseSampler::ShouldSample( } // If parent exists: - if (parent_context->sampled_flag) + if (parent_context->sampled) { return {Decision::RECORD_AND_SAMPLE, nullptr}; } diff --git a/sdk/src/trace/samplers/probability.cc b/sdk/src/trace/samplers/probability.cc new file mode 100644 index 0000000000..29795de6c3 --- /dev/null +++ b/sdk/src/trace/samplers/probability.cc @@ -0,0 +1,114 @@ +// Copyright 2020, Open Telemetry Authors +// Copyright 2017, OpenCensus 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 "opentelemetry/sdk/trace/samplers/probability.h" + +#include +#include +#include + +namespace +{ + namespace trace_api = opentelemetry::trace; + + /** + * Converts a probability in [0, 1] to a threshold in [0, UINT64_MAX] + * + * @param probability a required value top be converted to uint64_t. is + * bounded by 1 >= probability >= 0. + * @return Returns threshold value computed after converting probability to + * uint64_t datatype + */ + uint64_t CalculateThreshold(double probability) noexcept + { + if (probability <= 0.0) return 0; + if (probability >= 1.0) return UINT64_MAX; + + // We can't directly return probability * UINT64_MAX. + // + // UINT64_MAX is (2^64)-1, but as a double rounds up to 2^64. + // For probabilities >= 1-(2^-54), the product wraps to zero! + // Instead, calculate the high and low 32 bits separately. + const double product = UINT32_MAX * probability; + double hi_bits, lo_bits = ldexp(modf(product, &hi_bits), 32) + product; + return (static_cast(hi_bits) << 32) + + static_cast(lo_bits); + } + + /** + * @param trace_id a required value to be converted to uint64_t. trace_id must + * at least 8 bytes long + * @return Returns threshold value computed after converting trace_id to + * uint64_t datatype + */ + uint64_t CalculateThresholdFromBuffer(const trace_api::TraceId& trace_id) noexcept + { + // We only use the first 8 bytes of TraceId. + static_assert(trace_api::TraceId::kSize >= 8, "TraceID must be at least 8 bytes long."); + + uint64_t res = 0; + std::memcpy(&res, &trace_id, 8); + + double probability = (double) res / UINT64_MAX; + + return CalculateThreshold(probability); + } +} // namespace + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace trace +{ +ProbabilitySampler::ProbabilitySampler(double probability) +: threshold_(CalculateThreshold(probability)) +{ + if (probability > 1.0) probability = 1.0; + if (probability < 0.0) probability = 0.0; + sampler_description_ = "ProbabilitySampler{" + std::to_string(probability) + "}"; + } + +SamplingResult ProbabilitySampler::ShouldSample( + const trace_sdk::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 && !parent_context->is_remote) { + if (parent_context->sampled) { + return { Decision::RECORD_AND_SAMPLE, nullptr }; + } else { + return { Decision::NOT_RECORD, nullptr }; + } + } + + if (threshold_ == 0) return { Decision::NOT_RECORD, nullptr }; + + if (CalculateThresholdFromBuffer(trace_id) <= threshold_) + { + return { Decision::RECORD_AND_SAMPLE, nullptr }; + } + + return { Decision::NOT_RECORD, nullptr }; +} + +std::string ProbabilitySampler::GetDescription() const noexcept +{ + return sampler_description_; +} +} // namespace trace +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/test/trace/parent_or_else_sampler_test.cc b/sdk/test/trace/parent_or_else_sampler_test.cc index 152121f4c3..7c907a66ed 100644 --- a/sdk/test/trace/parent_or_else_sampler_test.cc +++ b/sdk/test/trace/parent_or_else_sampler_test.cc @@ -8,6 +8,7 @@ using opentelemetry::sdk::trace::AlwaysOffSampler; using opentelemetry::sdk::trace::AlwaysOnSampler; using opentelemetry::sdk::trace::Decision; using opentelemetry::sdk::trace::ParentOrElseSampler; +using opentelemetry::sdk::trace::SpanContext; TEST(ParentOrElseSampler, ShouldSample) { @@ -20,8 +21,8 @@ TEST(ParentOrElseSampler, ShouldSample) 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); + SpanContext parent_context_sampled(true, false); + SpanContext parent_context_nonsampled(false, false); // Case 1: Parent doesn't exist. Return result of delegateSampler() auto sampling_result = sampler_off.ShouldSample(nullptr, trace_id, "", span_kind, view); diff --git a/sdk/test/trace/probability_sampler_test.cc b/sdk/test/trace/probability_sampler_test.cc new file mode 100644 index 0000000000..5ff427db70 --- /dev/null +++ b/sdk/test/trace/probability_sampler_test.cc @@ -0,0 +1,224 @@ +#include "opentelemetry/sdk/trace/samplers/probability.h" +#include "src/common/random.h" + +#include +#include +#include + +using opentelemetry::sdk::common::Random; +using opentelemetry::sdk::trace::Decision; +using opentelemetry::sdk::trace::ProbabilitySampler; +using opentelemetry::sdk::trace::SpanContext; + +namespace +{ +/* + * Helper function for running probability sampler tests. + * Given a span context, sampler, and number of iterations this function + * will return the number of RECORD_AND_SAMPLE decision based on randomly + * generated traces. + * + * @param context a required valid span context + * @param sampler a required valid sampler + * @param iterations a requried number specifying the number of times to + * generate a random trace_id and check if it should sample using the provided + * provider and context + */ +int RunShouldSampleCountDecision(SpanContext &context, + ProbabilitySampler &sampler, + int iterations) +{ + int actual_count = 0; + + opentelemetry::trace::SpanKind span_kind = opentelemetry::trace::SpanKind::kInternal; + + using M = std::map; + M m1 = {{}}; + opentelemetry::trace::KeyValueIterableView view{m1}; + + srand(time(0)); + + for (int i = 0; i < iterations; ++i) + { + uint8_t buf[16] = {0}; + Random::GenerateRandomBuffer(buf); + + opentelemetry::trace::TraceId trace_id(buf); + + auto result = sampler.ShouldSample(&context, trace_id, "", span_kind, view); + if (result.decision == Decision::RECORD_AND_SAMPLE) + { + ++actual_count; + } + } + + return actual_count; +} +} // namespace + +TEST(ProbabilitySampler, ShouldSampleWithoutContext) +{ + opentelemetry::trace::TraceId invalid_trace_id; + + opentelemetry::trace::SpanKind span_kind = opentelemetry::trace::SpanKind::kInternal; + + using M = std::map; + M m1 = {{}}; + opentelemetry::trace::KeyValueIterableView view{m1}; + + ProbabilitySampler s1(0.01); + + auto sampling_result = s1.ShouldSample(nullptr, invalid_trace_id, "", span_kind, view); + + ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result.decision); + ASSERT_EQ(nullptr, sampling_result.attributes); + + constexpr uint8_t buf[] = {0, 0, 0, 0, 0, 0, 0, 0x80, 0, 0, 0, 0, 0, 0, 0, 0}; + opentelemetry::trace::TraceId valid_trace_id(buf); + + sampling_result = s1.ShouldSample(nullptr, valid_trace_id, "", span_kind, view); + + ASSERT_EQ(Decision::NOT_RECORD, sampling_result.decision); + ASSERT_EQ(nullptr, sampling_result.attributes); + + ProbabilitySampler s2(0.50000001); + + sampling_result = s2.ShouldSample(nullptr, valid_trace_id, "", span_kind, view); + + ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result.decision); + ASSERT_EQ(nullptr, sampling_result.attributes); + + ProbabilitySampler s3(0.49999999); + + sampling_result = s3.ShouldSample(nullptr, valid_trace_id, "", span_kind, view); + + ASSERT_EQ(Decision::NOT_RECORD, sampling_result.decision); + ASSERT_EQ(nullptr, sampling_result.attributes); + + ProbabilitySampler s4(0.50000000); + + sampling_result = s4.ShouldSample(nullptr, valid_trace_id, "", span_kind, view); + + ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result.decision); + ASSERT_EQ(nullptr, sampling_result.attributes); +} + +TEST(ProbabilitySampler, ShouldSampleWithContext) +{ + opentelemetry::trace::TraceId trace_id; + opentelemetry::trace::SpanKind span_kind = opentelemetry::trace::SpanKind::kInternal; + SpanContext c1(false, false); + SpanContext c2(true, false); + SpanContext c3(false, true); + SpanContext c4(true, true); + + using M = std::map; + M m1 = {{}}; + opentelemetry::trace::KeyValueIterableView view{m1}; + + ProbabilitySampler s1(0.01); + + auto sampling_result = s1.ShouldSample(&c1, trace_id, "", span_kind, view); + + ASSERT_EQ(Decision::NOT_RECORD, sampling_result.decision); + ASSERT_EQ(nullptr, sampling_result.attributes); + + sampling_result = s1.ShouldSample(&c2, trace_id, "", span_kind, view); + + ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result.decision); + ASSERT_EQ(nullptr, sampling_result.attributes); + + sampling_result = s1.ShouldSample(&c3, trace_id, "", span_kind, view); + + ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result.decision); + ASSERT_EQ(nullptr, sampling_result.attributes); + + sampling_result = s1.ShouldSample(&c4, trace_id, "", span_kind, view); + + ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result.decision); + ASSERT_EQ(nullptr, sampling_result.attributes); +} + +TEST(ProbabilitySampler, ProbabilitySamplerHalf) +{ + double probability = 0.5; + int iterations = 100000, expected_count = iterations * probability, variance = iterations * 0.01; + + SpanContext c(true, true); + ProbabilitySampler s(probability); + + int actual_count = RunShouldSampleCountDecision(c, s, iterations); + + ASSERT_TRUE(actual_count < (expected_count + variance)); + ASSERT_TRUE(actual_count > (expected_count - variance)); +} + +TEST(ProbabilitySampler, ProbabilitySamplerOnePercent) +{ + double probability = 0.01; + int iterations = 100000, expected_count = iterations * probability, variance = iterations * 0.01; + + SpanContext c(true, true); + ProbabilitySampler s(probability); + + int actual_count = RunShouldSampleCountDecision(c, s, iterations); + + ASSERT_TRUE(actual_count < (expected_count + variance)); + ASSERT_TRUE(actual_count > (expected_count - variance)); +} + +TEST(ProbabilitySampler, ProbabilitySamplerAll) +{ + double probability = 1.0; + int iterations = 100000, expected_count = iterations * probability; + + SpanContext c(true, true); + ProbabilitySampler s(probability); + + int actual_count = RunShouldSampleCountDecision(c, s, iterations); + + ASSERT_EQ(actual_count, expected_count); +} + +TEST(ProbabilitySampler, ProbabilitySamplerNone) +{ + double probability = 0.0; + int iterations = 100000, expected_count = iterations * probability; + + SpanContext c(true, true); + ProbabilitySampler s(probability); + + int actual_count = RunShouldSampleCountDecision(c, s, iterations); + + ASSERT_EQ(actual_count, expected_count); +} + +TEST(ProbabilitySampler, GetDescription) +{ + ProbabilitySampler s1(0.01); + ASSERT_EQ("ProbabilitySampler{0.010000}", s1.GetDescription()); + + ProbabilitySampler s2(0.00); + ASSERT_EQ("ProbabilitySampler{0.000000}", s2.GetDescription()); + + ProbabilitySampler s3(1.00); + ASSERT_EQ("ProbabilitySampler{1.000000}", s3.GetDescription()); + + ProbabilitySampler s4(0.102030405); + ASSERT_EQ("ProbabilitySampler{0.102030}", s4.GetDescription()); + + ProbabilitySampler s5(3.00); + ASSERT_EQ("ProbabilitySampler{1.000000}", s5.GetDescription()); + + ProbabilitySampler s6(-3.00); + ASSERT_EQ("ProbabilitySampler{0.000000}", s6.GetDescription()); + + ProbabilitySampler s7(1.00000000001); + ASSERT_EQ("ProbabilitySampler{1.000000}", s7.GetDescription()); + + ProbabilitySampler s8(-1.00000000001); + ASSERT_EQ("ProbabilitySampler{0.000000}", s8.GetDescription()); + + ProbabilitySampler s9(0.50); + ASSERT_EQ("ProbabilitySampler{0.500000}", s9.GetDescription()); +} From 2130ef045f52cd87b6b78caba1d1bd9a0a8e490c Mon Sep 17 00:00:00 2001 From: Nick Holbrook Date: Fri, 10 Jul 2020 13:02:11 -0600 Subject: [PATCH 02/10] Update SpanContext location / defination --- .../opentelemetry/trace/span_context.h | 53 +++++ sdk/include/opentelemetry/sdk/trace/sampler.h | 5 +- .../sdk/trace/samplers/always_off.h | 3 +- .../sdk/trace/samplers/always_on.h | 3 +- .../sdk/trace/samplers/parent_or_else.h | 3 +- .../sdk/trace/samplers/probability.h | 52 ---- .../opentelemetry/sdk/trace/span_context.h | 26 -- sdk/src/trace/samplers/parent_or_else.cc | 4 +- sdk/src/trace/samplers/probability.cc | 114 --------- sdk/test/trace/parent_or_else_sampler_test.cc | 2 +- sdk/test/trace/probability_sampler_test.cc | 224 ------------------ 11 files changed, 61 insertions(+), 428 deletions(-) create mode 100644 api/include/opentelemetry/trace/span_context.h delete mode 100644 sdk/include/opentelemetry/sdk/trace/samplers/probability.h delete mode 100644 sdk/include/opentelemetry/sdk/trace/span_context.h delete mode 100644 sdk/src/trace/samplers/probability.cc delete mode 100644 sdk/test/trace/probability_sampler_test.cc diff --git a/api/include/opentelemetry/trace/span_context.h b/api/include/opentelemetry/trace/span_context.h new file mode 100644 index 0000000000..362f19f6eb --- /dev/null +++ b/api/include/opentelemetry/trace/span_context.h @@ -0,0 +1,53 @@ +// Copyright 2020, 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. + +#pragma once + +#include "opentelemetry/nostd/unique_ptr.h" +#include "opentelemetry/trace/span_id.h" +#include "opentelemetry/trace/trace_flags.h" +#include "opentelemetry/trace/trace_id.h" + +namespace opentelemetry +{ +namespace trace +{ + +// SpanContext contains the state that must propagate to child Spans and across +// process boundaries. It contains the identifiers TraceId and SpanId, +// TraceFlags, TraceState, and whether it has a remote parent. +class SpanContext final +{ +public: + // An invalid SpanContext. + SpanContext(bool sampled_flag, bool has_remote_parent) : + trace_flags_(TraceFlags((uint8_t) sampled_flag)), remote_parent_(has_remote_parent) {}; + + const TraceId &trace_id() const noexcept { return trace_id_; } + const SpanId &span_id() const noexcept { return span_id_; } + const TraceFlags &trace_flags() const noexcept { return trace_flags_; } + + bool IsValid() const noexcept { return trace_id_.IsValid() && span_id_.IsValid(); } + + bool IsSampled() const noexcept { return trace_flags_.IsSampled(); } + bool HasRemoteParent() const noexcept { return remote_parent_; } + +private: + const TraceId trace_id_; + const SpanId span_id_; + const TraceFlags trace_flags_; + const bool remote_parent_ = false; +}; +} // namespace trace +} // namespace opentelemetry \ No newline at end of file diff --git a/sdk/include/opentelemetry/sdk/trace/sampler.h b/sdk/include/opentelemetry/sdk/trace/sampler.h index 8214173895..35a0927404 100644 --- a/sdk/include/opentelemetry/sdk/trace/sampler.h +++ b/sdk/include/opentelemetry/sdk/trace/sampler.h @@ -4,7 +4,7 @@ #include "opentelemetry/trace/span.h" #include "opentelemetry/trace/trace_id.h" #include "opentelemetry/version.h" -#include "opentelemetry/sdk/trace/span_context.h" +#include "opentelemetry/trace/span_context.h" #include #include @@ -16,7 +16,6 @@ namespace sdk namespace trace { namespace trace_api = opentelemetry::trace; -namespace trace_sdk = opentelemetry::sdk::trace; /** * A sampling Decision for a Span to be created. @@ -66,7 +65,7 @@ class Sampler * @since 0.1.0 */ - virtual SamplingResult ShouldSample(const trace_sdk::SpanContext *parent_context, + virtual SamplingResult ShouldSample(const trace_api::SpanContext *parent_context, trace_api::TraceId trace_id, nostd::string_view name, trace_api::SpanKind span_kind, diff --git a/sdk/include/opentelemetry/sdk/trace/samplers/always_off.h b/sdk/include/opentelemetry/sdk/trace/samplers/always_off.h index 4437db9d88..252cd9e4fd 100644 --- a/sdk/include/opentelemetry/sdk/trace/samplers/always_off.h +++ b/sdk/include/opentelemetry/sdk/trace/samplers/always_off.h @@ -8,7 +8,6 @@ namespace sdk namespace trace { namespace trace_api = opentelemetry::trace; -namespace trace_sdk = opentelemetry::sdk::trace; /** * The always off sampler always returns NOT_RECORD, effectively disabling @@ -21,7 +20,7 @@ class AlwaysOffSampler : public Sampler * @return Returns NOT_RECORD always */ SamplingResult ShouldSample( - const trace_sdk::SpanContext * /*parent_context*/, + const trace_api::SpanContext * /*parent_context*/, trace_api::TraceId /*trace_id*/, nostd::string_view /*name*/, trace_api::SpanKind /*span_kind*/, diff --git a/sdk/include/opentelemetry/sdk/trace/samplers/always_on.h b/sdk/include/opentelemetry/sdk/trace/samplers/always_on.h index 5d227eef13..5dd5f86457 100644 --- a/sdk/include/opentelemetry/sdk/trace/samplers/always_on.h +++ b/sdk/include/opentelemetry/sdk/trace/samplers/always_on.h @@ -8,7 +8,6 @@ namespace sdk namespace trace { namespace trace_api = opentelemetry::trace; -namespace trace_sdk = opentelemetry::sdk::trace; /** * The always on sampler is a default sampler which always return Decision::RECORD_AND_SAMPLE @@ -19,7 +18,7 @@ class AlwaysOnSampler : public Sampler /** * @return Always return Decision RECORD_AND_SAMPLE */ - inline SamplingResult ShouldSample(const trace_sdk::SpanContext * /*parent_context*/, + inline SamplingResult ShouldSample(const trace_api::SpanContext * /*parent_context*/, trace_api::TraceId /*trace_id*/, nostd::string_view /*name*/, trace_api::SpanKind /*span_kind*/, 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 3d63f256ce..aec9934604 100644 --- a/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h +++ b/sdk/include/opentelemetry/sdk/trace/samplers/parent_or_else.h @@ -9,7 +9,6 @@ namespace sdk namespace trace { namespace trace_api = opentelemetry::trace; -namespace trace_sdk = opentelemetry::sdk::trace; /** * The parent or else sampler is a composite sampler. ParentOrElse(delegateSampler) either respects @@ -23,7 +22,7 @@ class ParentOrElseSampler : public Sampler * delegateSampler for root spans * @return Returns NOT_RECORD always */ - SamplingResult ShouldSample(const trace_sdk::SpanContext * parent_context, + SamplingResult ShouldSample(const trace_api::SpanContext * parent_context, trace_api::TraceId trace_id, nostd::string_view name, trace_api::SpanKind span_kind, diff --git a/sdk/include/opentelemetry/sdk/trace/samplers/probability.h b/sdk/include/opentelemetry/sdk/trace/samplers/probability.h deleted file mode 100644 index 966880eab6..0000000000 --- a/sdk/include/opentelemetry/sdk/trace/samplers/probability.h +++ /dev/null @@ -1,52 +0,0 @@ -#pragma once - -#include "opentelemetry/sdk/trace/sampler.h" - -OPENTELEMETRY_BEGIN_NAMESPACE -namespace sdk -{ -namespace trace -{ -namespace trace_api = opentelemetry::trace; -namespace trace_sdk = opentelemetry::sdk::trace; - -/** - * The probability sampler, based on it's configuration, should either defer the - * decision to sample to it's parent, or compute and return a decision based on - * the provided trace_id and probability. - */ -class ProbabilitySampler : public Sampler -{ -public: - /** - * @param probability a required value, 1.0 >= probability >= 0.0, that given any - * random trace_id, ShouldSample will return RECORD_AND_SAMPLE - * @throws invalid_argument if probability is out of bounds [0.0, 1.0] - */ - explicit ProbabilitySampler(double probability); - - /** - * @return Returns either RECORD_AND_SAMPLE or NOT_RECORD based on current - * sampler configuration and provided parent_context / tracer_id. tracer_id - * is used as a pseudorandom value in conjunction with the predefined - * threshold to determine whether this trace should be sampled - */ - SamplingResult ShouldSample( - const trace_sdk::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 ProbabilitySampler{0.000100} - */ - std::string GetDescription() const noexcept override; - -private: - std::string sampler_description_; - const uint64_t threshold_; -}; -} // namespace trace -} // namespace sdk -OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/trace/span_context.h b/sdk/include/opentelemetry/sdk/trace/span_context.h deleted file mode 100644 index 7f79111263..0000000000 --- a/sdk/include/opentelemetry/sdk/trace/span_context.h +++ /dev/null @@ -1,26 +0,0 @@ -#pragma once - -OPENTELEMETRY_BEGIN_NAMESPACE -namespace sdk -{ -namespace trace -{ -namespace trace_api = opentelemetry::trace; - -// TODO: Remove this placeholder with real class -class SpanContext -{ -public: -explicit SpanContext(bool sampled_flag, bool is_remote) - : traceFlags(sampled_flag), is_remote(is_remote) {} - -uint8_t traceFlags; -bool is_remote; - -const uint8_t FLAG_SAMPLED = 1; // 00000001 - -bool sampled = (traceFlags & FLAG_SAMPLED) == FLAG_SAMPLED; -}; -} // namespace trace -} // namespace sdk -OPENTELEMETRY_END_NAMESPACE \ No newline at end of file diff --git a/sdk/src/trace/samplers/parent_or_else.cc b/sdk/src/trace/samplers/parent_or_else.cc index e1bdc51e69..2b56ab9d4b 100644 --- a/sdk/src/trace/samplers/parent_or_else.cc +++ b/sdk/src/trace/samplers/parent_or_else.cc @@ -11,7 +11,7 @@ ParentOrElseSampler::ParentOrElseSampler(std::shared_ptr delegate_sampl {} SamplingResult ParentOrElseSampler::ShouldSample( - const trace_sdk::SpanContext *parent_context, + const trace_api::SpanContext *parent_context, trace_api::TraceId trace_id, nostd::string_view name, trace_api::SpanKind span_kind, @@ -24,7 +24,7 @@ SamplingResult ParentOrElseSampler::ShouldSample( } // If parent exists: - if (parent_context->sampled) + if (parent_context->IsSampled()) { return {Decision::RECORD_AND_SAMPLE, nullptr}; } diff --git a/sdk/src/trace/samplers/probability.cc b/sdk/src/trace/samplers/probability.cc deleted file mode 100644 index 29795de6c3..0000000000 --- a/sdk/src/trace/samplers/probability.cc +++ /dev/null @@ -1,114 +0,0 @@ -// Copyright 2020, Open Telemetry Authors -// Copyright 2017, OpenCensus 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 "opentelemetry/sdk/trace/samplers/probability.h" - -#include -#include -#include - -namespace -{ - namespace trace_api = opentelemetry::trace; - - /** - * Converts a probability in [0, 1] to a threshold in [0, UINT64_MAX] - * - * @param probability a required value top be converted to uint64_t. is - * bounded by 1 >= probability >= 0. - * @return Returns threshold value computed after converting probability to - * uint64_t datatype - */ - uint64_t CalculateThreshold(double probability) noexcept - { - if (probability <= 0.0) return 0; - if (probability >= 1.0) return UINT64_MAX; - - // We can't directly return probability * UINT64_MAX. - // - // UINT64_MAX is (2^64)-1, but as a double rounds up to 2^64. - // For probabilities >= 1-(2^-54), the product wraps to zero! - // Instead, calculate the high and low 32 bits separately. - const double product = UINT32_MAX * probability; - double hi_bits, lo_bits = ldexp(modf(product, &hi_bits), 32) + product; - return (static_cast(hi_bits) << 32) + - static_cast(lo_bits); - } - - /** - * @param trace_id a required value to be converted to uint64_t. trace_id must - * at least 8 bytes long - * @return Returns threshold value computed after converting trace_id to - * uint64_t datatype - */ - uint64_t CalculateThresholdFromBuffer(const trace_api::TraceId& trace_id) noexcept - { - // We only use the first 8 bytes of TraceId. - static_assert(trace_api::TraceId::kSize >= 8, "TraceID must be at least 8 bytes long."); - - uint64_t res = 0; - std::memcpy(&res, &trace_id, 8); - - double probability = (double) res / UINT64_MAX; - - return CalculateThreshold(probability); - } -} // namespace - -OPENTELEMETRY_BEGIN_NAMESPACE -namespace sdk -{ -namespace trace -{ -ProbabilitySampler::ProbabilitySampler(double probability) -: threshold_(CalculateThreshold(probability)) -{ - if (probability > 1.0) probability = 1.0; - if (probability < 0.0) probability = 0.0; - sampler_description_ = "ProbabilitySampler{" + std::to_string(probability) + "}"; - } - -SamplingResult ProbabilitySampler::ShouldSample( - const trace_sdk::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 && !parent_context->is_remote) { - if (parent_context->sampled) { - return { Decision::RECORD_AND_SAMPLE, nullptr }; - } else { - return { Decision::NOT_RECORD, nullptr }; - } - } - - if (threshold_ == 0) return { Decision::NOT_RECORD, nullptr }; - - if (CalculateThresholdFromBuffer(trace_id) <= threshold_) - { - return { Decision::RECORD_AND_SAMPLE, nullptr }; - } - - return { Decision::NOT_RECORD, nullptr }; -} - -std::string ProbabilitySampler::GetDescription() const noexcept -{ - return sampler_description_; -} -} // namespace trace -} // namespace sdk -OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/test/trace/parent_or_else_sampler_test.cc b/sdk/test/trace/parent_or_else_sampler_test.cc index 7c907a66ed..93ce56dfb1 100644 --- a/sdk/test/trace/parent_or_else_sampler_test.cc +++ b/sdk/test/trace/parent_or_else_sampler_test.cc @@ -8,7 +8,7 @@ using opentelemetry::sdk::trace::AlwaysOffSampler; using opentelemetry::sdk::trace::AlwaysOnSampler; using opentelemetry::sdk::trace::Decision; using opentelemetry::sdk::trace::ParentOrElseSampler; -using opentelemetry::sdk::trace::SpanContext; +using opentelemetry::trace::SpanContext; TEST(ParentOrElseSampler, ShouldSample) { diff --git a/sdk/test/trace/probability_sampler_test.cc b/sdk/test/trace/probability_sampler_test.cc deleted file mode 100644 index 5ff427db70..0000000000 --- a/sdk/test/trace/probability_sampler_test.cc +++ /dev/null @@ -1,224 +0,0 @@ -#include "opentelemetry/sdk/trace/samplers/probability.h" -#include "src/common/random.h" - -#include -#include -#include - -using opentelemetry::sdk::common::Random; -using opentelemetry::sdk::trace::Decision; -using opentelemetry::sdk::trace::ProbabilitySampler; -using opentelemetry::sdk::trace::SpanContext; - -namespace -{ -/* - * Helper function for running probability sampler tests. - * Given a span context, sampler, and number of iterations this function - * will return the number of RECORD_AND_SAMPLE decision based on randomly - * generated traces. - * - * @param context a required valid span context - * @param sampler a required valid sampler - * @param iterations a requried number specifying the number of times to - * generate a random trace_id and check if it should sample using the provided - * provider and context - */ -int RunShouldSampleCountDecision(SpanContext &context, - ProbabilitySampler &sampler, - int iterations) -{ - int actual_count = 0; - - opentelemetry::trace::SpanKind span_kind = opentelemetry::trace::SpanKind::kInternal; - - using M = std::map; - M m1 = {{}}; - opentelemetry::trace::KeyValueIterableView view{m1}; - - srand(time(0)); - - for (int i = 0; i < iterations; ++i) - { - uint8_t buf[16] = {0}; - Random::GenerateRandomBuffer(buf); - - opentelemetry::trace::TraceId trace_id(buf); - - auto result = sampler.ShouldSample(&context, trace_id, "", span_kind, view); - if (result.decision == Decision::RECORD_AND_SAMPLE) - { - ++actual_count; - } - } - - return actual_count; -} -} // namespace - -TEST(ProbabilitySampler, ShouldSampleWithoutContext) -{ - opentelemetry::trace::TraceId invalid_trace_id; - - opentelemetry::trace::SpanKind span_kind = opentelemetry::trace::SpanKind::kInternal; - - using M = std::map; - M m1 = {{}}; - opentelemetry::trace::KeyValueIterableView view{m1}; - - ProbabilitySampler s1(0.01); - - auto sampling_result = s1.ShouldSample(nullptr, invalid_trace_id, "", span_kind, view); - - ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result.decision); - ASSERT_EQ(nullptr, sampling_result.attributes); - - constexpr uint8_t buf[] = {0, 0, 0, 0, 0, 0, 0, 0x80, 0, 0, 0, 0, 0, 0, 0, 0}; - opentelemetry::trace::TraceId valid_trace_id(buf); - - sampling_result = s1.ShouldSample(nullptr, valid_trace_id, "", span_kind, view); - - ASSERT_EQ(Decision::NOT_RECORD, sampling_result.decision); - ASSERT_EQ(nullptr, sampling_result.attributes); - - ProbabilitySampler s2(0.50000001); - - sampling_result = s2.ShouldSample(nullptr, valid_trace_id, "", span_kind, view); - - ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result.decision); - ASSERT_EQ(nullptr, sampling_result.attributes); - - ProbabilitySampler s3(0.49999999); - - sampling_result = s3.ShouldSample(nullptr, valid_trace_id, "", span_kind, view); - - ASSERT_EQ(Decision::NOT_RECORD, sampling_result.decision); - ASSERT_EQ(nullptr, sampling_result.attributes); - - ProbabilitySampler s4(0.50000000); - - sampling_result = s4.ShouldSample(nullptr, valid_trace_id, "", span_kind, view); - - ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result.decision); - ASSERT_EQ(nullptr, sampling_result.attributes); -} - -TEST(ProbabilitySampler, ShouldSampleWithContext) -{ - opentelemetry::trace::TraceId trace_id; - opentelemetry::trace::SpanKind span_kind = opentelemetry::trace::SpanKind::kInternal; - SpanContext c1(false, false); - SpanContext c2(true, false); - SpanContext c3(false, true); - SpanContext c4(true, true); - - using M = std::map; - M m1 = {{}}; - opentelemetry::trace::KeyValueIterableView view{m1}; - - ProbabilitySampler s1(0.01); - - auto sampling_result = s1.ShouldSample(&c1, trace_id, "", span_kind, view); - - ASSERT_EQ(Decision::NOT_RECORD, sampling_result.decision); - ASSERT_EQ(nullptr, sampling_result.attributes); - - sampling_result = s1.ShouldSample(&c2, trace_id, "", span_kind, view); - - ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result.decision); - ASSERT_EQ(nullptr, sampling_result.attributes); - - sampling_result = s1.ShouldSample(&c3, trace_id, "", span_kind, view); - - ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result.decision); - ASSERT_EQ(nullptr, sampling_result.attributes); - - sampling_result = s1.ShouldSample(&c4, trace_id, "", span_kind, view); - - ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result.decision); - ASSERT_EQ(nullptr, sampling_result.attributes); -} - -TEST(ProbabilitySampler, ProbabilitySamplerHalf) -{ - double probability = 0.5; - int iterations = 100000, expected_count = iterations * probability, variance = iterations * 0.01; - - SpanContext c(true, true); - ProbabilitySampler s(probability); - - int actual_count = RunShouldSampleCountDecision(c, s, iterations); - - ASSERT_TRUE(actual_count < (expected_count + variance)); - ASSERT_TRUE(actual_count > (expected_count - variance)); -} - -TEST(ProbabilitySampler, ProbabilitySamplerOnePercent) -{ - double probability = 0.01; - int iterations = 100000, expected_count = iterations * probability, variance = iterations * 0.01; - - SpanContext c(true, true); - ProbabilitySampler s(probability); - - int actual_count = RunShouldSampleCountDecision(c, s, iterations); - - ASSERT_TRUE(actual_count < (expected_count + variance)); - ASSERT_TRUE(actual_count > (expected_count - variance)); -} - -TEST(ProbabilitySampler, ProbabilitySamplerAll) -{ - double probability = 1.0; - int iterations = 100000, expected_count = iterations * probability; - - SpanContext c(true, true); - ProbabilitySampler s(probability); - - int actual_count = RunShouldSampleCountDecision(c, s, iterations); - - ASSERT_EQ(actual_count, expected_count); -} - -TEST(ProbabilitySampler, ProbabilitySamplerNone) -{ - double probability = 0.0; - int iterations = 100000, expected_count = iterations * probability; - - SpanContext c(true, true); - ProbabilitySampler s(probability); - - int actual_count = RunShouldSampleCountDecision(c, s, iterations); - - ASSERT_EQ(actual_count, expected_count); -} - -TEST(ProbabilitySampler, GetDescription) -{ - ProbabilitySampler s1(0.01); - ASSERT_EQ("ProbabilitySampler{0.010000}", s1.GetDescription()); - - ProbabilitySampler s2(0.00); - ASSERT_EQ("ProbabilitySampler{0.000000}", s2.GetDescription()); - - ProbabilitySampler s3(1.00); - ASSERT_EQ("ProbabilitySampler{1.000000}", s3.GetDescription()); - - ProbabilitySampler s4(0.102030405); - ASSERT_EQ("ProbabilitySampler{0.102030}", s4.GetDescription()); - - ProbabilitySampler s5(3.00); - ASSERT_EQ("ProbabilitySampler{1.000000}", s5.GetDescription()); - - ProbabilitySampler s6(-3.00); - ASSERT_EQ("ProbabilitySampler{0.000000}", s6.GetDescription()); - - ProbabilitySampler s7(1.00000000001); - ASSERT_EQ("ProbabilitySampler{1.000000}", s7.GetDescription()); - - ProbabilitySampler s8(-1.00000000001); - ASSERT_EQ("ProbabilitySampler{0.000000}", s8.GetDescription()); - - ProbabilitySampler s9(0.50); - ASSERT_EQ("ProbabilitySampler{0.500000}", s9.GetDescription()); -} From 78f74ed408dda2b93b2fac857aeccbadfe19e6f6 Mon Sep 17 00:00:00 2001 From: Nick Holbrook Date: Fri, 10 Jul 2020 13:18:56 -0600 Subject: [PATCH 03/10] Update spacing --- api/include/opentelemetry/trace/span_context.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/include/opentelemetry/trace/span_context.h b/api/include/opentelemetry/trace/span_context.h index 362f19f6eb..d30bcdf7c2 100644 --- a/api/include/opentelemetry/trace/span_context.h +++ b/api/include/opentelemetry/trace/span_context.h @@ -50,4 +50,4 @@ class SpanContext final const bool remote_parent_ = false; }; } // namespace trace -} // namespace opentelemetry \ No newline at end of file +} // namespace opentelemetry From 02c7cdfabb55d2fbe9e34663a2edd590082a6b54 Mon Sep 17 00:00:00 2001 From: Nick Holbrook Date: Fri, 10 Jul 2020 13:31:22 -0600 Subject: [PATCH 04/10] Update class references --- api/include/opentelemetry/trace/span_context.h | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/api/include/opentelemetry/trace/span_context.h b/api/include/opentelemetry/trace/span_context.h index d30bcdf7c2..bd79f494a7 100644 --- a/api/include/opentelemetry/trace/span_context.h +++ b/api/include/opentelemetry/trace/span_context.h @@ -23,6 +23,7 @@ namespace opentelemetry { namespace trace { +namespace trace_api = opentelemetry::trace; // SpanContext contains the state that must propagate to child Spans and across // process boundaries. It contains the identifiers TraceId and SpanId, @@ -32,11 +33,11 @@ class SpanContext final public: // An invalid SpanContext. SpanContext(bool sampled_flag, bool has_remote_parent) : - trace_flags_(TraceFlags((uint8_t) sampled_flag)), remote_parent_(has_remote_parent) {}; + trace_flags_(trace_api::TraceFlags((uint8_t) sampled_flag)), remote_parent_(has_remote_parent) {}; - const TraceId &trace_id() const noexcept { return trace_id_; } - const SpanId &span_id() const noexcept { return span_id_; } - const TraceFlags &trace_flags() const noexcept { return trace_flags_; } + const trace_api::TraceId &trace_id() const noexcept { return trace_id_; } + const trace_api::SpanId &span_id() const noexcept { return span_id_; } + const trace_api::TraceFlags &trace_flags() const noexcept { return trace_flags_; } bool IsValid() const noexcept { return trace_id_.IsValid() && span_id_.IsValid(); } @@ -44,9 +45,9 @@ class SpanContext final bool HasRemoteParent() const noexcept { return remote_parent_; } private: - const TraceId trace_id_; - const SpanId span_id_; - const TraceFlags trace_flags_; + const trace_api::TraceId trace_id_; + const trace_api::SpanId span_id_; + const trace_api::TraceFlags trace_flags_; const bool remote_parent_ = false; }; } // namespace trace From bd7f68cbd51d28513221538828180b135daa35a8 Mon Sep 17 00:00:00 2001 From: Nick Holbrook Date: Fri, 10 Jul 2020 14:34:01 -0600 Subject: [PATCH 05/10] Update namespace definition --- api/include/opentelemetry/trace/span_context.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/api/include/opentelemetry/trace/span_context.h b/api/include/opentelemetry/trace/span_context.h index bd79f494a7..86c94c2ff7 100644 --- a/api/include/opentelemetry/trace/span_context.h +++ b/api/include/opentelemetry/trace/span_context.h @@ -19,8 +19,7 @@ #include "opentelemetry/trace/trace_flags.h" #include "opentelemetry/trace/trace_id.h" -namespace opentelemetry -{ +OPENTELEMETRY_BEGIN_NAMESPACE namespace trace { namespace trace_api = opentelemetry::trace; @@ -51,4 +50,4 @@ class SpanContext final const bool remote_parent_ = false; }; } // namespace trace -} // namespace opentelemetry +OPENTELEMETRY_END_NAMESPACE From d90e878534077a689b8a49a29ea4314483bf9038 Mon Sep 17 00:00:00 2001 From: Nick Holbrook Date: Fri, 10 Jul 2020 15:25:26 -0600 Subject: [PATCH 06/10] Add unit tests --- .../opentelemetry/trace/span_context.h | 6 --- api/test/trace/BUILD | 11 ++++++ api/test/trace/CMakeLists.txt | 2 +- api/test/trace/span_context_test.cc | 38 +++++++++++++++++++ 4 files changed, 50 insertions(+), 7 deletions(-) create mode 100644 api/test/trace/span_context_test.cc diff --git a/api/include/opentelemetry/trace/span_context.h b/api/include/opentelemetry/trace/span_context.h index 86c94c2ff7..91de79ca72 100644 --- a/api/include/opentelemetry/trace/span_context.h +++ b/api/include/opentelemetry/trace/span_context.h @@ -34,18 +34,12 @@ class SpanContext final SpanContext(bool sampled_flag, bool has_remote_parent) : trace_flags_(trace_api::TraceFlags((uint8_t) sampled_flag)), remote_parent_(has_remote_parent) {}; - const trace_api::TraceId &trace_id() const noexcept { return trace_id_; } - const trace_api::SpanId &span_id() const noexcept { return span_id_; } const trace_api::TraceFlags &trace_flags() const noexcept { return trace_flags_; } - bool IsValid() const noexcept { return trace_id_.IsValid() && span_id_.IsValid(); } - bool IsSampled() const noexcept { return trace_flags_.IsSampled(); } bool HasRemoteParent() const noexcept { return remote_parent_; } private: - const trace_api::TraceId trace_id_; - const trace_api::SpanId span_id_; const trace_api::TraceFlags trace_flags_; const bool remote_parent_ = false; }; diff --git a/api/test/trace/BUILD b/api/test/trace/BUILD index ed3d584712..04baff671e 100644 --- a/api/test/trace/BUILD +++ b/api/test/trace/BUILD @@ -71,3 +71,14 @@ cc_test( "@com_google_googletest//:gtest_main", ], ) + +cc_test( + name = "span_context_test", + srcs = [ + "span_context_test.cc", + ], + deps = [ + "//api", + "@com_google_googletest//:gtest_main", + ], +) \ No newline at end of file diff --git a/api/test/trace/CMakeLists.txt b/api/test/trace/CMakeLists.txt index f722ae4557..3439146f7a 100644 --- a/api/test/trace/CMakeLists.txt +++ b/api/test/trace/CMakeLists.txt @@ -1,5 +1,5 @@ foreach(testname key_value_iterable_view_test noop_test provider_test - span_id_test trace_id_test trace_flags_test) + span_id_test trace_id_test trace_flags_test span_context_test) add_executable(${testname} "${testname}.cc") target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} opentelemetry_api) diff --git a/api/test/trace/span_context_test.cc b/api/test/trace/span_context_test.cc new file mode 100644 index 0000000000..9529c3e5de --- /dev/null +++ b/api/test/trace/span_context_test.cc @@ -0,0 +1,38 @@ +#include "opentelemetry/trace/span_context.h" + +#include + +using opentelemetry::trace::SpanContext; + +TEST(SpanContextTest, IsSampled) +{ + SpanContext s1(true, true); + + ASSERT_EQ(s1.IsSampled(), true); + + SpanContext s2(false, true); + + ASSERT_EQ(s2.IsSampled(), false); +} + +TEST(SpanContextTest, HasRemoteParent) +{ + SpanContext s1(true, true); + + ASSERT_EQ(s1.HasRemoteParent(), true); + + SpanContext s2(true, false); + + ASSERT_EQ(s2.HasRemoteParent(), false); +} + +TEST(SpanContextTest, TraceFlags) +{ + SpanContext s1(true, true); + + ASSERT_EQ(s1.trace_flags().flags(), 1); + + SpanContext s2(false, true); + + ASSERT_EQ(s2.trace_flags().flags(), 0); +} \ No newline at end of file From a5d461e296622bf4c8a6e63297678f0fc4aa528f Mon Sep 17 00:00:00 2001 From: Nick Holbrook Date: Fri, 10 Jul 2020 15:41:21 -0600 Subject: [PATCH 07/10] Added doc strings --- api/include/opentelemetry/trace/span_context.h | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/api/include/opentelemetry/trace/span_context.h b/api/include/opentelemetry/trace/span_context.h index 91de79ca72..64a3174e8a 100644 --- a/api/include/opentelemetry/trace/span_context.h +++ b/api/include/opentelemetry/trace/span_context.h @@ -24,19 +24,29 @@ namespace trace { namespace trace_api = opentelemetry::trace; -// SpanContext contains the state that must propagate to child Spans and across -// process boundaries. It contains the identifiers TraceId and SpanId, -// TraceFlags, TraceState, and whether it has a remote parent. +/* SpanContext contains the state that must propagate to child Spans and across + * process boundaries. It contains the identifiers TraceId and SpanId, + * TraceFlags, TraceState, and whether it has a remote parent. + */ class SpanContext final { public: - // An invalid SpanContext. + /* A temporary constructor for an invalid SpanContext. + * @param sampled_flag a required parameter specifying if child spans should be + * sampled + * @param has_remote_parent a required parameter specifying if this context has + * a remote parent + */ SpanContext(bool sampled_flag, bool has_remote_parent) : trace_flags_(trace_api::TraceFlags((uint8_t) sampled_flag)), remote_parent_(has_remote_parent) {}; + // @returns the trace_flags associated with this span_context const trace_api::TraceFlags &trace_flags() const noexcept { return trace_flags_; } + // @returns whether this context has the sampled flag set or not bool IsSampled() const noexcept { return trace_flags_.IsSampled(); } + + // @returns whether this context has a remote parent or not bool HasRemoteParent() const noexcept { return remote_parent_; } private: From 721c30c4431a781bea63569606c626b4bf6508b9 Mon Sep 17 00:00:00 2001 From: Nick Holbrook Date: Fri, 10 Jul 2020 16:00:53 -0600 Subject: [PATCH 08/10] Update spacing --- api/test/trace/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/test/trace/BUILD b/api/test/trace/BUILD index 04baff671e..b3f194baa0 100644 --- a/api/test/trace/BUILD +++ b/api/test/trace/BUILD @@ -81,4 +81,4 @@ cc_test( "//api", "@com_google_googletest//:gtest_main", ], -) \ No newline at end of file +) From 9ebc3ff84d8bf3ee66f27f4b3441fed8b0f9b68a Mon Sep 17 00:00:00 2001 From: Nick Holbrook Date: Mon, 13 Jul 2020 14:07:59 -0400 Subject: [PATCH 09/10] Add todo comment --- api/include/opentelemetry/trace/span_context.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/include/opentelemetry/trace/span_context.h b/api/include/opentelemetry/trace/span_context.h index 64a3174e8a..fc04ce17e6 100644 --- a/api/include/opentelemetry/trace/span_context.h +++ b/api/include/opentelemetry/trace/span_context.h @@ -27,6 +27,8 @@ namespace trace_api = opentelemetry::trace; /* SpanContext contains the state that must propagate to child Spans and across * process boundaries. It contains the identifiers TraceId and SpanId, * TraceFlags, TraceState, and whether it has a remote parent. + * + * TODO: This is currently a placeholder class and requires revisiting */ class SpanContext final { From 86ce47aec337665bc1e9f4a047af5c72dde5b339 Mon Sep 17 00:00:00 2001 From: Nick Holbrook Date: Mon, 13 Jul 2020 15:13:48 -0400 Subject: [PATCH 10/10] Add eof to span_context test --- api/test/trace/span_context_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/test/trace/span_context_test.cc b/api/test/trace/span_context_test.cc index 9529c3e5de..661bba00bc 100644 --- a/api/test/trace/span_context_test.cc +++ b/api/test/trace/span_context_test.cc @@ -35,4 +35,4 @@ TEST(SpanContextTest, TraceFlags) SpanContext s2(false, true); ASSERT_EQ(s2.trace_flags().flags(), 0); -} \ No newline at end of file +}