opentelemetry-cpp icon indicating copy to clipboard operation
opentelemetry-cpp copied to clipboard

Linker error when building otlp_grpc_exporter_test

Open svrnm opened this issue 2 years ago • 11 comments

Describe your environment

  • Alpine Linux v3.16 on Aarch64

Steps to reproduce

  • Commands I am running:
        sudo apk add cmake grpc-dev re2-dev protobuf-dev c-ares-dev curl-dev nlohmann-json thrift-dev boost-dev
        sudo apk add gtest-dev benchmark-dev
        git clone https://github.com/open-telemetry/opentelemetry-cpp.git
        cd opentelemetry-cpp
        cmake -B build \
                -DCMAKE_INSTALL_PREFIX=/usr \
                -DWITH_PROMETHEUS=OFF \
                -DBUILD_SHARED_LIBS=ON \
                -DCMAKE_POSITION_INDEPENDENT_CODE=ON \
                -DCMAKE_BUILD_TYPE=Release \
                -DBUILD_TESTING=ON \
                -DWITH_EXAMPLES=OFF \
                -DWITH_OTLP=ON \
                -DWITH_OTLP_HTTP=ON \
                -DWITH_JAEGER=ON \
                -DWITH_ZIPKIN=ON
        cmake --build build
    

What is the expected behavior? No error

What is the actual behavior? Linker error:

[ 83%] Linking CXX executable otlp_grpc_exporter_test
/usr/lib/gcc/aarch64-alpine-linux-musl/11.2.1/../../../../aarch64-alpine-linux-musl/bin/ld: CMakeFiles/otlp_grpc_exporter_test.dir/test/otlp_grpc_exporter_test.cc.o: undefined reference to symbol '_ZN4grpc6Status2OKE'
/usr/lib/gcc/aarch64-alpine-linux-musl/11.2.1/../../../../aarch64-alpine-linux-musl/bin/ld: /usr/lib/libgrpc++.so.1.46: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
make[2]: *** [exporters/otlp/CMakeFiles/otlp_grpc_exporter_test.dir/build.make:109: exporters/otlp/otlp_grpc_exporter_test] Error 1
make[1]: *** [CMakeFiles/Makefile2:4940: exporters/otlp/CMakeFiles/otlp_grpc_exporter_test.dir/all] Error 2
make: *** [Makefile:146: all] Error 2

Additional context I can work around this issue by setting export LDFLAGS="$LDFLAGS -Wl,--copy-dt-needed-entries" but I don't think that's a permanent solution

svrnm avatar Jan 30 '23 15:01 svrnm

@svrnm could you please try if the link error can be fixed by adding #include <grpcpp/grpcpp.h> to the beginning of below test file and right after the line #ifndef HAVE_CPP_STDLIB?

https://github.com/open-telemetry/opentelemetry-cpp/blob/main/exporters/otlp/test/otlp_grpc_exporter_test.cc

ThomsonTan avatar Feb 01 '23 18:02 ThomsonTan

~~@ThomsonTan it works with WITH_STL=ON set for cmake, I assume that's the effect you're looking for? (I can also try to apply the #ifndef as well)~~

svrnm avatar Feb 03 '23 11:02 svrnm

Ignore my last comment, it was stupid;-)

So, I tried adding grpcpp via the include but still got the error, but the following worked:

diff --git a/exporters/otlp/CMakeLists.txt b/exporters/otlp/CMakeLists.txt
index 49db605..fa15689 100644
--- a/exporters/otlp/CMakeLists.txt
+++ b/exporters/otlp/CMakeLists.txt
@@ -226,7 +226,7 @@ if(BUILD_TESTING)
   if(WITH_OTLP_GRPC)
     add_executable(otlp_grpc_exporter_test test/otlp_grpc_exporter_test.cc)
     target_link_libraries(
-      otlp_grpc_exporter_test ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}
+      otlp_grpc_exporter_test gRPC::grpc++ ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}
       ${GMOCK_LIB} opentelemetry_exporter_otlp_grpc)
     gtest_add_tests(
       TARGET otlp_grpc_exporter_test

I can raise a PR to get this fixed, just wanted to check first if this is the right approach?

svrnm avatar Feb 06 '23 09:02 svrnm

I'm not familar with alpine with musl toolchains, but according to #1891 and #1603 , linking gRPC::grpc++ into more than one executables and dynamic libraries may be dangerous, depends how we build gRPC and otep-cpp. I may be wrong but maybe -Wl,--copy-dt-needed-entries is a better solution here.

owent avatar Feb 07 '23 13:02 owent

@svrnm

The title says otlp_http_exporter_test ... this is about otlp_grpc_exporter_test, right ?

marcalff avatar Feb 07 '23 13:02 marcalff

@svrnm

The title says otlp_http_exporter_test ... this is about otlp_grpc_exporter_test, right ?

fixed

I'm not familar with alpine with musl toolchains, but according to https://github.com/open-telemetry/opentelemetry-cpp/pull/1891 and https://github.com/open-telemetry/opentelemetry-cpp/issues/1603 , linking gRPC::grpc++ into more than one executables and dynamic libraries may be dangerous, depends how we build gRPC and otep-cpp. I may be wrong but maybe -Wl,--copy-dt-needed-entries is a better solution here.

So the fix I applied for the package eventually is using WITH_STL=ON. If this issue is isolated to musl I guess we can close it as wont fix.

svrnm avatar Feb 07 '23 13:02 svrnm

@owent do you think the below dependence on grpc++ can be marked as public? Which may fix the issue as well?

https://github.com/open-telemetry/opentelemetry-cpp/blob/2544803a37d527acdc0f8b7af4b720336e7c14bc/exporters/otlp/CMakeLists.txt#L38-L39

ThomsonTan avatar Feb 09 '23 00:02 ThomsonTan

@owent do you think the below dependence on grpc++ can be marked as public? Which may fix the issue as well?

https://github.com/open-telemetry/opentelemetry-cpp/blob/2544803a37d527acdc0f8b7af4b720336e7c14bc/exporters/otlp/CMakeLists.txt#L38-L39

No, in my understanding, grpc++ can not be marked as public, the reason of #1603 is linking grpc into more than one dynamic libraries and executable.

owent avatar Feb 09 '23 01:02 owent

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

github-actions[bot] avatar Apr 10 '23 01:04 github-actions[bot]

@owent do you think the below dependence on grpc++ can be marked as public? Which may fix the issue as well? https://github.com/open-telemetry/opentelemetry-cpp/blob/2544803a37d527acdc0f8b7af4b720336e7c14bc/exporters/otlp/CMakeLists.txt#L38-L39

No, in my understanding, grpc++ can not be marked as public, the reason of #1603 is linking grpc into more than one dynamic libraries and executable.

@owent - I think the problem here is that the libgrpc++is not statically linked with opentelemetry_exporter_otlp_grpc_client as alpine repository only brings the shared library for gRPC:

# ldd libopentelemetry_exporter_otlp_grpc_client.so | grep ++
        libgrpc++.so.1.42 => /lib/libgrpc++.so.1.42 (0x00007fef8e811000)
        libstdc++.so.6 => /lib/libstdc++.so.6 (0x00007fef8dce3000)
# objdump -t libopentelemetry_exporter_otlp_grpc_client.so|grep OK
#

This makes gRPC the indirect dependency for libraries/executables linking with opentelemetry_exporter_otlp_grpc_client, hence the DSO missing from command line error. Should we modify our otlp test/example code to include libgrpc++ as a link dependency for all executables linking to opentelemetry_exporter_otlp_grpc_client? I haven't tested it, but I think this should work irrespective of whether grpc++ is statically or dynamically linked with opentelemetry_exporter_otlp_grpc_client.

lalitb avatar Jul 25 '23 06:07 lalitb

grpc++

I'm not sure if the problem of #1063 still exists in the latest gRPC(it changes a lot since then), but the problem is gRPC use static global variables in some codes, which will be construct and destruct multiple times when the codes are linked into more than one module on the platforms with ELF ABI. If the gRPC is built as dynamic libraries, it's will be always safe to make the dependency public, but it's unsafe when gRPC is built as static library.

Do you think we can make the dependency public only if we link the dynamic gRPC? It may solve the problem in this issue but may confuse users about how to link dependencies manually.

owent avatar Jul 27 '23 03:07 owent