opentelemetry-ruby-contrib icon indicating copy to clipboard operation
opentelemetry-ruby-contrib copied to clipboard

fix: use AS::N subscriber for serialize events

Open robbkidd opened this issue 1 year ago • 3 comments

Resolves #992

This mimicks a fair amount of the current ActionMailer and ActionView instrumentation.

The details for Context management (i.e. setting current span) are already handled by the OTel ActiveSupport instrumentation. Reuse the notifications subscriber here for ActiveModel serialization events.

Reworked the example app into two: one Rails which works with the usual SDK configuration and one standalone (no Rails) to demonstrate that the subscription needs to be made after the SDK configuration is complete. If the subscription is created during instrumentation install, the subscription's tracer will be a NO-OP API tracer and won't produce spans.

TODO:

  • [ ] review/update README and doc comments for accurate usage steps

robbkidd avatar Jul 19 '24 15:07 robbkidd

👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this pull request will be closed eventually by the stale bot

github-actions[bot] avatar Sep 08 '24 01:09 github-actions[bot]

@robbkidd mind taking a look at @kaylareopelle comments?

arielvalentin avatar Sep 08 '24 02:09 arielvalentin

👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this pull request will be closed eventually by the stale bot

github-actions[bot] avatar Oct 19 '24 01:10 github-actions[bot]

@robbkidd - Example code is cleared from me!

It looks like there's a few other things you and @arielvalentin chatted about related to the subscriber and Railties. Is there more you'd like to do to address those questions? I'm not 100% following with a quick read, but happy to take a deeper look if that would help.

kaylareopelle avatar Nov 26 '24 23:11 kaylareopelle

@kaylareopelle I'm good merge if you are

arielvalentin avatar Nov 27 '24 12:11 arielvalentin

@kaylareopelle I think the ideas @arielvalentin and I had in conversations in this PR are not blockers to this PR and can/ought to be done in follow-ups.

robbkidd avatar Nov 28 '24 00:11 robbkidd