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

[SDK] TracerProvider should own TracerContext, not share it #2221

Merged
Merged
Show file tree
Hide file tree
Changes from 2 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: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ Increment the:
* [SDK] MeterProvider should own MeterContext, not share it
[#2218](https://github.com/open-telemetry/opentelemetry-cpp/pull/2218)

* [SDK] TracerProvider should own TracerContext, not share it
[#2221](https://github.com/open-telemetry/opentelemetry-cpp/pull/2221)
marcalff marked this conversation as resolved.
Show resolved Hide resolved

Important changes:

* [REMOVAL] Remove the jaeger exporter
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