[BUILD] Fixes grpc linking for OTLP exporter's tests#3435
[BUILD] Fixes grpc linking for OTLP exporter's tests#3435marcalff merged 14 commits intoopen-telemetry:mainfrom
Conversation
✅ Deploy Preview for opentelemetry-cpp-api-docs canceled.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
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:
|
|
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. |
b2837c5 to
4d8ab09
Compare
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 - name: Run Tests (shared libs)
env:
BUILD_SHARED_LIBS: 'ON'
run: ./ci/do_ci.sh cmake.install.test |
6f86f0f to
c8cbb66
Compare
The problem of building shared libraries with conan is solved now. |
4f9d5ff to
f829e9f
Compare
There was a problem hiding this comment.
Requesting two changes based on #3435 (comment)
(edit - good to merge) Changes requested:
- (edit - this is done)
Remove handling ofprotobuf::libprotobufandgRPC::grpc++targets asINTERFACE_LIBRARYtype from this PR and in that case default to the prior behavior of leaving theadd_library<type>undefined (letBUILD_SHARED_LIBSdetermine 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 theubuntu_2404_conan_latestjob 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)
84506fc to
225ebde
Compare
Force build opentelemetry_proto and OTLP exporters as static libraries when protobuf or grpc are built static.
…protobuf as shared libraries
d5bf956 to
2e344f2
Compare
Force build opentelemetry_proto and OTLP exporters as static libraries when protobuf or grpc are built static.
Fixes #3405
Changes
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes