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

Replace runtime.jvm.gc.time/runtime.jvm.gc.count metrics with process.runtime.jvm.gc.duration histogram #6964

Merged
merged 5 commits into from
Nov 15, 2022

Conversation

jack-berg
Copy link
Member

Replaces #6362.

I've reduced the attributes to only record the gc name and the action that was taken (i.e. I've removed the gc cause). If needed we can add the cause later, but for now this should be sufficient to determine total time spent in GC, and categorize time spent as stop the world or parallel.

@jack-berg jack-berg requested a review from a team October 24, 2022 21:44
@PeterF778
Copy link
Contributor

I believe keeping the old GC time and count metrics (as non-monotonic counters) could be beneficial. This could be an option for those who want a quick look at these values without dealing with the complexities of histograms.

@PeterF778
Copy link
Contributor

On second thought, removing the old GC metrics would open a gap that could be filled by the new JMX Metric Insight (PR #6573) with optional JVM metrics grouped into the "jvm" platform.

@jack-berg
Copy link
Member Author

I believe keeping the old GC time and count metrics (as non-monotonic counters) could be beneficial. This could be an option for those who want a quick look at these values without dealing with the complexities of histograms.

The view API allows you change the aggregation for instruments. If you don't need the histogram buckets, you ca configure the histogram to have a single bucket, which essentially transforms it into a sort of "summary" containing the sum, count, min, and max. You can look at just the sum and count to get the current gc time and count.

trask pushed a commit that referenced this pull request Nov 2, 2022
In the 10/27 java sig we discussed that it would be valuable to
enumerate the attributes reported for memory pool and gc metrics when
different gcs are used.

I've went ahead and added a readme for the runtime metrics which
includes detailed information on the attributes reported. Note that I
also have the same data for gc metrics added in #6964 and #6963, but
will wait to add until those PRs are merged.
@jack-berg
Copy link
Member Author

What are folks' thoughts on merging this before or after open-telemetry/opentelemetry-specification#2903? I'm slightly partial to waiting until the spec PR is merged to avoid churn for users.

@trask trask modified the milestone: v1.20.0 Nov 8, 2022
@jack-berg
Copy link
Member Author

Spec PR has been merged and this is unblocked!

@trask trask changed the title Replace gc metrics with process.runtime.jvm.gc.time histogram Replace runtime.jvm.gc.time/runtime.jvm.gc.count metrics with process.runtime.jvm.gc.duration histogram Nov 15, 2022
@trask trask merged commit e39e5a6 into open-telemetry:main Nov 15, 2022
@yorikya
Copy link

yorikya commented Apr 16, 2023

Hey folks, after upgrading from io.opentelemetry.instrumentation/opentelemetry-runtime-metrics "1.16.0-alpha" --> io.opentelemetry.instrumentation/opentelemetry-runtime-metrics "1.24.0-alpha" I had a few issues with this metric:

  1. I don't get the gc:ps_marksweep tag anymore
  2. The numbers I am getting look strange, before the upgrade I had: avg:runtime.jvm.gc.time: 5.39K (gc:ps_scavenge) and after the upgrade I am getting process.runtime.jvm.gc.duration: 12 (gc:ps_scavenge). I know that the metric type has been changed from the gauge to the histogram
    Screenshot 2023-04-16 at 15 36 03
    but the numbers look odd to me

@trask
Copy link
Member

trask commented Apr 16, 2023

hi @yorikya! can you open a fresh issue for this?

@yorikya
Copy link

yorikya commented Apr 17, 2023

Sure, thank you #8305

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants