-
Notifications
You must be signed in to change notification settings - Fork 423
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
Clarifications needed around WITH_ABSEIL #3116
Comments
Findings, for discussion. APIAbseil is used for only one thing, which is to implement The class See file It is implemented using, by order of decreasing priority:
It is important to note that when compiling with OPENTELEMETRY_STL_VERSION >= 2017, External AbseilUsing Header files from external abseil uses inline C++ namespaces, so that:
For example:
Internal AbseilSee directory Per the README.md there,
This code is ancient, unmaintained, and was changed only to address build breaks caused by new compilers or new C++ standards. The internal abseil implementation is header only. Internal abseil headers use inline namespaces, so that:
It uses the same namespace This is because a namespace can be inlined to only 1 place, there can not be 2 inline pointing to 2 different implementations: Application that uses AbseilAn application may want to use Abseil for its own reasons, independently of the ABI for The problem is that a flag named It should be possible to:
and support combinations like (a-1, b-2) or (a-2, b-1), which is not possible today. |
Possible changes, for discussion. OPENTELEMETRY_WITH_ABSEILIn opentelemetry-cpp, use This allows to build an application WITH_ABSEIL=ON/OFF, independently of opentelemetry-cpp OPENTELEMETRY_WITH_ABSEIL=ON/OFF. Internal abseilIn the internal abseil implementation, do not use the inline form This is to allow, in the same binary:
so that the internal and external implementation can exist side by side without colliding on symbol names. With this, we should be able to build:
|
@lalitb @ThomsonTan @esigo @owent Please take a look, for discussion. |
There's a conflict between otel_absl and absl due to the inline namespace, which is the last issue in the main branch's code. I eliminated the external Abseil dependency when enabling gRPC or compiling otel-cpp with protobuf 3.22 or higher in #3041. Now, there is no conflict and it won't break the ABI. I think maybe we can replace all options of |
Yes, saw that. I agree with the fix.
Yes, but we need to do that in a separate PR, and make sure we don't break existing users in the transition. |
How to use opentelemetry-cpp with or without ABSEIL is unclear.
This is because:
all collide into the same flag,
WITH_ABSEIL
, while each topic is a separate concern.We need to clarify what is supported, what is not supported, and how to build cleanly a coherent binary while separating each concern.
The text was updated successfully, but these errors were encountered: