From a33bb550c68c235f8ac9eca1310e6485c65fe5c5 Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Wed, 10 Mar 2021 11:15:16 -0800 Subject: [PATCH 1/4] Propagate trace_flags from parent span to child span (#603) --- sdk/src/trace/span.cc | 13 ++++++---- sdk/src/trace/span.h | 17 ++++++------ sdk/src/trace/tracer.cc | 2 +- sdk/test/trace/tracer_test.cc | 49 +++++++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 14 deletions(-) diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index d866c97b89..ca5243fb18 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -64,7 +64,8 @@ Span::Span(std::shared_ptr &&tracer, const trace_api::SpanContextKeyValueIterable &links, const trace_api::StartSpanOptions &options, const trace_api::SpanContext &parent_span_context, - const opentelemetry::sdk::resource::Resource &resource) noexcept + const opentelemetry::sdk::resource::Resource &resource, + const bool sampled) noexcept : tracer_{std::move(tracer)}, processor_{processor}, recordable_{processor_->MakeRecordable()}, @@ -93,10 +94,12 @@ Span::Span(std::shared_ptr &&tracer, recordable_->SetIds(trace_id, span_id, trace_api::SpanId()); } - span_context_ = std::unique_ptr( - new trace_api::SpanContext(trace_id, span_id, trace_api::TraceFlags(), false, - is_parent_span_valid ? parent_span_context.trace_state() - : trace_api::TraceState::GetDefault())); + span_context_ = std::unique_ptr(new trace_api::SpanContext( + trace_id, span_id, + sampled ? trace_api::TraceFlags{trace_api::TraceFlags::kIsSampled} : trace_api::TraceFlags{}, + false, + is_parent_span_valid ? parent_span_context.trace_state() + : trace_api::TraceState::GetDefault())); attributes.ForEachKeyValue( [&](nostd::string_view key, opentelemetry::common::AttributeValue value) noexcept { diff --git a/sdk/src/trace/span.h b/sdk/src/trace/span.h index b44dcdb3bc..0f8e843e48 100644 --- a/sdk/src/trace/span.h +++ b/sdk/src/trace/span.h @@ -15,14 +15,15 @@ namespace trace_api = opentelemetry::trace; class Span final : public trace_api::Span { public: - explicit Span(std::shared_ptr &&tracer, - std::shared_ptr processor, - nostd::string_view name, - const opentelemetry::common::KeyValueIterable &attributes, - const trace_api::SpanContextKeyValueIterable &links, - const trace_api::StartSpanOptions &options, - const trace_api::SpanContext &parent_span_context, - const opentelemetry::sdk::resource::Resource &resource) noexcept; + Span(std::shared_ptr &&tracer, + std::shared_ptr processor, + nostd::string_view name, + const opentelemetry::common::KeyValueIterable &attributes, + const trace_api::SpanContextKeyValueIterable &links, + const trace_api::StartSpanOptions &options, + const trace_api::SpanContext &parent_span_context, + const opentelemetry::sdk::resource::Resource &resource, + const bool sampled = false) noexcept; ~Span() override; diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index 27e3aad58c..b85164b6ac 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -75,7 +75,7 @@ nostd::shared_ptr Tracer::StartSpan( { auto span = nostd::shared_ptr{ new (std::nothrow) Span{this->shared_from_this(), processor_.load(), name, attributes, - links, options, parent, resource_}}; + links, options, parent, resource_, true}}; // if the attributes is not nullptr, add attributes to the span. if (sampling_result.attributes) diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index d244ef2528..60544f6fdc 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -19,6 +19,7 @@ using opentelemetry::common::KeyValueIterableView; using opentelemetry::exporter::memory::InMemorySpanData; using opentelemetry::exporter::memory::InMemorySpanExporter; using opentelemetry::trace::SpanContext; +using opentelemetry::trace::TraceFlags; /** * A mock sampler that returns non-empty sampling results attributes. @@ -563,3 +564,51 @@ TEST(Tracer, ExpectParent) EXPECT_EQ(spandata_first->GetSpanId(), spandata_second->GetParentSpanId()); EXPECT_EQ(spandata_second->GetSpanId(), spandata_third->GetParentSpanId()); } + +TEST(Tracer, ExpectParentWithValidSpanContext) +{ + std::unique_ptr exporter(new InMemorySpanExporter()); + std::shared_ptr span_data = exporter->GetData(); + auto tracer = initTracer(std::move(exporter)); + auto spans = span_data.get()->GetSpans(); + + ASSERT_EQ(0, spans.size()); + + // produce valid SpanContext with pseudo span and trace Id. + uint8_t span_id_buf[trace_api::SpanId::kSize] = { + 1, + }; + trace_api::SpanId span_id{span_id_buf}; + uint8_t trace_id_buf[trace_api::TraceId::kSize] = { + 2, + }; + trace_api::TraceId trace_id{trace_id_buf}; + + trace_api::StartSpanOptions options; + options.parent = SpanContext(trace_id, span_id, TraceFlags{TraceFlags::kIsSampled}, true); + + auto span_first = tracer->StartSpan("span 1", options); + + EXPECT_EQ(span_first->GetContext().trace_flags().IsSampled(), true); + + options.parent = span_first->GetContext(); + auto span_second = tracer->StartSpan("span 2", options); + EXPECT_EQ(span_second->GetContext().trace_flags().IsSampled(), true); + + options.parent = span_second->GetContext(); + auto span_third = tracer->StartSpan("span 3", options); + EXPECT_EQ(span_third->GetContext().trace_flags().IsSampled(), true); + + span_third->End(); + span_second->End(); + span_first->End(); + + spans = span_data->GetSpans(); + ASSERT_EQ(3, spans.size()); + auto spandata_first = std::move(spans.at(2)); + auto spandata_second = std::move(spans.at(1)); + auto spandata_third = std::move(spans.at(0)); + + EXPECT_EQ(spandata_first->GetSpanId(), spandata_second->GetParentSpanId()); + EXPECT_EQ(spandata_second->GetSpanId(), spandata_third->GetParentSpanId()); +} From fa1f77eb9eee270449d9ff20705fc7121da24409 Mon Sep 17 00:00:00 2001 From: Muchene Date: Thu, 11 Mar 2021 09:31:01 -0500 Subject: [PATCH 2/4] Zipkin timestamp should be in microseconds according to the api (#601) --- exporters/zipkin/src/recordable.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/exporters/zipkin/src/recordable.cc b/exporters/zipkin/src/recordable.cc index eda23dcf89..074c903cd8 100644 --- a/exporters/zipkin/src/recordable.cc +++ b/exporters/zipkin/src/recordable.cc @@ -191,7 +191,8 @@ void Recordable::SetName(nostd::string_view name) noexcept void Recordable::SetStartTime(opentelemetry::core::SystemTimestamp start_time) noexcept { - span_["timestamp"] = start_time.time_since_epoch().count(); + span_["timestamp"] = + std::chrono::duration_cast(start_time.time_since_epoch()).count(); } void Recordable::SetDuration(std::chrono::nanoseconds duration) noexcept From 9aba9507b6f168f8e1625e5ac97580cac1e5c5a2 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 11 Mar 2021 20:25:42 +0530 Subject: [PATCH 3/4] Add TraceState in SamplerResult, and use it for creating span context. (#591) --- sdk/include/opentelemetry/sdk/trace/sampler.h | 5 ++++- .../opentelemetry/sdk/trace/samplers/always_off.h | 11 +++++++++-- .../opentelemetry/sdk/trace/samplers/always_on.h | 11 +++++++++-- sdk/src/trace/samplers/parent.cc | 4 ++-- sdk/src/trace/span.cc | 6 ++++-- sdk/src/trace/span.h | 2 ++ sdk/src/trace/tracer.cc | 6 +++--- sdk/test/trace/always_off_sampler_test.cc | 1 + sdk/test/trace/always_on_sampler_test.cc | 2 ++ sdk/test/trace/parent_sampler_test.cc | 10 ++++++++-- 10 files changed, 44 insertions(+), 14 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/sampler.h b/sdk/include/opentelemetry/sdk/trace/sampler.h index f44af1057f..d26b33bb97 100644 --- a/sdk/include/opentelemetry/sdk/trace/sampler.h +++ b/sdk/include/opentelemetry/sdk/trace/sampler.h @@ -5,6 +5,7 @@ #include "opentelemetry/trace/span_context.h" #include "opentelemetry/trace/span_context_kv_iterable.h" #include "opentelemetry/trace/trace_id.h" +#include "opentelemetry/trace/trace_state.h" #include "opentelemetry/version.h" #include @@ -41,6 +42,8 @@ struct SamplingResult Decision decision; // A set of span Attributes that will also be added to the Span. Can be nullptr. std::unique_ptr> attributes; + // The tracestate used by the span. + nostd::shared_ptr trace_state; }; /** @@ -61,7 +64,7 @@ class Sampler * @param name the name of the new Span. * @param spanKind the trace_api::SpanKind of the Span. * @param attributes list of AttributeValue with their keys. - * @param links TODO: Collection of links that will be associated with the Span to be created. + * @param links Collection of links that will be associated with the Span to be created. * @return sampling result whether span should be sampled or not. * @since 0.1.0 */ diff --git a/sdk/include/opentelemetry/sdk/trace/samplers/always_off.h b/sdk/include/opentelemetry/sdk/trace/samplers/always_off.h index f6e3221ce3..6a8b7f5581 100644 --- a/sdk/include/opentelemetry/sdk/trace/samplers/always_off.h +++ b/sdk/include/opentelemetry/sdk/trace/samplers/always_off.h @@ -20,14 +20,21 @@ class AlwaysOffSampler : public Sampler * @return Returns DROP always */ SamplingResult ShouldSample( - const trace_api::SpanContext & /*parent_context*/, + const trace_api::SpanContext &parent_context, trace_api::TraceId /*trace_id*/, nostd::string_view /*name*/, trace_api::SpanKind /*span_kind*/, const opentelemetry::common::KeyValueIterable & /*attributes*/, const trace_api::SpanContextKeyValueIterable & /*links*/) noexcept override { - return {Decision::DROP, nullptr}; + if (!parent_context.IsValid()) + { + return {Decision::DROP, nullptr, opentelemetry::trace::TraceState::GetDefault()}; + } + else + { + return {Decision::DROP, nullptr, parent_context.trace_state()}; + } } /** diff --git a/sdk/include/opentelemetry/sdk/trace/samplers/always_on.h b/sdk/include/opentelemetry/sdk/trace/samplers/always_on.h index fb5396e206..b717f9bdf7 100644 --- a/sdk/include/opentelemetry/sdk/trace/samplers/always_on.h +++ b/sdk/include/opentelemetry/sdk/trace/samplers/always_on.h @@ -19,14 +19,21 @@ class AlwaysOnSampler : public Sampler * @return Always return Decision RECORD_AND_SAMPLE */ inline SamplingResult ShouldSample( - const trace_api::SpanContext & /*parent_context*/, + const trace_api::SpanContext &parent_context, trace_api::TraceId /*trace_id*/, nostd::string_view /*name*/, trace_api::SpanKind /*span_kind*/, const opentelemetry::common::KeyValueIterable & /*attributes*/, const trace_api::SpanContextKeyValueIterable & /*links*/) noexcept override { - return {Decision::RECORD_AND_SAMPLE, nullptr}; + if (!parent_context.IsValid()) + { + return {Decision::RECORD_AND_SAMPLE, nullptr, opentelemetry::trace::TraceState::GetDefault()}; + } + else + { + return {Decision::RECORD_AND_SAMPLE, nullptr, parent_context.trace_state()}; + } } /** diff --git a/sdk/src/trace/samplers/parent.cc b/sdk/src/trace/samplers/parent.cc index 08095398a8..212a29e3c4 100644 --- a/sdk/src/trace/samplers/parent.cc +++ b/sdk/src/trace/samplers/parent.cc @@ -28,10 +28,10 @@ SamplingResult ParentBasedSampler::ShouldSample( // If parent exists: if (parent_context.IsSampled()) { - return {Decision::RECORD_AND_SAMPLE, nullptr}; + return {Decision::RECORD_AND_SAMPLE, nullptr, parent_context.trace_state()}; } - return {Decision::DROP, nullptr}; + return {Decision::DROP, nullptr, parent_context.trace_state()}; } nostd::string_view ParentBasedSampler::GetDescription() const noexcept diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index ca5243fb18..576d54f38c 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -65,6 +65,7 @@ Span::Span(std::shared_ptr &&tracer, const trace_api::StartSpanOptions &options, const trace_api::SpanContext &parent_span_context, const opentelemetry::sdk::resource::Resource &resource, + const nostd::shared_ptr trace_state, const bool sampled) noexcept : tracer_{std::move(tracer)}, processor_{processor}, @@ -98,8 +99,9 @@ Span::Span(std::shared_ptr &&tracer, trace_id, span_id, sampled ? trace_api::TraceFlags{trace_api::TraceFlags::kIsSampled} : trace_api::TraceFlags{}, false, - is_parent_span_valid ? parent_span_context.trace_state() - : trace_api::TraceState::GetDefault())); + trace_state ? trace_state + : is_parent_span_valid ? parent_span_context.trace_state() + : trace_api::TraceState::GetDefault())); attributes.ForEachKeyValue( [&](nostd::string_view key, opentelemetry::common::AttributeValue value) noexcept { diff --git a/sdk/src/trace/span.h b/sdk/src/trace/span.h index 0f8e843e48..01fb995517 100644 --- a/sdk/src/trace/span.h +++ b/sdk/src/trace/span.h @@ -23,6 +23,8 @@ class Span final : public trace_api::Span const trace_api::StartSpanOptions &options, const trace_api::SpanContext &parent_span_context, const opentelemetry::sdk::resource::Resource &resource, + const nostd::shared_ptr trace_state = + trace_api::TraceState::GetDefault(), const bool sampled = false) noexcept; ~Span() override; diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index b85164b6ac..93fdc6f680 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -73,9 +73,9 @@ nostd::shared_ptr Tracer::StartSpan( } else { - auto span = nostd::shared_ptr{ - new (std::nothrow) Span{this->shared_from_this(), processor_.load(), name, attributes, - links, options, parent, resource_, true}}; + auto span = nostd::shared_ptr{new (std::nothrow) Span{ + this->shared_from_this(), processor_.load(), name, attributes, links, options, parent, + resource_, sampling_result.trace_state, true}}; // if the attributes is not nullptr, add attributes to the span. if (sampling_result.attributes) diff --git a/sdk/test/trace/always_off_sampler_test.cc b/sdk/test/trace/always_off_sampler_test.cc index 822b81f93e..6fea04906d 100644 --- a/sdk/test/trace/always_off_sampler_test.cc +++ b/sdk/test/trace/always_off_sampler_test.cc @@ -27,6 +27,7 @@ TEST(AlwaysOffSampler, ShouldSample) ASSERT_EQ(Decision::DROP, sampling_result.decision); ASSERT_EQ(nullptr, sampling_result.attributes); + ASSERT_EQ("", sampling_result.trace_state->ToHeader()); } TEST(AlwaysOffSampler, GetDescription) diff --git a/sdk/test/trace/always_on_sampler_test.cc b/sdk/test/trace/always_on_sampler_test.cc index 7dad69acc4..9adb5258a5 100644 --- a/sdk/test/trace/always_on_sampler_test.cc +++ b/sdk/test/trace/always_on_sampler_test.cc @@ -34,6 +34,7 @@ TEST(AlwaysOnSampler, ShouldSample) ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result.decision); ASSERT_EQ(nullptr, sampling_result.attributes); + ASSERT_EQ("", sampling_result.trace_state->ToHeader()); // Test with a valid trace id and empty parent context sampling_result = sampler.ShouldSample( @@ -44,6 +45,7 @@ TEST(AlwaysOnSampler, ShouldSample) ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result.decision); ASSERT_EQ(nullptr, sampling_result.attributes); + ASSERT_EQ("", sampling_result.trace_state->ToHeader()); } TEST(AlwaysOnSampler, GetDescription) diff --git a/sdk/test/trace/parent_sampler_test.cc b/sdk/test/trace/parent_sampler_test.cc index ea9718bef5..692a1bf92d 100644 --- a/sdk/test/trace/parent_sampler_test.cc +++ b/sdk/test/trace/parent_sampler_test.cc @@ -31,9 +31,11 @@ TEST(ParentBasedSampler, ShouldSample) opentelemetry::common::KeyValueIterableView view{m1}; trace_api::SpanContextKeyValueIterableView links{l1}; - trace_api::SpanContext parent_context_sampled(trace_id, span_id, trace_api::TraceFlags{1}, false); + auto trace_state = opentelemetry::trace::TraceState::FromHeader("congo=t61rcWkgMzE"); + trace_api::SpanContext parent_context_sampled(trace_id, span_id, trace_api::TraceFlags{1}, false, + trace_state); trace_api::SpanContext parent_context_nonsampled(trace_id, span_id, trace_api::TraceFlags{0}, - false); + false, trace_state); // Case 1: Parent doesn't exist. Return result of delegateSampler() auto sampling_result = sampler_off.ShouldSample(trace_api::SpanContext::GetInvalid(), trace_id, @@ -43,16 +45,20 @@ TEST(ParentBasedSampler, ShouldSample) ASSERT_EQ(Decision::DROP, sampling_result.decision); ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result2.decision); + ASSERT_EQ("", sampling_result.trace_state->ToHeader()); + ASSERT_EQ("", sampling_result2.trace_state->ToHeader()); // Case 2: Parent exists and SampledFlag is true auto sampling_result3 = sampler_off.ShouldSample(parent_context_sampled, trace_id, "", span_kind, view, links); ASSERT_EQ(Decision::RECORD_AND_SAMPLE, sampling_result3.decision); + ASSERT_EQ("congo=t61rcWkgMzE", sampling_result3.trace_state->ToHeader()); // Case 3: Parent exists and SampledFlag is false auto sampling_result4 = sampler_on.ShouldSample(parent_context_nonsampled, trace_id, "", span_kind, view, links); ASSERT_EQ(Decision::DROP, sampling_result4.decision); + ASSERT_EQ("congo=t61rcWkgMzE", sampling_result4.trace_state->ToHeader()); } TEST(ParentBasedSampler, GetDescription) From b80cc91b9e9e3482b39c4e85fbabc19b0795225a Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Thu, 11 Mar 2021 20:06:31 -0800 Subject: [PATCH 4/4] Breathe needs to be initialized in parent sphinx project (#606) --- docs/public/conf.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/public/conf.py b/docs/public/conf.py index 7a3b4614a0..9b3071bf28 100644 --- a/docs/public/conf.py +++ b/docs/public/conf.py @@ -44,8 +44,15 @@ # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom # ones. extensions = [ + "breathe", ] +breathe_projects = { + "OpenTelemetry C++ API": "../../api/docs/doxyoutput/xml" +} + +breathe_default_project = "OpenTelemetry C++ API" + primary_domain = "cpp" higlight_language = "cpp"