From 644c99826d73174e6609aa24b7297443358488b1 Mon Sep 17 00:00:00 2001 From: Omri Zeneva Date: Thu, 20 Jan 2022 03:58:29 -0500 Subject: [PATCH 01/14] cmake: set WITH_JAEGER=ON by default Signed-off-by: Omri Zeneva --- CMakeLists.txt | 2 +- src/rgw/rgw_process.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f4d6e14fbbcf4..629f0286b1a83 100644 --- a/CMakeLists.txt +++ b/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() diff --git a/src/rgw/rgw_process.cc b/src/rgw/rgw_process.cc index a4d44267bfb4e..5f54fa3544538 100644 --- a/src/rgw/rgw_process.cc +++ b/src/rgw/rgw_process.cc @@ -401,7 +401,7 @@ int process_request(rgw::sal::Store* const store, } done: - if (op) { + if (op && s->trace) { s->trace->SetAttribute(tracing::rgw::RETURN, op->get_ret()); if (s->user) { s->trace->SetAttribute(tracing::rgw::USER_ID, s->user->get_id().id); From 2fecc2c132f4025f589c8851887130336b9f4e2b Mon Sep 17 00:00:00 2001 From: Omri Zeneva Date: Thu, 20 Jan 2022 04:01:11 -0500 Subject: [PATCH 02/14] common/tracer: return static noop span, instead of calling StartSpan(..) Signed-off-by: Omri Zeneva --- src/common/tracer.cc | 7 ++++--- src/common/tracer.h | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/common/tracer.cc b/src/common/tracer.cc index c9891726044c8..4d183eb72e793 100644 --- a/src/common/tracer.cc +++ b/src/common/tracer.cc @@ -11,6 +11,7 @@ namespace tracing { const opentelemetry::nostd::shared_ptr 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; @@ -39,7 +40,7 @@ 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) { @@ -54,7 +55,7 @@ jspan Tracer::add_span(opentelemetry::nostd::string_view span_name, const jspan& const auto parent_ctx = parent_span->GetContext(); return add_span(span_name, parent_ctx); } - 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 +64,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 { diff --git a/src/common/tracer.h b/src/common/tracer.h index bd095f310f618..242ae00e42d11 100644 --- a/src/common/tracer.h +++ b/src/common/tracer.h @@ -21,6 +21,7 @@ namespace tracing { class Tracer { private: const static opentelemetry::nostd::shared_ptr noop_tracer; + const static jspan noop_span; opentelemetry::nostd::shared_ptr tracer; public: @@ -59,7 +60,6 @@ void decode(jspan_context& span_ctx, ceph::buffer::list::const_iterator& bl); #include - class Value { public: template Value(T val) {} From 24ba2e18a75c2d9ea42c8faf18d3531597b46fc4 Mon Sep 17 00:00:00 2001 From: Omri Zeneva Date: Thu, 20 Jan 2022 04:03:53 -0500 Subject: [PATCH 03/14] osd: remove unused tracing spans Signed-off-by: Omri Zeneva --- src/osd/ECBackend.cc | 6 ------ src/osd/OSD.cc | 3 +-- src/osd/PrimaryLogPG.cc | 19 ------------------- src/osd/ReplicatedBackend.cc | 8 -------- src/osd/scheduler/OpSchedulerItem.cc | 1 - 5 files changed, 1 insertion(+), 36 deletions(-) diff --git a/src/osd/ECBackend.cc b/src/osd/ECBackend.cc index 06c75049f226a..2b350858f230a 100644 --- a/src/osd/ECBackend.cc +++ b/src/osd/ECBackend.cc @@ -951,7 +951,6 @@ void ECBackend::handle_sub_write( jspan span; if (msg) { msg->mark_event("sub_op_started"); - span = tracing::osd::tracer.add_span(__func__, msg->osd_parent_span); } trace.event("handle_sub_write"); @@ -1553,7 +1552,6 @@ void ECBackend::submit_transaction( jspan span; if (client_op) { op->trace = client_op->pg_trace; - span = tracing::osd::tracer.add_span("ECBackend::submit_transaction", client_op->osd_parent_span); } dout(10) << __func__ << ": op " << *op << " starting" << dendl; start_rmw(op, std::move(t)); @@ -2126,10 +2124,6 @@ bool ECBackend::try_reads_to_commit() messages.push_back(std::make_pair(i->osd, r)); } } - jspan span; - if (op->client_op) { - span = tracing::osd::tracer.add_span("EC sub write", op->client_op->osd_parent_span); - } if (!messages.empty()) { get_parent()->send_message_osd_cluster(messages, get_osdmap_epoch()); diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc index ef55e440b06c1..51c07df251160 100644 --- a/src/osd/OSD.cc +++ b/src/osd/OSD.cc @@ -7276,7 +7276,6 @@ void OSD::dispatch_session_waiting(const ceph::ref_t& session, OSDMapRe void OSD::ms_fast_dispatch(Message *m) { - auto dispatch_span = tracing::osd::tracer.start_trace(__func__); FUNCTRACE(cct); if (service.is_stopping()) { m->put(); @@ -7332,7 +7331,7 @@ void OSD::ms_fast_dispatch(Message *m) tracepoint(osd, ms_fast_dispatch, reqid.name._type, reqid.name._num, reqid.tid, reqid.inc); } - op->osd_parent_span = tracing::osd::tracer.add_span("op-request-created", dispatch_span); + op->osd_parent_span = tracing::osd::tracer.start_trace("op-request-created"); if (m->trace) op->osd_trace.init("osd op", &trace_endpoint, &m->trace); diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc index 1b60dfc0793aa..58a065f96dc79 100644 --- a/src/osd/PrimaryLogPG.cc +++ b/src/osd/PrimaryLogPG.cc @@ -1796,7 +1796,6 @@ void PrimaryLogPG::do_request( op->pg_trace.event("do request"); } - [[maybe_unused]] auto span = tracing::osd::tracer.add_span(__func__, op->osd_parent_span); // make sure we have a new enough map auto p = waiting_for_map.find(op->get_source()); @@ -2169,7 +2168,6 @@ void PrimaryLogPG::do_op(OpRequestRef& op) << " flags " << ceph_osd_flag_string(m->get_flags()) << dendl; - [[maybe_unused]] auto span = tracing::osd::tracer.add_span(__func__, op->osd_parent_span); // missing object? if (is_unreadable_object(head)) { @@ -4188,7 +4186,6 @@ void PrimaryLogPG::execute_ctx(OpContext *ctx) reqid.name._num, reqid.tid, reqid.inc); } - [[maybe_unused]] auto span = tracing::osd::tracer.add_span(__func__, ctx->op->osd_parent_span); int result = prepare_transaction(ctx); @@ -5966,10 +5963,6 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector& ops) dout(10) << "do_osd_op " << soid << " " << ops << dendl; - jspan span; - if (ctx->op) { - span = tracing::osd::tracer.add_span(__func__, ctx->op->osd_parent_span); - } ctx->current_osd_subop_num = 0; for (auto p = ops.begin(); p != ops.end(); ++p, ctx->current_osd_subop_num++, ctx->processed_subop_count++) { OSDOp& osd_op = *p; @@ -8924,10 +8917,6 @@ void PrimaryLogPG::finish_ctx(OpContext *ctx, int log_op_type, int result) << dendl; utime_t now = ceph_clock_now(); - jspan span; - if (ctx->op) { - span = tracing::osd::tracer.add_span(__func__, ctx->op->osd_parent_span); - } // Drop the reference if deduped chunk is modified if (ctx->new_obs.oi.is_dirty() && @@ -11314,10 +11303,6 @@ void PrimaryLogPG::op_applied(const eversion_t &applied_version) void PrimaryLogPG::eval_repop(RepGather *repop) { - jspan span; - if (repop->op) { - span = tracing::osd::tracer.add_span(__func__, repop->op->osd_parent_span); - } dout(10) << "eval_repop " << *repop << (repop->op && repop->op->get_req() ? "" : " (no op)") << dendl; @@ -11373,10 +11358,6 @@ void PrimaryLogPG::issue_repop(RepGather *repop, OpContext *ctx) << " o " << soid << dendl; - jspan span; - if (ctx->op) { - span = tracing::osd::tracer.add_span(__func__, ctx->op->osd_parent_span); - } repop->v = ctx->at_version; diff --git a/src/osd/ReplicatedBackend.cc b/src/osd/ReplicatedBackend.cc index 50a6c60d7f0f5..22bf4aea0f5f7 100644 --- a/src/osd/ReplicatedBackend.cc +++ b/src/osd/ReplicatedBackend.cc @@ -504,10 +504,6 @@ void ReplicatedBackend::submit_transaction( ceph_assert(insert_res.second); InProgressOp &op = *insert_res.first->second; - jspan span; - if (orig_op) { - span = tracing::osd::tracer.add_span("ReplicatedBackend::submit_transaction", orig_op->osd_parent_span); - } op.waiting_for_commit.insert( parent->get_acting_recovery_backfill_shards().begin(), @@ -1062,10 +1058,6 @@ void ReplicatedBackend::do_repop(OpRequestRef op) << " " << m->logbl.length() << dendl; - jspan span; - if (op) { - span = tracing::osd::tracer.add_span(__func__, op->osd_parent_span); - } // sanity checks ceph_assert(m->map_epoch >= get_info().history.same_interval_since); diff --git a/src/osd/scheduler/OpSchedulerItem.cc b/src/osd/scheduler/OpSchedulerItem.cc index 9f834e9077812..194e17f301c8e 100644 --- a/src/osd/scheduler/OpSchedulerItem.cc +++ b/src/osd/scheduler/OpSchedulerItem.cc @@ -30,7 +30,6 @@ void PGOpItem::run( PGRef& pg, ThreadPool::TPHandle &handle) { - [[maybe_unused]] auto span = tracing::osd::tracer.add_span("PGOpItem::run", op->osd_parent_span); osd->dequeue_op(pg, op, handle); pg->unlock(); } From 7be8be63501ba03da9b705238a9d3d3a518969ab Mon Sep 17 00:00:00 2001 From: Deepika Upadhyay Date: Tue, 1 Feb 2022 16:37:56 +0530 Subject: [PATCH 04/14] ceph.spec.in: make jaeger packages installation default true. specify --without-jaeger during rpm build process if jaeger package install is not desired. Signed-off-by: Deepika Upadhyay --- ceph.spec.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ceph.spec.in b/ceph.spec.in index 3042d6e0dc975..e25d4062ad32e 100644 --- a/ceph.spec.in +++ b/ceph.spec.in @@ -90,7 +90,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 From bee4d993fb67a8240a6ccdc8f6f7872b0e39236a Mon Sep 17 00:00:00 2001 From: Deepika Upadhyay Date: Tue, 1 Feb 2022 16:39:44 +0530 Subject: [PATCH 05/14] debian: make jaeger package installation as default jaeger opentelemetry deps that will be installed by default now: libyaml-dev > 0.6 libthrift-dev (thift deps: libevent-dev, bison, flex, boost(we use ceph compiled boost)) nlohmann-json3-dev removes: pkg.ceph.jaeger build package optional option Signed-off-by: Deepika Upadhyay --- debian/control | 10 +++++----- debian/rules | 6 +----- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/debian/control b/debian/control index f64664e4319ae..18986abacaa2c 100644 --- a/debian/control +++ b/debian/control @@ -8,7 +8,7 @@ Maintainer: Ceph Maintainers Uploaders: Ken Dreyer , Alfredo Deza , Build-Depends: automake, - bison , + bison, cmake (>= 3.10.2), cpio, cython3, @@ -16,7 +16,7 @@ Build-Depends: automake, default-jdk, dh-exec, dh-python, - flex , + flex, git, golang, gperf, @@ -37,7 +37,7 @@ Build-Depends: automake, libcap-ng-dev, libcunit1-dev, libcurl4-openssl-dev, - libevent-dev , + libevent-dev, libexpat1-dev, libffi-dev [!amd64] , 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) , + libyaml-cpp-dev (>= 0.6), libzstd-dev , libxmlsec1 , libxmlsec1-nss , libxmlsec1-openssl , libxmlsec1-dev , ninja-build, - nlohmann-json3-dev , + nlohmann-json3-dev, patch, pkg-config, prometheus , diff --git a/debian/rules b/debian/rules index 9f49deabb0e01..96245ae39c810 100755 --- a/debian/rules +++ b/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 From 028751bce606e337793e5ac12af7d0ebf77da657 Mon Sep 17 00:00:00 2001 From: Deepika Upadhyay Date: Tue, 1 Feb 2022 16:48:18 +0530 Subject: [PATCH 06/14] install-deps: make jaeger package install default, removes with_jaeger flag Signed-off-by: Deepika Upadhyay --- install-deps.sh | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/install-deps.sh b/install-deps.sh index dcf988696352c..5373a9260bfac 100755 --- a/install-deps.sh +++ b/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 <&1 | tee $DIR/yum-builddep.out @@ -455,7 +442,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 ;; *) From e63f1ba027aaf0bb168d22b5a04403353f102f44 Mon Sep 17 00:00:00 2001 From: Deepika Upadhyay Date: Thu, 3 Feb 2022 06:52:33 +0000 Subject: [PATCH 07/14] cmake: include opentelemetry include directories Signed-off-by: Deepika Upadhyay --- cmake/modules/BuildOpentelemetry.cmake | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cmake/modules/BuildOpentelemetry.cmake b/cmake/modules/BuildOpentelemetry.cmake index 6bd2380627192..68987e222e627 100644 --- a/cmake/modules/BuildOpentelemetry.cmake +++ b/cmake/modules/BuildOpentelemetry.cmake @@ -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 @@ -76,12 +75,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() From bbaafcd32c8c35143f397b459b76cb380cb5f036 Mon Sep 17 00:00:00 2001 From: Deepika Upadhyay Date: Fri, 4 Feb 2022 15:53:48 -0500 Subject: [PATCH 08/14] cmake: create jaeger_base target for consuming tracing libraries Signed-off-by: Deepika Upadhyay --- cmake/modules/BuildOpentelemetry.cmake | 4 ++-- src/CMakeLists.txt | 23 +++++++++++++++++------ src/crimson/CMakeLists.txt | 6 +++--- src/mon/CMakeLists.txt | 2 +- src/os/CMakeLists.txt | 4 ++-- src/rgw/CMakeLists.txt | 4 ++-- src/rgw/store/dbstore/CMakeLists.txt | 2 +- 7 files changed, 28 insertions(+), 17 deletions(-) diff --git a/cmake/modules/BuildOpentelemetry.cmake b/cmake/modules/BuildOpentelemetry.cmake index 68987e222e627..c528273a098e1 100644 --- a/cmake/modules/BuildOpentelemetry.cmake +++ b/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) + -DBoost_INCLUDE_DIR=${CMAKE_BINARY_DIR}/boost/include) set(opentelemetry_libs ${opentelemetry_BINARY_DIR}/sdk/src/trace/libopentelemetry_trace.a diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 61a8616d5a773..89c53ea46eb93 100644 --- a/src/CMakeLists.txt +++ b/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 diff --git a/src/crimson/CMakeLists.txt b/src/crimson/CMakeLists.txt index 4508491407d91..e17251564964e 100644 --- a/src/crimson/CMakeLists.txt +++ b/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} diff --git a/src/mon/CMakeLists.txt b/src/mon/CMakeLists.txt index b7232551ccb03..784b4c3ee0b34 100644 --- a/src/mon/CMakeLists.txt +++ b/src/mon/CMakeLists.txt @@ -42,5 +42,5 @@ target_link_libraries(mon heap_profiler fmt::fmt) if(WITH_JAEGER) - target_link_libraries(mon ${jaeger_base}) + target_link_libraries(mon jaeger_base) endif() diff --git a/src/os/CMakeLists.txt b/src/os/CMakeLists.txt index 204a29fea8ccd..9a6de9301054b 100644 --- a/src/os/CMakeLists.txt +++ b/src/os/CMakeLists.txt @@ -93,8 +93,8 @@ if(WITH_LTTNG) endif() if(WITH_JAEGER) - add_dependencies(os ${jaeger_base}) - target_link_libraries(os ${jaeger_base}) + add_dependencies(os jaeger_base) + target_link_libraries(os jaeger_base) endif() target_link_libraries(os kv) diff --git a/src/rgw/CMakeLists.txt b/src/rgw/CMakeLists.txt index 3f569deb20bbe..8a6cb7d258c21 100644 --- a/src/rgw/CMakeLists.txt +++ b/src/rgw/CMakeLists.txt @@ -251,8 +251,8 @@ if(WITH_LTTNG) endif() if(WITH_JAEGER) - add_dependencies(rgw_common ${jaeger_base}) - target_link_libraries(rgw_common PUBLIC ${jaeger_base}) + add_dependencies(rgw_common jaeger_base) + target_link_libraries(rgw_common PUBLIC jaeger_base) endif() if(WITH_RADOSGW_DBSTORE) diff --git a/src/rgw/store/dbstore/CMakeLists.txt b/src/rgw/store/dbstore/CMakeLists.txt index 18e032b73f54f..08dbe5f6fe675 100644 --- a/src/rgw/store/dbstore/CMakeLists.txt +++ b/src/rgw/store/dbstore/CMakeLists.txt @@ -21,7 +21,7 @@ target_include_directories(dbstore_lib PUBLIC "${CMAKE_SOURCE_DIR}/src/fmt/inclu target_include_directories(dbstore_lib PUBLIC "${CMAKE_SOURCE_DIR}/src/rgw") set(link_targets spawn) if(WITH_JAEGER) - list(APPEND link_targets ${jaeger_base}) + list(APPEND link_targets jaeger_base) endif() target_link_libraries(dbstore_lib PUBLIC ${link_targets}) From 19eff9a014f4224c52144c9c7de4b73a72ca32ea Mon Sep 17 00:00:00 2001 From: Deepika Upadhyay Date: Mon, 7 Feb 2022 08:12:12 -0500 Subject: [PATCH 09/14] cmake: use BOOST_ROOT if building with system boost Signed-off-by: Deepika Upadhyay --- cmake/modules/BuildOpentelemetry.cmake | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cmake/modules/BuildOpentelemetry.cmake b/cmake/modules/BuildOpentelemetry.cmake index c528273a098e1..ab28fa622e48c 100644 --- a/cmake/modules/BuildOpentelemetry.cmake +++ b/cmake/modules/BuildOpentelemetry.cmake @@ -12,8 +12,7 @@ function(build_opentelemetry) 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) + -DWITH_EXAMPLES=OFF) set(opentelemetry_libs ${opentelemetry_BINARY_DIR}/sdk/src/trace/libopentelemetry_trace.a @@ -40,8 +39,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) From 29ddad923f1b8ab2c569b9d3e82d666bc0a2dac1 Mon Sep 17 00:00:00 2001 From: Deepika Upadhyay Date: Mon, 7 Feb 2022 08:15:12 -0500 Subject: [PATCH 10/14] win32_build: do not build jaeger for windows, disable cmake option Signed-off-by: Deepika Upadhyay --- win32_build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/win32_build.sh b/win32_build.sh index d8b5f76f9818d..cace5fda9e80b 100755 --- a/win32_build.sh +++ b/win32_build.sh @@ -162,7 +162,7 @@ cmake -D CMAKE_PREFIX_PATH=$depsDirs \ -D WITH_GSSAPI=OFF -D WITH_XFS=OFF \ -D WITH_FUSE=OFF -D WITH_DOKAN=ON \ -D WITH_BLUESTORE=OFF -D WITH_LEVELDB=OFF \ - -D WITH_LTTNG=OFF -D WITH_BABELTRACE=OFF \ + -D WITH_LTTNG=OFF -D WITH_BABELTRACE=OFF -D WITH_JAEGER=OFF \ -D WITH_SYSTEM_BOOST=ON -D WITH_MGR=OFF -D WITH_KVS=OFF \ -D WITH_LIBCEPHFS=ON -D WITH_KRBD=OFF -D WITH_RADOSGW=OFF \ -D ENABLE_SHARED=$ENABLE_SHARED -D WITH_RBD=ON -D BUILD_GMOCK=ON \ From 41606cd05030ac95854b2b3515f56c28b328f478 Mon Sep 17 00:00:00 2001 From: Omri Zeneva Date: Tue, 15 Feb 2022 08:38:44 -0500 Subject: [PATCH 11/14] common/tracer: use batch_span_processor over simple processor the batch span processor holds batch of spans until timeout reached or it fills up, instead of sending each span alone which is a huge overhead Signed-off-by: Omri Zeneva --- src/common/tracer.cc | 16 ++++++++++------ src/common/tracer.h | 4 ---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/common/tracer.cc b/src/common/tracer.cc index 4d183eb72e793..f908ffe318526 100644 --- a/src/common/tracer.cc +++ b/src/common/tracer.cc @@ -6,7 +6,9 @@ #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 { @@ -21,10 +23,11 @@ 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(new opentelemetry::exporter::jaeger::JaegerExporter(opts)); - auto processor = std::unique_ptr(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(new opentelemetry::exporter::jaeger::JaegerExporter(exporter_options)); + auto processor = std::unique_ptr(new opentelemetry::sdk::trace::BatchSpanProcessor(std::move(jaeger_exporter), processor_options)); const auto provider = opentelemetry::nostd::shared_ptr(new opentelemetry::sdk::trace::TracerProvider(std::move(processor), jaeger_resource)); tracer = provider->GetTracer(service_name, OPENTELEMETRY_SDK_VERSION); } @@ -52,8 +55,9 @@ 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_span; } diff --git a/src/common/tracer.h b/src/common/tracer.h index 242ae00e42d11..1ad57e91f56a1 100644 --- a/src/common/tracer.h +++ b/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; using jspan_context = opentelemetry::trace::SpanContext; From d044c9ccd00ac70987c145d12c3eab99b837ddb0 Mon Sep 17 00:00:00 2001 From: Omri Zeneva Date: Sun, 13 Feb 2022 20:58:39 +0200 Subject: [PATCH 12/14] rgw,osd: convert tracer to be a global var since we are using batch span processor, the tracer does not send the spans by himself. so a single tracer is good enough. Signed-off-by: Omri Zeneva --- src/common/tracer.cc | 6 ------ src/common/tracer.h | 2 -- src/osd/OSD.cc | 1 - src/rgw/rgw_main.cc | 2 +- src/rgw/rgw_tracer.cc | 4 ---- src/rgw/rgw_tracer.h | 4 ---- 6 files changed, 1 insertion(+), 18 deletions(-) diff --git a/src/common/tracer.cc b/src/common/tracer.cc index f908ffe318526..1068bf5c2ccf0 100644 --- a/src/common/tracer.cc +++ b/src/common/tracer.cc @@ -33,12 +33,6 @@ void Tracer::init(opentelemetry::nostd::string_view service_name) { } } -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); diff --git a/src/common/tracer.h b/src/common/tracer.h index 1ad57e91f56a1..5d77a0dcf159c 100644 --- a/src/common/tracer.h +++ b/src/common/tracer.h @@ -25,7 +25,6 @@ class Tracer { 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` @@ -95,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) {} diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc index 51c07df251160..dc58cb438827d 100644 --- a/src/osd/OSD.cc +++ b/src/osd/OSD.cc @@ -4521,7 +4521,6 @@ int OSD::shutdown() utime_t duration = ceph_clock_now() - start_time_func; dout(0) <<"Slow Shutdown duration:" << duration << " seconds" << dendl; - tracing::osd::tracer.shutdown(); return r; } diff --git a/src/rgw/rgw_main.cc b/src/rgw/rgw_main.cc index f2fc502a70fb9..1189e3660b6b5 100644 --- a/src/rgw/rgw_main.cc +++ b/src/rgw/rgw_main.cc @@ -334,7 +334,7 @@ int radosgw_Main(int argc, const char **argv) derr << "ERROR: unable to initialize rgw tools" << dendl; return -r; } - + tracing::rgw::tracer.init("rgw"); rgw_init_resolver(); rgw::curl::setup_curl(fe_map); rgw_http_client_init(g_ceph_context); diff --git a/src/rgw/rgw_tracer.cc b/src/rgw/rgw_tracer.cc index 2d6720f591bac..7e12bb2e62dcc 100644 --- a/src/rgw/rgw_tracer.cc +++ b/src/rgw/rgw_tracer.cc @@ -7,11 +7,7 @@ namespace tracing { namespace rgw { -#ifdef HAVE_JAEGER -thread_local tracing::Tracer tracer("rgw"); -#else // !HAVE_JAEGER tracing::Tracer tracer; -#endif } // namespace rgw } // namespace tracing diff --git a/src/rgw/rgw_tracer.h b/src/rgw/rgw_tracer.h index 77d4689d257e3..9cbae8b9c6792 100644 --- a/src/rgw/rgw_tracer.h +++ b/src/rgw/rgw_tracer.h @@ -18,11 +18,7 @@ const auto TYPE = "type"; const auto REQUEST = "request"; const auto MULTIPART = "multipart_upload "; -#ifdef HAVE_JAEGER -extern thread_local tracing::Tracer tracer; -#else extern tracing::Tracer tracer; -#endif } // namespace rgw } // namespace tracing From f956b79aaafeeefac15ddb4ef8b7a68be82a724f Mon Sep 17 00:00:00 2001 From: Omri Zeneva Date: Sun, 13 Feb 2022 20:55:11 +0200 Subject: [PATCH 13/14] cmake: change build type of opentelemtry to Release Signed-off-by: Omri Zeneva --- cmake/modules/BuildOpentelemetry.cmake | 1 + 1 file changed, 1 insertion(+) diff --git a/cmake/modules/BuildOpentelemetry.cmake b/cmake/modules/BuildOpentelemetry.cmake index ab28fa622e48c..1bbdf870c17ae 100644 --- a/cmake/modules/BuildOpentelemetry.cmake +++ b/cmake/modules/BuildOpentelemetry.cmake @@ -12,6 +12,7 @@ function(build_opentelemetry) set(opentelemetry_CMAKE_ARGS -DCMAKE_POSITION_INDEPENDENT_CODE=ON -DWITH_JAEGER=ON -DBUILD_TESTING=OFF + -DCMAKE_BUILD_TYPE=Release -DWITH_EXAMPLES=OFF) set(opentelemetry_libs From dd57ad6c4d0005af8abae4e0698bb77d2087f5b6 Mon Sep 17 00:00:00 2001 From: Omri Zeneva Date: Mon, 25 Apr 2022 14:04:45 -0400 Subject: [PATCH 14/14] do_freebsd.sh: set WITH_JAEGER=OFF Signed-off-by: Omri Zeneva --- do_freebsd.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/do_freebsd.sh b/do_freebsd.sh index 097fe19e68f0b..442fd633f46e9 100755 --- a/do_freebsd.sh +++ b/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