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

[API/SDK] Provider cleanup #2664

Merged
merged 25 commits into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
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
54 changes: 54 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,60 @@ Increment the:
* [CI] Upgrade to clang-format 18
[#2684](https://github.com/open-telemetry/opentelemetry-cpp/pull/2684)

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

Important changes:

* [API/SDK] Provider cleanup
[#2664](https://github.com/open-telemetry/opentelemetry-cpp/pull/2664)
* Before this fix:
* The API class `opentelemetry::trace::Tracer` exposed methods such
as `ForceFlush()`, `ForceFlushWithMicroseconds()`, `Close()`
and `CloseWithMicroseconds()`.
* These methods are meant to be used when configuring the SDK,
and should not be part of the API. Exposing them was an oversight.
* Two of these methods are virtual, and therefore part of the ABI.
* After this fix:
* In `OPENTELEMETRY_ABI_VERSION_NO 1`, nothing is changed,
because removing this code would break the ABI.
* In `OPENTELEMETRY_ABI_VERSION_NO 2`, these methods are moved
from the API to the SDK. This is a breaking change for ABI version 2,
which is still experimental.
* In all cases, instrumenting an application should not
invoke flush or close on a tracer, do not use these methods.

Breaking changes:

* [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.
* If adjusting application code is impractical,
an alternate and temporary solution is to build with option
WITH_DEPRECATED_SDK_FACTORY=ON in CMake.
* Option WITH_DEPRECATED_SDK_FACTORY=ON will allow to build code
without application changes, posponing changes for later.
* WITH_DEPRECATED_SDK_FACTORY=ON is temporary, only to provide
an easier migration path. Expect this flag to be removed,
as early as by the next release.

Notes on experimental features:

* [#2372](https://github.com/open-telemetry/opentelemetry-cpp/issues/2372)
Expand Down
8 changes: 8 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,14 @@ 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
80 changes: 79 additions & 1 deletion DEPRECATED.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,85 @@ No date set yet for the Jaeger Propagator.

## [opentelemetry-cpp SDK]

N/A
### 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

## [opentelemetry-cpp Exporter]

Expand Down
5 changes: 5 additions & 0 deletions api/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ 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
4 changes: 4 additions & 0 deletions api/include/opentelemetry/plugin/tracer.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ class Tracer final : public trace::Tracer, public std::enable_shared_from_this<T
return nostd::shared_ptr<trace::Span>{new (std::nothrow) Span{this->shared_from_this(), span}};
}

#if OPENTELEMETRY_ABI_VERSION_NO == 1

void ForceFlushWithMicroseconds(uint64_t timeout) noexcept override
{
tracer_handle_->tracer().ForceFlushWithMicroseconds(timeout);
Expand All @@ -114,6 +116,8 @@ class Tracer final : public trace::Tracer, public std::enable_shared_from_this<T
tracer_handle_->tracer().CloseWithMicroseconds(timeout);
}

#endif /* OPENTELEMETRY_ABI_VERSION_NO */

private:
// Note: The order is important here.
//
Expand Down
4 changes: 4 additions & 0 deletions api/include/opentelemetry/trace/noop.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,13 @@ class OPENTELEMETRY_EXPORT NoopTracer final : public Tracer,
return noop_span;
}

#if OPENTELEMETRY_ABI_VERSION_NO == 1

void ForceFlushWithMicroseconds(uint64_t /*timeout*/) noexcept override {}

void CloseWithMicroseconds(uint64_t /*timeout*/) noexcept override {}

#endif /* OPENTELEMETRY_ABI_VERSION_NO */
};

/**
Expand Down
9 changes: 9 additions & 0 deletions api/include/opentelemetry/trace/tracer.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,13 @@ class Tracer
}
}

#if OPENTELEMETRY_ABI_VERSION_NO == 1

/*
* The following is removed from the API in ABI version 2.
* It belongs to the SDK.
*/

/**
* Force any buffered spans to flush.
* @param timeout to complete the flush
Expand All @@ -188,6 +195,8 @@ class Tracer
}

virtual void CloseWithMicroseconds(uint64_t timeout) noexcept = 0;

#endif /* OPENTELEMETRY_ABI_VERSION_NO */
};
} // namespace trace
OPENTELEMETRY_END_NAMESPACE
4 changes: 4 additions & 0 deletions api/test/singleton/singleton_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,13 @@ class MyTracer : public trace::Tracer
return result;
}

#if OPENTELEMETRY_ABI_VERSION_NO == 1

void ForceFlushWithMicroseconds(uint64_t /* timeout */) noexcept override {}

void CloseWithMicroseconds(uint64_t /* timeout */) noexcept override {}

#endif /* OPENTELEMETRY_ABI_VERSION_NO */
};

class MyTracerProvider : public trace::TracerProvider
Expand Down
2 changes: 2 additions & 0 deletions ci/do_ci.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ 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 @@ -157,6 +158,7 @@ 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: 4 additions & 0 deletions ci/do_ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ 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 @@ -152,6 +153,7 @@ 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 @@ -175,6 +177,7 @@ 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 @@ -196,6 +199,7 @@ 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
39 changes: 27 additions & 12 deletions examples/logs_simple/main.cc
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

#include "opentelemetry/exporters/ostream/log_record_exporter.h"
#include "opentelemetry/exporters/ostream/span_exporter_factory.h"
#include "opentelemetry/logs/provider.h"
#include "opentelemetry/sdk/logs/logger_provider.h"
#include "opentelemetry/sdk/logs/logger_provider_factory.h"
#include "opentelemetry/sdk/logs/processor.h"
#include "opentelemetry/sdk/logs/simple_log_record_processor_factory.h"
#include "opentelemetry/sdk/trace/exporter.h"
#include "opentelemetry/sdk/trace/processor.h"
#include "opentelemetry/sdk/trace/simple_processor_factory.h"
#include "opentelemetry/sdk/trace/tracer_provider.h"
#include "opentelemetry/sdk/trace/tracer_provider_factory.h"
#include "opentelemetry/trace/provider.h"

#include "opentelemetry/exporters/ostream/log_record_exporter.h"
#include "opentelemetry/logs/provider.h"
#include "opentelemetry/sdk/logs/logger_provider_factory.h"
#include "opentelemetry/sdk/logs/processor.h"
#include "opentelemetry/sdk/logs/simple_log_record_processor_factory.h"

#ifdef BAZEL_BUILD
# include "examples/common/logs_foo_library/foo_library.h"
#else
Expand All @@ -35,11 +36,18 @@ void InitTracer()
// Create ostream span exporter instance
auto exporter = trace_exporter::OStreamSpanExporterFactory::Create();
auto processor = trace_sdk::SimpleSpanProcessorFactory::Create(std::move(exporter));
std::shared_ptr<trace_api::TracerProvider> provider =
trace_sdk::TracerProviderFactory::Create(std::move(processor));

#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
trace_api::Provider::SetTracerProvider(provider);
std::shared_ptr<trace_api::TracerProvider> api_provider = provider;
trace_api::Provider::SetTracerProvider(api_provider);
}

void CleanupTracer()
Expand All @@ -54,11 +62,18 @@ void InitLogger()
auto exporter =
std::unique_ptr<logs_sdk::LogRecordExporter>(new logs_exporter::OStreamLogRecordExporter);
auto processor = logs_sdk::SimpleLogRecordProcessorFactory::Create(std::move(exporter));
std::shared_ptr<logs_api::LoggerProvider> provider(
logs_sdk::LoggerProviderFactory::Create(std::move(processor)));

#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
logs_api::Provider::SetLoggerProvider(provider);
std::shared_ptr<logs_api::LoggerProvider> api_provider = provider;
logs_api::Provider::SetLoggerProvider(api_provider);
}

void CleanupLogger()
Expand Down
Loading