open-telemetry / open-telemetry/opentelemetry-cpp
Use standard BUILD_SHARED_LIBS instead of OPENTELEMETRY_BUILD_DLL with MSVC/Windows
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 1.4k
- Forks
- 632
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 75
Description
tl;dr Basically use BUILD_SHARED_LIBS to get the same result as OPENTELEMETRY_BUILD_DLL does today.
The OPENTELEMETRY_BUILD_DLL macro should perhaps still be supported for compability, the idea is to be able to utilize standard cmake functionality to increase discoverability.
- Building otel-cpp with BUILD_SHARED_LIBS=ON today won't work as you all know.
- OPENTELEMETRY_BUILD_DLL which produce a single opentelemetry_cpp.dll does work, and is the only (?) viable option as long as the api remains header-only.
- packages managers like vcpkg would work out of the box if BUILD_SHARED_LIBS could be used (today, a custom "port" is neccessary).
To achieve this:
When BUILD_SHARED_LIBS is defined
- set OPENTELEMETRY_BUILD_DLL=ON
- new internal variable OPENTELEMETRY_LIBRARY_TYPE that
Is set to STATIC when BUILD_SHARED_LIBS=OFF and SHARED when BUILD_SHARED_LIBS=ON, except when OPENTELEMETRY_BUILD_DLL=ON (i.e. msvc) it is set to STATIC. All targets in opentelemetry where linkage varies, must use this macro:
add_library(mytarget ${OPENTELEMETRY_LIBRARY_TYPE} <sources..>). https://discourse.cmake.org/t/using-generator-expression-with-add-library/156/2
I think this technique would also surface more issues with the windows dll build: if e.g. common_*_foo_library did not use OPENTELEMETRY_LIBRARY_TYPE, they will be shared dll libraries in the build, thus build/run would test that using otel-cpp across dll-boundaries (.exe + dll-lib + opentelemetry_cpp.dll) works. We have already found bugs that I suspect would be found in CI with this approach.
That's it, really. What do you think?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by tracing the CMake handling of BUILD_SHARED_LIBS and OPENTELEMETRY_BUILD_DLL, then inspect every OpenTelemetry target whose linkage varies. Done means BUILD_SHARED_LIBS selects the corresponding library behavior, the compatibility macro remains supported as appropriate, and Windows builds exercise the executable, DLL library, and opentelemetry_cpp.dll boundaries.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cmake, cpp
- Domain
- build-system
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100