Provide a way to populate OPENTELEMETRY_STL_VERSION from __cplusplus.

[bcsgh]

Problem

Currently there is no handy way I have found to set OPENTELEMETRY_STL_VERSION correctly from a Bazel build that uses this repo as an external dependency, and that can be annoying.

Solution

In the (presumably) common case, this can easily be solved like this:

cc_library(
   name = "some_base_dep",
   ...
   defines=["OPENTELEMETRY_STL_VERSION=(__cplusplus/100)"]
)

If there are cases where that should be unset (which I assume is the exception rather than the rule), a select({...}) could be used to provide a configuration flag that removes that.

Alternatives

The primary alternative to this is the for every user to need to add something to the projects's .bazelrc file:

build --copt=-DOPENTELEMETRY_STL_VERSION=2017  # Needed by @com_github_aws_sdk//

The big issue with that is if the value is set to something that conflicts with the actual C++/std-library version.

Another alternative would be to add something like

 #if  !defined(OPENTELEMETRY_NO_STL) && !defined(DOPENTELEMETRY_STL_VERSION)
# define DOPENTELEMETRY_STL_VERSION (__cplusplus/100)
#endif

This is basically the same logic as the primary proposal, but imposed on all build systems (possibly including cases where there are better solutions).

[meastp]

@bcsgh I'm not a maintainer, but I think the nostd/ABI-compatible version is the default. I'm also building with WITH_STL=ON / OPENTELEMETRY_STL_VERSION=2023 and would like this mode to be default, and the nostd/ABI-stable mode to be opt-in (if this happens, we should add what you suggest to automatically infer the correct OPENTELEMETRY_STL_VERSION), imho. :)

Also, MSVC requires manually adding /Zc:__cplusplus to the command line for the __cplusplus macro to have a useful value.

[owent]

I think we can not set OPENTELEMETRY_STL_VERSION by __cplusplus. We should keep ABI compatibility when compiler support C++17, but we don't use it when build the prebuilt package.

[bcsgh]

@meastp Better than requiering /Zc:__cplusplus would be to select into deriving from _MSVC_LANG when building on MSVC.

[bcsgh]

While I would like to have OPENTELEMETRY_STL_VERSION default to the current C++ stdlib, what I'm asking for is a way to opt in to that under Bazel (where you have to go well out of your way if you want to not build everything from sources rendering ABI issues basically irrelevant).

Having the ability to opt in on non-Bazel build as well would also be nice and would likely avoid duplication of logic like the MSVC workaround issues mentioned above.

[marcalff]

Currently in CMake, the accepted values for WITH_STL are: "ON, OFF, CXX11, CXX14, CXX17, CXX20 or CXX23"

In the CMake build, OPENTELEMETRY_STL_VERSION is set according to WITH_STL, so that for example:

• WITH_STL=CXX23 sets OPENTELEMETRY_STL_VERSION=2023

If I understand correctly the request, in case of CMake, would be to have something like:

• WITH_STL=BEST, where the best available STL level is used, depending on the C++ level for the compiler.
• This would set:
  • OPENTELEMETRY_STL_VERSION=2014 for __cplusplus 2014xx (C++14 is the minimum required)
  • OPENTELEMETRY_STL_VERSION=2017 for __cplusplus 2017xx
  • OPENTELEMETRY_STL_VERSION=2020 for __cplusplus 2020xx
  • OPENTELEMETRY_STL_VERSION=2023 for __cplusplus 2023xx

Technically, this sounds feasible, it can be done.

The problem with this is that the notion of ABI is no longer properly defined, as it changes all the time, very likely causing even more chaos and confusion. The question to resolve is whether this should be done.

I am looking for arguments in support of WITH_STL=BESTas I am not convinced myself, please comment with a use case when it helps.

Note that changing the default value, currently WITH_STL=OFF, to something else is a separate discussion.

Now, about Bazel, there are other additional concerns:

• We (the opentelemetry-cpp maintainers) do not have enough expertise with bazel, and maintaining BUILD files today at the same level of CMakeList.txt is already a challenge.
• Speaking personally, my experience with bazel so far has been very frustrating, coming from CMake and autotools.

If bazel is used to build all dependencies at once, so that the mere concept of ABI binary compatibility is no longer an issue, and that an option similar to WITH_STL=BEST helps, we can consider having one.

How best to implement this in bazel is yet to be determined, and given (our) lack of expertise in bazel in general, the best way to make anything happen is, realistically, to propose a patch.

[bcsgh]

First off. The request is actually limited to only Bazel builds. Having similar functionality under CMake would be nice, but doesn't provide value to me. that said, the points raised WRT CMake should be addressed even under Bazel even if they end up with different answers.

Regarding ABI issues. Under Bazel that basically won't be a problem as getting Bazel to even try to link code that was built with different flags is, by design, hard to do. Also, as far as I know, every C++ stdlib post C++11 uses inline namespaces so that an attempt to link a library built with one version to a client built with another will fail at link time.

The primary uses case I'm considering is where this library is a transitive dependency that the end user would rather not care about. Just adding the thing they care about (which it self might not care that it only indirectly cares about this library) to their build should do the right thing: build this lib with the same C++ stdlib as everything else.

Ironically, my experience with CMake and autotools sounds much like yours with Bazel: "very frustrating".

As for a patch: https://github.com/bcsgh/opentelemetry-cpp/tree/bcsgh/with_cxx_stdlib (Not even tested yet, but I'll try to get to that later today.)

---

Some of that might change if you drag dynamic linking into the mix, but outside of a minuscule number of cases like sys-calls and plugins that can't be available at build time, why? <soapbox> I suspect I've literally spent more time and compute trying to debug issues resulting from dynamic linking of things that could have be static than have or ever could possibly be saved by using it. </soapbox>

[bcsgh]

Taking a pass at testing that patch I'm running into issues at HEAD (mostly because it seems to want to be built by a version of Bazel 2 major version behind) but everything that builds for me at head also builds with my patch under each option until it starts assuming a C++ version I don't have support for (i.e. up thought C++2017). I've added another patch that dumps an error if you ask for a version of C++ newer than what __cplusplus says is used in the current build environment. (I don't check for older because it technically works, but see below.)

---

All that said, I'm beginning to question the premise of OPENTELEMETRY_STL_VERSION even existing.

Under what conditions would setting OPENTELEMETRY_STL_VERSION to anything other than the current environments version of C++ solve a problem?

I can see leaving it un-set (i.e. don't use the stdlib) as a valid option, but telling it to assume a different version? Why?

If you tell it to use an older version, it won't actually do that; it will still use the the current version, but just a sub set of it that was available in the old version. That will compile just fine, but if you try to link that (static or dynamic) with something compiled with a different c++ version it should still fail. At best inline namespaces will result in link failures when manged names don't match. At worst (if inline namespaces aren't used) it will link a mishmash of versions and turn into a hash of UB.

If you tell it to use a newer version it will just fail to even compile.

The only situation I'm seeing that doesn't lead to problems is if the CBuild setup uses WITH_STL to both set the command line flags for the C++ version and also (redundantly) sets OPENTELEMETRY_STL_VERSION to match. If that's the case, then replacing OPENTELEMETRY_STL_VERSION with OPENTELEMETRY_USE_STL and re-working all the "which version?" logic to use __cplusplus directly would be a no-op under CBuild and a distinct improvement under Bazel.