rgw/tracing: end to end trace for multipart upload by zenomri · Pull Request #47370 · ceph/ceph

zenomri · 2022-07-31T08:11:52Z

The goal of this PR is to unify the trace created for MP upload in the RGW and the subsequent spans created for the same operation in OSDs

Librados already had the blkin_trace_info in some places, but this tracing system isn't used either in RGW or OSD, so we can replace it with open-telemetry trace information.

The blkin trace needs to be removed from all over the place. It needs to be kept for another PR to simplify the current changes made for end-to-end MP tracing.

Screenshot of how the MP trace looks like after this change:

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
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 cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows

new o-tel trace added for the message header, and encode/decode it for MSOD op Signed-off-by: Omri Zeneva <ozeneva@redhat.com>

in order to propagate the trace info I added the otel-trace as an extra param. in some places, there already was a blkin trace info, and since it is not used in other places we can safely change it to o-tel trace info. Signed-off-by: Omri Zeneva <ozeneva@redhat.com>

… aio_opearte in order to make it possible, I saved the trace info inside the sal::Object, so we can use it later when writing the object to rados it could be used also later for read ops Signed-off-by: Omri Zeneva <ozeneva@redhat.com>

Signed-off-by: Omri Zeneva <ozeneva@redhat.com>

… of the span Signed-off-by: Omri Zeneva <ozeneva@redhat.com>

Signed-off-by: Omri Zeneva <ozeneva@redhat.com>

github-actions · 2022-07-31T08:12:25Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

yuvalif · 2022-07-31T09:08:53Z

src/rgw/rgw_op.cc


    s->trace->SetAttribute(tracing::rgw::UPLOAD_ID, multipart_upload_id);
    multipart_trace = tracing::rgw::tracer.add_span(name(), upload->get_trace());
+    s->object->set_trace(multipart_trace->GetContext());


lets start from regular object trace. please move out of the "if multipart" block, and set the regular trace as its parent

yuvalif · 2022-07-31T09:13:02Z

src/osd/OSD.cc

        reqid.name._num, reqid.tid, reqid.inc);
  }
-  op->osd_parent_span = tracing::osd::tracer.start_trace("op-request-created");
+  op->osd_parent_span = tracing::osd::tracer.add_span("op-request-created", m->otel_trace);


please keep current behavior. if m->otel_trace is null, just start a new trace

yuvalif · 2022-07-31T09:25:36Z

src/librados/IoCtxImpl.cc

 				     ::ObjectOperation *o, AioCompletionImpl *c,
 				     const SnapContext& snap_context, int flags,
-                                     const blkin_trace_info *trace_info)
+                                     const jspan_context *trace_info)


please keep blkin until we deprecate it completely

yuvalif · 2022-07-31T09:26:07Z

src/librados/IoCtxImpl.h

  int aio_operate(const object_t& oid, ::ObjectOperation *o,
 		  AioCompletionImpl *c, const SnapContext& snap_context,
-		  int flags, const blkin_trace_info *trace_info = nullptr);
+		  int flags, const jspan_context *trace_info = nullptr);


please keep blkin for now

assume you mean we can add the jspan_context, but leave blkin for later removal? I don't want us to have to do an unsafe cast to/from a blkin pointer type

removing blkin separately somehow feels cleaner, but at the same time, I've confirmed with core that blkin can be unceremoniously removed, just saying

assume you mean we can add the jspan_context, but leave blkin for later removal? I don't want us to have to do an unsafe cast to/from a blkin pointer type

agree. lets add another function (for now)

removing blkin separately somehow feels cleaner, but at the same time, I've confirmed with core that blkin can be unceremoniously removed, just saying

yes, buts lets remove that in a separate commit, including the removal of all functions that use it, remove it from the git submodules or OS dependencies, make etc.
the only place we should keep it (or a place holder of it) is the wire protocol, so we won't crash if someone sends a message with a blkin trace.

…putobj operations this commit also include changes for the librados interfaces to support the otel trace info Signed-off-by: Omri Zeneva <ozeneva@redhat.com>

yuvalif · 2022-08-03T08:37:38Z

src/rgw/rgw_op.cc


  rgw_placement_rule *pdest_placement = &s->dest_placement;

+  s->object->set_trace(s->trace->GetContext());


what if s->trace is null?

this scenario is not possible. s->trace is initialized unconditionally before the op execution(in rgw_process.cc)

in case we compile with WITH_JAEGER=OFF then s->trace won't be a ptr, but a stub struct (with -> overloading)

zenomri · 2022-08-04T07:00:57Z

thank for the review. I will open a new PR with the right new branch name (trace for Put op instead of Multipart)

Omri Zeneva added 6 commits July 31, 2022 03:35

msg: add opentelemetry trace to Messages

edc297a

new o-tel trace added for the message header, and encode/decode it for MSOD op Signed-off-by: Omri Zeneva <ozeneva@redhat.com>

osd: use the o-tel trace of the msg as a parent span of the osd trace

1274262

Signed-off-by: Omri Zeneva <ozeneva@redhat.com>

common/tracer: add nullptr check in add_span before checking validity…

6fe2b1b

… of the span Signed-off-by: Omri Zeneva <ozeneva@redhat.com>

cmake: add opentelemetry to include path

d98915d

Signed-off-by: Omri Zeneva <ozeneva@redhat.com>

zenomri requested a review from a team as a code owner July 31, 2022 08:11

github-actions bot added build/ops common core rgw labels Jul 31, 2022

zenomri requested review from cbodley, ideepika and yuvalif July 31, 2022 08:12

github-actions bot added the needs-rebase label Jul 31, 2022

yuvalif reviewed Jul 31, 2022

View reviewed changes

rgw: set trace for single putobj operation instead of just multipart …

cc3a88a

…putobj operations this commit also include changes for the librados interfaces to support the otel trace info Signed-off-by: Omri Zeneva <ozeneva@redhat.com>

yuvalif reviewed Aug 3, 2022

View reviewed changes

zenomri closed this Aug 4, 2022

zenomri mentioned this pull request Aug 4, 2022

rgw/tracing: end to end trace for put obj op #47457

Merged

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rgw/tracing: end to end trace for multipart upload #47370

rgw/tracing: end to end trace for multipart upload #47370
zenomri wants to merge 7 commits intoceph:mainfrom
zenomri:wip-omri-full-multipart-trace

zenomri commented Jul 31, 2022 •

edited

Loading

Uh oh!

github-actions bot commented Jul 31, 2022

Uh oh!

yuvalif Jul 31, 2022

Uh oh!

yuvalif Jul 31, 2022

Uh oh!

yuvalif Jul 31, 2022 •

edited

Loading

Uh oh!

yuvalif Jul 31, 2022

Uh oh!

mattbenjamin Jul 31, 2022

Uh oh!

mattbenjamin Jul 31, 2022

Uh oh!

yuvalif Aug 1, 2022 •

edited

Loading

Uh oh!

yuvalif Aug 3, 2022

Uh oh!

zenomri Aug 4, 2022

Uh oh!

zenomri commented Aug 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		rgw_placement_rule *pdest_placement = &s->dest_placement;

		s->object->set_trace(s->trace->GetContext());

Conversation

zenomri commented Jul 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

github-actions bot commented Jul 31, 2022

Uh oh!

yuvalif Jul 31, 2022

Choose a reason for hiding this comment

Uh oh!

yuvalif Jul 31, 2022

Choose a reason for hiding this comment

Uh oh!

yuvalif Jul 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuvalif Jul 31, 2022

Choose a reason for hiding this comment

Uh oh!

mattbenjamin Jul 31, 2022

Choose a reason for hiding this comment

Uh oh!

mattbenjamin Jul 31, 2022

Choose a reason for hiding this comment

Uh oh!

yuvalif Aug 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuvalif Aug 3, 2022

Choose a reason for hiding this comment

Uh oh!

zenomri Aug 4, 2022

Choose a reason for hiding this comment

Uh oh!

zenomri commented Aug 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zenomri commented Jul 31, 2022 •

edited

Loading

yuvalif Jul 31, 2022 •

edited

Loading

yuvalif Aug 1, 2022 •

edited

Loading