appsignal-ruby icon indicating copy to clipboard operation
appsignal-ruby copied to clipboard

Track GC total time

Open tombruijn opened this issue 2 years ago • 1 comments

In PR https://github.com/appsignal/appsignal-ruby/pull/852 we added tracking for gc_total_time, renamed to gc_time in PR #874 . But then in PR https://github.com/appsignal/public_config/pull/38 I've removed the GC total time graph from the Ruby VM dashboard because it's empty by default. To avoid confusion and an increase in support conversations I've removed the graph.

To make this metric work, the app needs to call GC::Profiler.enable to enable the GC profiler that Appsignal::GarbageCollectionProfiler relies on. This is not on by default in any app.

We have a way to enable this in the Ruby gem with the undocumented enable_gc_instrumentation config option. We never shipped and documented this. My understanding was that it wasn't done (nothing is done with the data on the frontend) and it wasn't tested enough. I don't fully know the impact of enabling the GC Profiler by default and in the moment (Thursday afternoon 2022-07-28) I didn't want to ship this to every app using our Ruby gem.

We need to figure out the impact of enabling the GC profiler, decide if we want to enable it by default or put it behind the enable_gc_instrumentation config option, and document that. We have decided not to enable it by default. (#876)

Also required for this is PR #867 that fixes the gc_time metric. To update the dashboard on the server side we're blocked on https://github.com/appsignal/appsignal-server/issues/4770.

To do

  • [x] Decide what to do with the Transaction level GC instrumentation (ask Thijs)
  • [x] Add docs for:
    • [x] ~enable_gc_instrumentation config option, or not if we decide to remove it from the Ruby gem~
    • [x] GC::Profiler.enable requirement and usage for GC instrumentation dashboard: https://github.com/appsignal/appsignal-docs/pull/648
  • [x] Deploy new graph to dashboard
    • [x] Merge linked PR
    • [x] Submit PR with updated submodule to appsignal-server repo
  • [x] Release Ruby gem 3.1.4
  • [ ] Deploy server and run updates
  • [ ] Merge docs PR

And then decide how and when to update the existing dashboards.

tombruijn avatar Jul 29 '22 07:07 tombruijn

While performing the daily checks some issues were found with this issue.

  • This issue has not had any activity in 14 days. Please provide a status update if it is still relevant. Closed it if it is no longer relevant. Or move it to another column if it's blocked or requires another look at it. - (More info)

New issue guide | Backlog management | Rules | Feedback

backlog-helper[bot] avatar Aug 30 '22 07:08 backlog-helper[bot]