Skip to content

Commit

Permalink
[REMOVAL] Remove build option WITH_DEPRECATED_SDK_FACTORY (#2717)
Browse files Browse the repository at this point in the history
  • Loading branch information
marcalff authored Jul 17, 2024
1 parent 0a43c1f commit 611906e
Show file tree
Hide file tree
Showing 25 changed files with 39 additions and 517 deletions.
1 change: 0 additions & 1 deletion .github/workflows/iwyu.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ jobs:
-DWITH_STL=CXX14 \
-DCMAKE_CXX_INCLUDE_WHAT_YOU_USE="include-what-you-use;-w;-Xiwyu;--mapping_file=${TOPDIR}/.iwyu.imp;" \
-DBUILD_TESTING=OFF \
-DWITH_DEPRECATED_SDK_FACTORY=OFF \
-DBUILD_W3CTRACECONTEXT_TEST=OFF \
-DWITH_OTLP_GRPC=OFF \
-DWITH_OTLP_HTTP=ON \
Expand Down
38 changes: 38 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,44 @@ Increment the:
* [CI] Add a clang-tidy build
[#3001](https://github.com/open-telemetry/opentelemetry-cpp/pull/3001)

* [REMOVAL] Remove build option `WITH_DEPRECATED_SDK_FACTORY`
[#2717](https://github.com/open-telemetry/opentelemetry-cpp/pull/2717)

Breaking changes:

* [REMOVAL] Remove build option `WITH_DEPRECATED_SDK_FACTORY`
[#2717](https://github.com/open-telemetry/opentelemetry-cpp/pull/2717)

* As announced in opentelemetry-cpp previous release 1.16.0,
CMake option `WITH_DEPRECATED_SDK_FACTORY` was temporary,
and to be removed by the next release.
* This option is now removed.
* Code configuring the SDK must be adjusted, as previously described:

* [API/SDK] Provider cleanup
[#2664](https://github.com/open-telemetry/opentelemetry-cpp/pull/2664)

* Before this fix:
* SDK factory methods such as:
* opentelemetry::sdk::trace::TracerProviderFactory::Create()
* opentelemetry::sdk::metrics::MeterProviderFactory::Create()
* opentelemetry::sdk::logs::LoggerProviderFactory::Create()
* opentelemetry::sdk::logs::EventLoggerProviderFactory::Create()

returned an API object (opentelemetry::trace::TracerProvider)
to the caller.

* After this fix, these methods return an SDK level object
(opentelemetry::sdk::trace::TracerProvider) to the caller.
* Returning an SDK object is necessary for the application to
cleanup and invoke SDK level methods, such as ForceFlush(),
on a provider.
* The application code that configures the SDK, by calling
the various provider factories, may need adjustment.
* All the examples have been updated, and in particular no
longer perform static_cast do convert an API object to an SDK object.
Please refer to examples for guidance on how to adjust.

## [1.16.1 2024-07-17]

* [BUILD] Add bazel missing BUILD file
Expand Down
19 changes: 0 additions & 19 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -159,14 +159,6 @@ message(STATUS "OPENTELEMETRY_VERSION=${OPENTELEMETRY_VERSION}")

option(WITH_NO_DEPRECATED_CODE "Do not include deprecated code" OFF)

# This option is temporary, and will be removed. Set
# WITH_DEPRECATED_SDK_FACTORY=OFF to migrate to the new SDK code.
option(WITH_DEPRECATED_SDK_FACTORY "Use deprecated SDK provider factory" ON)

if(WITH_DEPRECATED_SDK_FACTORY)
message(WARNING "WITH_DEPRECATED_SDK_FACTORY=ON is temporary and deprecated")
endif()

set(WITH_STL
"OFF"
CACHE STRING "Which version of the Standard Library for C++ to use")
Expand Down Expand Up @@ -204,13 +196,6 @@ if(NOT WITH_STL STREQUAL "OFF")
endif()
endif()

if(DEFINED WITH_OTLP)
message(
FATAL_ERROR
"WITH_OTLP is deprecated. Please set either WITH_OTLP_GRPC=ON, WITH_OTLP_HTTP=ON, or both to ON."
)
endif()

option(WITH_OTLP_GRPC_SSL_MTLS_PREVIEW
"Whether to enable mTLS support fro gRPC" OFF)

Expand Down Expand Up @@ -294,10 +279,6 @@ option(

option(WITH_FUNC_TESTS "Whether to build functional tests" ON)

if(DEFINED WITH_LOGS_PREVIEW)
message(WARNING "WITH_LOGS_PREVIEW is removed because logs signal is stable")
endif()

option(WITH_ASYNC_EXPORT_PREVIEW "Whether to enable async export" OFF)

# Exemplar specs status is experimental, so behind feature flag by default
Expand Down
80 changes: 1 addition & 79 deletions DEPRECATED.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,85 +88,7 @@ No date set yet for the Jaeger Propagator.

## [opentelemetry-cpp SDK]

### SDK ProviderFactory cleanup

#### Announcement (SDK ProviderFactory cleanup)

* Version: 1.15.0
* Date: 2024-06-03
* PR: [API/SDK] Provider cleanup
[#2664](https://github.com/open-telemetry/opentelemetry-cpp/pull/2664)

This PR introduces changes to SDK ProviderFactory methods.

#### Motivation (SDK ProviderFactory cleanup)

SDK Factory methods for signal providers, such as:

* opentelemetry::sdk::trace::TracerProviderFactory
* opentelemetry::sdk::metrics::MeterProviderFactory
* opentelemetry::sdk::logs::LoggerProviderFactory
* opentelemetry::sdk::logs::EventLoggerProviderFactory

currently returns a unique pointer on a API class.

This is incorrect, the proper return type should be
a unique pointer on a SDK class instead.

#### Scope (SDK ProviderFactory cleanup)

All the current Create methods in:

* class opentelemetry::sdk::trace::TracerProviderFactory
* class opentelemetry::sdk::metrics::MeterProviderFactory
* class opentelemetry::sdk::logs::LoggerProviderFactory
* class opentelemetry::sdk::logs::EventLoggerProviderFactory

are marked as deprecated, as they return an API object.

Instead, another set of Create methods is provided,
with a different return type, an SDK object.

Both sets can not be exposed at the same time,
as this would cause build breaks,
so a compilation flag is defined to select which methods to use.

When OPENTELEMETRY_DEPRECATED_SDK_FACTORY is defined,
the old, deprecated, methods are available.

When OPENTELEMETRY_DEPRECATED_SDK_FACTORY is not defined,
the new methods are available.

The scope of this deprecation and removal,
is to remove the flag OPENTELEMETRY_DEPRECATED_SDK_FACTORY itself,
which implies that only the new set of Create() methods,
returning an SDK object, are supported.

#### Mitigation (SDK ProviderFactory cleanup)

Build without defining flag OPENTELEMETRY_DEPRECATED_SDK_FACTORY.

Existing code, such as:

```cpp
std::shared_ptr<opentelemetry::trace::TracerProvider> tracer_provider;
tracer_provider = opentelemetry::sdk::trace::TracerProviderFactory::Create(...);
```

should be adjusted to:

```cpp
std::shared_ptr<opentelemetry::sdk::trace::TracerProvider> tracer_provider;
tracer_provider = opentelemetry::sdk::trace::TracerProviderFactory::Create(...);
```

#### Planned removal (SDK ProviderFactory cleanup)

Flag OPENTELEMETRY_DEPRECATED_SDK_FACTORY is introduced in release 1.16.0,
to provide a migration path.

This flag is meant to be temporary, and short lived.
Expect removal by release 1.17.0
N/A

## [opentelemetry-cpp Exporter]

Expand Down
5 changes: 0 additions & 5 deletions api/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,6 @@ if(WITH_NO_DEPRECATED_CODE)
INTERFACE OPENTELEMETRY_NO_DEPRECATED_CODE)
endif()

if(WITH_DEPRECATED_SDK_FACTORY)
target_compile_definitions(opentelemetry_api
INTERFACE OPENTELEMETRY_DEPRECATED_SDK_FACTORY)
endif()

if(WITH_ABSEIL)
target_compile_definitions(opentelemetry_api INTERFACE HAVE_ABSEIL)
target_link_libraries(
Expand Down
2 changes: 0 additions & 2 deletions ci/do_ci.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ switch ($action) {
cmake $SRC_DIR `
-DOTELCPP_MAINTAINER_MODE=ON `
-DWITH_NO_DEPRECATED_CODE=ON `
-DWITH_DEPRECATED_SDK_FACTORY=OFF `
-DVCPKG_TARGET_TRIPLET=x64-windows `
"-DCMAKE_TOOLCHAIN_FILE=$VCPKG_DIR/scripts/buildsystems/vcpkg.cmake"
$exit = $LASTEXITCODE
Expand All @@ -160,7 +159,6 @@ switch ($action) {
-DCMAKE_CXX_STANDARD=20 `
-DOTELCPP_MAINTAINER_MODE=ON `
-DWITH_NO_DEPRECATED_CODE=ON `
-DWITH_DEPRECATED_SDK_FACTORY=OFF `
-DVCPKG_TARGET_TRIPLET=x64-windows `
"-DCMAKE_TOOLCHAIN_FILE=$VCPKG_DIR/scripts/buildsystems/vcpkg.cmake"
$exit = $LASTEXITCODE
Expand Down
4 changes: 0 additions & 4 deletions ci/do_ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ elif [[ "$1" == "cmake.maintainer.sync.test" ]]; then
-DWITH_ASYNC_EXPORT_PREVIEW=OFF \
-DOTELCPP_MAINTAINER_MODE=ON \
-DWITH_NO_DEPRECATED_CODE=ON \
-DWITH_DEPRECATED_SDK_FACTORY=OFF \
-DWITH_OTLP_HTTP_COMPRESSION=ON \
${IWYU} \
"${SRC_DIR}"
Expand All @@ -153,7 +152,6 @@ elif [[ "$1" == "cmake.maintainer.async.test" ]]; then
-DWITH_ASYNC_EXPORT_PREVIEW=ON \
-DOTELCPP_MAINTAINER_MODE=ON \
-DWITH_NO_DEPRECATED_CODE=ON \
-DWITH_DEPRECATED_SDK_FACTORY=OFF \
-DWITH_OTLP_HTTP_COMPRESSION=ON \
${IWYU} \
"${SRC_DIR}"
Expand All @@ -177,7 +175,6 @@ elif [[ "$1" == "cmake.maintainer.cpp11.async.test" ]]; then
-DWITH_ASYNC_EXPORT_PREVIEW=ON \
-DOTELCPP_MAINTAINER_MODE=ON \
-DWITH_NO_DEPRECATED_CODE=ON \
-DWITH_DEPRECATED_SDK_FACTORY=OFF \
-DWITH_OTLP_HTTP_COMPRESSION=ON \
"${SRC_DIR}"
make -k -j $(nproc)
Expand All @@ -199,7 +196,6 @@ elif [[ "$1" == "cmake.maintainer.abiv2.test" ]]; then
-DWITH_ASYNC_EXPORT_PREVIEW=OFF \
-DOTELCPP_MAINTAINER_MODE=ON \
-DWITH_NO_DEPRECATED_CODE=ON \
-DWITH_DEPRECATED_SDK_FACTORY=OFF \
-DWITH_ABI_VERSION_1=OFF \
-DWITH_ABI_VERSION_2=ON \
-DWITH_OTLP_HTTP_COMPRESSION=ON \
Expand Down
10 changes: 0 additions & 10 deletions examples/logs_simple/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,8 @@ void InitTracer()
auto exporter = trace_exporter::OStreamSpanExporterFactory::Create();
auto processor = trace_sdk::SimpleSpanProcessorFactory::Create(std::move(exporter));

#ifdef OPENTELEMETRY_DEPRECATED_SDK_FACTORY
std::shared_ptr<opentelemetry::trace::TracerProvider> provider =
opentelemetry::sdk::trace::TracerProviderFactory::Create(std::move(processor));
#else
std::shared_ptr<opentelemetry::sdk::trace::TracerProvider> provider =
opentelemetry::sdk::trace::TracerProviderFactory::Create(std::move(processor));
#endif /* OPENTELEMETRY_DEPRECATED_SDK_FACTORY */

// Set the global trace provider
const std::shared_ptr<trace_api::TracerProvider> &api_provider = provider;
Expand All @@ -70,13 +65,8 @@ void InitLogger()
std::unique_ptr<logs_sdk::LogRecordExporter>(new logs_exporter::OStreamLogRecordExporter);
auto processor = logs_sdk::SimpleLogRecordProcessorFactory::Create(std::move(exporter));

#ifdef OPENTELEMETRY_DEPRECATED_SDK_FACTORY
std::shared_ptr<opentelemetry::logs::LoggerProvider> provider(
opentelemetry::sdk::logs::LoggerProviderFactory::Create(std::move(processor)));
#else
std::shared_ptr<opentelemetry::sdk::logs::LoggerProvider> provider(
opentelemetry::sdk::logs::LoggerProviderFactory::Create(std::move(processor)));
#endif /* OPENTELEMETRY_DEPRECATED_SDK_FACTORY */

// Set the global logger provider
const std::shared_ptr<logs_api::LoggerProvider> &api_provider = provider;
Expand Down
9 changes: 0 additions & 9 deletions examples/metrics_simple/metrics_ostream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,7 @@ void InitMetrics(const std::string &name)
auto reader =
metrics_sdk::PeriodicExportingMetricReaderFactory::Create(std::move(exporter), options);

#ifdef OPENTELEMETRY_DEPRECATED_SDK_FACTORY
auto u_provider = opentelemetry::sdk::metrics::MeterProviderFactory::Create();
auto *provider = static_cast<opentelemetry::sdk::metrics::MeterProvider *>(u_provider.get());
#else
auto provider = opentelemetry::sdk::metrics::MeterProviderFactory::Create();
#endif /* OPENTELEMETRY_DEPRECATED_SDK_FACTORY */

provider->AddMetricReader(std::move(reader));

Expand Down Expand Up @@ -118,11 +113,7 @@ void InitMetrics(const std::string &name)
provider->AddView(std::move(histogram_instrument_selector), std::move(histogram_meter_selector),
std::move(histogram_view));

#ifdef OPENTELEMETRY_DEPRECATED_SDK_FACTORY
std::shared_ptr<opentelemetry::metrics::MeterProvider> api_provider(std::move(u_provider));
#else
std::shared_ptr<opentelemetry::metrics::MeterProvider> api_provider(std::move(provider));
#endif /* OPENTELEMETRY_DEPRECATED_SDK_FACTORY */

metrics_api::Provider::SetMeterProvider(api_provider);
}
Expand Down
13 changes: 0 additions & 13 deletions examples/otlp/file_log_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,8 @@ namespace
opentelemetry::exporter::otlp::OtlpFileExporterOptions opts;
opentelemetry::exporter::otlp::OtlpFileLogRecordExporterOptions log_opts;

#ifdef OPENTELEMETRY_DEPRECATED_SDK_FACTORY
std::shared_ptr<opentelemetry::trace::TracerProvider> tracer_provider;
std::shared_ptr<opentelemetry::logs::LoggerProvider> logger_provider;
#else
std::shared_ptr<opentelemetry::sdk::trace::TracerProvider> tracer_provider;
std::shared_ptr<opentelemetry::sdk::logs::LoggerProvider> logger_provider;
#endif /* OPENTELEMETRY_DEPRECATED_SDK_FACTORY */

void InitTracer()
{
Expand All @@ -71,11 +66,7 @@ void CleanupTracer()
// We call ForceFlush to prevent to cancel running exportings, It's optional.
if (tracer_provider)
{
#ifdef OPENTELEMETRY_DEPRECATED_SDK_FACTORY
static_cast<opentelemetry::sdk::trace::TracerProvider *>(tracer_provider.get())->ForceFlush();
#else
tracer_provider->ForceFlush();
#endif /* OPENTELEMETRY_DEPRECATED_SDK_FACTORY */
}

tracer_provider.reset();
Expand All @@ -99,11 +90,7 @@ void CleanupLogger()
// We call ForceFlush to prevent to cancel running exportings, It's optional.
if (logger_provider)
{
#ifdef OPENTELEMETRY_DEPRECATED_SDK_FACTORY
static_cast<opentelemetry::sdk::logs::LoggerProvider *>(logger_provider.get())->ForceFlush();
#else
logger_provider->ForceFlush();
#endif /* OPENTELEMETRY_DEPRECATED_SDK_FACTORY */
}

logger_provider.reset();
Expand Down
8 changes: 0 additions & 8 deletions examples/otlp/file_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,7 @@ namespace
{
opentelemetry::exporter::otlp::OtlpFileExporterOptions opts;

#ifdef OPENTELEMETRY_DEPRECATED_SDK_FACTORY
std::shared_ptr<opentelemetry::trace::TracerProvider> provider;
#else
std::shared_ptr<opentelemetry::sdk::trace::TracerProvider> provider;
#endif /* OPENTELEMETRY_DEPRECATED_SDK_FACTORY */

void InitTracer()
{
Expand All @@ -54,11 +50,7 @@ void CleanupTracer()
// We call ForceFlush to prevent to cancel running exportings, It's optional.
if (provider)
{
#ifdef OPENTELEMETRY_DEPRECATED_SDK_FACTORY
static_cast<opentelemetry::sdk::trace::TracerProvider *>(provider.get())->ForceFlush();
#else
provider->ForceFlush();
#endif /* OPENTELEMETRY_DEPRECATED_SDK_FACTORY */
}

provider.reset();
Expand Down
13 changes: 0 additions & 13 deletions examples/otlp/grpc_log_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,8 @@ namespace
opentelemetry::exporter::otlp::OtlpGrpcExporterOptions opts;
opentelemetry::exporter::otlp::OtlpGrpcLogRecordExporterOptions log_opts;

#ifdef OPENTELEMETRY_DEPRECATED_SDK_FACTORY
std::shared_ptr<opentelemetry::trace::TracerProvider> tracer_provider;
std::shared_ptr<opentelemetry::logs::LoggerProvider> logger_provider;
#else
std::shared_ptr<opentelemetry::sdk::trace::TracerProvider> tracer_provider;
std::shared_ptr<opentelemetry::sdk::logs::LoggerProvider> logger_provider;
#endif /* OPENTELEMETRY_DEPRECATED_SDK_FACTORY */

void InitTracer()
{
Expand All @@ -63,11 +58,7 @@ void CleanupTracer()
// We call ForceFlush to prevent to cancel running exportings, It's optional.
if (tracer_provider)
{
#ifdef OPENTELEMETRY_DEPRECATED_SDK_FACTORY
static_cast<opentelemetry::sdk::trace::TracerProvider *>(tracer_provider.get())->ForceFlush();
#else
tracer_provider->ForceFlush();
#endif /* OPENTELEMETRY_DEPRECATED_SDK_FACTORY */
}

tracer_provider.reset();
Expand All @@ -92,11 +83,7 @@ void CleanupLogger()
// We call ForceFlush to prevent to cancel running exportings, It's optional.
if (logger_provider)
{
#ifdef OPENTELEMETRY_DEPRECATED_SDK_FACTORY
static_cast<opentelemetry::sdk::logs::LoggerProvider *>(logger_provider.get())->ForceFlush();
#else
logger_provider->ForceFlush();
#endif /* OPENTELEMETRY_DEPRECATED_SDK_FACTORY */
}

logger_provider.reset();
Expand Down
Loading

1 comment on commit 611906e

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp sdk Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 611906e Previous: 0a43c1f Ratio
BM_LockFreeBuffer/2 8570609.092712402 ns/iter 4105279.0849882276 ns/iter 2.09

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.