Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Associate valid SpanId/SpanContext with Spans irrespective of the Sampling decision. #879

Merged
merged 6 commits into from
Jul 1, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions api/include/opentelemetry/trace/noop.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@ class NoopSpan final : public Span
{
public:
explicit NoopSpan(const std::shared_ptr<Tracer> &tracer) noexcept
: tracer_{tracer}, span_context_{SpanContext::GetInvalid()}
: tracer_{tracer}, span_context_{new SpanContext(false, false)}
{}

explicit NoopSpan(const std::shared_ptr<Tracer> &tracer,
nostd::unique_ptr<SpanContext> span_context) noexcept
: tracer_{tracer}, span_context_{std::move(span_context)}
{}

void SetAttribute(nostd::string_view /*key*/,
Expand All @@ -55,11 +60,11 @@ class NoopSpan final : public Span

bool IsRecording() const noexcept override { return false; }

SpanContext GetContext() const noexcept override { return span_context_; }
SpanContext GetContext() const noexcept override { return *span_context_.get(); }
lalitb marked this conversation as resolved.
Show resolved Hide resolved

private:
std::shared_ptr<Tracer> tracer_;
SpanContext span_context_;
nostd::unique_ptr<SpanContext> span_context_;
};

/**
Expand Down
43 changes: 32 additions & 11 deletions api/test/trace/noop_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@

#include <gtest/gtest.h>

using opentelemetry::common::SystemTimestamp;
using opentelemetry::trace::NoopTracer;
using opentelemetry::trace::SpanContext;
using opentelemetry::trace::Tracer;
namespace trace_api = opentelemetry::trace;
namespace nonstd = opentelemetry::nostd;
namespace common = opentelemetry::common;

TEST(NoopTest, UseNoopTracers)
{
std::shared_ptr<Tracer> tracer{new NoopTracer{}};
std::shared_ptr<trace_api::Tracer> tracer{new trace_api::NoopTracer{}};
auto s1 = tracer->StartSpan("abc");

std::map<std::string, std::string> attributes1;
Expand All @@ -41,20 +40,42 @@ TEST(NoopTest, UseNoopTracers)

s1->UpdateName("test_name");

SystemTimestamp t1;
common::SystemTimestamp t1;
s1->AddEvent("test_time_stamp", t1);

s1->GetContext();
}

TEST(NoopTest, StartSpan)
{
std::shared_ptr<Tracer> tracer{new NoopTracer{}};
std::shared_ptr<trace_api::Tracer> tracer{new trace_api::NoopTracer{}};

std::map<std::string, std::string> attrs = {{"a", "3"}};
std::vector<std::pair<SpanContext, std::map<std::string, std::string>>> links = {
{SpanContext(false, false), attrs}};
std::map<std::string, std::string> attrs = {{"a", "3"}};
std::vector<std::pair<trace_api::SpanContext, std::map<std::string, std::string>>> links = {
{trace_api::SpanContext(false, false), attrs}};
auto s1 = tracer->StartSpan("abc", attrs, links);

auto s2 = tracer->StartSpan("efg", {{"a", 3}}, {{SpanContext(false, false), {{"b", 4}}}});
auto s2 =
tracer->StartSpan("efg", {{"a", 3}}, {{trace_api::SpanContext(false, false), {{"b", 4}}}});
}

TEST(NoopTest, CreateSpanValidSpanContext)
{
// Create valid spancontext for NoopSpan

constexpr uint8_t buf_span[] = {1, 2, 3, 4, 5, 6, 7, 8};
constexpr uint8_t buf_trace[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16};
auto trace_id = trace_api::TraceId{buf_trace};
auto span_id = trace_api::SpanId{buf_span};
auto span_context = nonstd::unique_ptr<trace_api::SpanContext>(
new trace_api::SpanContext{trace_id, span_id, trace_api::TraceFlags{true}, false});
std::shared_ptr<trace_api::Tracer> tracer{new trace_api::NoopTracer{}};
auto s1 =
nonstd::shared_ptr<trace_api::Span>(new trace_api::NoopSpan(tracer, std::move(span_context)));
auto stored_span_context = s1->GetContext();
EXPECT_EQ(stored_span_context.span_id(), span_id);
EXPECT_EQ(stored_span_context.trace_id(), trace_id);

s1->AddEvent("even1"); // noop
s1->End(); // noop
}
22 changes: 12 additions & 10 deletions sdk/src/trace/tracer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,25 +46,27 @@ nostd::shared_ptr<trace_api::Span> Tracer::StartSpan(

auto sampling_result = context_->GetSampler().ShouldSample(parent_context, trace_id, name,
options.kind, attributes, links);
auto trace_flags = sampling_result.decision == Decision::DROP
? trace_api::TraceFlags{}
: trace_api::TraceFlags{trace_api::TraceFlags::kIsSampled};

auto span_context = std::unique_ptr<trace_api::SpanContext>(new trace_api::SpanContext(
trace_id, span_id, trace_flags, false,
sampling_result.trace_state ? sampling_result.trace_state
: is_parent_span_valid ? parent_context.trace_state()
: trace_api::TraceState::GetDefault()));

if (sampling_result.decision == Decision::DROP)
{
// Don't allocate a no-op span for every DROP decision, but use a static
// singleton for this case.
static nostd::shared_ptr<trace_api::Span> noop_span(
new trace_api::NoopSpan{this->shared_from_this()});
// create no-op span with valid span-context.

auto noop_span = nostd::shared_ptr<trace_api::Span>{
new (std::nothrow) trace_api::NoopSpan(this->shared_from_this(), std::move(span_context))};
return noop_span;
}
else
{

auto span_context = std::unique_ptr<trace_api::SpanContext>(new trace_api::SpanContext(
trace_id, span_id, trace_api::TraceFlags{trace_api::TraceFlags::kIsSampled}, false,
sampling_result.trace_state ? sampling_result.trace_state
: is_parent_span_valid ? parent_context.trace_state()
: trace_api::TraceState::GetDefault()));

auto span = nostd::shared_ptr<trace_api::Span>{
new (std::nothrow) Span{this->shared_from_this(), name, attributes, links, options,
parent_context, std::move(span_context)}};
Expand Down
8 changes: 7 additions & 1 deletion sdk/test/trace/tracer_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,14 @@ TEST(Tracer, StartSpanSampleOff)
auto tracer_off = initTracer(std::move(exporter), new AlwaysOffSampler());

// This span will not be recorded.
tracer_off->StartSpan("span 2")->End();
auto span = tracer_off->StartSpan("span 2");

// Always generate a valid span-context (span-id)
auto context = span->GetContext();
EXPECT_TRUE(context.IsValid());
EXPECT_FALSE(context.IsSampled());

span->End();
// The span doesn't write any span data because the sampling decision is alway
// DROP.
ASSERT_EQ(0, span_data->GetSpans().size());
Expand Down