GlobalLogHandler::GetLogHandler() singleton is not unique. #1409
Comments
|
To my understanding, ELF ABI will let all This problem is simular to #1105, we can not do it now because we need |
|
A possibility is that I am not using the proper build / link flags when building, will investigate more locally. |
|
@marcalff - Did you try this further. There should be ideally single global singleton used in the case of more than one library. |
|
Hi @lalitb, welcome back. I have spent a lot of time trying to get logs from the opentelemetry-cpp libraries, but never could make it work. I am aware that some logs are protected by additional flags, as in: and even with Then, out of desperation, I moved (in a local branch) the This allowed me to make progress investigating my own application. From the technical discussion, I agree that there should be only one single global singleton used, even when multiple opentelemetry-*.so shared libraries are involved, but this does not match my observation. If it helps, the structure of my application is as follows:
Build environment used: opentelemetry-cpp built with Local patch applied to make things work: |
|
Sorry for my mistake, GlobalLogHandler ia part of |
Greetings.
The title is correct.
In my application, the code is calling:
(1)
GlobalLogHandler::SetLogHandler(), to set a custom handler(2)
OTEL_INTERNAL_LOG_XXX(), to print internal logs.These calls produce logs in the custom handler, as expected so far.
But then, later:
(3)
OTEL_INTERNAL_LOG_XXX()calls from opentelemetry librariesend up calling the default handler
DefaultLogHandlerinstead of the customone, which defeats the purpose of a singleton
SetLogHandler().Analysis:
GlobalLogHandler::GetHandlerAndLevel()is implemented inline in a headerfile, not a .cc file.
When building static libraries, this "singleton" ends up in multiple
libraries:
Note, interestingly, how the
GlobalLogHandler::GetHandlerAndLevel()code isnot part of
libopentelemetry_common.a, but defined (not referenced) in every use instead.(4)
man nmgives, for my platform and for"u":I am willing to accept that the linker, when linking several
libopentelemetry_*.astatic libraries into a final binary, will resolve duplicates, making the singleton unique.However, there is more to it.
Now with dynamic libraries:
My reading of (4) is that the linker, when producing a -- single --
opentelemetry_xyz.solibrary, will ensure the symbol there is unique ... for this .so alone.The linker is never given all the code at once, and can not enforce uniqueness for all the *.so libraries combined.
Looking further, at runtime: how is
dlopen()supposed to reconcile uniqueness of a symbol, when defined in multiple *.so shared libraries ?Note how the symbol is "W" (weak)
This possibly explains why multiple instances of the same singleton are not reported as duplicate symbols.
I was not able to investigate if/how
dlopen()resolves this, and whether there are indeed multiple instances of the singleton in the final loaded binary or not, but this is highly suspicious, and the behavior seen is consistent with duplicates.Suggestions for a fix:
Abandon the idea of defining singleton in header files, use a *.cc file instead.
For
GlobalLogHandler::GetHandlerAndLevel(),implement this in
global_log_handler.cc, and adjust makefiles accordingly.Now, this raises the interesting questions of every other singleton defined in
header files, in particular:
For example:
trace::Provider::GetTracerProvider()trace::Provider::GetLock()are at risk of not being unique.
Please consider moving every singleton implementation in a *.cc file,
so it gets defined in only one library (.a or .so), not many.
A good home for these singletons can be
libopentelemetry_common.so, possiblylibopentelemetry_version.soRegards.
The text was updated successfully, but these errors were encountered: