Skip to content

feat(active-job): Add performance metrics to process span#2198

Open
bdewater-thatch wants to merge 1 commit intoopen-telemetry:mainfrom
thatch-health:feat/active-job-perf-metrics
Open

feat(active-job): Add performance metrics to process span#2198
bdewater-thatch wants to merge 1 commit intoopen-telemetry:mainfrom
thatch-health:feat/active-job-perf-metrics

Conversation

@bdewater-thatch
Copy link
Copy Markdown
Contributor

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.

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).

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Apr 10, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: bdewater-thatch / name: Bart de Water (44dded4)

@bdewater-thatch bdewater-thatch force-pushed the feat/active-job-perf-metrics branch 3 times, most recently from abd8bc7 to 468f0c2 Compare April 10, 2026 18:18
@github-actions github-actions bot added the ci label Apr 10, 2026
@bdewater-thatch bdewater-thatch force-pushed the feat/active-job-perf-metrics branch from 468f0c2 to b868c9f Compare April 10, 2026 18:19
@github-actions github-actions bot removed the ci label Apr 10, 2026
@bdewater-thatch bdewater-thatch force-pushed the feat/active-job-perf-metrics branch from b868c9f to d1e115e Compare April 10, 2026 18:33
@bdewater-thatch bdewater-thatch changed the title feat(active_job): Add performance metrics to process span feat(active-job): Add performance metrics to process span Apr 10, 2026
@bdewater-thatch bdewater-thatch force-pushed the feat/active-job-perf-metrics branch 3 times, most recently from 41ebc51 to 71ab41f Compare April 10, 2026 18:40
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.
@bdewater-thatch bdewater-thatch force-pushed the feat/active-job-perf-metrics branch from 71ab41f to 44dded4 Compare April 10, 2026 18:42
Copy link
Copy Markdown
Contributor

@thompson-tomo thompson-tomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@bdewater-thatch
Copy link
Copy Markdown
Contributor Author

bdewater-thatch commented Apr 11, 2026

@thompson-tomo thanks for having a look. IMO it is useful to know for a specific job execution what these numbers are, as well as seeing them in aggregate. TIL about exemplars

I can add (or complete change over to metrics, but I didn't see any examples in this repo to work from. I did find a bunch of old pull requests but I'll need to know what the desired direction is.

@arielvalentin
Copy link
Copy Markdown
Contributor

@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.

@bdewater-thatch
Copy link
Copy Markdown
Contributor Author

On today's SIG call we discussed this PR and agreed to an opt-in, I'll work on that this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants