Enable generating deb, rpm, NuGet, tgz, zip package through cmake build

[lalitb]

Add support for generating platform specific binary packages:

Linux : deb , rpm, tgx
Windows : NuGet, zip

While we still don't deliver any such binary package, just facilitate the package generation if someone wants to use it.

[codecov]

Codecov Report

Merging #1662 (fad13a4) into main (dfff693) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1662   +/-   ##
=======================================
  Coverage   85.79%   85.79%           
=======================================
  Files         171      171           
  Lines        5240     5240           
=======================================
  Hits         4495     4495           
  Misses        745      745           

[ThomsonTan]

Will we consider add the same generation for bazel?

[lalitb]

Will we consider add the same generation for bazel?

We use cpack config generator to generate these packages, I don't know if bazel provide any such support.

[owent]

Should we also set CPACK_PACKAGE_HOMEPAGE_URL
to https://opentelemetry.io/docs/instrumentation/cpp/ or https://opentelemetry.io ?
And is it better to move the shared options to a standalone file? I think CPACK_RPM_PACKAGE_DESCRIPTION, CPACK_PACKAGE_VENDOR, CPACK_PACKAGE_VERSION_MAJOR, CPACK_PACKAGE_VERSION_MINOR and etc. could be set by all generators.

[marcalff]

Raised a PR to discuss, if this is the good idea to add binary deb/rpm generation as past of cmake build. While we still don't deliver any such binary package, just facilitate the package generation if someone wants to use it.

Ok in principle, but in my understanding packaging can become really complicated really fast, so this needs analysis.

For example:

• do we ship dependencies (nlohman_json, grpc, curl, abseil, etc) inside the otelcpp package, or do we add dependencies on these packages ?
• do we ship only one package, compiled with every exporter (zipkin, prometheus, elastic search, etc) or different packages for different exporters individually ?
• what about options that affect the build, like WITH_STL or WITH_GSL, which one do we pick for "the" package ?

Packaging will be needed at some point, especially to help adoption from distributions, so it is a valid and valuable thing to do.

Before we get to packaging, I think the following is needed as a pre-requisite:

• make sure to upgrade dependencies to the most up to date code, and stay up to date
• avoid exposing dependencies to users if possible (for example, not require nlohmann_json from public header files), to lower complexity
• for the SDK in particular, clarify which header file is public and which header file is private to the SDK implementation, so that only public header files are shipped with the SDK package.

[lalitb]

Thanks @marcalff , these are valid concerns raised. Trying to answer some of them

do we ship dependencies (nlohman_json, grpc, curl, abseil, etc) inside the otelcpp package, or do we add dependencies on these packages ?

I think we shouldn't ship dependencies inside the package. As of now, we are adding these dependencies to the package, so the package installation should fail if they are missing.

do we ship only one package, compiled with every exporter (zipkin, prometheus, elastic search, etc) or different packages for different exporters individually ?

We don't release these components separately or have different version numbers for these components. In that case, it makes sense to release them as a single package. Also to some extent, it is customizable using cmake build. So the below command will generate API only package.

$ cmake .. -DWITH_API_ONLY=ON && make && cpack -G DEB

And to have a package for Zipkin exporter

$ cmake .. DWITH_ZIPKIN=ON && make && cpack -G DEB

what about options that affect the build, like WITH_STL or WITH_GSL, which one do we pick for "the" package ?

We can let the user decide which to pick based on the cmake options used.

for the SDK in particular, clarify which header file is public and which header file is private to the SDK implementation, so that only public header files are shipped with the SDK package.

Agree. This is highly driven by our cmake install rules defined in CMakeLists.txt. We do try to define the exclude rules for header files at some of the places, but this needs to be investigated more thoroughly.

[sirzooro]

I tried to build rpm package on CentOS8/RedHat8 using my custom code listed in #1752 . I have noticed following issues when I did this. Please address them in this pull request or other one(s) as necessary:

1. cmake installed by default on my system (3.20.2) adds some extra paths which caused conflicts with system packages when tried to my opentelemetry-cpp rpm. I had to exclude them using following lines. First three lines are needed when CPACK_PACKAGING_INSTALL_PREFIX is set to /usr/local (like in this pull request), last one when it is set to /usr (as for official CentOS/RH packages).

list(APPEND CPACK_RPM_EXCLUDE_FROM_AUTO_FILELIST_ADDITION "/usr/local")
list(APPEND CPACK_RPM_EXCLUDE_FROM_AUTO_FILELIST_ADDITION "/usr/local/include")
list(APPEND CPACK_RPM_EXCLUDE_FROM_AUTO_FILELIST_ADDITION "/usr/local/lib64")
list(APPEND CPACK_RPM_EXCLUDE_FROM_AUTO_FILELIST_ADDITION "/usr/lib64/cmake")

2. Generated package does not have dependency on protobuf-devel. I did not check this further how to fix this i CMakeFile as I solved this in other way in my build scripts. I have found this when trying to build my app in docker, where I have to install all required packages before build.
3. Libraries usually have at least 3 packages: main one (with compiled .so files needed to run apps), -devel with files needed to compile other apps/libs with this library (e.g. header files), and -debuginfo with extracted debug information needed by tools like gdb.
4. Libraries usually have version number attached to file name, and symlink from .so file(s) to current version. OpenTelemetry libs probably also should use this naming scheme, e.g. /lib64/libopentelemetry_common.so.1.7.0.

[lalitb]

Should we also set CPACK_PACKAGE_HOMEPAGE_URL to https://opentelemetry.io/docs/instrumentation/cpp/ or https://opentelemetry.io ? And is it better to move the shared options to a standalone file? I think CPACK_RPM_PACKAGE_DESCRIPTION, CPACK_PACKAGE_VENDOR, CPACK_PACKAGE_VERSION_MAJOR, CPACK_PACKAGE_VERSION_MINOR and etc. could be set by all generators.

Good point. Have set CPACK_PACKAGE_HOMEPAGE_URL to https://opentelemetry.io/", and moved all shared variables together.

[owent]

Why only set cpack variables on Unix like systems?We can still use CPack archive generator on most system and use NuGet generator on Windows.

[flier]

Agree, at least we can get a tarball on MacOS.

[lalitb]

Good point. Have added TARBALL cmake option to create tarball on Linux and Mac. I am still testing the changes for NuGet generation, so if it takes more time will add it in separate PR.

[lalitb]

Without protobuf-devel compilation of client code fails with following error. Maybe this should be fixed in a different way, by moving #include "google/protobuf/message.h" to .cpp file and adding necessary forward declarations to .h file if needed.

@sirzooro Help me understand it here. Even if we move the protobuf/grpc headers from public otel-cpp headers to cc file ( which is definitely a right step, and I will raise a PR for this), the linking will still fail if protobuf or grpc is not installed. Regarding adding protobuf-devel or grpc dependency in rpm specs file (through CPack), how can we do that as there is no rpm package for these libraries which can be installed to meet the dependency.

[sirzooro]

Without protobuf-devel compilation of client code fails with following error. Maybe this should be fixed in a different way, by moving #include "google/protobuf/message.h" to .cpp file and adding necessary forward declarations to .h file if needed.

@sirzooro Help me understand it here. Even if we move the protobuf/grpc headers from public otel-cpp headers to cc file ( which is definitely a right step, and I will raise a PR for this), the linking will still fail if protobuf or grpc is not installed. Regarding adding protobuf-devel or grpc dependency in rpm specs file (through CPack), how can we do that as there is no rpm package for these libraries which can be installed to meet the dependency.

opentelemetry-cpp library needs protobuf and protobuf-devel to be built, this is fine. My point is that when when I have rpm ready, it should be enough to install opentelemetry-cpp and protobuf only in order to build client apps which use it. Now I have to also install protobuf-devel too. PR which you are going to create will fix this. You can verify this fix in your CI tests by building opentelemetry-cpp rpm first, then start new docker instance, install it inside it (all dependencies like protobuf will be installed automatically too) and try to build some test app. Last step should succeed without having to explicitly install protobuf-devel inside that docker.

[sirzooro]

I have tested changes on CentOS8 and RedHat8 and was able to build rpm without any issues.

[lalitb]

opentelemetry-cpp library needs protobuf and protobuf-devel to be built, this is fine. My point is that when when I have rpm ready, it should be enough to install opentelemetry-cpp and protobuf only in order to build client apps which use it. Now I have to also install protobuf-devel too. PR which you are going to create will fix this. You can verify this fix in your CI tests by building opentelemetry-cpp rpm first, then start new docker instance, install it inside it (all dependencies like protobuf will be installed automatically too) and try to build some test app. Last step should succeed without having to explicitly install protobuf-devel inside that docker.

Got it, thanks for explanation, it makes sense now. Have incorporated the suggested changes.

@owent - I have also incorporated the changes suggested by you. Can you please review it again when you have time. Thanks.

[owent]

message("-- /etc/os_release : ${Name}=${Value}")

opentelemetry-cpp library needs protobuf and protobuf-devel to be built, this is fine. My point is that when when I have rpm ready, it should be enough to install opentelemetry-cpp and protobuf only in order to build client apps which use it. Now I have to also install protobuf-devel too. PR which you are going to create will fix this. You can verify this fix in your CI tests by building opentelemetry-cpp rpm first, then start new docker instance, install it inside it (all dependencies like protobuf will be installed automatically too) and try to build some test app. Last step should succeed without having to explicitly install protobuf-devel inside that docker.

Got it, thanks for explanation, it makes sense now. Have incorporated the suggested changes.

@owent - I have also incorporated the changes suggested by you. Can you please review it again when you have time. Thanks.

There is still a typo problem above message("-- /etc/os_release : ${Name}=${Value}"). Should it be message("-- /etc/os-release : ${Name}=${Value}") ?

[lalitb]

There is still a typo problem above message("-- /etc/os_release : ${Name}=${Value}"). Should it be message("-- /etc/os-release : ${Name}=${Value}")

Thanks @owent . This got missed, have fixed it now.

[owent]

LGTM

[marcalff]

In my (limited) understanding of packaging, this PR produces a package which contents depends on the CMake options used at build time (with/without STL, various exporters, etc).

While this is a good step towards packaging, my concern is that this will need to be revised later, so we should be careful about user expectations, in particular if someone starts to take this into a distribution.

Ok to merge to main, with a big note to clarify that packaging is still experimental, and subject to changes.

In the long term, I think we will need:

• a package for opentelemetry-cpp-api, to be used (alone) by libraries instrumenting their code
• a package for opentelemetry-cpp-sdk, to be used when linking an application with the sdk
• several packages, opentelemetry-cpp-exporter-name, to use exporters as needed.

[lalitb]

In my (limited) understanding of packaging, this PR produces a package which contents depends on the CMake options used at build time (with/without STL, various exporters, etc).

While this is a good step towards packaging, my concern is that this will need to be revised later, so we should be careful about user expectations, in particular if someone starts to take this into a distribution.

Ok to merge to main, with a big note to clarify that packaging is still experimental, and subject to changes.

In the long term, I think we will need:

• a package for opentelemetry-cpp-api, to be used (alone) by libraries instrumenting their code
• a package for opentelemetry-cpp-sdk, to be used when linking an application with the sdk
• several packages, opentelemetry-cpp-exporter-name, to use exporters as needed.

Thanks for the comments @marcalff. I agree, It would be good to add a note that the packaging is experimental for now, and users may have to customize it further before using it as distribution. In general, package generation using CMake is used across C++/C projects like opencv, maridb, zerodb, poco and is well received within community. This allows us to generate api only, api + sdk only, api + sdk + exporter-name package, maybe we can further customize the package-name based on the CMake options provided. This is also lesser maintenance effort than directly providing package-specs files for each of the packaging system.

[lalitb]

Added a note -

OpenTelemetry C++ supports generating plateform specific binary packages from CMake
configuration. The packages generated through this mayn't be production ready, and user
may have to customize it further before using it as distribution.