Skip to content
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

Build googletest at buildtime, relying on CMake #3267

Open
santiagorr opened this issue Feb 11, 2025 · 4 comments
Open

Build googletest at buildtime, relying on CMake #3267

santiagorr opened this issue Feb 11, 2025 · 4 comments
Labels
triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@santiagorr
Copy link
Contributor

I believe it would be great to better integrate googletest into the opentelemetry-cpp's build process, so when distro/system-wide googletest source code is available, it can be built along with opentelemetry-cpp.

As googletest upstream recommends, gtest and gmock should be compiled with the same flags than the tested code (http://groups.google.com/group/googletestframework/browse_thread/thread/668eff1cebf5309d). Upstream doesn't recommend to rely on precompiled libraries.

I found a related issue while packaging opentelemetry-cpp to debian, getting tests that end up with FATAL, even if they should pass. E.g.:

test 515
        Start 515: exporter.otlp.OtlpHttpExporterTest.Shutdown

515: Test command: /home/santiago/cpp/opentelemetry-cpp-1.19.0/obj-x86_64-linux-gnu/exporters/otlp/otlp_http_exporter_test "--gtest_filter=OtlpHttpExporterTest.Shutdown"
515: Working Directory: /home/santiago/cpp/opentelemetry-cpp-1.19.0/obj-x86_64-linux-gnu/exporters/otlp
515: Test timeout computed to be: 1500
515: Running main() from ./googletest/src/gtest_main.cc
515: Note: Google Test filter = OtlpHttpExporterTest.Shutdown
515: [==========] Running 1 test from 1 test suite.
515: [----------] Global test environment set-up.
515: [----------] 1 test from OtlpHttpExporterTest
515: [ RUN      ] OtlpHttpExporterTest.Shutdown
515: [Error] File: ./exporters/otlp/src/otlp_http_exporter.cc:150 [OTLP TRACE HTTP Exporter] ERROR: Export 0 trace span(s) failed, exporter is shutdown
515: [       OK ] OtlpHttpExporterTest.Shutdown (0 ms)
515: [----------] 1 test from OtlpHttpExporterTest (0 ms total)
515: 
515: [----------] Global test environment tear-down
515: [==========] 1 test from 1 test suite ran. (0 ms total)
515: [  PASSED  ] 1 test.
515: 
515: [ FATAL ] ./googletest/include/gtest/internal/gtest-port.h:1811:: pthread_key_delete(key_)failed with error 22
515/609 Test #515: exporter.otlp.OtlpHttpExporterTest.Shutdown ..........................................................Subprocess aborted***Exception:   0.01 sec
Running main() from ./googletest/src/gtest_main.cc
Note: Google Test filter = OtlpHttpExporterTest.Shutdown
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from OtlpHttpExporterTest
[ RUN      ] OtlpHttpExporterTest.Shutdown
[Error] File: ./exporters/otlp/src/otlp_http_exporter.cc:150 [OTLP TRACE HTTP Exporter] ERROR: Export 0 trace span(s) failed, exporter is shutdown
[       OK ] OtlpHttpExporterTest.Shutdown (0 ms)
[----------] 1 test from OtlpHttpExporterTest (0 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (0 ms total)
[  PASSED  ] 1 test.

[ FATAL ] ./googletest/include/gtest/internal/gtest-port.h:1811:: pthread_key_delete(key_)failed with error 22

I understand that for opentelemetry-cpp's CI, googletest is setup with ./ci/setup_googletest.sh, but it is suboptimal from a distro packaging PoV.

I'd propose to rely on CMake's very features (ExternalProject / FindGTest) to build and find the googletest libraries. I belive

@github-actions github-actions bot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Feb 11, 2025
@lalitb lalitb added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 19, 2025
@lalitb
Copy link
Member

lalitb commented Feb 19, 2025

@santiagorr Thanks for the issue. Would you like to contribute the PR?

@santiagorr
Copy link
Contributor Author

@santiagorr Thanks for the issue. Would you like to contribute the PR?

I am far from being a cmake expert. I could try, if nobody else is eager to address it, but it would be great if someone with more cmake experience is available.

Thanks for the 'triage/accepted' label!

@ThomsonTan
Copy link
Contributor

Thanks for raising the issue, @santiagorr.

We have googletest as submodule under https://github.com/open-telemetry/opentelemetry-cpp/tree/main/third_party.

Would that be very different than the ExternalProject / FindGTest approach?

@sjinks
Copy link
Contributor

sjinks commented Feb 21, 2025

Would that be very different than the ExternalProject / FindGTest approach?

No. This is one of the approaches recommended by Google:

Add GoogleTest as a git submodule or equivalent. This may not always be possible or appropriate. Git submodules, for example, have their own set of advantages and drawbacks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

4 participants