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

Gauge that allows setting value #1126

Closed
xiaochuanyu opened this issue May 1, 2017 · 9 comments
Closed

Gauge that allows setting value #1126

xiaochuanyu opened this issue May 1, 2017 · 9 comments
Labels
Milestone

Comments

@xiaochuanyu
Copy link

xiaochuanyu commented May 1, 2017

I find the current API for gauges awkward to use in certain situations where I simply just to want to set the gauge to arbitrary values. It is not always convenient to specify upfront a lambda or inner class required for providing values to a Gauge.

To illustrate, I extend the contrived example from the Getting Started page as follows:

public class QueueManager {
    private final Queue queue;
   
    public QueueManager(MetricRegistry metrics, String name) {
        this.queue = new Queue();
        sizeGauge = metrics.register(MetricRegistry.name(QueueManager.class, name, "size"), new SettableLongGauge());
    }

    public void doSomethingToQueue() {
         // ... 
         sizeGauge.setValue(something);
    }

    public void doSomethingElseToQueue() {
         // ... 
         sizeGauge.setValue(somethingElse);
    }
}

In order to have an API like above I currently copy-paste around the classes from here:
https://github.com/signalfx/signalfx-java/tree/master/signalfx-codahale/src/main/java/com/signalfx/codahale/metrics

I'd like to propose in include this in the main library itself.

@emste
Copy link

emste commented May 16, 2017

To give another idea and/or help for someone who wants to work around this: If you are using spring boot actuator, their dropwizard wrapper does support updating a gauge: DropwizardMetricServices.java#L202

If you are not using the spring boot framework, you can still take a look at the implementation: There is a custom SimpleGauge class that implements dropwizards Gauge interface and provides a method to directly set the current Gauge value.

@joschi
Copy link
Member

joschi commented May 24, 2020

@xiaochuanyu Would you be able to provide a pull request for this feature?

@xiaochuanyu
Copy link
Author

@joschi
I'm not familiar with the code base here but I can give it a try.
Would appreciate if there is guidance on where to start.

@the-thing
Copy link
Contributor

@xiaochuanyu

Look around the code first - it's not complex at all. All things have their own place. Here is my suggestion, how you could go about it.

You probably want to start with creating a newly documented interface that extends existing com.codahale.metrics.Gauge with setValue method. Then you want to have a default implementation of this settable gauge that holds the reference to the value (needs to be thread safe). Check if MetricRegistry needs new methods for the new gauge or existing ones can be retrofitted without breaking backwards compatibility. Add tests for the new implementation. Additionally, prove the new interface is compatible with metric registry and reporters (investigate what needs adding or changing).

Things to think about. There are existing Gauge implementations that might need attention. Do they need to support new feature?

  • JmxAttributeGauge - there is no need for this to be settable as this is internal to JVM and it will pull values from JVM
  • CachedGauge - there is not much reason of setting the value on a cached gauge as this is supposed to load values from slower resources
  • RatioGauge - for the ratio gauge to be settable you would have to expose two setters which would not comply with the new interface (this could be another task if required)
  • DerivativeGauge - this cannot be extended without breaking compatibility as support for reverse transformation is required T -> F. If this is necessary a new implementation of e.g. SettableDerivativeGaugeneeds to be implemented (possibly by extending DerivativeGauge).

Hope this helps. Let me know if you are planning to work on this, if not I will be happy to pick it up.

@xiaochuanyu
Copy link
Author

@the-thing thanks for the very detailed explanations.
I'm probably gonna take a stab at the implementation this week. I'll let you know if I don't get to that.

@xushiyan
Copy link

@xiaochuanyu Thanks for the raising the issue and making the changes. It'll be a useful update. @the-thing Seeing that the issue is tagged to 4.2.0 release, wonder if there is any estimated release date? Thanks.

@the-thing
Copy link
Contributor

@xushiyan I'm not to say as I'm not a member of dropwizard. The pull requests requires reviews from members as well. Usually things get merged pretty fast as long as the approval comes in.

@xushiyan
Copy link

xushiyan commented Jun 24, 2020

@xushiyan I'm not to say as I'm not a member of dropwizard. The pull requests requires reviews from members as well. Usually things get merged pretty fast as long as the approval comes in.

@the-thing Thanks for the info!
@joschi Would you kindly suggest an estimate on 4.2.0 release date please?

@github-actions github-actions bot added the Stale label Dec 22, 2020
@dropwizard dropwizard deleted a comment from github-actions bot Jan 2, 2021
@joschi joschi removed the Stale label Jan 2, 2021
@joschi joschi removed the help wanted label Jan 2, 2021
@joschi
Copy link
Member

joschi commented Jan 2, 2021

Merged in #1607 (f4d84b6).

@joschi joschi closed this as completed Jan 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants