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

Issues compiling opentelemetry-cpp with Jaeger on Ubuntu 20.04 and 21.04 finding thrift #889

Closed
ghost opened this issue Jul 2, 2021 · 13 comments
Labels
do-not-stale help wanted Good for taking. Extra help will be provided by maintainers

Comments

@ghost
Copy link

ghost commented Jul 2, 2021

Describe your environment
Ubuntu 20.04 (WSL) and 21.04 on a Virtual Machine

Steps to reproduce
Build opentelemetry-cpp with the following CMAKE flags:
-G "Ninja"
-DCMAKE_TOOLCHAIN_FILE=${HOME}/alphadev/vcpkg/scripts/buildsystems/vcpkg.cmake
-DCMAKE_POSITION_INDEPENDENT_CODE=ON -DWITH_JAEGER=ON -DWITH_PROMETHEUS=OFF
-DBUILD_TESTING=OFF -DJSON_Install=ON
-DCMAKE_C_COMPILER=/usr/bin/gcc -DCMAKE_CXX_COMPILER=/usr/bin/g++

What is the expected behavior?
Clean compile.

What is the actual behavior?
Command error: Using external nlohmann::json
Building with nostd types...
CMake Error at exporters/jaeger/CMakeLists.txt:3 (find_package):
asked CMake to find a package configuration file provided by "Thrift", but
CMake did not find one.
By not providing "FindThrift.cmake" in CMAKE_MODULE_PATH this project has
Could not find a package configuration file provided by "Thrift" with any
of the following names:
ThriftConfig.cmake
thrift-config.cmake
Add the installation prefix of "Thrift" to CMAKE_PREFIX_PATH or set
"Thrift_DIR" to a directory containing one of the above files. If "Thrift"
provides a separate development package or SDK, be sure it has been
installed.

Additional context

thrift is installed using vcpkg.

This is working on Windows, but fails on Linux.

It is probably something I have set wrong, but the upper case T at the beginning of Thrift made me wonder. since the package installs as "thrift".

Output of vcpkg list:
thrift:x64-linux 0.13.0 Apache Thrift is a software project spanning a v...

@ghost ghost added the bug Something isn't working label Jul 2, 2021
@lalitb
Copy link
Member

lalitb commented Jul 4, 2021

vcpkg toolchain configuration using CMAKE_TOOLCHAIN_FILE is only supported for Windows. For Linux platforms, dependency should be already met externally before building opentelemetry-cpp.
Removing bug label as this is expected behavior. Though this can be documented in Jagger Readme.md.

@lalitb lalitb removed the bug Something isn't working label Jul 4, 2021
@ghost
Copy link
Author

ghost commented Jul 6, 2021

By way of feedback, the sheer number of tools and configuration issues we have encountered in trying to build opentelemetry-cpp (and now adding the Jaeger collector), is somewhat overwhelming. Building across platforms (Win32, Win64, Linux-x64) has required that we literally create a meta-build system to get everything in place; and then we can think about building our own code.

We started using a purely CMake build script for our project, but quickly found that we needed Ninja, vcpkg (for some Linux packages too), Git modules and a whole host of nested project dependencies that never seem to work the first time because of some new build or packaging system.

While this is partly an open source ecosystem problem, having "yet another build and/or packaging system" has made a mess of the build process. gRPC has similar issues. They recommend using CMAKE's FetchContent_xxxx macros, but they fail on Abseil.

I would encourage a project-wide review of what it takes to build opentelemetry-cpp on each target from scratch, including dependencies; especially since it is early enough not to break existing installations. The CI/CD process is going to be quite painful for new adopters like us.

$0.02

@ThomsonTan
Copy link
Contributor

@Kurt-Rayner-Alpha thanks for your feedback on the difficulty of managing dependence, this is a good topic for us to discuss.

Meanwhile, could you please try below file to install thrift on your Linux build machine?

https://github.com/open-telemetry/opentelemetry-cpp/blob/main/ci/setup_thrift.sh

@ghost
Copy link
Author

ghost commented Jul 6, 2021 via email

@lalitb
Copy link
Member

lalitb commented Jul 7, 2021

It could still be something in my scripting. I’ll continue to look at it.

Thanks @Kurt-Rayner-Alpha . Seems we would need your help in troubleshooting, as I tested it locally on Linux, and cmake toolchain works fine after installing the thrift package through vcpkg

root@513454c370ae:/opentelemetry-cpp/build# cmake .. -DWITH_JAEGER=ON -DBUILD_TESTING=OFF -DCMAKE_TOOLCHAIN_FILE=/vcpkg/scripts/buildsystems/vcpkg.cmake ..
-- Building for architecture ARCH=x64
Using local nlohmann::json from submodule
-- Using the single-header code from /opentelemetry-cpp/third_party/nlohmann-json/single_include/
Building with nostd types...
/vcpkg/scripts/buildsystems/vcpkg.cmake
-- Found thrift: /vcpkg/installed/x64-linux
-- Configuring done
-- Generating done
-- Build files have been written to: /opentelemetry-cpp/build
root@513454c370ae:/opentelemetry-cpp/build#
root@513454c370ae:/opentelemetry-cpp/build#
root@513454c370ae:/opentelemetry-cpp/build#
root@513454c370ae:/opentelemetry-cpp/build#
root@513454c370ae:/opentelemetry-cpp/build#
root@513454c370ae:/opentelemetry-cpp/build#
root@513454c370ae:/opentelemetry-cpp/build# cmake .. -DWITH_JAEGER=ON -DBUILD_TESTING=OFF ..
-- Building for architecture ARCH=x64
Using local nlohmann::json from submodule
-- Using the single-header code from /opentelemetry-cpp/third_party/nlohmann-json/single_include/
Building with nostd types...
-- Could NOT find CURL (missing: CURL_LIBRARY CURL_INCLUDE_DIR)
CMake Error at exporters/jaeger/CMakeLists.txt:3 (find_package):
  Could not find a package configuration file provided by "Thrift" with any
  of the following names:

    ThriftConfig.cmake
    thrift-config.cmake

  Add the installation prefix of "Thrift" to CMAKE_PREFIX_PATH or set
  "Thrift_DIR" to a directory containing one of the above files.  If "Thrift"
  provides a separate development package or SDK, be sure it has been
  installed.


-- Configuring incomplete, errors occurred!
See also "/opentelemetry-cpp/build/CMakeFiles/CMakeOutput.log".
See also "/opentelemetry-cpp/build/CMakeFiles/CMakeError.log".
root@513454c370ae:/opentelemetry-cpp/build#

@lalitb
Copy link
Member

lalitb commented Jul 20, 2021

@Kurt-Rayner-Alpha - Did you find anything further on this, as this is not reproducible for us locally ?

@ghost
Copy link
Author

ghost commented Jul 20, 2021 via email

@github-actions
Copy link

This issue was marked as stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Nov 11, 2021
@github-actions
Copy link

Closed as inactive. Feel free to reopen if this is still an issue.

@TonyRoussel
Copy link

FYI, had the same issue when trying to use opentelemetry-cpp as a submodule, and manage to circumvent by setting in my CMakeLists set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/opentelemetry-cpp/cmake/modules)

I'm not sure but it's possible that calling mark_as_advanced(CMAKE_TOOLCHAIN_FILE) in the root CMakeLists make impossible to go into the if-branch that is suppose to set cmake module path

@lalitb lalitb reopened this Feb 14, 2022
@github-actions github-actions bot removed the Stale label Feb 15, 2022
@github-actions
Copy link

This issue was marked as stale due to lack of activity. It will be closed in 7 days if no furthur activity occurs.

@github-actions github-actions bot added the Stale label Apr 16, 2022
@lalitb lalitb added help wanted Good for taking. Extra help will be provided by maintainers and removed Stale labels Apr 16, 2022
@github-actions
Copy link

This issue was marked as stale due to lack of activity. It will be closed in 7 days if no furthur activity occurs.

@marcalff
Copy link
Member

marcalff commented Feb 3, 2023

The Jaeger exporter is now deprecated, and is to be removed, see #1938

Closing this issue.

@marcalff marcalff closed this as not planned Won't fix, can't repro, duplicate, stale Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-stale help wanted Good for taking. Extra help will be provided by maintainers
Projects
None yet
Development

No branches or pull requests

4 participants