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

JMX reset operation on metrics #143

Closed
kgs opened this issue Jan 17, 2012 · 25 comments
Closed

JMX reset operation on metrics #143

kgs opened this issue Jan 17, 2012 · 25 comments

Comments

@kgs
Copy link

kgs commented Jan 17, 2012

It would be really great to have operation "reset" on every metric exposed via JMX.

@amir20
Copy link

amir20 commented Mar 14, 2012

Would this be as simple as adding Resettable interface with this in Meter?

    @Override
    public void reset() {
        count.set(0);
    }

I would be more than happy to do this and send you a pull request. It is some what messing up our counts. :)

@stampy88
Copy link

I could really use this feature also, and be happy to do it as well.

@codahale
Copy link
Contributor

Definitely not. Concurrency and reset operations don't play nicely.

@smougenot
Copy link

IMO there is a huge difference between API provides something and coders use it.
If for some use cases a user need the reset : just indicate the risks.

Use case :

  • test cases in a project using Metrics.

@chrbayer84
Copy link

Agree with smougenot. I just need this for a test case. Call the method __reset() or hide it in an implementation method so that the user needs to cast to get to it or the like. No one should call this in production.

@aryaKetan
Copy link

If I am using a scheduled reporter to flush out metrics to a persistent storage like Opentsdb or graphite, then I need to reset metrics , otherwise my counts will always include historical event-counts.
How do I solve for that without reset Metrics ??

@costimuraru
Copy link

+1 to what @aryaKetan said

@ryantenney
Copy link
Contributor

Timer and Meter both use exponentially weighted moving averages and histograms with exponentially decaying reservoirs to create a bias toward more recent data, thereby making it unnecessary to ever reset the metrics.

A histogram with an exponentially decaying reservoir produces quantiles which are representative of (roughly) the last five minutes of data. It does so by using a forward-decaying priority reservoir with an exponential weighting towards newer data.

@ryantenney ryantenney removed this from the 3.0.0.Beta1 milestone Aug 21, 2014
@himanshug
Copy link

@ryantenney : problem with exponentially decaying reservoir is that if no new data comes in, it will keep on giving the same numbers all the times. For example, let say you update a timer with 5 and 7 (then don't put anything at all) , then no matter when you see (even after x hours), timer will still show the average to be 6 which is not representative of last 5 mins by any means.
so, it works only if data is arriving all the time.

@himanshug
Copy link

also, what will be your suggested workaround if not an implemented solution.

@mohit-gupta
Copy link

I'm facing similar issue while migrating from Yammer 2.2.0 to Codahale 3.0.2. In my use case, need to calculate the exacts event-counts and time average in a specific period of time.
Had been using the reset/clear methods as supported by the earlier version ( yammer ). But the updated library does not support these methods. So, what's the suggested workaround now? Shall I remove and register the metric after every interval?

@ryantenney
Copy link
Contributor

@mohit-gupta For the scenario you describe it sounds like you should be using a timer with a SlidingTimeWindowReservoir.

@mohit-gupta
Copy link

@ryantenney thanks for the quick reply. Couple of things:

  1. SlidingTimeWindowReservoir higher memory requirements has it keeps all the measurements in the window in-memory which becomes unacceptable at large number of events. Also, just need mean-time during the period.
  2. Counters also don't have reset operation.
    Does it makes sense to use AtomicLong for counting and own implementation for mean time ( just need to keep sum and event-count in memory) and register them in MetricRegistry as Gauge. This way they can be reset after the interval.

Also, intrigued to know why the reset operations were supported before but not how. Concurrency issues should have been there before as well.

Thanks in Advance!

@mohit-gupta
Copy link

@codahale @ryantenney would appreciate any help!

thnks

@tguzik
Copy link
Contributor

tguzik commented Nov 26, 2014

Is the real requirement to reset the registry or just to get the deltas of the values between two points in time?

If it is the latter then I guess that the quick'n'dirty solution would be to compare immutable snapshots of the registry from two points in time. While this could work a-ok for simple values (Counters), there would have to be a more elaborate way to handle values based on Reservoir.

@roytruelove
Copy link

+1 on this. Very sad but I can't use this library without the ability to reset.

@wrprice
Copy link

wrprice commented Mar 19, 2015

+1

@amanhigh
Copy link

After reading all responses i see only way to reset is using some or other hack. A Simple Counter which resets after reporting data or fixed time interval becomes difficult to realize.

One way is described here.
http://evertrue.github.io/blog/2013/10/14/resettable-counters-with-codahale-metrics/

@jmason
Copy link

jmason commented Mar 24, 2015

There's another workaround, BTW.

If the desire is to zero Histograms and Timers after each report, this can be accomplished by using the HdrHistogramResetOnSnapshotReservoir from https://bitbucket.org/marshallpierce/hdrhistogram-metrics-reservoir/ ; this is pretty clean.

Alternatively, if you want to reset counters, another way is to subclass the desired Reporter class, use that subclass in your own app, and add an override processCounter() method which uses:

        long count = counter.getCount();
        // report the value in the usual way
        counter.dec(count);  // reset the counter for the next iteration

(pretty similar to @amanhigh 's suggestion, but without the creation of a Gauge.)

@jan-molak
Copy link

@jmason - thanks for your suggestion, Justin. We're facing the same difficulty with gauges and counters being reported to the db even though their state hasn't been updated (ie. the client reporting them is down).

I can't see the processCounter() you're referring to in the 3.1.0 code base, could you please point me to the right place?
There is for example the ScheduledReporter::report method, but then resetting the state of stats at that stage would mess up any other reporters listening on the same MetricRegistry...

It's possible to add a ResettingReporter that would affect the state of the MetricRegistry when it's called, but this would result in similar synchronisation issues as described above.
It looks like every reporter is running on a separate thread, and with a separate polling schedule - which gives @codahale more flexibility (you can have as many reporters as you want and don't need to care about their schedule), but takes away the control over the process.

I think the reason why Etsy's statsd supports resetting metrics upon a flush is that because the way they've implemented it it's the "MetricRegistry"that's in control, rather than the reporters. Their equivalent of MetricRegistry sends a flush event to all its listeners, and then resets the metrics.
This way you don't have the flexibility of running reporters with different polling intervals*, but you can make sure that if the metric hasn't been updated within a given interval - it wouldn't be (incorrectly) reported to whatever db you're using.

Thinking about it, it seems like it might make sense for something to look after the state of the MetricRegistry and periodically clean up the state of metrics that haven't been updated recently?** Although this would probably result in synchronisation issues anyway.

Alternatively, each Reporter could receive a snapshot of state of the MetricRegistry with metrics cleaned up according to it's own flush interval. This way in the ScheduledReporter rather than getting the data directly from the MetricRegistry:

    public void report() {
        synchronized (this) {
            report(registry.getGauges(filter),
                    registry.getCounters(filter),
                    registry.getHistograms(filter),
                    registry.getMeters(filter),
                    registry.getTimers(filter));
        }
    }

we could obtain a snapshot from somewhere first:

    public void report() {
        synchronized (this) {
            MetricRegistry snapshot = registry.getSnapshotFor(period); 
                  // the snapshot would filter out any stats 
                  // that haven't been updated within the specified period
                  // This could also be configurable per reporter; as a template method for instance
            report(snapshot.getGauges(filter),
                    snapshot.getCounters(filter),
                    snapshot.getHistograms(filter),
                    snapshot.getMeters(filter),
                    snapshot.getTimers(filter));
        }
    }

Thoughts? Would that be a sensible approach? Would you consider such pull request, @codahale, @ryantenney


(*) well, you could have, but it would be tricky to implement

(**) recently as in: take the lowest common denominator of all the reporters' intervals and delete all the metrics that haven't been updated within that period?

@jmason
Copy link

jmason commented Apr 21, 2015

'There is for example the ScheduledReporter::report method, but then resetting the state of stats at that stage would mess up any other reporters listening on the same MetricRegistry...'

Yeah -- that's a limitation we've chosen to accept in order to be able to do this (we're using a StatsdReporter which delivers to statsd, which requires deltas rather than absolute values). It would be nice to come up with an approach that didn't bring that limitation.

@DanielYWoo
Copy link

+1

@wyzssw
Copy link

wyzssw commented Sep 10, 2015

I WANT RESET +1

@wyzssw
Copy link

wyzssw commented Sep 10, 2015

TO COUNT DAU

@nailding1
Copy link

I ALSO WANT RESET !!!

@dropwizard dropwizard locked and limited conversation to collaborators Oct 21, 2015
@dropwizard dropwizard locked and limited conversation to collaborators Oct 21, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests