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

Create SDK TRACE context, a shared reference to tracer pipelines between Tracer/TracerProvider. Update tests. #590

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from 8 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ Increment the:

## [Unreleased]

* Added TracerContext to so Tracer and TracerProvider share SDK configuration and
updates to exporters apply to all tracers #590

## [0.0.1] 2020-12-16

### Added
Expand Down
2 changes: 1 addition & 1 deletion api/include/opentelemetry/trace/span_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ class SpanContext final
trace_api::TraceId trace_id_;
trace_api::SpanId span_id_;
trace_api::TraceFlags trace_flags_;
nostd::shared_ptr<trace_api::TraceState> trace_state_;
bool is_remote_ = false;
nostd::shared_ptr<trace_api::TraceState> trace_state_;
};
} // namespace trace
OPENTELEMETRY_END_NAMESPACE
1 change: 0 additions & 1 deletion api/include/opentelemetry/trace/trace_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ class TraceState

std::size_t begin{0};
std::size_t end{0};
bool invalid_header = false;
while (begin < header.size() && ts->num_entries_ < kMaxKeyValuePairs)
{
// find list-member
Expand Down
3 changes: 1 addition & 2 deletions examples/simple/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ void initTracer()
auto processor = std::shared_ptr<sdktrace::SpanProcessor>(
new sdktrace::SimpleSpanProcessor(std::move(exporter)));
auto provider = nostd::shared_ptr<opentelemetry::trace::TracerProvider>(
new sdktrace::TracerProvider(processor, opentelemetry::sdk::resource::Resource::Create({}),
std::make_shared<opentelemetry::sdk::trace::AlwaysOnSampler>()));
new sdktrace::TracerProvider(processor));

// Set the global trace provider
opentelemetry::trace::Provider::SetTracerProvider(provider);
Expand Down
3 changes: 2 additions & 1 deletion ext/test/w3c_tracecontext_test/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ void initTracer()
new opentelemetry::exporter::trace::OStreamSpanExporter);
auto processor = std::shared_ptr<sdktrace::SpanProcessor>(
new sdktrace::SimpleSpanProcessor(std::move(exporter)));
auto context = std::make_shared<sdktrace::TracerContext>(processor);
auto provider = nostd::shared_ptr<opentelemetry::trace::TracerProvider>(
new sdktrace::TracerProvider(processor));
new sdktrace::TracerProvider(context));
// Set the global trace provider
opentelemetry::trace::Provider::SetTracerProvider(provider);
}
Expand Down
5 changes: 3 additions & 2 deletions ext/test/zpages/tracez_data_aggregator_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ class TracezDataAggregatorTest : public ::testing::Test
void SetUp() override
{
std::shared_ptr<TracezSpanProcessor> processor(new TracezSpanProcessor());
auto resource = opentelemetry::sdk::resource::Resource::Create({});
tracer = std::shared_ptr<opentelemetry::trace::Tracer>(new Tracer(processor, resource));
auto resource = opentelemetry::sdk::resource::Resource::Create({});
auto context = std::make_shared<TracerContext>(processor, resource);
tracer = std::shared_ptr<opentelemetry::trace::Tracer>(new Tracer(context));
tracez_data_aggregator = std::unique_ptr<TracezDataAggregator>(
new TracezDataAggregator(processor, milliseconds(10)));
}
Expand Down
10 changes: 5 additions & 5 deletions ext/test/zpages/tracez_processor_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,11 @@ class TracezProcessor : public ::testing::Test
{
processor = std::shared_ptr<TracezSpanProcessor>(new TracezSpanProcessor());
auto resource = opentelemetry::sdk::resource::Resource::Create({});

tracer = std::shared_ptr<opentelemetry::trace::Tracer>(new Tracer(processor, resource));
auto spans = processor->GetSpanSnapshot();
running = spans.running;
completed = std::move(spans.completed);
auto context = std::make_shared<TracerContext>(processor, resource);
tracer = std::shared_ptr<opentelemetry::trace::Tracer>(new Tracer(context));
auto spans = processor->GetSpanSnapshot();
running = spans.running;
completed = std::move(spans.completed);

span_names = {"s0", "s2", "s1", "s1", "s"};
}
Expand Down
2 changes: 2 additions & 0 deletions sdk/include/opentelemetry/sdk/common/atomic_unique_ptr.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ class AtomicUniquePtr
public:
AtomicUniquePtr() noexcept {}

explicit AtomicUniquePtr(std::unique_ptr<T> &&other) noexcept : ptr_(other.release()) {}

~AtomicUniquePtr() noexcept { Reset(); }

T &operator*() const noexcept { return *Get(); }
Expand Down
18 changes: 6 additions & 12 deletions sdk/include/opentelemetry/sdk/trace/tracer.h
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
#pragma once

#include "opentelemetry/sdk/common/atomic_shared_ptr.h"
#include "opentelemetry/sdk/resource/resource.h"
#include "opentelemetry/sdk/trace/processor.h"
#include "opentelemetry/sdk/trace/samplers/always_on.h"
#include "opentelemetry/sdk/trace/tracer_context.h"
#include "opentelemetry/trace/noop.h"
#include "opentelemetry/trace/tracer.h"
#include "opentelemetry/version.h"
Expand All @@ -23,28 +21,26 @@ class Tracer final : public trace_api::Tracer, public std::enable_shared_from_th
* @param processor The span processor for this tracer. This must not be a
* nullptr.
*/
explicit Tracer(std::shared_ptr<SpanProcessor> processor,
const opentelemetry::sdk::resource::Resource &resource,
std::shared_ptr<Sampler> sampler = std::make_shared<AlwaysOnSampler>()) noexcept;
explicit Tracer(std::shared_ptr<sdk::trace::TracerContext> context) noexcept;

/**
* Set the span processor associated with this tracer.
* @param processor The new span processor for this tracer. This must not be
* a nullptr.
*/
void SetProcessor(std::shared_ptr<SpanProcessor> processor) noexcept;
void SetProcessor(std::unique_ptr<SpanProcessor> processor) noexcept;

/**
* Obtain the span processor associated with this tracer.
* @return The span processor for this tracer.
*/
std::shared_ptr<SpanProcessor> GetProcessor() const noexcept;
SpanProcessor *GetProcessor() const noexcept;

/**
* Obtain the sampler associated with this tracer.
* @return The sampler for this tracer.
*/
std::shared_ptr<Sampler> GetSampler() const noexcept;
Sampler *GetSampler() const noexcept;

nostd::shared_ptr<trace_api::Span> StartSpan(
nostd::string_view name,
Expand All @@ -57,9 +53,7 @@ class Tracer final : public trace_api::Tracer, public std::enable_shared_from_th
void CloseWithMicroseconds(uint64_t timeout) noexcept override;

private:
opentelemetry::sdk::AtomicSharedPtr<SpanProcessor> processor_;
const std::shared_ptr<Sampler> sampler_;
const opentelemetry::sdk::resource::Resource &resource_;
std::shared_ptr<sdk::trace::TracerContext> context_;
};
} // namespace trace
} // namespace sdk
Expand Down
77 changes: 77 additions & 0 deletions sdk/include/opentelemetry/sdk/trace/tracer_context.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
#pragma once

#include "opentelemetry/sdk/common/atomic_shared_ptr.h"
#include "opentelemetry/sdk/common/atomic_unique_ptr.h"
#include "opentelemetry/sdk/resource/resource.h"
#include "opentelemetry/sdk/trace/processor.h"
#include "opentelemetry/sdk/trace/samplers/always_on.h"
#include "opentelemetry/version.h"

OPENTELEMETRY_BEGIN_NAMESPACE
namespace sdk
{
namespace trace
{
/**
* A class which stores the TracerProvider context.
*
* This class meets the following design criteria:
* - A shared reference between TracerProvider and Tracers instantiated.
* - A thread-safe class that allows updating/altering processor/exporter pipelines
* and sampling config.
* - The owner/destroyer of Processors/Exporters. These will remain active until
* this class is destroyed. I.e. Sampling, Exporting, flushing etc. are all ok if this
* object is alive, and they willl work together. If this object is destroyed, then
* no shared references to Processor, Exporter, Recordable etc. should exist, and all
* associated pipelines will have been flushed.
*
*
* TODOs - This should allow more than one processor pipeline to be attached.
*/
class TracerContext
{
public:
explicit TracerContext(std::shared_ptr<SpanProcessor> processor,
opentelemetry::sdk::resource::Resource resource =
opentelemetry::sdk::resource::Resource::Create({}),
std::unique_ptr<Sampler> sampler =
std::unique_ptr<AlwaysOnSampler>(new AlwaysOnSampler)) noexcept;
/**
* Obtain the span processor associated with this tracer context.
* <p>
* This does NOT give ownership to the caller and must be used within the lifecycle of
* a shared `TracerContext` object.
*/
SpanProcessor *GetProcessor() const noexcept;
jsuereth marked this conversation as resolved.
Show resolved Hide resolved
/**
* Set the span processor associated with this tracer.
* @param processor The new span processor for this tracer. This must not be
* a nullptr.
*/
void SetProcessor(std::shared_ptr<SpanProcessor> processor) noexcept;

/**
* Obtain the sampler associated with this tracer.
* @return The sampler for this tracer.
*/
Sampler *GetSampler() const noexcept;

/**
* Obtain the resource associated with this tracer context.
* @return The resource for this tracer context.
*/
const opentelemetry::sdk::resource::Resource &GetResource() const noexcept;

void ForceFlushWithMicroseconds(uint64_t timeout) noexcept;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a way to force-flush spans in some ephemeral compute environments. There's been SOME additions to the spec around ForceFlush. I think we need this to support that version of the spec.


private:
// Note: Currently Z-Pages Exporter relies on sharing processor w/ HTTP server.
// Likely this should be decoupled going forward, for cleaner shutdown semantics
// and ownership on pipelines.
opentelemetry::sdk::AtomicSharedPtr<SpanProcessor> processor_;
opentelemetry::sdk::resource::Resource resource_;
opentelemetry::sdk::common::AtomicUniquePtr<Sampler> sampler_;
jsuereth marked this conversation as resolved.
Show resolved Hide resolved
};
} // namespace trace
} // namespace sdk
OPENTELEMETRY_END_NAMESPACE
28 changes: 17 additions & 11 deletions sdk/include/opentelemetry/sdk/trace/tracer_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "opentelemetry/sdk/trace/processor.h"
#include "opentelemetry/sdk/trace/samplers/always_on.h"
#include "opentelemetry/sdk/trace/tracer.h"
#include "opentelemetry/sdk/trace/tracer_context.h"
#include "opentelemetry/trace/tracer_provider.h"

OPENTELEMETRY_BEGIN_NAMESPACE
Expand All @@ -26,11 +27,17 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider
* @param sampler The sampler for this tracer provider. This must
* not be a nullptr.
*/
explicit TracerProvider(
std::shared_ptr<SpanProcessor> processor,
opentelemetry::sdk::resource::Resource &&resource =
opentelemetry::sdk::resource::Resource::Create({}),
std::shared_ptr<Sampler> sampler = std::make_shared<AlwaysOnSampler>()) noexcept;
explicit TracerProvider(std::shared_ptr<SpanProcessor> processor,
opentelemetry::sdk::resource::Resource resource =
opentelemetry::sdk::resource::Resource::Create({}),
std::unique_ptr<Sampler> sampler =
std::unique_ptr<AlwaysOnSampler>(new AlwaysOnSampler)) noexcept;

/**
* Initialize a new tracer provider with a specified context
* @param context The shared tracer configuration/pipeline for this provider.
*/
explicit TracerProvider(std::shared_ptr<sdk::trace::TracerContext> context) noexcept;

opentelemetry::nostd::shared_ptr<opentelemetry::trace::Tracer> GetTracer(
nostd::string_view library_name,
Expand All @@ -41,19 +48,19 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider
* @param processor The new span processor for this tracer provider. This
* must not be a nullptr.
*/
void SetProcessor(std::shared_ptr<SpanProcessor> processor) noexcept;
void SetProcessor(std::unique_ptr<SpanProcessor> processor) noexcept;

/**
* Obtain the span processor associated with this tracer provider.
* @return The span processor for this tracer provider.
*/
std::shared_ptr<SpanProcessor> GetProcessor() const noexcept;
SpanProcessor *GetProcessor() const noexcept;

/**
* Obtain the sampler associated with this tracer provider.
* @return The sampler for this tracer provider.
*/
std::shared_ptr<Sampler> GetSampler() const noexcept;
Sampler *GetSampler() const noexcept;

/**
* Obtain the resource associated with this tracer provider.
Expand All @@ -67,10 +74,9 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider
bool Shutdown() noexcept;

private:
opentelemetry::sdk::AtomicSharedPtr<SpanProcessor> processor_;
std::shared_ptr<sdk::trace::TracerContext> context_;
// TODO: We should have one-tracer per-instrumentation library, per specification.
std::shared_ptr<opentelemetry::trace::Tracer> tracer_;
const std::shared_ptr<Sampler> sampler_;
const opentelemetry::sdk::resource::Resource resource_;
};
} // namespace trace
} // namespace sdk
Expand Down
9 changes: 7 additions & 2 deletions sdk/src/trace/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
add_library(
opentelemetry_trace
tracer_provider.cc tracer.cc span.cc batch_span_processor.cc
samplers/parent.cc samplers/trace_id_ratio.cc)
tracer_provider.cc
tracer.cc
span.cc
batch_span_processor.cc
samplers/parent.cc
samplers/trace_id_ratio.cc
tracer_context.cc)

set_target_properties(opentelemetry_trace PROPERTIES EXPORT_NAME trace)

Expand Down
11 changes: 4 additions & 7 deletions sdk/src/trace/span.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,13 @@ trace_api::SpanId GenerateRandomSpanId()
}

Span::Span(std::shared_ptr<Tracer> &&tracer,
std::shared_ptr<SpanProcessor> 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
const trace_api::SpanContext &parent_span_context) noexcept
: tracer_{std::move(tracer)},
processor_{processor},
recordable_{processor_->MakeRecordable()},
recordable_{tracer_->GetProcessor()->MakeRecordable()},
start_steady_time{options.start_steady_time},
has_ended_{false}
{
Expand Down Expand Up @@ -114,7 +111,7 @@ Span::Span(std::shared_ptr<Tracer> &&tracer,
recordable_->SetStartTime(NowOr(options.start_system_time));
start_steady_time = NowOr(options.start_steady_time);
// recordable_->SetResource(resource_); TODO
processor_->OnStart(*recordable_, parent_span_context);
tracer_->GetProcessor()->OnStart(*recordable_, parent_span_context);
}

Span::~Span()
Expand Down Expand Up @@ -201,7 +198,7 @@ void Span::End(const trace_api::EndSpanOptions &options) noexcept
recordable_->SetDuration(std::chrono::steady_clock::time_point(end_steady_time) -
std::chrono::steady_clock::time_point(start_steady_time));

processor_->OnEnd(std::move(recordable_));
tracer_->GetProcessor()->OnEnd(std::move(recordable_));
recordable_.reset();
}

Expand Down
8 changes: 3 additions & 5 deletions sdk/src/trace/span.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <mutex>

#include "opentelemetry/sdk/trace/tracer.h"
#include "opentelemetry/sdk/trace/tracer_context.h"
#include "opentelemetry/version.h"

OPENTELEMETRY_BEGIN_NAMESPACE
Expand All @@ -16,13 +17,11 @@ class Span final : public trace_api::Span
{
public:
explicit Span(std::shared_ptr<Tracer> &&tracer,
std::shared_ptr<SpanProcessor> 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;
const trace_api::SpanContext &parent_span_context) noexcept;

~Span() override;

Expand All @@ -49,8 +48,7 @@ class Span final : public trace_api::Span
trace_api::SpanContext GetContext() const noexcept override { return *span_context_.get(); }

private:
std::shared_ptr<trace_api::Tracer> tracer_;
std::shared_ptr<SpanProcessor> processor_;
std::shared_ptr<Tracer> tracer_;
mutable std::mutex mu_;
std::unique_ptr<Recordable> recordable_;
opentelemetry::core::SteadyTimestamp start_steady_time;
Expand Down
Loading