[API/SDK] Provider cleanup (#2664)

Author: marcalff

CHANGELOG.md

@@ -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)

CMakeLists.txt

@@ -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")

DEPRECATED.md

@@ -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]
 

api/CMakeLists.txt

@@ -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(

api/include/opentelemetry/plugin/tracer.h

@@ -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);
@@ -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.
   //

api/include/opentelemetry/trace/noop.h

@@ -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 */
 };
 
 /**

api/include/opentelemetry/trace/tracer.h

@@ -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
@@ -188,6 +195,8 @@ class Tracer
   }
 
   virtual void CloseWithMicroseconds(uint64_t timeout) noexcept = 0;
+
+#endif /* OPENTELEMETRY_ABI_VERSION_NO */
 };
 }  // namespace trace
 OPENTELEMETRY_END_NAMESPACE

api/test/singleton/singleton_test.cc

@@ -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

ci/do_ci.ps1

@@ -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
@@ -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

ci/do_ci.sh

@@ -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}"
@@ -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}"
@@ -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)
@@ -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 \