Please add number version in SONAMEs. #3110
Comments
github-actions
bot
added
the
needs-triage
|
Thanks for the report. Agreed, opentelemetry-cpp needs to have this. In more details:
|
marcalff
added
triage/accepted
Unfortunately, it could be more complicated than that 😞 Whenever we break the ABI, the SOVERSION has to change.
More formal rules are available here. Say, we had current=1, revision=0, age=0 in the very beginning. The SOVERSION is 1. But since the code breaks the ABI of libopentelemetry-logs only, the SOVERSION for other libraries remains the same. |
|
A test script to check for ABI changes between different releases (adapt as needed to add more tags or to build various exporters): #!/bin/sh
mkdir -p dumps
first=v1.10.0
tags="v1.11.0 v1.12.0 v1.13.0 v1.14.0 v1.14.1 v1.14.2 v1.15.0 v1.16.0 v1.16.1 v1.17.0"
for tag in ${first} ${tags}; do
rm -rf build
git checkout "${tag}"
cmake -B build -DBUILD_TESTING=OFF -DWITH_BENCHMARK=OFF -DCMAKE_CXX_FLAGS="-Og -g" -DBUILD_SHARED_LIBS=ON -DWITH_EXAMPLES=OFF
cmake --build build -j
cmake --install build --prefix "dist/${tag}"
for i in "dist/${tag}/lib/"*.so; do abi-dumper "${i}" -o "dumps/${tag}-$(basename "${i}").dump" -lver "${tag}"; done
git reset --hard "${tag}"
done
prev="${first}"
for tag in ${tags}; do
for lib in "dist/${prev}/lib/"*.so; do
name="$(basename "${lib}" .so)"
base="$(basename "${lib}")"
abi-compliance-checker -l "${name}" -old "dumps/${prev}-${base}.dump" -new dumps/${tag}-${base}.dump --extended
done
prev="${tag}"
done |
|
The ABI has changed several times: libopentelemetry_metricsBinary compatibility: 99.8% Binary compatibility: 99.8% Binary compatibility: 99.5% Binary compatibility: 98.9% Binary compatibility: 99.9% Binary compatibility: 97.2% Binary compatibility: 99.3% Binary compatibility: 99.7% libopentelemetry_exporter_in_memoryBinary compatibility: 97.7% libopentelemetry_exporter_ostream_spanBinary compatibility: 97.9% Binary compatibility: 99.6% libopentelemetry_traceBinary compatibility: 98% Binary compatibility: 99.5% Binary compatibility: 99.3% Binary compatibility: 99.6% libopentelemetry_commonBinary compatibility: 99.1% Binary compatibility: 99% libopentelemetry_exporter_ostream_metricsBinary compatibility: 98.3% Binary compatibility: 99.2% libopentelemetry_logsBinary compatibility: 99.5% Binary compatibility: 99.5% libopentelemetry_exporter_ostream_logsBinary compatibility: 99% libopentelemetry_resourcesBinary compatibility: 98.4% |
|
@sjinks Thanks for commenting on this. To clarify a few expectations, see details below. Typical libraryIn a typical library, there are:
and the term
so an application compiled against header X can link (dynamically) with version Y. ABI compatibility in opentelemetry-cppOpentelemetry-cpp consist of:
Typical application code using opentelemetry-cpp falls into several categories:
Expectations are as follows: Instrumented codeInstrumented code is only allowed to use the public Some libraries, say libA, libB, libC, can be compiled against opentelemetry-cpp API headers versions 1.12, 1.15, 1.17 respectively, and are all expected to work properly when linked inside an application that uses version 1.18 for example. This is what we call abi compatibility for short: Application main() functionIn an application that deploys opentelemetry-cpp, the main function initializes the SDK and add exporters, thereby initializing the global singleton provider objects defined in the API, to point to an actual (not noop) implementation. To do this, the main() needs to use header files from the SDK and exporters. Here we do NOT provide ABI compatibility, so a change in SDK or exporter public headers is acceptable. Custom exporterSomeone may want to implement an exporter for a custom protocol, or to a different backend. To do this, the exporter implementation needs to use header files from the SDK, hopefully only using SDK header files considered public. Here we do NOT provide ABI compatibility, so a change in SDK or exporter public headers is acceptable. Custom SDKSomeone may want to extend or change the SDK implementation itself. To do this, the custom sdk implementation needs to use the full header files from the SDK, including headers considered private. Here we do provide compatibility of any kind. It is correct to say the To be more clear, the What we aim to maintain is the In other words:
Dynamic linking was added for windows, but is experimental and unstable, given that there is no stable SDK or exporter api. I tried to implement some tooling, but never got time to finish it. See repository: |
|
Orthogonal discussion: Currently we ship different libraries for traces, metrics, logs, common libraries, etc for the SDK. These libraries have inter dependencies (error handler), and cross signal dependencies (metrics depends on traces, because of examplars). A preliminary refactoring before using SONAME is to ship only one library for the entire SDK, to reconcile this. |
Because there is no For example, between v1.16.1 and v1.17.0, the size of Also, abseil says:
Thus, the parts of the API that depend on
Linux distros start to adopt OpenTelemetry, which is great. However, they prefer to ship dynamic libraries, not static ones. Which means that the applications using OpenTelemetry will use dynamic linking.
My point was that
won't probably work :-( Every library will have to use its own SONAME, and even for For example, libopentelemetry_metrics will be smth like |
Agreed, this is a good discussion. So, to summarize, there are 2 ABI compatibility issues that are important to address:
|
@sjinks Do you have more details on this ? I am looking at the git history, but can not find when/why this happened.
Yes, |
I used this script: #!/bin/sh
mkdir -p dumps
first="v1.16.1"
tags="v1.17.0"
for tag in ${first} ${tags}; do
rm -rf build
git checkout "${tag}"
cmake -B build -DBUILD_TESTING=OFF -DWITH_BENCHMARK=OFF -DCMAKE_CXX_FLAGS="-Og -g" -DBUILD_SHARED_LIBS=ON -DWITH_EXAMPLES=OFF
cmake --build build -j
cmake --install build --prefix "dist/${tag}"
for i in "dist/${tag}/lib/"*.so; do abi-dumper "${i}" -o "dumps/${tag}-$(basename "${i}").dump" -lver "${tag}"; done
git reset --hard "${tag}"
done
prev="${first}"
for tag in ${tags}; do
for lib in "dist/${prev}/lib/"*.so; do
name="$(basename "${lib}" .so)"
base="$(basename "${lib}")"
abi-compliance-checker -l "${name}" -old "dumps/${prev}-${base}.dump" -new dumps/${tag}-${base}.dump --extended
done
prev="${tag}"
doneand then checked the report for |
|
Thanks for the scripts. I tried to reproduce locally, and got this: The following command: allocated up to 110 Gb of memory (I have a big machine), and is then killed. When trying without the I am using: |
Dear OpenTelemetry Team,
While packaging opentelemetry-cpp for Debian, I noticed the SDK's SONAME lacks a numerical version. While I understand the library has been carefully designed and considering ABI stability, IMO, it would be useful to add a numerical version in case you need to introduce breaking changes in the future. Bumping the SONAME would help to have smooth library transitions.
Could you please consider this?
Than you
The text was updated successfully, but these errors were encountered: