[GitHub] [qpid-proton] jiridanek commented on a change in pull request #319: [WIP] Add the OpenTelemetry cpp library dependency.

[GitBox <gi...@apache.org>]

jiridanek commented on a change in pull request #319:
URL: https://github.com/apache/qpid-proton/pull/319#discussion_r648031232



##########
File path: cpp/CMakeLists.txt
##########
@@ -36,6 +36,14 @@ else()
   set(CONNECT_CONFIG_SRC src/connect_config_dummy.cpp)
 endif()
 
+# Check for OPENTELEMETRY-CPP for distributed tracing
+find_package(opentelemetry-cpp)
+cmake_dependent_option(ENABLE_OPENTELEMETRYCPP "Use opentelemetry for distributed tracing" ON "OPENTELEMETRY-CPP_FOUND" OFF)
+
+if (ENABLE_OPENTELEMETRYCPP)
+  include_directories(${OPENTELEMETRY_CPP_INCLUDE_DIRS})

Review comment:
       This is "old-style" CMake. Nowadays, you should use `target_link_libraries` (https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#transitive-usage-requirements) later when you compile something that needs OpenTelemetry. Adding an OpenTelemetry-exported target this way will also add the include directories to the compilation.
   
   If you need to adjust includes yourself (which should not be necessary here), you should use `target_include_directories` (https://cmake.org/cmake/help/v2.8.12/cmake.html#command:target_include_directories).
   
   These things are available even in CMake 2.8.12 which we need to support in Proton.
   
   # Docs
   
   Here are some documents that introduce "Modern CMake". If these things can be used in CMake 2.8.12, then they should be used.
   
   * https://cliutils.gitlab.io/modern-cmake/
   * https://gist.github.com/mbinna/c61dbb39bca0e4fb7d1f73b0d66a4fd1




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org