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

Fixes grpc linking for OTLP exporter's tests

Open owent opened this issue 6 months ago • 6 comments

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
  • [x] Unit tests have been added
  • [ ] Changes in public API reviewed

owent avatar May 22 '25 12:05 owent

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

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

netlify[bot] avatar May 22 '25 12:05 netlify[bot]

Codecov Report

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

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

Additional details and impacted files

Impacted file tree graph

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

codecov[bot] avatar May 22 '25 12:05 codecov[bot]

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.

dbarker avatar May 30 '25 16:05 dbarker

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.

owent avatar May 30 '25 17:05 owent

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

dbarker avatar May 30 '25 17:05 dbarker

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

owent avatar Jun 07 '25 17:06 owent