Basic Jaeger Tracing Added #1

suab321321 · 2020-03-15T09:41:20Z

Signed-off-by: Abhinav Singh [email protected]

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

Signed-off-by: Abhinav Singh <[email protected]>

…321/ceph into suab321321_jaegerTracing Signed-off-by: Abhinav Singh <[email protected]>

Signed-off-by: Abhinav Singh <[email protected]>

ideepika · 2020-05-10T20:08:53Z

src/rgw/rgw_civetweb_frontend.cc

-                            *env.auth_registry, &client_io, env.olog,
-                            null_yield, scheduler.get() ,&http_ret);
+  int ret;
+  #ifdef WITH_JAEGER


instead of having function definition changed everywhere with 2 alternatives, can you try testing adding span to RGWSimpleRequest https://github.com/ideepika/ceph/blob/jaeger-rebase-8.0/src/rgw/rgw_rest_client.h#L10:L10

ideepika · 2020-05-10T20:16:18Z

src/rgw/rgw_rest_swift.cc

@@ -1257,6 +1257,21 @@ int RGWDeleteObj_ObjStore_SWIFT::verify_permission()
  }
 }

+int RGWDeleteObj_ObjStore_SWIFT::verify_permission(Jager_Tracer& tracer, const Span& parent_span)
+{
+  Span span = tracer.child_span("rgw_rest_swift.cc RGWDeleteObj_ObjStore_SWIFT::verify_permission", parent_span);


you mentioned you were going to try using a global tracer, can you explain your approach here?

@ideepika the tracer is global, but the spans are not, each "Root" span denotes a new client request and that "Root" span will contain its serveral follow_up spans and child spans.
In the above example I m creating a new child span, parent_span is the span created in the function which is calling this function RGWDeleteObj_ObjStore_SWIFT::verify_permission

@suab321321 I understand that, my question is : is it important to pass the tracer to the function, or are you keeping the tracer globally unique for rgw?

@ideepika yes it is mandatory to pass the tracer as an argument, even if I make the tracer an extern and use this as global tracer, but I wont be able to make the Span an extern variable.I understand that by making and tracer and Span global I dont have to pass the variable as argument which in turn will prevent the creating of overloaded function, but the span variable is an issue here

Signed-off-by: Abhinav Singh <[email protected]>

ideepika

Hey @suab321321 !
Nice work till now, a few suggestions:

can you cleanup commits, with eliminating changes which are unrelated to jaeger.
can you also add a screenshot of jaeger trace with each commit addition, that way it would be easier to follow your code updates.
we can test the span traversal using req_state as we discuss, since that would change a lot of this code, I would rather help you out in that first, let's keep review stalled uptil then.

ideepika · 2020-05-19T16:02:49Z

src/rgw/rgw_op.cc

@@ -6725,7 +6725,7 @@ int RGWDeleteObj::verify_permission()
 int RGWDeleteObj::verify_permission(Jager_Tracer& tracer, const Span& parent_span)


s/Jager_Tracer/Jaeger_Tracer

ideepika · 2020-05-19T16:06:54Z

src/rgw/rgw_op.cc

+int RGWListBuckets::verify_permission(Jager_Tracer& tracer, const Span& parent_span)
+{
+  Span span = tracer.child_span("rgw_op.cc RGWListBuckets::verify_permission", parent_span);
+  rgw::Partition partition = rgw::Partition::aws;


can you remove non-jaeger changes in the further cleanups, it would make work easier for reviewer, hence better focus on feedback.

Signed-off-by: Abhinav Singh <[email protected]>

ideepika · 2020-06-15T17:48:50Z

src/common/common_init.cc

+	{
+		Span span=tracer.child_span("common_init.cc common_preinit()",parent_span);
+	  // set code environment
+	  ANNOTATE_BENIGN_RACE_SIZED(&g_code_env, sizeof(g_code_env), "g_code_env");


are these changes part of your code? If not can you clean them up?

ideepika · 2020-06-15T17:49:36Z

src/common/jaegerTracer.h

+    std::unique_ptr<opentracing::Span> childSpan(const char *, const std::unique_ptr<opentracing::Span> &);
+    std::unique_ptr<opentracing::Span> followUpSpan(const char *, const std::unique_ptr<opentracing::Span> &);
+    ~jTracer(){
+        opentracing::Tracer::Global()->Close();


looks good!

ideepika · 2020-06-15T18:00:15Z

src/global/global_init.cc

@@ -165,13 +167,103 @@ void global_pre_init(
  g_conf().complain_about_parse_error(g_ceph_context);
 }

+#ifdef WITH_JAEGER
+  void global_pre_init(


can you remove unrelated changes, it would really help in reviewing your work better + faster

ideepika · 2020-07-20T08:05:02Z

src/include/tracer.h

@@ -0,0 +1,64 @@
+#ifndef TRACER_H_


I see there are two files src/common/jaegerTracer and this one, can you move them to src/common/JaegerTracer{.h/cc}

Changes addressing comments in PR - commit to be squashed prior to merge Signed-off-by: Paul Cuzner <[email protected]>

suab321321 added 3 commits March 15, 2020 15:09

Basic Jaeger Tracing Added

fd36b14

Signed-off-by: Abhinav Singh <[email protected]>

Basic Jaeger Tracing Added

b878b91

Signed-off-by: Abhinav Singh <[email protected]>

Merge branch 'suab321321_jaegerTracing' of https://github.com/suab321…

05e1442

…321/ceph into suab321321_jaegerTracing Signed-off-by: Abhinav Singh <[email protected]>

suab321321 force-pushed the suab321321_jaegerTracing branch from a54fd8a to 05e1442 Compare March 15, 2020 09:47

suab321321 force-pushed the suab321321_jaegerTracing branch 4 times, most recently from 05e1442 to d0a8211 Compare April 22, 2020 19:37

"Jaeger Tracing of Bucket List Request"

eeebe21

Signed-off-by: Abhinav Singh <[email protected]>

suab321321 force-pushed the suab321321_jaegerTracing branch from d0a8211 to eeebe21 Compare April 22, 2020 19:40

suab321321 added 6 commits April 29, 2020 05:14

Tracer for object Uploading in Container added

f10c6f4

Signed-off-by: Abhinav Singh <[email protected]>

Removing Uneccessary code

1aa2724

Signed-off-by: Abhinav Singh <[email protected]>

Error Uploading Object to Container resolved

725a5a4

Signed-off-by: Abhinav Singh <[email protected]>

Converted variables into snake_case and removed non-essential code

a3ba8d7

Signed-off-by: Abhinav Singh <[email protected]>

RGW_OP_GET_OBJ tracer and cleanup

f605172

Signed-off-by: Abhinav Singh <[email protected]>

RGW_OP_DELETE_OBJ Tracer added

1ec9c76

Signed-off-by: Abhinav Singh <[email protected]>

ideepika reviewed May 10, 2020

View reviewed changes

suab321321 added 9 commits May 11, 2020 11:44

RGW_OP_DELETE_BUCKET Tracer Added

a98ce02

Signed-off-by: Abhinav Singh <[email protected]>

S3 and Swift tag added

1117ba5

Signed-off-by: Abhinav Singh <[email protected]>

Injecting trace further deep down functions for RGW_LIST_BUCKETS

d70d0f8

Signed-off-by: Abhinav Singh <[email protected]>

Added more tracer to into deep functions RGW_LIST_BUCKET

b83e969

Signed-off-by: Abhinav Singh <[email protected]>

Added more traces in depth in RGWPutObj operation

7f68f4d

Signed-off-by: Abhinav Singh <[email protected]>

Added more traces to in-depth functions of RGW_PUTObj

8c60410

Signed-off-by: Abhinav Singh <[email protected]>

More Tracer added in-depth fucntion RGW_DELETE_BUCKET

ea7e1e0

Signed-off-by: Abhinav Singh <[email protected]>

Added more traces in depth of RGWCreateBucket

2b1bb05

Signed-off-by: Abhinav Singh <[email protected]>

Added more in depth traces to RGW_DeleteObj

3a331ff

Signed-off-by: Abhinav Singh <[email protected]>

ideepika suggested changes May 19, 2020

View reviewed changes

suab321321 added 2 commits May 21, 2020 18:03

Tracer Added in req_state and ListBuckets tracing added

a42be55

Signed-off-by: Abhinav Singh <[email protected]>

Tracer for ListBucket

c88c714

Signed-off-by: Abhinav Singh <[email protected]>

DeleteContainer

7d76769

Signed-off-by: Abhinav Singh <[email protected]>

ideepika suggested changes Jun 15, 2020

View reviewed changes

ideepika reviewed Jul 20, 2020

View reviewed changes

suab321321 pushed a commit that referenced this pull request Aug 27, 2020

ceph-volume: minor updates prior to merge #1

60e110f

Changes addressing comments in PR - commit to be squashed prior to merge Signed-off-by: Paul Cuzner <[email protected]>

suab321321 force-pushed the master branch from ab03d4f to fb7729c Compare August 28, 2020 11:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic Jaeger Tracing Added #1

Basic Jaeger Tracing Added #1

suab321321 commented Mar 15, 2020 •

edited

Loading

ideepika May 10, 2020

ideepika May 10, 2020

suab321321 May 10, 2020 •

edited

Loading

ideepika May 11, 2020

suab321321 May 11, 2020

ideepika left a comment

ideepika May 19, 2020

ideepika May 19, 2020

ideepika Jun 15, 2020

ideepika Jun 15, 2020

ideepika Jun 15, 2020

ideepika Jul 20, 2020

		@@ -6725,7 +6725,7 @@ int RGWDeleteObj::verify_permission()
		int RGWDeleteObj::verify_permission(Jager_Tracer& tracer, const Span& parent_span)

Basic Jaeger Tracing Added #1

Are you sure you want to change the base?

Basic Jaeger Tracing Added #1

Conversation

suab321321 commented Mar 15, 2020 • edited Loading

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suab321321 May 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ideepika left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suab321321 commented Mar 15, 2020 •

edited

Loading

suab321321 May 10, 2020 •

edited

Loading