feat(active-job): Add performance metrics to process span#2198
feat(active-job): Add performance metrics to process span#2198bdewater-thatch wants to merge 1 commit intoopen-telemetry:mainfrom
Conversation
|
|
abd8bc7 to
468f0c2
Compare
468f0c2 to
b868c9f
Compare
b868c9f to
d1e115e
Compare
41ebc51 to
71ab41f
Compare
Record cpu_time, idle_time, gc_time, and allocations as span attributes on the perform.active_job process span, mirroring the metrics that ActiveSupport::Notifications::Event captures internally.
71ab41f to
44dded4
Compare
thompson-tomo
left a comment
There was a problem hiding this comment.
As a first point of discussion I am not sure if we should be adding them as attributes on the span. Creating them as metrics seems more appropriate.
|
@thompson-tomo thanks for having a look. I can |
|
@bdewater-thatch thanks for this contribution. This is not the first time someone has requested or submitted a PR to add Rails internal measurements as span attributes. Although I agree that the Rails internal measurements should be derived from existing spans, I find there is mixed messaging in the spec about recording metric type values on span attributes. E.g. database batch size and response return rows or HTTP body size, resend count but you can also record any arbitrary HTTP header. They seem to draw the line on latency attributes, which would very likely be more interesting and familiar to Rails users. I used to be much more opinionated that these Rails measured performance values should not be added as span attributes but not I'm not so sure. As a compromise here I'd like to offer up a suggestion that allows users the option to opt-in level: https://opentelemetry.io/docs/specs/semconv/general/attribute-requirement-level/ I haven't looked at how other SDKs are implementing opt-in yet but I think this is a good use case for us to consider with the caveat that there may be a need to have more complex configurations and these attributes would be absent by default. |
|
On today's SIG call we discussed this PR and agreed to an opt-in, I'll work on that this week. |
Record cpu_time, idle_time, gc_time, and allocations as span attributes on the
perform.active_jobprocess span, mirroring the metrics that ActiveSupport::Notifications::Event captures internally.We don't get these for free because these are only captured on the EventObject path (
ActiveSupport::Notifications.subscribe('perform.active_job') { |event| ... }) where OTel uses the Evented path instead (fanout.rb).