Skip to content

Commit

Permalink
[SDK] TracerProvider should own TracerContext, not share it (open-tel…
Browse files Browse the repository at this point in the history
  • Loading branch information
marcalff authored Jul 8, 2023
1 parent d6cebe1 commit 8b61318
Show file tree
Hide file tree
Showing 15 changed files with 60 additions and 40 deletions.
13 changes: 12 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ Increment the:
* [SDK] MeterProvider should own MeterContext, not share it
[#2218](https://github.com/open-telemetry/opentelemetry-cpp/pull/2218)

Important changes:
* [SDK] TracerProvider should own TracerContext, not share it
[#2221](https://github.com/open-telemetry/opentelemetry-cpp/pull/2221)

Breaking changes:

* [REMOVAL] Remove the jaeger exporter
[#2031](https://github.com/open-telemetry/opentelemetry-cpp/pull/2031)
Expand All @@ -37,6 +40,14 @@ Important changes:
`MeterContext`, instead of a `shared_ptr`.
* Please adjust SDK configuration code accordingly.

* [SDK] TracerProvider should own TracerContext, not share it
[#2221](https://github.com/open-telemetry/opentelemetry-cpp/pull/2221)
* The `TracerProvider` constructor now takes a `unique_ptr` on
`TracerContext`, instead of a `shared_ptr`.
* The `LoggerProvider` constructor now takes a `unique_ptr` on
`LoggerContext`, instead of a `shared_ptr`.
* Please adjust SDK configuration code accordingly.

## [1.9.1] 2023-05-26

* [DEPRECATION] Drop C++11 support
Expand Down
4 changes: 2 additions & 2 deletions examples/grpc/tracer_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,10 @@ void InitTracer()
std::vector<std::unique_ptr<opentelemetry::sdk::trace::SpanProcessor>> processors;
processors.push_back(std::move(processor));
// Default is an always-on sampler.
std::shared_ptr<opentelemetry::sdk::trace::TracerContext> context =
std::unique_ptr<opentelemetry::sdk::trace::TracerContext> context =
opentelemetry::sdk::trace::TracerContextFactory::Create(std::move(processors));
std::shared_ptr<opentelemetry::trace::TracerProvider> provider =
opentelemetry::sdk::trace::TracerProviderFactory::Create(context);
opentelemetry::sdk::trace::TracerProviderFactory::Create(std::move(context));
// Set the global trace provider
opentelemetry::trace::Provider::SetTracerProvider(provider);

Expand Down
4 changes: 2 additions & 2 deletions examples/http/tracer_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ void InitTracer()
std::vector<std::unique_ptr<opentelemetry::sdk::trace::SpanProcessor>> processors;
processors.push_back(std::move(processor));
// Default is an always-on sampler.
std::shared_ptr<opentelemetry::sdk::trace::TracerContext> context =
std::unique_ptr<opentelemetry::sdk::trace::TracerContext> context =
opentelemetry::sdk::trace::TracerContextFactory::Create(std::move(processors));
std::shared_ptr<opentelemetry::trace::TracerProvider> provider =
opentelemetry::sdk::trace::TracerProviderFactory::Create(context);
opentelemetry::sdk::trace::TracerProviderFactory::Create(std::move(context));
// Set the global trace provider
opentelemetry::trace::Provider::SetTracerProvider(provider);

Expand Down
7 changes: 4 additions & 3 deletions ext/test/w3c_tracecontext_test/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ void initTracer()
new trace_sdk::SimpleSpanProcessor(std::move(exporter)));
std::vector<std::unique_ptr<trace_sdk::SpanProcessor>> processors;
processors.push_back(std::move(processor));
auto context = std::make_shared<trace_sdk::TracerContext>(std::move(processors));
auto provider =
nostd::shared_ptr<trace_api::TracerProvider>(new trace_sdk::TracerProvider(context));
auto context = std::unique_ptr<trace_sdk::TracerContext>(
new trace_sdk::TracerContext(std::move(processors)));
auto provider = nostd::shared_ptr<trace_api::TracerProvider>(
new trace_sdk::TracerProvider(std::move(context)));
// Set the global trace provider
trace_api::Provider::SetTracerProvider(provider);
}
Expand Down
6 changes: 3 additions & 3 deletions sdk/include/opentelemetry/sdk/logs/logger_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
# include <memory>
# include <vector>

# include "opentelemetry/sdk/logs/processor.h"
# include "opentelemetry/sdk/resource/resource.h"
# include "opentelemetry/version.h"

Expand All @@ -17,7 +18,6 @@ namespace sdk
{
namespace logs
{
class LogRecordProcessor;

/**
* A class which stores the LoggerContext context.
Expand All @@ -40,10 +40,10 @@ class LoggerContext
opentelemetry::sdk::resource::Resource::Create({})) noexcept;

/**
* Attaches a log processor to list of configured processors to this tracer context.
* Attaches a log processor to list of configured processors to this logger context.
* Processor once attached can't be removed.
* @param processor The new log processor for this tracer. This must not be
* a nullptr. Ownership is given to the `TracerContext`.
* a nullptr. Ownership is given to the `LoggerContext`.
*
* Note: This method is not thread safe.
*/
Expand Down
6 changes: 3 additions & 3 deletions sdk/include/opentelemetry/sdk/logs/logger_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ class LoggerProvider final : public opentelemetry::logs::LoggerProvider

/**
* Initialize a new logger provider with a specified context
* @param context The shared logger configuration/pipeline for this provider.
* @param context The owned logger configuration/pipeline for this provider.
*/
explicit LoggerProvider(std::shared_ptr<sdk::logs::LoggerContext> context) noexcept;
explicit LoggerProvider(std::unique_ptr<LoggerContext> context) noexcept;

~LoggerProvider() override;

Expand Down Expand Up @@ -106,7 +106,7 @@ class LoggerProvider final : public opentelemetry::logs::LoggerProvider
private:
// order of declaration is important here - loggers should destroy only after context.
std::vector<std::shared_ptr<opentelemetry::sdk::logs::Logger>> loggers_;
std::shared_ptr<sdk::logs::LoggerContext> context_;
std::shared_ptr<LoggerContext> context_;
std::mutex lock_;
};
} // namespace logs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class OPENTELEMETRY_EXPORT LoggerProviderFactory
* Create a LoggerProvider.
*/
static std::unique_ptr<opentelemetry::logs::LoggerProvider> Create(
std::shared_ptr<sdk::logs::LoggerContext> context);
std::unique_ptr<LoggerContext> context);
};

} // namespace logs
Expand Down
4 changes: 2 additions & 2 deletions sdk/include/opentelemetry/sdk/trace/tracer_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider

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

~TracerProvider() override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class OPENTELEMETRY_EXPORT TracerProviderFactory
/* Create with a tracer context. */

static std::unique_ptr<opentelemetry::trace::TracerProvider> Create(
std::shared_ptr<TracerContext> context);
std::unique_ptr<TracerContext> context);
};

} // namespace trace
Expand Down
14 changes: 6 additions & 8 deletions sdk/src/logs/logger_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,29 +24,27 @@ LoggerProvider::LoggerProvider(std::unique_ptr<LogRecordProcessor> &&processor,
{
std::vector<std::unique_ptr<LogRecordProcessor>> processors;
processors.emplace_back(std::move(processor));
context_ = std::make_shared<sdk::logs::LoggerContext>(std::move(processors), std::move(resource));
context_ = std::make_shared<LoggerContext>(std::move(processors), std::move(resource));
OTEL_INTERNAL_LOG_DEBUG("[LoggerProvider] LoggerProvider created.");
}

LoggerProvider::LoggerProvider(std::vector<std::unique_ptr<LogRecordProcessor>> &&processors,
opentelemetry::sdk::resource::Resource resource) noexcept
: context_{
std::make_shared<sdk::logs::LoggerContext>(std::move(processors), std::move(resource))}
: context_{std::make_shared<LoggerContext>(std::move(processors), std::move(resource))}
{}

LoggerProvider::LoggerProvider() noexcept
: context_{std::make_shared<sdk::logs::LoggerContext>(
std::vector<std::unique_ptr<LogRecordProcessor>>{})}
: context_{std::make_shared<LoggerContext>(std::vector<std::unique_ptr<LogRecordProcessor>>{})}
{}

LoggerProvider::LoggerProvider(std::shared_ptr<sdk::logs::LoggerContext> context) noexcept
: context_{context}
LoggerProvider::LoggerProvider(std::unique_ptr<LoggerContext> context) noexcept
: context_(std::move(context))
{}

LoggerProvider::~LoggerProvider()
{
// Logger hold the shared pointer to the context. So we can not use destructor of LoggerContext to
// Shutdown and flush all pending recordables when we hasve more than one loggers.These
// Shutdown and flush all pending recordables when we have more than one loggers. These
// recordables may use the raw pointer of instrumentation_scope_ in Logger
if (context_)
{
Expand Down
6 changes: 4 additions & 2 deletions sdk/src/logs/logger_provider_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#ifdef ENABLE_LOGS_PREVIEW

# include "opentelemetry/sdk/logs/logger_provider_factory.h"
# include "opentelemetry/sdk/logs/logger_context.h"
# include "opentelemetry/sdk/logs/logger_provider.h"
# include "opentelemetry/sdk/resource/resource.h"

Expand Down Expand Up @@ -46,9 +47,10 @@ std::unique_ptr<opentelemetry::logs::LoggerProvider> LoggerProviderFactory::Crea
}

std::unique_ptr<opentelemetry::logs::LoggerProvider> LoggerProviderFactory::Create(
std::shared_ptr<sdk::logs::LoggerContext> context)
std::unique_ptr<LoggerContext> context)
{
std::unique_ptr<opentelemetry::logs::LoggerProvider> provider(new LoggerProvider(context));
std::unique_ptr<opentelemetry::logs::LoggerProvider> provider(
new LoggerProvider(std::move(context)));
return provider;
}

Expand Down
4 changes: 2 additions & 2 deletions sdk/src/trace/tracer_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ namespace trace
namespace resource = opentelemetry::sdk::resource;
namespace trace_api = opentelemetry::trace;

TracerProvider::TracerProvider(std::shared_ptr<sdk::trace::TracerContext> context) noexcept
: context_{context}
TracerProvider::TracerProvider(std::unique_ptr<TracerContext> context) noexcept
: context_(std::move(context))
{
OTEL_INTERNAL_LOG_DEBUG("[TracerProvider] TracerProvider created.");
}
Expand Down
6 changes: 4 additions & 2 deletions sdk/src/trace/tracer_provider_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "opentelemetry/sdk/trace/processor.h"
#include "opentelemetry/sdk/trace/random_id_generator_factory.h"
#include "opentelemetry/sdk/trace/samplers/always_on_factory.h"
#include "opentelemetry/sdk/trace/tracer_context.h"
#include "opentelemetry/sdk/trace/tracer_provider.h"

namespace trace_api = opentelemetry::trace;
Expand Down Expand Up @@ -87,9 +88,10 @@ std::unique_ptr<opentelemetry::trace::TracerProvider> TracerProviderFactory::Cre
}

std::unique_ptr<trace_api::TracerProvider> TracerProviderFactory::Create(
std::shared_ptr<sdk::trace::TracerContext> context)
std::unique_ptr<TracerContext> context)
{
std::unique_ptr<trace_api::TracerProvider> provider(new trace_sdk::TracerProvider(context));
std::unique_ptr<trace_api::TracerProvider> provider(
new trace_sdk::TracerProvider(std::move(context)));
return provider;
}

Expand Down
6 changes: 4 additions & 2 deletions sdk/test/logs/logger_provider_sdk_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ TEST(LoggerProviderSDK, Shutdown)
std::vector<std::unique_ptr<LogRecordProcessor>> processors;
processors.push_back(std::move(processor));

LoggerProvider lp(std::make_shared<LoggerContext>(std::move(processors)));
std::unique_ptr<LoggerContext> context(new LoggerContext(std::move(processors)));
LoggerProvider lp(std::move(context));

EXPECT_TRUE(lp.Shutdown());
EXPECT_TRUE(processor_ptr->IsShutdown());
Expand All @@ -185,7 +186,8 @@ TEST(LoggerProviderSDK, ForceFlush)
std::vector<std::unique_ptr<LogRecordProcessor>> processors;
processors.push_back(std::move(processor));

LoggerProvider lp(std::make_shared<LoggerContext>(std::move(processors)));
std::unique_ptr<LoggerContext> context(new LoggerContext(std::move(processors)));
LoggerProvider lp(std::move(context));

EXPECT_TRUE(lp.ForceFlush());
}
Expand Down
16 changes: 10 additions & 6 deletions sdk/test/trace/tracer_provider_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ TEST(TracerProvider, GetTracer)
std::unique_ptr<SpanProcessor> processor(new SimpleSpanProcessor(nullptr));
std::vector<std::unique_ptr<SpanProcessor>> processors;
processors.push_back(std::move(processor));
TracerProvider tp1(std::make_shared<TracerContext>(std::move(processors), Resource::Create({})));
std::unique_ptr<TracerContext> context1(
new TracerContext(std::move(processors), Resource::Create({})));
TracerProvider tp1(std::move(context1));
auto t1 = tp1.GetTracer("test");
auto t2 = tp1.GetTracer("test");
auto t3 = tp1.GetTracer("different", "1.0.0");
Expand Down Expand Up @@ -49,10 +51,11 @@ TEST(TracerProvider, GetTracer)
std::unique_ptr<SpanProcessor> processor2(new SimpleSpanProcessor(nullptr));
std::vector<std::unique_ptr<SpanProcessor>> processors2;
processors2.push_back(std::move(processor2));
TracerProvider tp2(
std::make_shared<TracerContext>(std::move(processors2), Resource::Create({}),
std::unique_ptr<Sampler>(new AlwaysOffSampler()),
std::unique_ptr<IdGenerator>(new RandomIdGenerator)));
std::unique_ptr<TracerContext> context2(
new TracerContext(std::move(processors2), Resource::Create({}),
std::unique_ptr<Sampler>(new AlwaysOffSampler()),
std::unique_ptr<IdGenerator>(new RandomIdGenerator)));
TracerProvider tp2(std::move(context2));
#ifdef OPENTELEMETRY_RTTI_ENABLED
auto sdkTracer2 = dynamic_cast<Tracer *>(tp2.GetTracer("test").get());
#else
Expand Down Expand Up @@ -81,7 +84,8 @@ TEST(TracerProvider, Shutdown)
std::vector<std::unique_ptr<SpanProcessor>> processors;
processors.push_back(std::move(processor));

TracerProvider tp1(std::make_shared<TracerContext>(std::move(processors)));
std::unique_ptr<TracerContext> context1(new TracerContext(std::move(processors)));
TracerProvider tp1(std::move(context1));

EXPECT_TRUE(tp1.Shutdown());

Expand Down

0 comments on commit 8b61318

Please sign in to comment.