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)

Most upvoted comments

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.

private <T> void registerGauge(String name, T obj, ToDoubleFunction<T> f, String description, Iterable<Tag> tags) {
        Gauge gauge = meterRegistry.find(name).tags(tags).gauge();
        if (gauge != null) {
            meterRegistry.remove(gauge);
        }

        Gauge.builder(name, obj, f)
                .description(description)
                .tags(tags)
                .register(meterRegistry);
    }

@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.

AtomicReference stateRef = new AtomicReference(new State());
Gauge
    .builder("test.gauge", stateRef, ref -> ref.get().getValue())
    .register(registry);

You can get your state object by stateRef.get() and replace it by stateRef.set(new State()). Btw Micrometer does support KafkaMetrics (and Kafka removing the references is a pain), I recommend you to check it.

can you please share what was the motivation to use week reference for gauge ?

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.

what will heppen if implementation was working without week reference …

If Micrometer would use strong reference, it could cause memory leaks.

I think shifting the responsibility from the framework micrometer to the application that use micrometer

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.

and from heap size point of view we are ending with the same heap size for the java process

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.

why the framework can not hold this map of weak-referenc objects internaliy

I hope I answered this above.

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

I think I do. Also, please take a look at MultiGauge, it might do what you need to register lots of gauges.

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 ?

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);

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 …?

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.)

can we have guauge with strong reference … so on find we will have way to update the state object ?

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.

There is an implementation diffents between the two method for creating gague …, can not tell if this by intention or not … and suggest to open a new issue on that …

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 the lag variable).

why from user prespective we need more the single way to create a meter , keep thing simple provide single way to do it

Those things are quite different: the first one uses a Number as the state object, the second one uses a Supplier<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).

if you chose to provide more than one way to build a metric object make sure that both object are working the same underhood .

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 the Gauge correctly and you have a strong reference to the lag 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:

AtomicInteger myGauge = meterRegistry.gauge("myGauge", new AtomicInteger(0));
myGauge.set(1);

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 and dec 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 is

  • when user login, MetricsTool.gauge("user", "level", "3").inc(1)
  • when user logout, 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.

  def dec(name: String, tags: String*)
         (value: Double = 1.0): Unit = metrics { ms =>
    ms.registry.gauge(name, Tags.of(tags: _*), new AtomicDouble).addAndGet(-value)
  }

Further, such implementation doesn’t use strong reference so the ref AtomicDouble will disappeared after GC.

Would you like give me some advices? @shakuzen