Rename gc_total_time to gc_time #874
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Update naming to only be about the time measured. It no longer reports the total time, but reports the delta of two measurements. Part of #868
|
✔️ All good! |
I forgot to add it to the original commit. Let's squash merge the PR this is a part of. [skip ci]
unflxw
approved these changes
Aug 4, 2022
| type: "change" | ||
| --- | ||
|
|
||
| Rename the (so far privately reported) `gc_total_time` metric to `gc_time`. It no longer reports the total time of Garbage Collection measured, but only the time between two (minutely) measurements. |
There was a problem hiding this comment.
What is meant by "privately reported"?
There was a problem hiding this comment.
I mean we don't show it anywhere. We added it for the Ruby VM dashboard but then took the graph display it out at the last minute. People could make a graph themselves, but they'd have to find it in the reported metrics list and then they probably don't know what the values mean. So privately because it wasn't shown anywhere and it's not documented anywhere.
luismiramirez
approved these changes
Aug 4, 2022
tombruijn
added a commit
to appsignal/public_config
that referenced
this pull request
Aug 4, 2022
Add the Garbage Collection time graph back to the Ruby VM dashboard after it was added and removed in PR #38. I've updated the metric name to `gc_time` in Ruby gem PR: appsignal/appsignal-ruby#874 This graph will not have any data by default. We need a way to redirect users to our docs on how to enable this. We also need to write docs for this, basically saying: Add `GC::Profiler.enable` to your app, but not in production or not for long in production because it may adversely impact the performance of your app. Also, to update existing dashboards, I refer to the dashboard update issue we should pick up: appsignal/appsignal-server#4770 Part of appsignal/appsignal-ruby#868
tombruijn
added a commit
to appsignal/public_config
that referenced
this pull request
Aug 4, 2022
Add the Garbage Collection time graph back to the Ruby VM dashboard after it was added and removed in PR #38. I've updated the metric name to `gc_time` in Ruby gem PR: appsignal/appsignal-ruby#874 This graph will not have any data by default. We need a way to redirect users to our docs on how to enable this. We cannot link to this from the graph description. We also need to write docs for this, basically saying: Add `GC::Profiler.enable` to your app, but not in production or not for long in production because it may adversely impact the performance of your app. Also, to update existing dashboards, I refer to the dashboard update issue we should pick up: appsignal/appsignal-server#4770 Part of appsignal/appsignal-ruby#868
tombruijn
added a commit
to appsignal/public_config
that referenced
this pull request
Aug 5, 2022
Add the Garbage Collection time graph back to the Ruby VM dashboard after it was added and removed in PR #38. I've updated the metric name to `gc_time` in Ruby gem PR because it doesn't report the total time anymore: appsignal/appsignal-ruby#874 appsignal/appsignal-ruby#867 This graph will not have any data by default. We need a way to direct users to our docs on how to enable this. We cannot link to this from the graph description that I could tell, the graph description does not support any link format. We need to write some docs for this, basically saying: Add `GC::Profiler.enable` to your app, but not in production or not for long in production because it may negatively impact the performance of your app. Also, to update existing dashboards, I refer to the dashboard update issue we should pick up: appsignal/appsignal-server#4770 Part of appsignal/appsignal-ruby#868
tombruijn
added a commit
to appsignal/public_config
that referenced
this pull request
Aug 15, 2022
Add the Garbage Collection time graph back to the Ruby VM dashboard after it was added and removed in PR #38. I've updated the metric name to `gc_time` in Ruby gem PR because it doesn't report the total time anymore: appsignal/appsignal-ruby#874 appsignal/appsignal-ruby#867 This graph will not have any data by default. We need a way to direct users to our docs on how to enable this. We cannot link to this from the graph description that I could tell, the graph description does not support any link format. We need to write some docs for this, basically saying: Add `GC::Profiler.enable` to your app, but not in production or not for long in production because it may negatively impact the performance of your app. Also, to update existing dashboards, I refer to the dashboard update issue we should pick up: appsignal/appsignal-server#4770 Part of appsignal/appsignal-ruby#868
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update naming to only be about the time measured.
It no longer reports the total time, but reports the delta of two
measurements.
Part of #868