jaegertracing build/ops integration #31358

ideepika · 2019-11-04T13:48:05Z

Milestones

Build Jaeger as External Project ( all cmake at scalable level)
OSD trace using marker spans
Off the wire context transfer configuration
Documentation
RGW path trace
Use Case Documentation
Reviews, Cleanups and Documentation update
Code Coverage and Tests

Checklist

References tracker ticket
Updates documentation if necessary
Includes tests for new functionality or reproducer for bug

Show available Jenkins commands

jenkins retest this please
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard backend
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox

CMakeLists.txt

cmake/modules/FindYAMLCPP.cmake

cmake/modules/FindJAEGER.cmake

cmake/modules/FindOPENTRACE.cmake

stale · 2020-03-14T16:20:05Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

jdurgin

looks good in general, a few minor things noted

ceph.spec.in

src/common/tracer.cc

jdurgin · 2020-12-11T01:41:40Z

src/common/tracer.cc

+    if(!tracer){
+	YAML::Node yaml;
+	try{
+	  yaml = YAML::LoadFile("../src/jaegertracing/config.yml");


this should probably be a config option. in the future it'd be very useful to be able to change the jaeger config at runtime as well, not sure how the opentracing library handles that

src/osd/PrimaryLogPG.cc

src/osd/OSD.cc

src/osd/OpRequest.h

src/osd/OSD.cc

src/vstart.sh

* This commit introduces Jaegertracing library as package libjaeger, pickwhich would be consumed by other ceph pacakges such as ceph-common0 * adds the following dependencies, which would be build from source using ExternalProjectHelper.cmake +IncludeJaeger.cmake + Build<package>.cmake scripts: jaegertracing: v0.6.0 [added as a submodule] opentracing: v1.6.0 [added as a submodule] thrift: 0.13.0 [added as a submodule] yaml-cpp: 0.6.0 json(optional) * updates Boost to be installed instead of being build only, because jaegertracing them during their build process. * ceph.spec.in: introduces a default enabled jaeger packaging option, which could be disabled using --without-jaeger flag during rpmbuild * note: libjaeger package if enabled will be a dependency on ceph-common, ceph-mon, rgw_common and transitively will be a dependency for modules that have them as a dependency. Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>

Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>

* add tracing header files { common/tracer.{h/cc} } Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>

Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>

ideepika · 2020-12-12T11:31:21Z

looks green mostly:
http://pulpito.front.sepia.ceph.com/ideepika-2020-12-11_19:11:18-rados-wip-jaegertracing-in-ceph-distro-basic-smithi/

ronen-fr · 2020-12-13T16:13:19Z

CMakeLists.txt

@@ -364,6 +364,11 @@ if(WITH_BLKIN)
  include_directories(SYSTEM src/blkin/blkin-lib)
 endif(WITH_BLKIN)

+option(WITH_JAEGER "Enable jaegertracing and it's dependent libraries" OFF)


ronen-fr · 2020-12-13T16:20:09Z

cmake/modules/BuildJaeger.cmake

@@ -0,0 +1,65 @@
+# This module builds Jaeger after it's dependencies are installed and discovered


src/vstart.sh

Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>

jdurgin

Looks good! let's merge this and handle follow-on work in further PRs.

tchaikov · 2020-12-16T09:46:18Z

@ideepika @jdurgin this change breaks the build

-- Checking for one of the modules 'yaml-cpp'
CMake Error at /usr/share/cmake-3.10/Modules/FindPackageHandleStandardArgs.cmake:137 (message):
  Could NOT find yaml-cpp: Found unsuitable version "", but required is at
  least "0.5.1" (found yaml-cpp_LIBRARY-NOTFOUND)
Call Stack (most recent call first):
  /usr/share/cmake-3.10/Modules/FindPackageHandleStandardArgs.cmake:376 (_FPHSA_FAILURE_MESSAGE)
  cmake/modules/Findyaml-cpp.cmake:46 (find_package_handle_standard_args)
  src/CMakeLists.txt:331 (_find_package)
  src/seastar/cmake/SeastarDependencies.cmake:98 (find_package)
  src/seastar/CMakeLists.txt:330 (seastar_find_dependencies)


-- Configuring incomplete, errors occurred!
See also "/home/jenkins-build/build/workspace/ceph-pull-requests/build/CMakeFiles/CMakeOutput.log".
See also "/home/jenkins-build/build/workspace/ceph-pull-requests/build/CMakeFiles/CMakeError.log".
+ exit 1

tchaikov · 2020-12-16T09:47:21Z

@ideepika
could you prefix the title of your commit message with the subcomponent your are changing ? see https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#3-describe-your-changes

we don't have a component named "core" or "jaeger". would you please be more specific in future?

ideepika · 2020-12-16T10:11:12Z

@ideepika @jdurgin this change breaks the build

-- Checking for one of the modules 'yaml-cpp'
CMake Error at /usr/share/cmake-3.10/Modules/FindPackageHandleStandardArgs.cmake:137 (message):
  Could NOT find yaml-cpp: Found unsuitable version "", but required is at
  least "0.5.1" (found yaml-cpp_LIBRARY-NOTFOUND)
Call Stack (most recent call first):
  /usr/share/cmake-3.10/Modules/FindPackageHandleStandardArgs.cmake:376 (_FPHSA_FAILURE_MESSAGE)
  cmake/modules/Findyaml-cpp.cmake:46 (find_package_handle_standard_args)
  src/CMakeLists.txt:331 (_find_package)
  src/seastar/cmake/SeastarDependencies.cmake:98 (find_package)
  src/seastar/CMakeLists.txt:330 (seastar_find_dependencies)


-- Configuring incomplete, errors occurred!
See also "/home/jenkins-build/build/workspace/ceph-pull-requests/build/CMakeFiles/CMakeOutput.log".
See also "/home/jenkins-build/build/workspace/ceph-pull-requests/build/CMakeFiles/CMakeError.log".
+ exit 1

@tchaikov seems like missed to catch it, i will fix it asap, thanks!

ideepika · 2020-12-16T12:34:57Z

@ideepika
could you prefix the title of your commit message with the subcomponent your are changing ? see https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#3-describe-your-changes

we don't have a component named "core" or "jaeger". would you please be more specific in future?

thanks, will keep in mind!

tchaikov · 2020-12-17T00:06:43Z

FTBFS addressed by #38609

ideepika force-pushed the wip-jaegertracing-in-ceph branch from aa5bc84 to 5d7710f Compare November 5, 2019 07:28

tchaikov added the build/ops label Nov 5, 2019

tchaikov requested review from tchaikov and neha-ojha November 5, 2019 12:45

neha-ojha changed the title ~~Wip jaegertracing in ceph~~ [WIP] jaegertracing in ceph Nov 5, 2019

wjwithagen reviewed Nov 5, 2019

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

tchaikov reviewed Nov 22, 2019

View reviewed changes

cmake/modules/FindYAMLCPP.cmake Outdated Show resolved Hide resolved

cmake/modules/FindJAEGER.cmake Outdated Show resolved Hide resolved

cmake/modules/FindOPENTRACE.cmake Outdated Show resolved Hide resolved

cmake/modules/FindOPENTRACE.cmake Outdated Show resolved Hide resolved

ideepika force-pushed the wip-jaegertracing-in-ceph branch 2 times, most recently from 6c16bdd to 958e4c4 Compare December 16, 2019 12:52

ideepika force-pushed the wip-jaegertracing-in-ceph branch 3 times, most recently from 5f4ac82 to d6632b0 Compare January 7, 2020 10:34

ideepika force-pushed the wip-jaegertracing-in-ceph branch from d6632b0 to d838fe8 Compare January 14, 2020 15:26

stale bot added the stale label Mar 14, 2020

ideepika removed the stale label Mar 16, 2020

ideepika force-pushed the wip-jaegertracing-in-ceph branch 2 times, most recently from 1344ed7 to b01f984 Compare March 17, 2020 14:29

ideepika force-pushed the wip-jaegertracing-in-ceph branch from b01f984 to c069c92 Compare April 7, 2020 12:55

ideepika force-pushed the wip-jaegertracing-in-ceph branch 2 times, most recently from 25dc32c to 5c324b4 Compare April 21, 2020 10:27

ideepika requested review from tchaikov and wjwithagen April 21, 2020 12:07

ideepika force-pushed the wip-jaegertracing-in-ceph branch from 1a45ff1 to 2705237 Compare May 11, 2020 12:43

stale bot added the stale label Jul 11, 2020

ideepika removed the stale label Jul 11, 2020

ceph deleted a comment from stale bot Jul 11, 2020

ideepika force-pushed the wip-jaegertracing-in-ceph branch 2 times, most recently from 37764d5 to 151a60a Compare July 14, 2020 10:53

ideepika requested a review from a team as a code owner July 14, 2020 10:53

github-actions bot added common mon rgw labels Dec 3, 2020

ideepika force-pushed the wip-jaegertracing-in-ceph branch 3 times, most recently from 5b41797 to 440f7aa Compare December 10, 2020 14:48

ideepika requested a review from tchaikov December 10, 2020 15:25

jdurgin requested changes Dec 11, 2020

View reviewed changes

Deepika Upadhyay added 6 commits December 11, 2020 07:55

ceph.spec.in: refactor s/Cephs/Ceph

3c5abab

Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>

core: tracing basic i/o path for OSD

01997e7

* add tracing header files { common/tracer.{h/cc} } Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>

osd: refactor missing spaces

4b8c92a

Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>

debian/control: use yaml-dev >= 0.6

1e6e1f9

Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>

jaeger: make tracing default turned off, until teuthology integration

85ca7b3

Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>

ideepika force-pushed the wip-jaegertracing-in-ceph branch from 8faba0e to a3b42dd Compare December 11, 2020 10:13

ronen-fr reviewed Dec 13, 2020

View reviewed changes

src/vstart.sh Outdated Show resolved Hide resolved

vstart: add jaeger option + misc refactoring

9f5d725

Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>

ideepika force-pushed the wip-jaegertracing-in-ceph branch from a3b42dd to 9f5d725 Compare December 15, 2020 16:13

jdurgin approved these changes Dec 15, 2020

View reviewed changes

jdurgin merged commit 8a74a93 into ceph:master Dec 16, 2020

This was referenced Dec 16, 2020

cmake: fix build failure due to yaml-cpp not available #38609

Merged

rpm,deb: conditionally package crimson-store-nbd in ceph-test #38608

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

jaegertracing build/ops integration #31358

jaegertracing build/ops integration #31358

ideepika commented Nov 4, 2019 •

edited

stale bot commented Mar 14, 2020

jdurgin left a comment

jdurgin Dec 11, 2020

ideepika Dec 11, 2020

ideepika commented Dec 12, 2020

ronen-fr Dec 13, 2020

ronen-fr Dec 13, 2020

jdurgin left a comment

tchaikov commented Dec 16, 2020

tchaikov commented Dec 16, 2020

ideepika commented Dec 16, 2020

ideepika commented Dec 16, 2020

tchaikov commented Dec 17, 2020

		@@ -0,0 +1,65 @@
		# This module builds Jaeger after it's dependencies are installed and discovered

jaegertracing build/ops integration #31358

jaegertracing build/ops integration #31358

Conversation

ideepika commented Nov 4, 2019 • edited

Milestones

Checklist

stale bot commented Mar 14, 2020

jdurgin left a comment

Choose a reason for hiding this comment

jdurgin Dec 11, 2020

Choose a reason for hiding this comment

ideepika Dec 11, 2020

Choose a reason for hiding this comment

ideepika commented Dec 12, 2020

ronen-fr Dec 13, 2020

Choose a reason for hiding this comment

ronen-fr Dec 13, 2020

Choose a reason for hiding this comment

jdurgin left a comment

Choose a reason for hiding this comment

tchaikov commented Dec 16, 2020

tchaikov commented Dec 16, 2020

ideepika commented Dec 16, 2020

ideepika commented Dec 16, 2020

tchaikov commented Dec 17, 2020

ideepika commented Nov 4, 2019 •

edited