From 1ef77609bd99136f71cd6b0f0eb8a838fd0741ac Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Fri, 7 Jul 2023 15:03:19 +0200 Subject: [PATCH 01/11] Proof of concept. --- CMakeLists.txt | 26 ++- api/CMakeLists.txt | 4 + api/include/opentelemetry/version.h | 5 +- docs/abi-version-policy.md | 321 ++++++++++++++++++++++++++++ 4 files changed, 348 insertions(+), 8 deletions(-) create mode 100644 docs/abi-version-policy.md diff --git a/CMakeLists.txt b/CMakeLists.txt index fb483f5a58..96947757dd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -89,17 +89,27 @@ if(VCPKG_CHAINLOAD_TOOLCHAIN_FILE) include("${VCPKG_CHAINLOAD_TOOLCHAIN_FILE}") endif() +option(WITH_ABI_VERSION_2_PREVIEW "EXPERIMENTAL: ABI version 2 preview" OFF) + file(READ "${CMAKE_CURRENT_LIST_DIR}/api/include/opentelemetry/version.h" OPENTELEMETRY_CPP_HEADER_VERSION_H) -if(OPENTELEMETRY_CPP_HEADER_VERSION_H MATCHES - "OPENTELEMETRY_ABI_VERSION_NO[ \t\r\n]+\"?([0-9]+)\"?") - math(EXPR OPENTELEMETRY_ABI_VERSION_NO ${CMAKE_MATCH_1}) + +if (WITH_ABI_VERSION_2_PREVIEW) + set(OPENTELEMETRY_ABI_VERSION_NO "2") else() - message( - FATAL_ERROR - "OPENTELEMETRY_ABI_VERSION_NO not found on ${CMAKE_CURRENT_LIST_DIR}/api/include/opentelemetry/version.h" - ) + if(OPENTELEMETRY_CPP_HEADER_VERSION_H MATCHES + "OPENTELEMETRY_ABI_VERSION_NO[ \t\r\n]+\"?([0-9]+)\"?") + math(EXPR OPENTELEMETRY_ABI_VERSION_NO ${CMAKE_MATCH_1}) + else() + message( + FATAL_ERROR + "OPENTELEMETRY_ABI_VERSION_NO not found on ${CMAKE_CURRENT_LIST_DIR}/api/include/opentelemetry/version.h" + ) + endif() endif() + +message(STATUS "OPENTELEMETRY_ABI_VERSION_NO=${OPENTELEMETRY_ABI_VERSION_NO}") + if(OPENTELEMETRY_CPP_HEADER_VERSION_H MATCHES "OPENTELEMETRY_VERSION[ \t\r\n]+\"?([^\"]+)\"?") set(OPENTELEMETRY_VERSION ${CMAKE_MATCH_1}) @@ -110,6 +120,8 @@ else() ) endif() +message(STATUS "OPENTELEMETRY_VERSION=${OPENTELEMETRY_VERSION}") + option(WITH_NO_DEPRECATED_CODE "Do not include deprecated code" OFF) option(WITH_STL "Whether to use Standard Library for C++ latest features" OFF) diff --git a/api/CMakeLists.txt b/api/CMakeLists.txt index 91d129e179..8ff3ac2e4f 100644 --- a/api/CMakeLists.txt +++ b/api/CMakeLists.txt @@ -98,6 +98,10 @@ if(WITH_ASYNC_EXPORT_PREVIEW) target_compile_definitions(opentelemetry_api INTERFACE ENABLE_ASYNC_EXPORT) endif() +if(WITH_ABI_VERSION_2_PREVIEW) + target_compile_definitions(opentelemetry_api INTERFACE OPENTELEMETRY_ABI_VERSION_NO=2) +endif() + # A better place should be in sdk, not api if(WITH_OTLP_HTTP_SSL_PREVIEW) target_compile_definitions(opentelemetry_api diff --git a/api/include/opentelemetry/version.h b/api/include/opentelemetry/version.h index 2178591728..0ff8d71d32 100644 --- a/api/include/opentelemetry/version.h +++ b/api/include/opentelemetry/version.h @@ -6,7 +6,10 @@ #include "opentelemetry/common/macros.h" #include "opentelemetry/detail/preprocessor.h" -#define OPENTELEMETRY_ABI_VERSION_NO 1 +#ifndef OPENTELEMETRY_ABI_VERSION_NO +# define OPENTELEMETRY_ABI_VERSION_NO 1 +#endif + #define OPENTELEMETRY_VERSION "1.9.1" #define OPENTELEMETRY_VERSION_MAJOR 1 #define OPENTELEMETRY_VERSION_MINOR 9 diff --git a/docs/abi-version-policy.md b/docs/abi-version-policy.md new file mode 100644 index 0000000000..6ff36766f6 --- /dev/null +++ b/docs/abi-version-policy.md @@ -0,0 +1,321 @@ +# Application Binary Interface (ABI) version policy + +## Goals + +### Instrumented applications + +Once a given release of opentelemetry-cpp is published as stable, +subsequent releases are expected to provide compatibility, +to avoid disruption. + +Compatibility at the source code level (API) is expected, +so that no code change in an application already instrumented is required +to adopt a newer release. + +Also, compatibility at the binary level (ABI) is expected, +so that an instrumented application already compiled against +opentelemetry-cpp API headers from an older version, and distributed as a +binary package, can be linked against the SDK library from a newer version. + +In other words, once an application is instrumented using the +opentelemetry-cpp API, adopting a newer version: + +* should not require source code changes, +* should not require building and distributing a new package. + +### opentelemetry-cpp + +The opentelemetry-cpp project itself needs to deliver fixes and features on +a continual basis. + +Reasons to change an API can be external: + +* new APIs added in the specification, for new features +* changes in APIs in the specifications, extending existing features. + +Changes can also be caused by internal issues: + +* fix technical design issues with an API (incorrect types used, missing + parameters) + +Regardless of the root cause for a change (bug or feature), +changes to the existing APIs are unavoidable. + +For the opentelemetry-cpp project to stay healthy, +there must be a way to deliver ABI breaking fixes while preserving +compatibility for users. + +This is achieved with ABI versions. + +## Concept of ABI version + +### Inline namespaces + +For the sake of illustration, let's consider a fictious API such as: + +```cpp +namespace opentelemetry +{ + namespace common + { + class OtelUtil + { + virtual void DoSomething() = 0; + }; + } +} +``` + +An application can be instrumented to use it: + +```cpp +opentelemetry::common::OtelUtil *p = ...; +p->DoSomething(); +``` + +The problem here is that the binary package produced during the build +contains references to symbols such as: + +```cpp +opentelemetry::common::OtelUtil::DoSomething() +``` + +Because all symbols are in the same `opentelemetry::` namespace, it becomes +impossible to deliver changes (new or different symbols) while at the same +time not deliver changes (to preserve binary compatibility). + +This is resolved by the use of inline namespaces in C++, leading to +dedicated ABI versions. + +Revised example: + +```cpp +namespace opentelemetry +{ + inline namespace v1 + { + namespace common + { + class OtelUtil + { + virtual void DoSomething() = 0; + }; + } + } +} +``` + +```cpp +opentelemetry::common::OtelUtil *p = ...; +p->DoSomething(); +``` + +When building, the compiler translates `opentelemetry::common` to +`opentelemetry::v1::common` + +The symbols delivered by the opentelemetry library, and used by the +instrumented application, are: + +```cpp +opentelemetry::v1::common::OtelUtil::DoSomething() +``` + +With the help of the `OPENTELEMETRY_BEGIN_NAMESPACE` macro, +the source code can be abstracted to: + +```cpp +#define OPENTELEMETRY_ABI_VERSION_NO 1 + +OPENTELEMETRY_BEGIN_NAMESPACE +{ + namespace common + { + class OtelUtil + { + virtual void DoSomething() = 0; + }; + } +} +OPENTELEMETRY_END_NAMESPACE +``` + +Adding a new API to the OtelUtil class is an ABI breaking change (the C++ +virtual table is different). + +This change can be delivered, but in a different namespace, which defines +a different (extended) ABI: + +```cpp +#define OPENTELEMETRY_ABI_VERSION_NO 2 + +OPENTELEMETRY_BEGIN_NAMESPACE +{ + namespace common + { + class OtelUtil + { + virtual void DoSomething() = 0; + virtual void DoSomethingMore() = 0; + }; + } +} +OPENTELEMETRY_END_NAMESPACE +``` + +Compiling this declaration produce the following symbols: + +```cpp +opentelemetry::v2::common::OtelUtil::DoSomething() +opentelemetry::v2::common::OtelUtil::DoSomethingMore() +``` + +### ABI version + +By defining an inline namespace per ABI, it is possible to deliver +an ABI v2 implementation independently of ABI v1, keeping v1 unchanged. + +Code compiled against the v1 ABI can continue to link against a library +providing v1 symbols, while code compiled against the v2 ABI can link +against a library providing the v2 symbols. + +An application, when building, can choose to: + +* either build against the v1 interface +* or build against the v2 interface + +Using v1 ensures compatibility, at the API and ABI level. + +Using v2 allows to benefit from new features. + +The choice is made by the application owner when building, +and not by the opentelemetry-cpp library. + +This, in essence, describes the technical 'ABI version' building block used +to deliver breaking changes. + +How to use this versioning feature, provided by the C++ language with inline +namespaces, is described in the next section. + +## Versioning policy + +### Version scope + +Due to dependencies between classes, having an ABI version per class is not +viable. + +For example, class TracerProvider depends on +class Tracer, as it builds tracers. + +If class Tracer comes in multiple versions, +then class TracerProvider needs to also comes in multiple versions, +and these versions are correlated: + +* v1::TracerProvider creates v1::Tracer instances +* v2::TracerProvider creates v2::Tracer instances + +The next logical scope is to consider ABI versions per signal. + +This is not viable either, because of interdependencies between signals: + +* metrics can use traces in examplars +* eventually, the trace implementation can emit internal metrics +* all signals can use common utility classes + +In conclusion, the scope of a version is the entire opentelemetry-cpp project. + +### Source code + +The number of breaking change that affects an APIs is expected to be very low, +with only specific methods affected directly. + +It is very undesirable to: + +* create a different git branch per ABI version, in effect forking the + entire code base +* create a different header file per ABI version, in effect forking + the entire include headers + +just to handle a few modified APIs. + +As a result, differences between ABI versions are handled using C +preprocessor macros, in the few places where it is necessary. + +For example: + +```cpp +OPENTELEMETRY_BEGIN_NAMESPACE +{ + namespace common + { + class OtelUtil + { + virtual void DoSomething() = 0; + +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + // Added in ABI v2 + virtual void DoSomethingMore() = 0; +#endif + + }; + } +} +OPENTELEMETRY_END_NAMESPACE +``` + +The same source code, when compiled with different values of +OPENTELEMETRY_ABI_VERSION_NO, produces the proper declarations for ABI v1 or +ABI v2. + +This solution reduces the maintenance burden on the source code itself. + + + +TODO + +## Versions lifecycle + +For a given ABI, the lifecycle is: + +* EXPERIMENTAL +* STABLE +* DEPRECATED +* REMOVED + +In the EXPERIMENTAL status, +any change to the ABI can be implemented, without notice. + +There are no compatibility guarantees. +This status is meant as a preview, until the ABI is declared STABLE. + +In the STABLE status, +changes to the ABI are forbidden, to guarantee stability. + +In the DEPRECATED status, the ABI is still functional and supported, +but instrumented applications are encouraged to migrate to a newer ABI. + +In the REMOVED status, +the given ABI is no longer available. + +The following sections describe the migration path from one ABI (v1) to the +next (v2). + +### STABLE V1 + +TODO + +### STABLE V1, EXPERIMENTAL V2 + +TODO + +### STABLE V1, STABLE V2 + +TODO + +### DEPRECATED V1, STABLE V2 + +TODO + +### (REMOVED V1), STABLE V2 + +TODO + From 3d33126f39043174b2958ede43ab9a61a0bb8279 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Fri, 7 Jul 2023 16:12:27 +0200 Subject: [PATCH 02/11] Continued. --- CMakeLists.txt | 7 ++- api/CMakeLists.txt | 8 ++- docs/abi-version-policy.md | 123 ++++++++++++++++++++++++++++++++++--- 3 files changed, 124 insertions(+), 14 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 96947757dd..4ffc55c561 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -89,13 +89,16 @@ if(VCPKG_CHAINLOAD_TOOLCHAIN_FILE) include("${VCPKG_CHAINLOAD_TOOLCHAIN_FILE}") endif() -option(WITH_ABI_VERSION_2_PREVIEW "EXPERIMENTAL: ABI version 2 preview" OFF) +option(WITH_ABI_VERSION_1 "ABI version 1" ON) +option(WITH_ABI_VERSION_2 "EXPERIMENTAL: ABI version 2 preview" OFF) file(READ "${CMAKE_CURRENT_LIST_DIR}/api/include/opentelemetry/version.h" OPENTELEMETRY_CPP_HEADER_VERSION_H) -if (WITH_ABI_VERSION_2_PREVIEW) +if(WITH_ABI_VERSION_2) set(OPENTELEMETRY_ABI_VERSION_NO "2") +elseif(WITH_ABI_VERSION_1) + set(OPENTELEMETRY_ABI_VERSION_NO "1") else() if(OPENTELEMETRY_CPP_HEADER_VERSION_H MATCHES "OPENTELEMETRY_ABI_VERSION_NO[ \t\r\n]+\"?([0-9]+)\"?") diff --git a/api/CMakeLists.txt b/api/CMakeLists.txt index 8ff3ac2e4f..cc4eb5361e 100644 --- a/api/CMakeLists.txt +++ b/api/CMakeLists.txt @@ -98,8 +98,12 @@ if(WITH_ASYNC_EXPORT_PREVIEW) target_compile_definitions(opentelemetry_api INTERFACE ENABLE_ASYNC_EXPORT) endif() -if(WITH_ABI_VERSION_2_PREVIEW) - target_compile_definitions(opentelemetry_api INTERFACE OPENTELEMETRY_ABI_VERSION_NO=2) +if(WITH_ABI_VERSION_2) + target_compile_definitions(opentelemetry_api + INTERFACE OPENTELEMETRY_ABI_VERSION_NO=2) +elseif(WITH_ABI_VERSION_1) + target_compile_definitions(opentelemetry_api + INTERFACE OPENTELEMETRY_ABI_VERSION_NO=1) endif() # A better place should be in sdk, not api diff --git a/docs/abi-version-policy.md b/docs/abi-version-policy.md index 6ff36766f6..4db9214cad 100644 --- a/docs/abi-version-policy.md +++ b/docs/abi-version-policy.md @@ -268,11 +268,7 @@ ABI v2. This solution reduces the maintenance burden on the source code itself. - - -TODO - -## Versions lifecycle +## Versions lifecycle For a given ABI, the lifecycle is: @@ -301,21 +297,128 @@ next (v2). ### STABLE V1 -TODO +In this state, only one ABI version is available, and it is closed to +changes. + +Instrumented applications are built against ABI v1 by default. + +opentelemetry-cpp produces a library for ABI v1 by default. + +Fixes introducing breaking changes can __not__ be delivered. + +This is the current status as of opentelemetry-cpp version 1.9.1 ### STABLE V1, EXPERIMENTAL V2 -TODO +In this state, two ABI versions are available. + +CMake offers the following options: + +``` +option(WITH_ABI_VERSION_1 "ABI version 1" ON) +option(WITH_ABI_VERSION_2 "EXPERIMENTAL: ABI version 2 preview" OFF) +``` + +Instrumented applications are built against ABI v1 by default, +but may ask to use ABI v2 instead. + +opentelemetry-cpp produces a library for ABI v1 by default, +but can be configured to provide ABI v2 instead. + +Fixes introducing breaking changes can only be delivered in the experimental +ABI v2. ### STABLE V1, STABLE V2 -TODO +In this state, two ABI versions are available, +the ABI offered by default is the conservative ABI v1. + +CMake offers the following options: + +``` +option(WITH_ABI_VERSION_1 "ABI version 1" ON) +option(WITH_ABI_VERSION_2 "ABI version 2" OFF) +``` + +Instrumented applications are built against ABI v1 by default, +but may ask to use ABI v2 instead. + +opentelemetry-cpp produces a library for ABI v1 by default, +but can be configured to provide ABI v2 instead. + +Fixes introducing breaking changes can not be delivered ABI v2, because it +is declared stable. These fixes can either wait, or an experimental ABI v3 +can be created. ### DEPRECATED V1, STABLE V2 -TODO +In this state, two ABI versions are available, +the ABI offered by default is the newer ABI v2. + +CMake offers the following options: + +``` +option(WITH_ABI_VERSION_1 "DEPRECATED: ABI version 1" OFF) +option(WITH_ABI_VERSION_2 "ABI version 2" ON) +``` + +Instrumented applications are built against ABI v2 by default, +but may ask to use ABI v1 instead. + +opentelemetry-cpp produces a library for ABI v2 by default, +but can be configured to provide ABI v1 instead. ### (REMOVED V1), STABLE V2 -TODO +In this state, only ABI v2 is available. +ABI v1 is no longer supported. + +CMake offers the following options: + +``` +option(WITH_ABI_VERSION_2 "ABI version 2" ON) +``` + +Instrumented applications and the opentelemetry-cpp library are build using +ABI v2. + +Now that ABI v1 no longer exists, the code: + +```cpp +OPENTELEMETRY_BEGIN_NAMESPACE +{ + namespace common + { + class OtelUtil + { + virtual void DoSomething() = 0; + +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + // Added in ABI v2 + virtual void DoSomethingMore() = 0; +#endif + + }; + } +} +OPENTELEMETRY_END_NAMESPACE +``` + +can be simplified to: + +```cpp +OPENTELEMETRY_BEGIN_NAMESPACE +{ + namespace common + { + class OtelUtil + { + virtual void DoSomething() = 0; + virtual void DoSomethingMore() = 0; + + }; + } +} +OPENTELEMETRY_END_NAMESPACE +``` From 1903e7e8d376ede5e73d4deb39e5941bd6dcf6f4 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Fri, 7 Jul 2023 18:17:20 +0200 Subject: [PATCH 03/11] Added details. --- docs/abi-version-policy.md | 56 ++++++++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 5 deletions(-) diff --git a/docs/abi-version-policy.md b/docs/abi-version-policy.md index 4db9214cad..c19a7988ae 100644 --- a/docs/abi-version-policy.md +++ b/docs/abi-version-policy.md @@ -51,7 +51,7 @@ This is achieved with ABI versions. ### Inline namespaces -For the sake of illustration, let's consider a fictious API such as: +For the sake of illustration, let's consider a fictitious API such as: ```cpp namespace opentelemetry @@ -314,7 +314,7 @@ In this state, two ABI versions are available. CMake offers the following options: -``` +```cmake option(WITH_ABI_VERSION_1 "ABI version 1" ON) option(WITH_ABI_VERSION_2 "EXPERIMENTAL: ABI version 2 preview" OFF) ``` @@ -335,7 +335,7 @@ the ABI offered by default is the conservative ABI v1. CMake offers the following options: -``` +```cmake option(WITH_ABI_VERSION_1 "ABI version 1" ON) option(WITH_ABI_VERSION_2 "ABI version 2" OFF) ``` @@ -357,7 +357,7 @@ the ABI offered by default is the newer ABI v2. CMake offers the following options: -``` +```cmake option(WITH_ABI_VERSION_1 "DEPRECATED: ABI version 1" OFF) option(WITH_ABI_VERSION_2 "ABI version 2" ON) ``` @@ -375,7 +375,7 @@ ABI v1 is no longer supported. CMake offers the following options: -``` +```cmake option(WITH_ABI_VERSION_2 "ABI version 2" ON) ``` @@ -422,3 +422,49 @@ OPENTELEMETRY_BEGIN_NAMESPACE OPENTELEMETRY_END_NAMESPACE ``` +## Practical Example + +### Fixing issue #2033 + +The problem is to change the MeterProvider::GetMeter() prototype, +to follow specification changes. + +See the issue description for all details: + +* [Metrics API/SDK] Add InstrumentationScope attributes in MeterProvider::GetMeter() #2033 + +#### API change + +In the API, class MeterProvider is changed as follows: + +```cpp +class MeterProvider +{ +public: + virtual ~MeterProvider() = default; + /** + * Gets or creates a named Meter instance. + * + * Optionally a version can be passed to create a named and versioned Meter + * instance. + */ +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + virtual nostd::shared_ptr GetMeter(nostd::string_view library_name, + nostd::string_view library_version = "", + nostd::string_view schema_url = "", + const common::KeyValueIterable *attributes = nullptr) noexcept = 0; +#else + virtual nostd::shared_ptr GetMeter(nostd::string_view library_name, + nostd::string_view library_version = "", + nostd::string_view schema_url = "") noexcept = 0; +#endif +}; + +``` + +#### SDK change + +TODO + + + From 31f428875ca8b236f5941fe747284939d2431da7 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Mon, 10 Jul 2023 21:34:46 +0200 Subject: [PATCH 04/11] Continue doc. --- docs/abi-version-policy.md | 52 +++++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/docs/abi-version-policy.md b/docs/abi-version-policy.md index c19a7988ae..438e93d230 100644 --- a/docs/abi-version-policy.md +++ b/docs/abi-version-policy.md @@ -458,13 +458,63 @@ public: nostd::string_view library_version = "", nostd::string_view schema_url = "") noexcept = 0; #endif + + /* ... */ }; ``` +Note how the ABI changes, while the API stays compatible, requiring no code +change in the caller when providing up to 3 parameters. + #### SDK change -TODO +In the SDK class declaration, implement the expected API. + +``` +class MeterProvider final : public opentelemetry::metrics::MeterProvider +{ +public: + + /* ... */ + +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + nostd::shared_ptr GetMeter( + nostd::string_view name, + nostd::string_view version = "", + nostd::string_view schema_url = "", + const opentelemetry::common::KeyValueIterable *attributes = nullptr) noexcept override; +#else + nostd::shared_ptr GetMeter( + nostd::string_view name, + nostd::string_view version = "", + nostd::string_view schema_url = "") noexcept override; +#endif + + /* ... */ +}; +``` +In the SDK implementation: +* either get the new parameters from the extended ABI v2 method +* or provide default values for the old ABI v1 method +``` +nostd::shared_ptr MeterProvider::GetMeter( + nostd::string_view name, + nostd::string_view version, + nostd::string_view schema_url +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + , + const opentelemetry::common::KeyValueIterable *attributes +#endif + ) noexcept +{ +#if OPENTELEMETRY_ABI_VERSION_NO < 2 + const opentelemetry::common::KeyValueIterable *attributes = nullptr; +#endif + + /* common implementation, use attributes */ +} +``` From f17d9321a8ba429e7ea0f1fb76bbebc98eed9e7f Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Mon, 10 Jul 2023 21:39:55 +0200 Subject: [PATCH 05/11] Build cleanup --- api/CMakeLists.txt | 2 ++ docs/abi-version-policy.md | 7 ++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/api/CMakeLists.txt b/api/CMakeLists.txt index eebaf0a900..fba94adcb4 100644 --- a/api/CMakeLists.txt +++ b/api/CMakeLists.txt @@ -101,6 +101,8 @@ endif() if(WITH_REMOVE_METER_PREVIEW) target_compile_definitions(opentelemetry_api INTERFACE ENABLE_REMOVE_METER_PREVIEW) +endif() + if(WITH_ABI_VERSION_2) target_compile_definitions(opentelemetry_api INTERFACE OPENTELEMETRY_ABI_VERSION_NO=2) diff --git a/docs/abi-version-policy.md b/docs/abi-version-policy.md index 438e93d230..e9bc2c7f47 100644 --- a/docs/abi-version-policy.md +++ b/docs/abi-version-policy.md @@ -431,7 +431,8 @@ to follow specification changes. See the issue description for all details: -* [Metrics API/SDK] Add InstrumentationScope attributes in MeterProvider::GetMeter() #2033 +* [Metrics API/SDK] + Add InstrumentationScope attributes in MeterProvider::GetMeter() #2033 #### API change @@ -471,7 +472,7 @@ change in the caller when providing up to 3 parameters. In the SDK class declaration, implement the expected API. -``` +```cpp class MeterProvider final : public opentelemetry::metrics::MeterProvider { public: @@ -500,7 +501,7 @@ In the SDK implementation: * either get the new parameters from the extended ABI v2 method * or provide default values for the old ABI v1 method -``` +```cpp nostd::shared_ptr MeterProvider::GetMeter( nostd::string_view name, nostd::string_view version, From bd75d86561bdeb54d662d8e3c958d8c99f5b2787 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Tue, 5 Sep 2023 14:23:49 +0200 Subject: [PATCH 06/11] Adjust version after merge --- docs/abi-version-policy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/abi-version-policy.md b/docs/abi-version-policy.md index e9bc2c7f47..f907ac978d 100644 --- a/docs/abi-version-policy.md +++ b/docs/abi-version-policy.md @@ -306,7 +306,7 @@ opentelemetry-cpp produces a library for ABI v1 by default. Fixes introducing breaking changes can __not__ be delivered. -This is the current status as of opentelemetry-cpp version 1.9.1 +This is the current status as of opentelemetry-cpp version 1.11.0 ### STABLE V1, EXPERIMENTAL V2 From 18efa93f6e8913b8046daa474d58fdd91f4c4c52 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Wed, 6 Sep 2023 22:22:53 +0200 Subject: [PATCH 07/11] Implement review comments. --- CMakeLists.txt | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index bd90a66084..e94be502e6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -98,6 +98,31 @@ option(WITH_ABI_VERSION_2 "EXPERIMENTAL: ABI version 2 preview" OFF) file(READ "${CMAKE_CURRENT_LIST_DIR}/api/include/opentelemetry/version.h" OPENTELEMETRY_CPP_HEADER_VERSION_H) +if(WITH_ABI_VERSION_1 AND WITH_ABI_VERSION_2) +# +# We do not want to have WITH_ABI_VERSION = "1" or "2", +# and instead prefer two distinct flags, +# WITH_ABI_VERSION_1 and WITH_ABI_VERSION_2. +# +# This allows: +# - to have a specific option description for each ABI +# - to mark experimental/stable/deprecated on flags, not on values +# - to search for exact abi usage move easily, discouraging: +# cmake -DWITH_ABI_VERSION=${ARG} +# +# While not supported yet, +# having distinct WITH_ABI_VERSION_1 and WITH_ABI_VERSION_2 flags +# opens the possibility to support multiple ABI concurrently, +# which is why separate flags are used here. +# +# For now, only one ABI is supported in a build. +# + message( + FATAL_ERROR + "Set either WITH_ABI_VERSION_1 or WITH_ABI_VERSION_2, not both") +endif() + + if(WITH_ABI_VERSION_2) set(OPENTELEMETRY_ABI_VERSION_NO "2") elseif(WITH_ABI_VERSION_1) From 353a3d535aff4449f6257a0053c376a30eb9873b Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Wed, 6 Sep 2023 22:34:31 +0200 Subject: [PATCH 08/11] format --- CMakeLists.txt | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e94be502e6..ed07a64259 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -99,30 +99,16 @@ file(READ "${CMAKE_CURRENT_LIST_DIR}/api/include/opentelemetry/version.h" OPENTELEMETRY_CPP_HEADER_VERSION_H) if(WITH_ABI_VERSION_1 AND WITH_ABI_VERSION_2) -# -# We do not want to have WITH_ABI_VERSION = "1" or "2", -# and instead prefer two distinct flags, -# WITH_ABI_VERSION_1 and WITH_ABI_VERSION_2. -# -# This allows: -# - to have a specific option description for each ABI -# - to mark experimental/stable/deprecated on flags, not on values -# - to search for exact abi usage move easily, discouraging: -# cmake -DWITH_ABI_VERSION=${ARG} -# -# While not supported yet, -# having distinct WITH_ABI_VERSION_1 and WITH_ABI_VERSION_2 flags -# opens the possibility to support multiple ABI concurrently, -# which is why separate flags are used here. -# -# For now, only one ABI is supported in a build. -# + # While not supported yet, having distinct WITH_ABI_VERSION_1 and + # WITH_ABI_VERSION_2 flags opens the possibility to support multiple ABI + # concurrently, which is why separate flags are used here. + # + # For now, only one ABI is supported in a build. + # message( - FATAL_ERROR - "Set either WITH_ABI_VERSION_1 or WITH_ABI_VERSION_2, not both") + FATAL_ERROR "Set either WITH_ABI_VERSION_1 or WITH_ABI_VERSION_2, not both") endif() - if(WITH_ABI_VERSION_2) set(OPENTELEMETRY_ABI_VERSION_NO "2") elseif(WITH_ABI_VERSION_1) From 264e51e32c7d85ed85a9b4139f8d2108d571fb7f Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Wed, 13 Sep 2023 12:50:45 +0200 Subject: [PATCH 09/11] Rewording, misc cleanup --- CMakeLists.txt | 22 +++++++++++++++++---- api/CMakeLists.txt | 9 ++------- docs/abi-version-policy.md | 39 +++++++++++++++++++++++--------------- 3 files changed, 44 insertions(+), 26 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ed07a64259..2a2083c945 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -98,12 +98,26 @@ option(WITH_ABI_VERSION_2 "EXPERIMENTAL: ABI version 2 preview" OFF) file(READ "${CMAKE_CURRENT_LIST_DIR}/api/include/opentelemetry/version.h" OPENTELEMETRY_CPP_HEADER_VERSION_H) +# +# We do not want to have WITH_ABI_VERSION = "1" or "2", +# and instead prefer two distinct flags, +# WITH_ABI_VERSION_1 and WITH_ABI_VERSION_2. +# +# This allows: +# +# - to have a specific option description for each ABI +# - to mark experimental/stable/deprecated on flags, for clarity +# - to search for exact abi usage move easily, discouraging: +# cmake -DWITH_ABI_VERSION=${ARG} +# +# While not supported, +# having distinct WITH_ABI_VERSION_1 and WITH_ABI_VERSION_2 flags +# also opens the possibility to support multiple ABI concurrently, +# should that become necessary. +# if(WITH_ABI_VERSION_1 AND WITH_ABI_VERSION_2) - # While not supported yet, having distinct WITH_ABI_VERSION_1 and - # WITH_ABI_VERSION_2 flags opens the possibility to support multiple ABI - # concurrently, which is why separate flags are used here. # - # For now, only one ABI is supported in a build. + # Only one ABI is supported in a build. # message( FATAL_ERROR "Set either WITH_ABI_VERSION_1 or WITH_ABI_VERSION_2, not both") diff --git a/api/CMakeLists.txt b/api/CMakeLists.txt index b61a94b184..613c89082c 100644 --- a/api/CMakeLists.txt +++ b/api/CMakeLists.txt @@ -94,13 +94,8 @@ if(WITH_REMOVE_METER_PREVIEW) INTERFACE ENABLE_REMOVE_METER_PREVIEW) endif() -if(WITH_ABI_VERSION_2) - target_compile_definitions(opentelemetry_api - INTERFACE OPENTELEMETRY_ABI_VERSION_NO=2) -elseif(WITH_ABI_VERSION_1) - target_compile_definitions(opentelemetry_api - INTERFACE OPENTELEMETRY_ABI_VERSION_NO=1) -endif() +target_compile_definitions(opentelemetry_api + INTERFACE OPENTELEMETRY_ABI_VERSION_NO=${OPENTELEMETRY_ABI_VERSION_NO}) # A better place should be in sdk, not api if(WITH_OTLP_HTTP_SSL_PREVIEW) diff --git a/docs/abi-version-policy.md b/docs/abi-version-policy.md index f907ac978d..1dcd1c91df 100644 --- a/docs/abi-version-policy.md +++ b/docs/abi-version-policy.md @@ -322,37 +322,41 @@ option(WITH_ABI_VERSION_2 "EXPERIMENTAL: ABI version 2 preview" OFF) Instrumented applications are built against ABI v1 by default, but may ask to use ABI v2 instead. -opentelemetry-cpp produces a library for ABI v1 by default, -but can be configured to provide ABI v2 instead. +opentelemetry-cpp produces a library for stable ABI v1 by default, +but can be configured to provide experimental ABI v2 instead. Fixes introducing breaking changes can only be delivered in the experimental ABI v2. -### STABLE V1, STABLE V2 +### STABLE V1, STABLE V2, EXPERIMENTAL V3 -In this state, two ABI versions are available, +In this state, two stable ABI versions are available, the ABI offered by default is the conservative ABI v1. +Fixes introducing breaking changes can no longer be delivered ABI v2, +because it is declared stable. +An experimental ABI v3 is created. + CMake offers the following options: ```cmake option(WITH_ABI_VERSION_1 "ABI version 1" ON) option(WITH_ABI_VERSION_2 "ABI version 2" OFF) +option(WITH_ABI_VERSION_3 "EXPERIMENTAL: ABI version 3 preview" OFF) ``` -Instrumented applications are built against ABI v1 by default, -but may ask to use ABI v2 instead. +Instrumented applications are built against stable ABI v1 by default, +but may ask to use the now stable ABI v2 instead. opentelemetry-cpp produces a library for ABI v1 by default, but can be configured to provide ABI v2 instead. -Fixes introducing breaking changes can not be delivered ABI v2, because it -is declared stable. These fixes can either wait, or an experimental ABI v3 -can be created. +Fixes introducing breaking changes can only be delivered in the experimental +ABI v3. -### DEPRECATED V1, STABLE V2 +### DEPRECATED V1, STABLE V2, EXPERIMENTAL V3 -In this state, two ABI versions are available, +In this state, two stable ABI versions are available, the ABI offered by default is the newer ABI v2. CMake offers the following options: @@ -360,23 +364,28 @@ CMake offers the following options: ```cmake option(WITH_ABI_VERSION_1 "DEPRECATED: ABI version 1" OFF) option(WITH_ABI_VERSION_2 "ABI version 2" ON) +option(WITH_ABI_VERSION_3 "EXPERIMENTAL: ABI version 3 preview" OFF) ``` -Instrumented applications are built against ABI v2 by default, -but may ask to use ABI v1 instead. +Instrumented applications are built against stable ABI v2 by default, +but may ask to use stable ABI v1 instead. opentelemetry-cpp produces a library for ABI v2 by default, but can be configured to provide ABI v1 instead. -### (REMOVED V1), STABLE V2 +Fixes introducing breaking changes can only be delivered in the experimental +ABI v3. + +### (REMOVED V1), STABLE V2, EXPERIMENTAL V3 -In this state, only ABI v2 is available. +In this state, the only stable ABI available is v2. ABI v1 is no longer supported. CMake offers the following options: ```cmake option(WITH_ABI_VERSION_2 "ABI version 2" ON) +option(WITH_ABI_VERSION_3 "EXPERIMENTAL: ABI version 3 preview" OFF) ``` Instrumented applications and the opentelemetry-cpp library are build using From 7f2ceee826bca3ff7eabc029f4b5311b5d855ef6 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Wed, 13 Sep 2023 14:17:33 +0200 Subject: [PATCH 10/11] format --- CMakeLists.txt | 21 ++++++++++----------- api/CMakeLists.txt | 5 +++-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2a2083c945..f579efe431 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -99,21 +99,20 @@ file(READ "${CMAKE_CURRENT_LIST_DIR}/api/include/opentelemetry/version.h" OPENTELEMETRY_CPP_HEADER_VERSION_H) # -# We do not want to have WITH_ABI_VERSION = "1" or "2", -# and instead prefer two distinct flags, -# WITH_ABI_VERSION_1 and WITH_ABI_VERSION_2. +# We do not want to have WITH_ABI_VERSION = "1" or "2", and instead prefer two +# distinct flags, WITH_ABI_VERSION_1 and WITH_ABI_VERSION_2. # # This allows: # -# - to have a specific option description for each ABI -# - to mark experimental/stable/deprecated on flags, for clarity -# - to search for exact abi usage move easily, discouraging: -# cmake -DWITH_ABI_VERSION=${ARG} +# * to have a specific option description for each ABI +# * to mark experimental/stable/deprecated on flags, for clarity +# * to search for exact abi usage move easily, discouraging: # -# While not supported, -# having distinct WITH_ABI_VERSION_1 and WITH_ABI_VERSION_2 flags -# also opens the possibility to support multiple ABI concurrently, -# should that become necessary. +# * cmake -DWITH_ABI_VERSION=${ARG} +# +# While not supported, having distinct WITH_ABI_VERSION_1 and WITH_ABI_VERSION_2 +# flags also opens the possibility to support multiple ABI concurrently, should +# that become necessary. # if(WITH_ABI_VERSION_1 AND WITH_ABI_VERSION_2) # diff --git a/api/CMakeLists.txt b/api/CMakeLists.txt index 613c89082c..f5f1fbf897 100644 --- a/api/CMakeLists.txt +++ b/api/CMakeLists.txt @@ -94,8 +94,9 @@ if(WITH_REMOVE_METER_PREVIEW) INTERFACE ENABLE_REMOVE_METER_PREVIEW) endif() -target_compile_definitions(opentelemetry_api - INTERFACE OPENTELEMETRY_ABI_VERSION_NO=${OPENTELEMETRY_ABI_VERSION_NO}) +target_compile_definitions( + opentelemetry_api + INTERFACE OPENTELEMETRY_ABI_VERSION_NO=${OPENTELEMETRY_ABI_VERSION_NO}) # A better place should be in sdk, not api if(WITH_OTLP_HTTP_SSL_PREVIEW) From 032033e10249a5c24993504e9b3d8f0cc812bacc Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Thu, 14 Sep 2023 10:09:19 +0200 Subject: [PATCH 11/11] Fixed typo. --- docs/abi-version-policy.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/abi-version-policy.md b/docs/abi-version-policy.md index 1dcd1c91df..f3d66bb4f1 100644 --- a/docs/abi-version-policy.md +++ b/docs/abi-version-policy.md @@ -333,7 +333,7 @@ ABI v2. In this state, two stable ABI versions are available, the ABI offered by default is the conservative ABI v1. -Fixes introducing breaking changes can no longer be delivered ABI v2, +Fixes introducing breaking changes can no longer be delivered in ABI v2, because it is declared stable. An experimental ABI v3 is created. @@ -424,7 +424,6 @@ OPENTELEMETRY_BEGIN_NAMESPACE { virtual void DoSomething() = 0; virtual void DoSomethingMore() = 0; - }; } }