micrometer: MeterRegistry.gauge not returning value reference of already existing Gauge
When running the following code:
public void testGauge(MeterRegistry meterRegistry) {
// Creating the Gauge
AtomicInteger value1 = meterRegistry.gauge("myTestGauge", new AtomicInteger(0));
// Getting the same Gauge
AtomicInteger value2 = meterRegistry.gauge("myTestGauge", new AtomicInteger(1));
value1.set(2);
value2.set(3);
System.out.println(meterRegistry.get("myTestGauge").gauge().value());
}
I get a value of 2 - when I would expect a value of 3.
It appears that the underlining code for meterRegistry.gauge checks to see if the Gauge already exists and if so uses that Gauge in the code internally. However, the method always returns the reference object you pass in - while not updating the Gauge’s reference. Wouldn’t this be problematic when actually calling value2.set(3). Wouldn’t that set not get monitored - as no Gauge is technically referencing it?
It would be nice to have a method where we present a value if the Gauge is not present. If the Gauge is present, it should just return back the reference object for the Gauge.
Thanks, -Dave
About this issue
- Original URL
- State: open
- Created 4 years ago
- Reactions: 4
- Comments: 32 (17 by maintainers)
This is terribly inconsistent. You can call Timer() and get back the same timer you created earlier. You can use the Guage.builder.register() and get back the same Guage stateObject you created earlier so why is this method different? If you want to get technical, the javadoc is wrong. It claims “will keep a weak reference to the object” when, in fact, it doesn’t maintain a reference to the passed in stateObject at all. If you want to fix this in docs, yes, please explain in the javadoc this irrational behavior. “If the guage already exists, this method does nothing and returns the value object you sent in - not the one associated with the existing guage…”
Maybe we can provide a need to register more than once. We are using the gauge builder with an object reference and a function to be called on that object. The trouble we are having is that the object reference is itself subject to change due to context switches (in a Kafka streams application), that we do not control. We therefore have to update the reference to that object inside the gauge. Our workaround (see code below): Whenever there is a context switch, we search for the gauge in order to check whether it has already been registered, delete it if that’s the case, and register the gauge again with the new reference. This works, but is a bit cumbersome. We would greatly appreciate it, if there was a way to simply reregister a gauge with a different object reference.
@julianrschmidt You can wrap your KafkaMetric or any other “state” object into an
AtomicReference
and pass that to your Gauge; this would enable you to replace the object that provides the value for you.You can get your state object by
stateRef.get()
and replace it bystateRef.set(new State())
. Btw Micrometer does support KafkaMetrics (and Kafka removing the references is a pain), I recommend you to check it.I think to not cause a memory leak. Micrometer should not prevent your objects from collection since it knows nothing about your objects or their lifecycle. The control over them must be in your hands not in Micrometer’s.
If Micrometer would use strong reference, it could cause memory leaks.
Yes, that’s the point, see my answer above: Micrometer should not prevent your objects from collection since it knows nothing about your objects or their lifecycle. The control over them must be in your hands not in Micrometer’s.
If things can be collected but Micrometer prevents GC to collect them that can cause a memory leak which can mean significant GC overhead or out of memory errors.
I hope I answered this above.
I think I do. Also, please take a look at
MultiGauge
, it might do what you need to register lots of gauges.I think the point of view is the opposite here. You have something (the state object) that you manage, control, and you know how to get. You pass it to Micrometer, why should it give back to you? You already have it I doubt it is only possible to fetch it at Gauge creation. A Gauge is not a “container” and also see above: you should not really interact with it. There are also scenarios where doing this would be quite weird:
Gauge.builder("test", () -> 1).register(registry);
I think I might lost track of the context here. What is this solving and why would you re-register a gauge? (You can set the value of your state object and if you want to replace that, there is always
AtomicReference
.)Yes, you can use a
Gauge
with strong reference, you need to tell explicitly that you want this see its builder:.strongReference(true)
but I’m not sure I understand the second part of this.Please open a new issue for this with a minimal reproducer in Java (please double check that you are using the
Gauge
correctly and you have a strong reference to thelag
variable).Those things are quite different: the first one uses a
Number
as the state object, the second one uses aSupplier<Number>
(with no “real” state object). If you meant why we have.register
and Builder too: register is just a shortcut for the most common/simple things (also, we can’t remove it).They are different things (
Number
vs.Supplier<Number>
) and they should work slightly differently. Though not as differently that they should cause issues to you if you use them properly so please create a new issue with a reproducer (please double check that you are using theGauge
correctly and you have a strong reference to thelag
variable).@shakuzen - can you please share what was the motivation to use week reference for gauge ? what will heppen if implementation was working without week reference …
regarding @jonatan-ivanov suggestion , keeping a map in memory (key --> weakReference) by the application it self … is as what I think shifting the responsibility from the framework micrometer to the application that use micrometer …and from heap size point of view we are ending with the same heap size for the java process …
why the framework can not hold this map of weak-referenc objects internaliy … , do you think it make sense the application will hold a map of gauge weak-reference object ? if I have 1000 combination on gauge object due to tag values which can be change
the only way I saw to access the weak-reference is upon gauge creation … can we get the weak-reference by api call from gauge ? the only solution I see now is what suggested above … find the current gauge remove it and create a new one … but is this the best solution …?
can we have guauge with strong reference … so on find we will have way to update the state object ?
@shakuzen Consider following code:
Looks valid? Yes, absolutely typical usage. But with current behavior it could be non working due to previous
.gauge("myGauge", ...)
call.Any workaround?
I’m also confusing with such behavior. Gauge in micrometer has significant difference comparing to dropwizard and prometheus.
Gauge in prometheus provides convenient
inc
anddec
methods, and dropwizard has the similar stuff but named counter. Since counter in micrometer should keep monotonous, so I’m trying to use gauge to trace something which can up and down, and my expected API isMetricsTool.gauge("user", "level", "3").inc(1)
MetricsTool.gauge("user", "level", "3").dec(1)
So far, I didn’t find a easy approach to get such implementation.
Here is one approach that I have tried, but failed because the
Metrics.gauge
behavior mentioned by this issue.Further, such implementation doesn’t use strong reference so the ref
AtomicDouble
will disappeared after GC.Would you like give me some advices? @shakuzen