feat(active_job): add spans for Continuation#2361
Conversation
fa429f2 to
d8fa1a6
Compare
hannahramadan
left a comment
There was a problem hiding this comment.
Thank you, Bart ◡̈
I'd like to revert the prefixes back to messaging vs rails. I think we should be consistent until #2368 is settled. If it is decided that we want the new naming, then we can make that update across the board.
|
Actually @hannahramadan that is more broad of a change. Even if we were to stick with messaging the naming convention in this PR currently is what is needed as semconv doesn't prefix vendor specific attributes with the domain. |
I agree with @hannahramadan, the README documents the current attributes across the library: opentelemetry-ruby-contrib/instrumentation/active_job/README.md Lines 81 to 89 in b1fe968 These attributes may not be aligned with the current semantic conventions, but they are consistent. If we add in the I recommend we use Since we're split, maybe we can get some other points of view to chime in. @simi, @xuan-cao-swi what do you think? |
👍 |
|
For reference the collector which runs into this problem all the time follow the policy of following semconv rules for new attributes & existing attributes remain untouched. This enables the reduction of future breaking changes and additional work for users. The collector then At a later point functionality is added to enable the user to opt-in to the new naming for existing attributes. Hence not sure if the inconsistency in namespace is a big issue for users as I would've expected it to be raised as an issue there.
I am not following #2368 is about changing from using the messaging domain (including span types) as it doesn't fit the expectations of messaging. Even if we stick with messaging for the time being, the active_job attributes need to come out of that namespace. As mid ground, why don't we first add a flag use_rails_namespace which if true switches the active_job attributes from messaging to rails and new attributes are always rails.*. That way for the user who needs consistency they can get it via opt-in and we reduce future breaking changes. |
02bac0f to
f007fb3
Compare
|
I have named the attributes back to |
|
@thompson-tomo - I hear what you're saying with the collector, I just don't think that's what Ruby users expect. If anyone is viewing this conversation and thinks otherwise, I'd love to hear about their experience. We haven't solidified what we want new conventions to be for this library, or even if we want to change them. Until #2368 is decided, I do not want to change the attribute naming strategy for Active Job. I also don't want that discussion to hold up support for Active Job Continuations. Let's move the rest of our discussion about attribute naming for this instrumentation to that issue. |
Add tracing support for Rails 8.1 Active Job Continuation steps. Each step in a continuable job gets its own child span under the perform span, with attributes for step name, cursor, resumed, and interrupted state.
f007fb3 to
bf22a6d
Compare
Rails 8.1 added Active Job Continuation for interruptible and resumable jobs. This PR adds instrumentation for the events emitted when the
stepDSL is used: https://edgeguides.rubyonrails.org/active_support_instrumentation.html#active-job