[BUILD] Fixes grpc linking for OTLP exporter's tests #3435

owent · 2025-05-22T12:14:51Z

Force build opentelemetry_proto and OTLP exporters as static libraries when protobuf or grpc are built static.

Fixes #3405

Changes

Build opentelemetry_proto and OTLP exporters as static libraries when protobuf or gRPC are statically built.

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

netlify · 2025-05-22T12:15:05Z

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

Name	Link
🔨 Latest commit	`991561b`
🔍 Latest deploy log	https://app.netlify.com/projects/opentelemetry-cpp-api-docs/deploys/68603d576800680008987a3e

codecov · 2025-05-22T12:38:34Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.95%. Comparing base (82bddef) to head (991561b).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3435   +/-   ##
=======================================
  Coverage   89.95%   89.95%           
=======================================
  Files         219      219           
  Lines        7051     7051           
=======================================
  Hits         6342     6342           
  Misses        709      709

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

cmake/opentelemetry-proto.cmake

cmake/tools.cmake

cmake/opentelemetry-proto.cmake

exporters/otlp/CMakeLists.txt

dbarker · 2025-05-30T16:34:57Z

Thanks for working on this @owent. I've added some feedback. Can you cherry-pick the tests commit from the testing PR #3427 into this PR so we can verify the changes?

I've tested this PR locally and the tests now pass with otel-cpp shared libs built with protobuf/grpc static libs (grpc 1.67 protobuf 5.27). The tests still fail to build with otel-cpp shared libs built with protobuf/grpc shared libs though.

owent · 2025-05-30T17:31:02Z

Thanks for working on this @owent. I've added some feedback. Can you cherry-pick the tests commit from the testing PR #3427 into this PR so we can verify the changes?

I've tested this PR locally and the tests now pass with otel-cpp shared libs built with protobuf/grpc static libs (grpc 1.67 protobuf 5.27). The tests still fail to build with otel-cpp shared libs built with protobuf/grpc shared libs though.

Thanks. I just tested #3405 before. I will be on holiday these days. And I will test #3427 after that.

dbarker · 2025-05-30T17:54:54Z

Thanks for working on this @owent. I've added some feedback. Can you cherry-pick the tests commit from the testing PR #3427 into this PR so we can verify the changes?
I've tested this PR locally and the tests now pass with otel-cpp shared libs built with protobuf/grpc static libs (grpc 1.67 protobuf 5.27). The tests still fail to build with otel-cpp shared libs built with protobuf/grpc shared libs though.

Thanks. I just tested #3405 before. I will be on holiday these days. And I will test #3427 after that.

Sounds good. We could address the test failures from #3427 in two PRs. This PR should fix the shared otel-cpp and static protobuf/grpc builds. Then a second PR can tackle the shared otel-cpp with shared protobuf/grpc libs build.

I think we can merge this PR with passing Run Tests (shared libs) steps added to the cmake_install.yml workflow jobs with the latest grpc/protobuf versions (ubuntu_2404_latest and ubuntu_2404_conan_latest). Can you add the following step to those jobs?

    - name: Run Tests (shared libs)
      env:
        BUILD_SHARED_LIBS: 'ON'
      run: ./ci/do_ci.sh cmake.install.test

owent · 2025-06-07T17:08:33Z

Thanks for working on this @owent. I've added some feedback. Can you cherry-pick the tests commit from the testing PR #3427 into this PR so we can verify the changes?

I've tested this PR locally and the tests now pass with otel-cpp shared libs built with protobuf/grpc static libs (grpc 1.67 protobuf 5.27). The tests still fail to build with otel-cpp shared libs built with protobuf/grpc shared libs though.

The problem of building shared libraries with conan is solved now.
And I also cherry-pick #3427 into this PR to check the building.
Could you please have a look when you have time?@dbarker

cmake/opentelemetry-proto.cmake

dbarker

Requesting two changes based on #3435 (comment)

(edit - good to merge) ~~Changes requested:~~

(edit - this is done) Remove handling of protobuf::libprotobuf and gRPC::grpc++ targets as INTERFACE_LIBRARY type from this PR and in that case default to the prior behavior of leaving the add_library <type> undefined (let BUILD_SHARED_LIBS determine the type).
(edit - conan shared libs was test removed and we can add more testing in a separate PR) ~~Remove the Run Tests (shared libs) step from the ubuntu_2404_conan_latest job and move that step to the ubuntu_2404_latest job~~.
(edit - this is fixed) ~~Remove OPENTELEMETRY_OTLP_PROTO_LIB_TYPE from opentelemetry-proto.cmake - please see #3435 (comment)~~

cmake/opentelemetry-proto.cmake

Force build opentelemetry_proto and OTLP exporters as static libraries when protobuf or grpc are built static.

…protobuf as shared libraries

dbarker

LGTM. Thanks for your time on this @owent

owent requested a review from a team as a code owner May 22, 2025 12:14

owent mentioned this pull request May 22, 2025

Add test for building otel-cpp shared libraries against shared libraries of grpc and protobuf #3405

Closed

dbarker reviewed May 30, 2025

View reviewed changes

cmake/opentelemetry-proto.cmake Show resolved Hide resolved

dbarker reviewed May 30, 2025

View reviewed changes