Add child_spans for Sidekiq Queue instrumentation

[frederikspang]

Implements #2322 for Sidekiq.

Description

Adds support for Sentry Queues page (With predefined span op names)

https://docs.sentry.io/platforms/ruby/guides/sidekiq/tracing/instrumentation/custom-instrumentation/queues-module/

[rwojsznis]

Oh wow, exactly what I was looking for 😅 🙌

can we somehow help to get this shipped? 💪

[frederikspang]

@solnic I've added two basic specs. Latency is a bit difficult, without introducing some timecop kind of tool. Which we do have in sentry-ruby, so we could just introduce it here if we wanted to.

[frederikspang]

@solnic After looking at these, I think queue.process may actually replace op: queue.sidekiq instead of being a child span. They will always be redundant - But we should set origin or some other field to sidekiq, description or name maybe, to differentiate the processor. (ie. using another processor alongside Sidekiq - or doing queries in Sentry UI across applications)

[nateberkopec]

Looks like it needs a changelog too

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.16%. Comparing base (27d7384) to head (5e000c6). Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2403   +/-   ##
=======================================
  Coverage   98.15%   98.16%           
=======================================
  Files         126      126           
  Lines        4728     4741   +13     
=======================================
+ Hits         4641     4654   +13     
  Misses         87       87           

Components	Coverage Δ
sentry-ruby	98.51% <ø> (ø)
sentry-rails	97.05% <ø> (ø)
sentry-sidekiq	97.63% <100.00%> (+0.15%)	:arrow_up:
sentry-resque	92.85% <ø> (ø)
sentry-delayed_job	95.65% <ø> (ø)
sentry-opentelemetry	99.31% <ø> (ø)

Files with missing lines	Coverage Δ
...iq/lib/sentry/sidekiq/sentry_context_middleware.rb	98.21% <100.00%> (+0.53%)	:arrow_up:

[solnic]

@solnic After looking at these, I think queue.process may actually replace op: queue.sidekiq instead of being a child span. They will always be redundant

• But we should set origin or some other field to sidekiq, description or name maybe, to differentiate the processor. (ie. using another processor alongside Sidekiq - or doing queries in Sentry UI across applications)

I've been testing this out and I don't see queue.sidekiq/queue.process. This is what I see instead:

[solnic]

Latency is a bit difficult, without introducing some timecop kind of tool. Which we do have in sentry-ruby, so we could just introduce it here if we wanted to.

Thanks for adding the specs. I think it would be good to use timecop there actually.

[frederikspang]

@solnic After looking at these, I think queue.process may actually replace op: queue.sidekiq instead of being a child span. They will always be redundant

• But we should set origin or some other field to sidekiq, description or name maybe, to differentiate the processor. (ie. using another processor alongside Sidekiq - or doing queries in Sentry UI across applications)

I've been testing this out and I don't see queue.sidekiq/queue.process. This is what I see instead:

You're right! I changed that after my comment, before your review here:

https://github.com/getsentry/sentry-ruby/pull/2403/commits/cd3a1df20eb357498f91bf4217c1667e4d4839aa

I'll have a look at the other comments as soon as possible.

[frederikspang]

@solnic I have updated PR with feedback adjusted!

Edit: I see specs are broken. I'll look into that as well!

[frederikspang]

@solnic The specs failing seems related to https://github.com/sidekiq/sidekiq/pull/6430 - Was able to reproduce locally, after clearing Lock file and bundle install again.

Somewhat unrelated to this PR, although it's noisy.

[solnic]

@solnic The specs failing seems related to sidekiq/sidekiq#6430 - Was able to reproduce locally, after clearing Lock file and bundle install again.

Somewhat unrelated to this PR, although it's noisy.

Please update to the latest master. I just checked locally and it passes. Once CI is green I'll merge it in.

[frederikspang]

@solnic Done!

[nateberkopec]

🙌 Thanks everyone! This is such a great feature.