Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails integration: trace Active Storage events #1558

Closed
georgeclaghorn opened this issue Sep 1, 2021 · 2 comments · Fixed by #1588
Closed

Rails integration: trace Active Storage events #1558

georgeclaghorn opened this issue Sep 1, 2021 · 2 comments · Fixed by #1588
Assignees
Projects
Milestone

Comments

@georgeclaghorn
Copy link

georgeclaghorn commented Sep 1, 2021

Rails’s Active Storage component generates the following ActiveSupport::Notifications events:

  • service_upload.active_storage
  • service_download.active_storage
  • service_streaming_download.active_storage
  • service_download_chunk.active_storage
  • service_delete.active_storage
  • service_delete_prefixed.active_storage
  • service_exist.active_storage
  • service_url.active_storage
  • service_mirror.active_storage
  • service_update_metadata.active_storage
  • preview.active_storage
  • analyze.active_storage

More on these events is available here. All are worth tracing, except maybe service_url, as they’re likely to be responsible for a significant portion of runtime in transactions where they occur. For example, here’s a request in Sentry where uploading to S3 takes the majority of the total duration but isn’t represented:

Sentry performance tab screenshot

I think this should be relatively straightforward using the existing subscriber system in sentry-rails.

@st0012 st0012 added this to To do in 4.x via automation Sep 2, 2021
@st0012 st0012 added this to the 4.8.0 milestone Sep 2, 2021
@st0012 st0012 self-assigned this Sep 2, 2021
@st0012
Copy link
Collaborator

st0012 commented Sep 2, 2021

@georgeclaghorn thanks for the suggestion. I'll add it in version 4.8.0 🙂

@st0012
Copy link
Collaborator

st0012 commented Oct 2, 2021

@georgeclaghorn I've implemented the feature in #1588. But because I can only test it against the simple example app locally, it'd be great if you can also test this branch (even just locally) before I merge it 🙂

4.x automation moved this from To do to Done Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
4.x
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants