opentelemetry-cpp icon indicating copy to clipboard operation
opentelemetry-cpp copied to clipboard

[CMAKE] Unify third party dependency management

Open dbarker opened this issue 6 months ago • 4 comments

Update third party dependency management in CMake.

Rationale:

  • To to give users full control over how opentelemetry-cpp satisfies its dependencies in cmake
    • For those using package managers (conan, vcpkg, ...) only use the packages installed by the manager.
    • For those wanting a single build tree use CMake's FetchContent for all dependencies.
  • The CMake build does not apply a common strategy for adding third party dependencies
    • Some are submodules and added to the build tree, some require packages to be installed, and some use the ExternalProject module.
  • Updating and managing what dependency versions are tested in CI is challenging.
    • The third_party_release file only accounts for some of the dependencies and the versions are not fully used in the CMake build, instead the github ci workflow files have a lot of dependency versions in them.
    • Custom bash scripts (setup_googletest.sh, setup_grpc.sh, ..) are mixed with apt installs for Linux, while vcpkg is used on Windows bringing in dependency versions from the tools/vcpkg submodule.

Goals:

  • Provide new options to have otel-cpp find all dependencies as installed packages or fetch them with FetchContent
  • Create a single cmake project to build/install all dependencies based on the version tags file.
  • Move tested versions out of the github workflow files and test with the versions from third_party_release

Changes

  • Add an otel_add_thirdparty_package function unify how third party dependencies are found/fetched.
    • Create a cmake script file for each dependency to ensure all required targets are imported/installed
    • Enable fetching from submodules or git repo with the version tags in the third_party_release.
  • Fix handling of WITH_API_ONLY so it overrides any options that may import third party packages. Add api only test to verify.
  • Remove the install_windows_deps CMake function and ARCH detection
    • The bootstrapping use case is addressed through fetching all dependencies with standard CMake FetchContent from github.
  • Clean up cmake dependency linking
    • Use the standard CURL::libcurl target
    • Use the explicit prometheus-cpp pull and core targets
    • Use either the shared or static opentracing-cpp target
  • Add cmake options to force fetch dependencies or to require all local dependencies (following the model used by Protobuf)
    • If only local dependencies should be used then set OTELCPP_THIRDPARTY_FIND_PACKAGE_ONLY=ON to prevent fetching from github or submodules.
    • If only supplying dependencies from FetchContent is desired then set OTELCPP_THIRDPARTY_FETCH_CONTENT_ONLY=ON
  • Update github ci workflow to test fetching all dependencies from github (no git submodules).
  • Add a cmake project to build/install all dependencies based on the selected git tag file.
    • Remove the googletest, abseil, protobuf, and grpc install scripts and replace with a single install_thirdparty script.
  • Add the ms-gsl dependency to the vcpkg setup ci script and fix the api cmake file so the component will find this dependency if WITH_GSL=ON

For significant contributions please make sure you have completed the following items:

  • [ ] CHANGELOG.md updated for non-trivial changes
  • [ ] Unit tests have been added
  • [ ] Changes in public API reviewed

dbarker avatar Jun 03 '25 02:06 dbarker

Deploy Preview for opentelemetry-cpp-api-docs canceled.

Name Link
Latest commit 06a30154faa5e5952417077a3eb1bbf3f10cd0ff
Latest deploy log https://app.netlify.com/projects/opentelemetry-cpp-api-docs/deploys/6850a154d82263000866128e

netlify[bot] avatar Jun 03 '25 02:06 netlify[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 89.92%. Comparing base (021490e) to head (06a3015). Report is 8 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3460   +/-   ##
=======================================
  Coverage   89.92%   89.92%           
=======================================
  Files         219      219           
  Lines        7042     7042           
=======================================
  Hits         6332     6332           
  Misses        710      710           
:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Jun 03 '25 02:06 codecov[bot]

Why do we need add a dedicated function otel_add_external_project for third party, could just use otel_add_component?

ThomsonTan avatar Jun 05 '25 20:06 ThomsonTan

Why do we need add a dedicated function otel_add_external_project for third party, could just use otel_add_component?

@ThomsonTan They serve two different purposes.

The otel_add_external_project function wraps the CMake find_package and FetchContent_* functions and sets some properties needed for install. The rationale for this is to unify support for importing the required targets by finding packages or fetching from git submodules/repos.

The otel_add_component function supports grouping otel-cpp targets to create otel-cpp components for install/packaging and this step depends on having valid third party dependency targets imported.

dbarker avatar Jun 05 '25 22:06 dbarker

Closing this PR in favor of breaking up the changes into multiple PRs:

  1. Adding a third-party install cmake project and script (#3486)
  2. Find/Fetch googletest and benchmark (#3485)
  3. Find/Fetch prometheus-cpp and cleanup to explicitly link core and pull targets (#3522)
  4. Find/Fetch Curl and find zlib (#3526)
  5. Find/Fetch OpenTracing and support the opentracing-static target (#3518)
  6. Find/Fetch nlohmann-json and cmake cleanup (#3523)
  7. Find/Fetch ms-gsl and test (#3521)
  8. Find/Fetch Protobuf/Grpc (#3533)
  9. Setting PROJECT_VERSION for opentelemetry-cpp and cache OPENTELEMETRY_* variables for in-tree builds (#3605)

TODO:

  • Fix API only cmake builds (api only must override all other options) and test

dbarker avatar Jun 27 '25 00:06 dbarker