fix: use AS::N subscriber for serialize events #1075
Conversation
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.
robbkidd
requested review from
fbogsany,
mwear,
robertlaurin,
dazuma,
ericmustin,
arielvalentin,
ahayworth,
plantfansam,
simi,
kaylareopelle and
xuan-cao-swi
as code owners
...ializers/test/opentelemetry/instrumentation/active_model_serializers/instrumentation_test.rb
Show resolved
Hide resolved
...el_serializers/lib/opentelemetry/instrumentation/active_model_serializers/instrumentation.rb
Show resolved
Hide resolved
...el_serializers/lib/opentelemetry/instrumentation/active_model_serializers/instrumentation.rb
Show resolved
Hide resolved
...el_serializers/lib/opentelemetry/instrumentation/active_model_serializers/instrumentation.rb
Show resolved
Hide resolved
...tive_model_serializers/lib/opentelemetry/instrumentation/active_model_serializers/railtie.rb
Show resolved
Hide resolved
|
👋 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 |
|
@robbkidd mind taking a look at @kaylareopelle comments? |
|
👋 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 |
arielvalentin
added
keep
Co-authored-by: Kayla Reopelle <[email protected]>
... but leave OTLP exporter gem in dependencies so that a curious person can override without code changes to send to some OTLP receiver by setting the appropriate environment variables.
|
@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 I'm good merge if you are |
|
@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. |
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: