Skip to content

[CMAKE] Unify third party dependency management #3460

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

dbarker
Copy link
Member

@dbarker dbarker commented Jun 3, 2025

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

Copy link

netlify bot commented Jun 3, 2025

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

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

Copy link

codecov bot commented Jun 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

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           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ThomsonTan
Copy link
Contributor

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

@dbarker
Copy link
Member Author

dbarker commented Jun 5, 2025

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 dbarker force-pushed the cmake_find_third_party_cleanup branch from 6012dff to af673b9 Compare June 9, 2025 01:30
dbarker added 7 commits June 9, 2025 12:54
…ontent and find_package with the otel_add_thirdparty_package function. Remove the static third party dependency file. Move the version parsing to a version script and set the project version in the main command. Clean up cmake target usage.
…s. Adds a install_thirdparty.sh bash script. Use the script in ci and remove the separate install scripts for abseil, googletest, protobuf, and grpc. Adds third party tag files and removes references to specific third party versions from ci workflows.
… overrides third party package finds/fetches as required
@dbarker dbarker force-pushed the cmake_find_third_party_cleanup branch from 0e2def7 to 94875f1 Compare June 9, 2025 19:11
@dbarker dbarker marked this pull request as ready for review June 9, 2025 20:50
@dbarker dbarker requested a review from a team as a code owner June 9, 2025 20:50
@dbarker dbarker marked this pull request as draft June 13, 2025 17:03
@dbarker dbarker added the pr:do-not-merge This PR is not ready to be merged. label Jun 17, 2025
@dbarker
Copy link
Member Author

dbarker commented Jun 27, 2025

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

  1. Adding a third-party install cmake project and script ([CMAKE] Add thirdparty install cmake project and install bash script #3486)
  2. Find/Fetch googletest and benchmark ([CMAKE] clean up googletest and benchmark dependency management #3485)
  3. Find/Fetch prometheus-cpp and cleanup to explicitly link core and pull targets ([CMAKE] Add CMake script to find or fetch prometheus-cpp #3522)
  4. Find/Fetch Curl and find zlib ([CMAKE] Add CMake scripts to find or fetch curl and find zlib #3526)
  5. Find/Fetch OpenTracing and support the opentracing-static target ([CMAKE] Address the vcpkg opentelemetry-cpp port CMake patches #3518)
  6. Find/Fetch nlohmann-json and cmake cleanup ([CMAKE] find or fetch nlohmann-json #3523)
  7. Find/Fetch ms-gsl and test ([CMAKE] Add CMake script to find or fetch Microsoft.GSL  #3521)
  8. Find/Fetch Protobuf/Grpc ([CMAKE] Add CMake scripts to find or fetch protobuf and grpc #3533)

TODO:

  • Setting PROJECT_VERSION for opentelemetry-cpp and cache OPENTELEMETRY_* variables for in-tree builds
  • Fix API only cmake builds (api only must override all other options) and test

@dbarker dbarker closed this Jun 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:do-not-merge This PR is not ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants