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

JvmHeapPressureMetrics have incorrect base unit #3236

Closed
fkrull opened this issue Jun 17, 2022 · 9 comments
Closed

JvmHeapPressureMetrics have incorrect base unit #3236

fkrull opened this issue Jun 17, 2022 · 9 comments
Labels
bug A general bug instrumentation An issue that is related to instrumenting a component module: micrometer-core An issue that is related to our core module release notes Noteworthy change to call out in the release notes
Milestone

Comments

@fkrull
Copy link

fkrull commented Jun 17, 2022

Describe the bug
The unit for the metrics "jvm.memory.usage.after.gc" and "jvm.gc.overhead" in JvmHeapPressureMetrics.java is set as percent, but the values are in range [0...1]. This confuses Datadog (and possibly other registries?) which shows these metrics as e.g. "0.1%" instead of 10%.

Environment

  • Micrometer 1.9.0
  • Datadog

Expected behavior
The unit should match the value range so that Datadog can display it properly. Datadog seems to support a "fraction" unit that works for this range.

@shakuzen shakuzen added registry: datadog A Datadog Registry related issue waiting-for-triage labels Jul 6, 2022
@marcingrzejszczak
Copy link
Contributor

We can't change the values in a way that would be backward compatible. Are you saying that what we send to Datadog is in a format that is not correct?

@marcingrzejszczak marcingrzejszczak added the waiting for feedback We need additional information before we can continue label Dec 27, 2023
@fkrull
Copy link
Author

fkrull commented Dec 27, 2023

No, I'm saying that the value range does not match the unit. The values for that metric are 0-1, but the unit is "percent" so Datadog expects 0-100. So then when you graph these metrics, Datadog will automatically show you readings like "1% GC overhead" because of the unit it receives, and then you're like, "oh, 1%, that's not very much at all" when in fact it is 100% because the value range doesn't match the unit. And then you have to know that the value range and the unit don't match and multiply with 100 or explicitly override the unit to make it show correct things.

(Unless any of this changed in the past year and a half, I can't really check right now.)

Copy link

github-actions bot commented Jan 4, 2024

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

Copy link

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 11, 2024
@jonatan-ivanov jonatan-ivanov removed waiting for feedback We need additional information before we can continue waiting-for-triage feedback-reminder labels Jan 19, 2024
izeye added a commit to izeye/micrometer that referenced this issue Feb 22, 2024
@izeye
Copy link
Contributor

izeye commented Feb 22, 2024

It seems to be technically wrong and misleading as reported by @fkrull. I created #4765 to try to resolve it.

@shakuzen shakuzen reopened this Feb 27, 2024
@shakuzen shakuzen added bug A general bug module: micrometer-core An issue that is related to our core module instrumentation An issue that is related to instrumenting a component and removed closed-as-inactive registry: datadog A Datadog Registry related issue labels Feb 27, 2024
@shakuzen shakuzen changed the title Metrics in JvmHeapPressureMetrics display incorrectly in Datadog due to percent/[0...1] confusion JvmHeapPressureMetrics have incorrect base unit Feb 27, 2024
@shakuzen shakuzen added this to the 1.13.0-M2 milestone Feb 27, 2024
@shakuzen shakuzen added the release notes Noteworthy change to call out in the release notes label Feb 27, 2024
@shakuzen
Copy link
Member

shakuzen commented Feb 27, 2024

Thanks for the clarification @fkrull and sorry the issue was closed. Thanks to @izeye's pull request, going forward the unit will not be specified. If you would like to test it out, the changes are available in 1.13.0-SNAPSHOT in the snapshot repo now before the 1.13.0-M2 pre-release.

Edit: fix version numbers

@izeye
Copy link
Contributor

izeye commented Feb 27, 2024

@shakuzen Typos in the versions for 1.13.0-SNAPSHOT and 1.13.0-M2.

@shakuzen
Copy link
Member

@izeye Thanks for catching that. I've edited my comment now.

@fkrull
Copy link
Author

fkrull commented Mar 2, 2024

@shakuzen @izeye I can't really test this out any more, but thanks for getting this fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A general bug instrumentation An issue that is related to instrumenting a component module: micrometer-core An issue that is related to our core module release notes Noteworthy change to call out in the release notes
Projects
None yet
Development

No branches or pull requests

5 participants