Skip to content
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

Linker error when building otlp_grpc_exporter_test #1940

Open
svrnm opened this issue Jan 30, 2023 · 11 comments
Open

Linker error when building otlp_grpc_exporter_test #1940

svrnm opened this issue Jan 30, 2023 · 11 comments
Assignees
Labels
bug Something isn't working do-not-stale help wanted Good for taking. Extra help will be provided by maintainers

Comments

@svrnm
Copy link
Member

svrnm commented Jan 30, 2023

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 svrnm added the bug Something isn't working label Jan 30, 2023
@ThomsonTan
Copy link
Contributor

@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

@svrnm
Copy link
Member Author

svrnm commented Feb 3, 2023

@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
Copy link
Member Author

svrnm commented Feb 6, 2023

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?

@owent
Copy link
Member

owent commented Feb 7, 2023

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.

@marcalff
Copy link
Member

marcalff commented Feb 7, 2023

@svrnm

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

@svrnm svrnm changed the title Linker error when building otlp_http_exporter_test Linker error when building otlp_grpc_exporter_test Feb 7, 2023
@svrnm
Copy link
Member Author

svrnm commented Feb 7, 2023

@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 #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.

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.

@ThomsonTan
Copy link
Contributor

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

target_link_libraries(opentelemetry_exporter_otlp_grpc_client
PRIVATE gRPC::grpc++)

@owent
Copy link
Member

owent commented Feb 9, 2023

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

target_link_libraries(opentelemetry_exporter_otlp_grpc_client
PRIVATE gRPC::grpc++)

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.

@github-actions
Copy link

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

@github-actions github-actions bot added the Stale label Apr 10, 2023
@esigo esigo added do-not-stale and removed Stale labels Apr 10, 2023
@lalitb lalitb added the help wanted Good for taking. Extra help will be provided by maintainers label May 17, 2023
@lalitb
Copy link
Member

lalitb commented Jul 25, 2023

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

target_link_libraries(opentelemetry_exporter_otlp_grpc_client
PRIVATE gRPC::grpc++)

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.

@owent
Copy link
Member

owent commented Jul 27, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working do-not-stale help wanted Good for taking. Extra help will be provided by maintainers
Projects
None yet
Development

No branches or pull requests

6 participants