Merge pull request ceph#44684 from zenomri/wip-omri-tracing-compiled

tracer: set tracing compiled in by default

CMakeLists.txt

@@ -408,7 +408,7 @@ if(WITH_BLKIN)
   include_directories(SYSTEM src/blkin/blkin-lib)
 endif(WITH_BLKIN)
 
-option(WITH_JAEGER "Enable jaegertracing and it's dependent libraries" OFF)
+option(WITH_JAEGER "Enable jaegertracing and it's dependent libraries" ON)
 if(WITH_JAEGER)
   set(HAVE_JAEGER TRUE)
 endif()

ceph.spec.in

@@ -86,7 +86,7 @@
 %endif
 %endif
 %bcond_with seastar
-%bcond_with jaeger
+%bcond_without jaeger
 %if 0%{?fedora} || 0%{?suse_version} >= 1500
 # distros that ship cmd2 and/or colorama
 %bcond_without cephfs_shell

cmake/modules/BuildOpentelemetry.cmake

@@ -6,14 +6,14 @@ function(target_create _target _lib)
 endfunction()
 
 function(build_opentelemetry)
-  set(opentelemetry_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/opentelemetry-cpp")
+  set(opentelemetry_SOURCE_DIR "${PROJECT_SOURCE_DIR}/src/opentelemetry-cpp")
   set(opentelemetry_BINARY_DIR "${CMAKE_CURRENT_BINARY_DIR}/opentelemetry-cpp")
   set(opentelemetry_cpp_targets opentelemetry_trace opentelemetry_exporter_jaeger_trace)
   set(opentelemetry_CMAKE_ARGS -DCMAKE_POSITION_INDEPENDENT_CODE=ON
                                -DWITH_JAEGER=ON
                                -DBUILD_TESTING=OFF
-                               -DWITH_EXAMPLES=OFF
-                               -DBoost_INCLUDE_DIR=${CMAKE_BINARY_DIR}/boost/include)
+                               -DCMAKE_BUILD_TYPE=Release
+                               -DWITH_EXAMPLES=OFF)
 
   set(opentelemetry_libs
       ${opentelemetry_BINARY_DIR}/sdk/src/trace/libopentelemetry_trace.a
@@ -27,7 +27,6 @@ function(build_opentelemetry)
                                 ${opentelemetry_SOURCE_DIR}/exporters/jaeger/include/
                                 ${opentelemetry_SOURCE_DIR}/ext/include/
                                 ${opentelemetry_SOURCE_DIR}/sdk/include/)
-  include_directories(SYSTEM ${opentelemetry_include_dir})
   # TODO: add target based propogation
   set(opentelemetry_deps opentelemetry_trace opentelemetry_resources opentelemetry_common
                          opentelemetry_exporter_jaeger_trace http_client_curl
@@ -41,8 +40,11 @@ function(build_opentelemetry)
                  ${opentelemetry_cpp_targets})
   endif()
 
-  if(NOT WITH_SYSTEM_BOOST)
+  if(WITH_SYSTEM_BOOST)
+    list(APPEND opentelemetry_CMAKE_ARGS -DBOOST_ROOT=${BOOST_ROOT})
+  else()
     list(APPEND dependencies Boost)
+    list(APPEND opentelemetry_CMAKE_ARGS -DBoost_INCLUDE_DIR=${CMAKE_BINARY_DIR}/boost/include)
   endif()
 
   include(ExternalProject)
@@ -76,12 +78,12 @@ function(build_opentelemetry)
 
   # will do all linking and path setting fake include path for
   # interface_include_directories since this happens at build time
-  file(MAKE_DIRECTORY
-       "${opentelemetry_BINARY_DIR}/opentelemetry-cpp/exporters/jaeger/include")
+  file(MAKE_DIRECTORY ${opentelemetry_include_dir})
   add_library(opentelemetry::libopentelemetry INTERFACE IMPORTED)
   add_dependencies(opentelemetry::libopentelemetry opentelemetry-cpp)
   set_target_properties(
     opentelemetry::libopentelemetry
     PROPERTIES
-      INTERFACE_LINK_LIBRARIES "${opentelemetry_deps}")
+      INTERFACE_LINK_LIBRARIES "${opentelemetry_deps}"
+      INTERFACE_INCLUDE_DIRECTORIES "${opentelemetry_include_dir}")
 endfunction()

