opentelemetry-cpp
opentelemetry-cpp copied to clipboard
[POC RFC] header only API for singletons
Fix header only API for singletons (#1520)
Changes
DRAFT -- implement header only API singletons.
- [ ]
CHANGELOG.md
updated for non-trivial changes - [ ] Unit tests have been added
- [ ] Changes in public API reviewed
Codecov Report
Merging #1525 (2eed305) into main (b520480) will decrease coverage by
0.08%
. The diff coverage is100.00%
.
Additional details and impacted files
@@ Coverage Diff @@
## main #1525 +/- ##
==========================================
- Coverage 85.02% 84.94% -0.07%
==========================================
Files 156 156
Lines 4977 4959 -18
==========================================
- Hits 4231 4212 -19
- Misses 746 747 +1
Impacted Files | Coverage Δ | |
---|---|---|
api/include/opentelemetry/baggage/baggage.h | 97.28% <100.00%> (-0.04%) |
:arrow_down: |
...ntelemetry/context/propagation/global_propagator.h | 100.00% <100.00%> (ø) |
|
...pi/include/opentelemetry/context/runtime_context.h | 97.54% <100.00%> (-0.05%) |
:arrow_down: |
api/include/opentelemetry/metrics/provider.h | 100.00% <100.00%> (ø) |
|
api/include/opentelemetry/trace/provider.h | 100.00% <100.00%> (ø) |
|
api/include/opentelemetry/trace/trace_state.h | 97.54% <100.00%> (-0.05%) |
:arrow_down: |
ext/src/http/client/curl/http_client_curl.cc | 80.31% <0.00%> (-1.13%) |
:arrow_down: |
api/include/opentelemetry/trace/noop.h | 93.11% <0.00%> (+6.90%) |
:arrow_up: |
Did a quick test with this PR on Linux, by creating an instrumented library with -fvisible=hidden
flag and the library is able to use the singleton tracer provider from the application. So changes seem to work on Linux.
Need to check on Windows, but based on what @owent has already tested here, it may not work with PE ABI as __declspec(dllexport)
needs to be held inside the compile unit.
I have add more examples here, and __declspec (selectany)
of MSVC works well, but __attribute__ ((selectany))
of GCC on Windows do not work as expected,do I miss anything?
Resources:
Paper from Ulrich Drepper, see section 2.2 export control, page 17:
https://akkadia.org/drepper/dsohowto.pdf
Gnu visibility doc
https://gcc.gnu.org/wiki/Visibility
Resources:
Paper from Ulrich Drepper, see section 2.2 export control, page 17:
https://akkadia.org/drepper/dsohowto.pdf
Gnu visibility doc
https://gcc.gnu.org/wiki/Visibility
I have a quick look at https://akkadia.org/drepper/dsohowto.pdf, I think it's for ELF ABI of Linux, not PE ABI of Windows.
And the visibility
attribute of gcc document(https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes) also says The possible values of visibility_type correspond to the visibility settings in the ELF gABI. .
My question is: is there any way to make the singleton unique with GCC on Windows?
@owent - We agreed this PR to be scoped for Linux and Mac. Let us know if you have any comment on that.
@owent - We agreed this PR to be scoped for Linux and Mac. Let us know if you have any comment on that.
Fine by me. I think we can document this if we do not support build shared library with gcc and clang on Windows, can we also force set BUILD_SHARED_LIB
to OFF
when we use gcc and clang on Windows to reduce misuse?
@lalitb @marcalff
@lalitb , @esigo , @owent
I added a singleton_test unit test, and makefiles for both CMake and Bazel.
This test does not build properly on windows platforms, and I can not figure out why.
"component_c" is compiled as a shared library / DDL (expected), but then when linking the final singleton_test binary, the makefile wants a component_c.lib instead of component_c.ddl, which is incorrect.
On linux, the same makefile with cmake properly builds static libraries (component_a, component_b) and shared libraries (component_c, d, e and f) as expected.
For bazel, I could not find a proper way to express in the BUILD file how to link a test binary singleton_test with a mix of static (component_a and b) and shared (component_c, d, e and f) libraries.
Any help appreciated, the goal is to have first a unit test that builds everywhere.
Regards, -- Marc
@lalitb , @esigo , @owent
I added a singleton_test unit test, and makefiles for both CMake and Bazel.
This test does not build properly on windows platforms, and I can not figure out why.
"component_c" is compiled as a shared library / DDL (expected), but then when linking the final singleton_test binary, the makefile wants a component_c.lib instead of component_c.ddl, which is incorrect.
On linux, the same makefile with cmake properly builds static libraries (component_a, component_b) and shared libraries (component_c, d, e and f) as expected.
For bazel, I could not find a proper way to express in the BUILD file how to link a test binary singleton_test with a mix of static (component_a and b) and shared (component_c, d, e and f) libraries.
Any help appreciated, the goal is to have first a unit test that builds everywhere.
Regards, -- Marc
Do we need declare do_something_in_c
as default visibility or __declspec(dllexport)/__declspec(dllimport)
? I find component_c
is built as a shared library but export nothing.
To my knowledge, MSVC will create both .lib
and .dll
for dynamic libraries. .lib
is for linking and .dll
is for runtime.
Do we need declare
do_something_in_c
as default visibility or__declspec(dllexport)/__declspec(dllimport)
? I findcomponent_c
is built as a shared library but export nothing.
Good point, fixing this.
Thanks
This PR contains proof of concept code, for a general fix.
It is now closed, for the benefit of:
- PR #1604, a fix for gcc and clang, ready for merge
- PR #1595, a tentative fix for windows, still a proof of concept.
Work for windows should continue in PR #1595.