Open lenin-jaganathan opened 1 year ago
I am not sure such behavior was created. But ideally, using the below snippet for class unloading will help solve this issue.
I see that jvm.classes.loaded metric is the number of classes that are loaded currently and this being a gauge makes sense.
jvm.classes.unloaded - is the total number of classes unloaded since the JVM has started, so this should not be a gauge but rather a counter.
Will it be informational to also expose "the total number of classes loaded" by the JVM?
Upvoting this
Some questions:
jvm.classes.loaded
is a gaugejvm.classes.unloaded
is a counter not a gauge, right?So is this issue about adding the "the total number of classes loaded" ? Are you upvoting this @patpatpat123 ?
Hello @marcingrzejszczak ,
Thank you for your input.
First of all, you are correct about the two metrics:
jvm_classes_loaded metric_type=gauge statistic=value
jvm_classes_unloaded metric_type=counter statistic=count
And yes, it would be great to have metrics unifying those, to allow building visuals such as a loaded vs. unloaded counter.
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.
I am voting for the below 2 things,
jvm.classes.unloaded
to make the statement valid for Step registries. Currently, it says "classes loaded from JVM start" but this will not be true for a StepRegistry.ClassLoadingMXBean#getTotalLoadedClassCount
. Ideally, this metric should be named "jvm.classes.loaded" but we already have a metric with the same name. We need to sort that out.Thanks for the update. Are you willing to file a PR for this @lenin-jaganathan ?
I think there is an open question on how we handle the names,
jvm.classes.unloaded
- is a sorted thing.
The opposite of this metric would be jvm.classes.loaded
using info from ClassLoadingMXBean#getTotalLoadedClassCount
but that name is already taken. Today, we already have a metric named jvm.classes.loaded
which reports the number of classes loaded currently. Ideally, this should have been jvm.classes.count
or jvm.classes.current.count
. I am not sure how will be able to handle this change. This will be a breaking change but in metric space emitting a metric with the same name but representing different info would be very dangerous.
Also see what OTEL does in this space.
We could do sth like this
jvm.classes.unloaded
for a counter of total unloaded classesjvm.classes.loaded
would be the number of classes currently loadedjvm.classes.loaded.count
could be the counter of total loaded classesNow if someone wants to be OTel compliant they can create a MeterFilter
and rename the metrics. If we decide one day that we want to migrate that will also mean that we would have to provide such a MeterFilter
and give users a couple of minor versions to migrate to it and then change the names in core and deprecate the MeterFilter
.
WDYT @shakuzen @jonatan-ivanov ?
I think if the names for classes loaded and unloaded don't match, it will cause a bit of confusion. However, since both of them are counters, I am not opposed to having a .count
suffix for both of them.
I am proposing the below changes for the class loading metrics,
jvm.classes.loaded
- Gauge - Gives the number of classes currently loaded (Keep it as is)jvm.classes.unloaded.count
- FunctionCounter - Tracks the number of classes unloaded (Rename existing metric jvm.classes.unloaded
to jvm.classes.unloaded.count
- this avoids confusion but will be a breaking change)jvm.classes.loaded.count
- FunctionCounter - Tracks the number of classes loaded (Generate a new metric)@marcingrzejszczak / @shakuzen / @jonatan-ivanov Let me know what you guys think. Apart from the breaking change, there is no problem with the above. This is similar to what was proposed in but just that we add .count
to the counter.
One another option would be to emit, jvm.classes.unloaded
as an additional metric for one/two minor versions before removing it. But, that really does not mean a lot because unlike class deprecation I don't think people will be closely following the internal details.
I would call this metric name change out in release notes and move with option 1. Let me know what you guys think.
Describe the bug Currently, the JVM class unloading metric is being registered as a Function counter. The description says "The total number of classes unloaded since the Java virtual machine has started execution" which will not be true when this metric is used in a Step Meter Registry.
Environment
To Reproduce How to reproduce the bug: Start the java application with the micrometer JVM binder enabled. Use any step registry to register these metrics. After one step is completed, the unloaded class becomes zero.
Expected behavior jvm.classes.unloaded - should represent the total number of classes unloaded since the Java virtual machine has started execution