debian/control

@@ -8,15 +8,15 @@ Maintainer: Ceph Maintainers <[email protected]>
 Uploaders: Ken Dreyer <[email protected]>,
            Alfredo Deza <[email protected]>,
 Build-Depends: automake,
-               bison  <pkg.ceph.jaeger>,
+               bison,
                cmake (>= 3.10.2),
                cpio,
                cython3,
                debhelper (>= 10),
                default-jdk,
                dh-exec,
                dh-python,
-               flex  <pkg.ceph.jaeger>,
+               flex,
                git,
                golang,
                gperf,
@@ -37,7 +37,7 @@ Build-Depends: automake,
                libcap-ng-dev,
                libcunit1-dev,
                libcurl4-openssl-dev,
-               libevent-dev <pkg.ceph.jaeger>,
+               libevent-dev,
                libexpat1-dev,
                libffi-dev [!amd64] <pkg.ceph.check>,
                libfmt-dev (>= 6.1.2),
@@ -72,14 +72,14 @@ Build-Depends: automake,
                librdkafka-dev,
                luarocks,
                libthrift-dev (>= 0.13.0),
-               libyaml-cpp-dev (>= 0.6) <pkg.ceph.crimson> <pkg.ceph.jaeger>,
+               libyaml-cpp-dev (>= 0.6),
                libzstd-dev <pkg.ceph.check>,
                libxmlsec1 <pkg.ceph.check>,
                libxmlsec1-nss <pkg.ceph.check>,
                libxmlsec1-openssl <pkg.ceph.check>,
                libxmlsec1-dev <pkg.ceph.check>,
                ninja-build,
-               nlohmann-json3-dev <pkg.ceph.jaeger>,
+               nlohmann-json3-dev,
                patch,
                pkg-config,
                prometheus <pkg.ceph.check>,

debian/rules

@@ -16,15 +16,11 @@ ifeq (,$(findstring WITH_SEASTAR,$(CEPH_EXTRA_CMAKE_ARGS)))
 else
   export CEPH_OSD_BASENAME = crimson-osd
 endif
-ifeq ($(filter pkg.ceph.jaeger,$(DEB_BUILD_PROFILES)),)
-  extraopts += -DWITH_JAEGER=OFF
-else
-  extraopts += -DWITH_JAEGER=ON
-endif
 ifneq ($(filter pkg.ceph.arrow,$(DEB_BUILD_PROFILES)),)
   extraopts += -DWITH_SYSTEM_ARROW=ON
 endif
 
+extraopts += -DWITH_JAEGER=ON
 extraopts += -DWITH_SYSTEM_UTF8PROC=ON
 extraopts += -DWITH_OCF=ON -DWITH_LTTNG=ON
 extraopts += -DWITH_MGR_DASHBOARD_FRONTEND=OFF

do_freebsd.sh

@@ -60,6 +60,7 @@ mkdir ${BUILD_DIR}
 	-D WITH_MGR=YES \
 	-D WITH_RDMA=OFF \
 	-D WITH_SPDK=OFF \
+	-D WITH_JAEGER=OFF \
 	2>&1 | tee cmake.log
 
 echo -n "start building: "; date

install-deps.sh

@@ -34,17 +34,12 @@ function munge_ceph_spec_in {
     shift
     local for_make_check=$1
     shift
-    local with_jaeger=$1
-    shift
     local OUTFILE=$1
     sed -e 's/@//g' < ceph.spec.in > $OUTFILE
     # http://rpm.org/user_doc/conditional_builds.html
     if $with_seastar; then
         sed -i -e 's/%bcond_with seastar/%bcond_without seastar/g' $OUTFILE
     fi
-    if $with_jaeger; then
-        sed -i -e 's/%bcond_with jaeger/%bcond_without jaeger/g' $OUTFILE
-    fi
     if $with_zbd; then
         sed -i -e 's/%bcond_with zbd/%bcond_without zbd/g' $OUTFILE
     fi
@@ -63,10 +58,6 @@ function munge_debian_control {
 	    grep -v babeltrace debian/control > $control
 	    ;;
     esac
-    if $with_jaeger; then
-	sed -i -e 's/^# Jaeger[[:space:]]//g' $control
-	sed -i -e 's/^# Crimson      libyaml-cpp-dev,/d' $control
-    fi
     echo $control
 }
 
@@ -316,7 +307,6 @@ if [ x$(uname)x = xFreeBSDx ]; then
     exit
 else
     [ $WITH_SEASTAR ] && with_seastar=true || with_seastar=false
-    [ $WITH_JAEGER ] && with_jaeger=true || with_jaeger=false
     [ $WITH_ZBD ] && with_zbd=true || with_zbd=false
     [ $WITH_PMEM ] && with_pmem=true || with_pmem=false
     [ $WITH_RADOSGW_MOTR ] && with_rgw_motr=true || with_rgw_motr=false
@@ -368,9 +358,6 @@ else
 	if $with_seastar; then
 	    build_profiles+=",pkg.ceph.crimson"
 	fi
-	if $with_jaeger; then
-	    build_profiles+=",pkg.ceph.jaeger"
-	fi
 
         in_jenkins && cat <<EOF
 CI_DEBUG: for_make_check=$for_make_check
@@ -437,7 +424,7 @@ EOF
                 fi
                 ;;
         esac
-        munge_ceph_spec_in $with_seastar $with_zbd $for_make_check $with_jaeger $DIR/ceph.spec
+        munge_ceph_spec_in $with_seastar $with_zbd $for_make_check $DIR/ceph.spec
         # for python3_pkgversion macro defined by python-srpm-macros, which is required by python3-devel
         $SUDO dnf install -y python3-devel
         $SUDO $builddepcmd $DIR/ceph.spec 2>&1 | tee $DIR/yum-builddep.out
@@ -457,7 +444,7 @@ EOF
         echo "Using zypper to install dependencies"
         zypp_install="zypper --gpg-auto-import-keys --non-interactive install --no-recommends"
         $SUDO $zypp_install systemd-rpm-macros rpm-build || exit 1
-        munge_ceph_spec_in $with_seastar false $for_make_check $with_jaeger $DIR/ceph.spec
+        munge_ceph_spec_in $with_seastar false $for_make_check $DIR/ceph.spec
         $SUDO $zypp_install $(rpmspec -q --buildrequires $DIR/ceph.spec) || exit 1
         ;;
     *)

src/CMakeLists.txt

@@ -406,9 +406,11 @@ if(WITH_JAEGER)
   find_package(thrift 0.13.0 REQUIRED)
   include(BuildOpentelemetry)
   build_opentelemetry()
-  set(jaeger_base opentelemetry::libopentelemetry thrift::libthrift)
-  add_dependencies(common-objs ${jaeger_base})
-  target_link_libraries(common-objs ${jaeger_base})
+  add_library(jaeger_base INTERFACE)
+  target_link_libraries(jaeger_base INTERFACE opentelemetry::libopentelemetry
+    thrift::libthrift)
+  add_dependencies(common-objs jaeger_base)
+  target_link_libraries(common-objs jaeger_base)
 endif()
 
 CHECK_C_COMPILER_FLAG("-fvar-tracking-assignments" HAS_VTA)
@@ -472,7 +474,7 @@ if(WITH_DPDK)
 endif()
 
 if(WITH_JAEGER)
-  list(APPEND ceph_common_deps ${jaeger_base})
+  list(APPEND ceph_common_deps jaeger_base)
 endif()
 
 if(WIN32)
@@ -498,7 +500,10 @@ endif()
 
 add_library(common STATIC ${ceph_common_objs})
 target_link_libraries(common ${ceph_common_deps})
-add_dependencies(common legacy-option-headers ${jaeger_base})
+add_dependencies(common legacy-option-headers)
+if(WITH_JAEGER)
+add_dependencies(common jaeger_base)
+endif()
 
 if (WIN32)
   # Statically building ceph-common on Windows fails. We're temporarily
@@ -512,7 +517,13 @@ target_link_libraries(ceph-common ${ceph_common_deps})
 if(ENABLE_COVERAGE)
   target_link_libraries(ceph-common gcov)
 endif(ENABLE_COVERAGE)
-add_dependencies(ceph-common legacy-option-headers ${jaeger_base})
+
+add_dependencies(ceph-common legacy-option-headers)
+
+if(WITH_JAEGER)
+add_dependencies(ceph-common jaeger_base)
+endif()
+
 # appease dpkg-shlibdeps
 set_target_properties(ceph-common PROPERTIES
   SOVERSION 2

src/common/tracer.cc

@@ -6,11 +6,14 @@
 #include "tracer.h"
 
 #ifdef HAVE_JAEGER
-
+#include "opentelemetry/sdk/trace/batch_span_processor.h"
+#include "opentelemetry/sdk/trace/tracer_provider.h"
+#include "opentelemetry/exporters/jaeger/jaeger_exporter.h"
 
 namespace tracing {
 
 const opentelemetry::nostd::shared_ptr<opentelemetry::trace::Tracer> Tracer::noop_tracer = opentelemetry::trace::Provider::GetTracerProvider()->GetTracer("no-op", OPENTELEMETRY_SDK_VERSION);
+const jspan Tracer::noop_span = noop_tracer->StartSpan("noop");
 
 using bufferlist = ceph::buffer::list;
 
@@ -20,26 +23,21 @@ Tracer::Tracer(opentelemetry::nostd::string_view service_name) {
 
 void Tracer::init(opentelemetry::nostd::string_view service_name) {
   if (!tracer) {
-    const opentelemetry::exporter::jaeger::JaegerExporterOptions opts;
-    auto jaeger_exporter  = std::unique_ptr<opentelemetry::sdk::trace::SpanExporter>(new opentelemetry::exporter::jaeger::JaegerExporter(opts));
-    auto processor = std::unique_ptr<opentelemetry::sdk::trace::SpanProcessor>(new opentelemetry::sdk::trace::SimpleSpanProcessor(std::move(jaeger_exporter)));
+    const opentelemetry::exporter::jaeger::JaegerExporterOptions exporter_options;
+    const opentelemetry::sdk::trace::BatchSpanProcessorOptions processor_options;
     const auto jaeger_resource = opentelemetry::sdk::resource::Resource::Create(std::move(opentelemetry::sdk::resource::ResourceAttributes{{"service.name", service_name}}));
+    auto jaeger_exporter = std::unique_ptr<opentelemetry::sdk::trace::SpanExporter>(new opentelemetry::exporter::jaeger::JaegerExporter(exporter_options));
+    auto processor = std::unique_ptr<opentelemetry::sdk::trace::SpanProcessor>(new opentelemetry::sdk::trace::BatchSpanProcessor(std::move(jaeger_exporter), processor_options));
     const auto provider = opentelemetry::nostd::shared_ptr<opentelemetry::trace::TracerProvider>(new opentelemetry::sdk::trace::TracerProvider(std::move(processor), jaeger_resource));
     tracer = provider->GetTracer(service_name, OPENTELEMETRY_SDK_VERSION);
   }
 }
 
-void Tracer::shutdown() {
-  if (tracer) {
-    tracer->CloseWithMicroseconds(1);
-  }
-}
-
 jspan Tracer::start_trace(opentelemetry::nostd::string_view trace_name) {
   if (is_enabled()) {
     return tracer->StartSpan(trace_name);
   }
-  return noop_tracer->StartSpan(trace_name);
+  return noop_span;
 }
 
 jspan Tracer::start_trace(opentelemetry::nostd::string_view trace_name, bool trace_is_enabled) {
@@ -51,10 +49,11 @@ jspan Tracer::start_trace(opentelemetry::nostd::string_view trace_name, bool tra
 
 jspan Tracer::add_span(opentelemetry::nostd::string_view span_name, const jspan& parent_span) {
   if (is_enabled() && parent_span->IsRecording()) {
-    const auto parent_ctx = parent_span->GetContext();
-    return add_span(span_name, parent_ctx);
+    opentelemetry::trace::StartSpanOptions span_opts;
+    span_opts.parent = parent_span->GetContext();
+    return tracer->StartSpan(span_name, span_opts);
   }
-  return noop_tracer->StartSpan(span_name);
+  return noop_span;
 }
 
 jspan Tracer::add_span(opentelemetry::nostd::string_view span_name, const jspan_context& parent_ctx) {
@@ -63,7 +62,7 @@ jspan Tracer::add_span(opentelemetry::nostd::string_view span_name, const jspan_
     span_opts.parent = parent_ctx;
     return tracer->StartSpan(span_name, span_opts);
   }
-  return noop_tracer->StartSpan(span_name);
+  return noop_span;
 }
 
 bool Tracer::is_enabled() const {

src/common/tracer.h

@@ -7,11 +7,7 @@
 #include "include/buffer.h"
 
 #ifdef HAVE_JAEGER
-
 #include "opentelemetry/trace/provider.h"
-#include "opentelemetry/exporters/jaeger/jaeger_exporter.h"
-#include "opentelemetry/sdk/trace/simple_processor.h"
-#include "opentelemetry/sdk/trace/tracer_provider.h"
 
 using jspan = opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>;
 using jspan_context = opentelemetry::trace::SpanContext;
@@ -21,14 +17,14 @@ namespace tracing {
 class Tracer {
  private:
   const static opentelemetry::nostd::shared_ptr<opentelemetry::trace::Tracer> noop_tracer;
+  const static jspan noop_span;
   opentelemetry::nostd::shared_ptr<opentelemetry::trace::Tracer> tracer;
 
  public:
   Tracer() = default;
   Tracer(opentelemetry::nostd::string_view service_name);
 
   void init(opentelemetry::nostd::string_view service_name);
-  void shutdown();
 
   bool is_enabled() const;
   // creates and returns a new span with `trace_name`
@@ -59,7 +55,6 @@ void decode(jspan_context& span_ctx, ceph::buffer::list::const_iterator& bl);
 #include <string_view>
 
 
-
 class Value {
  public:
   template <typename T> Value(T val) {}
@@ -99,7 +94,6 @@ struct Tracer {
   jspan add_span(std::string_view, const jspan&) { return {}; }
   jspan add_span(std::string_view span_name, const jspan_context& parent_ctx) { return {}; }
   void init(std::string_view service_name) {}
-  void shutdown() {}
 };
   inline void encode(const jspan_context& span, bufferlist& bl, uint64_t f=0) {}
   inline void decode(jspan_context& span_ctx, ceph::buffer::list::const_iterator& bl) {}

src/crimson/CMakeLists.txt

@@ -134,9 +134,9 @@ set(crimson_common_deps
   Boost::random
   json_spirit)
 
+set(crimson_common_public_deps crimson::cflags)
 if(WITH_JAEGER)
-  include_directories(SYSTEM ${CMAKE_BINARY_DIR}/external/include)
-  list(APPEND crimson_common_deps ${jaeger_base})
+  list(APPEND crimson_common_public_deps jaeger_base)
 endif()
 
 if(NOT WITH_SYSTEM_BOOST)
@@ -145,7 +145,7 @@ endif()
 
 target_link_libraries(crimson-common
   PUBLIC
-    crimson::cflags
+    ${crimson_common_public_deps}
   PRIVATE
     crc32
     ${crimson_common_deps}