Use standard BUILD_SHARED_LIBS instead of OPENTELEMETRY_BUILD_DLL with MSVC/Windows

[meastp]

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.

1. Building otel-cpp with BUILD_SHARED_LIBS=ON today won't work as you all know.
2. 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.
3. 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

1. set OPENTELEMETRY_BUILD_DLL=ON
2. 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?

[owent]

Good point. I think we can set the default value of OPENTELEMETRY_BUILD_DLL to ON when BUILD_SHARED_LIBS is ON. But just wondering do we need change add_library calls to add_library(mytarget ${OPENTELEMETRY_LIBRARY_TYPE} <sources..>)? According to https://cmake.org/cmake/help/latest/variable/BUILD_SHARED_LIBS.html this variable will cause add_library() to create shared libraries if on and we need do nothing. OPENTELEMETRY_BUILD_DLL is only used to make otel-cpp be built into single DLL.

[meastp]

@owent Yes, I think the default needs to be modified (when building on windows + BUILD_SHARED_LIBS) because the libraries that are merged* into opentelemetry_cpp(.dll) need to be built as static libraries.

If they (api, sdk (trace, metrics, common etc.), exporters ...) are built as dlls themselves, they can not be merged* into a common dll later (opentelemetry_cpp.dll).

Please correct me if this is wrong (if I am, the modifications to make this work are even smaller).

*not sure if this is the right term

[meastp]

@marcalff @owent Was the reason(s) for why ${OPENTELEMETRY_LIBRARY_TYPE} is needed clearly explained?

If you agree, I'll sketch it up in a PR.

[owent]

@marcalff @owent Was the reason(s) for why ${OPENTELEMETRY_LIBRARY_TYPE} is needed clearly explained?

If you agree, I'll sketch it up in a PR.

Good point. I think it's better to add new function (such as otel_add_library) to replace add_library, and then we can do more common jobs later, not just for OPENTELEMETRY_LIBRARY_TYPE.

[meastp]

@owent Sure, that's very reasonable - I'll make it a function.

@marcalff Any opinion?

[meastp]

I'll start looking at drafting a PR when #2476 is completed :)

[github-actions]

This issue was marked as stale due to lack of activity.