rcrowley / go-metrics

Go port of Coda Hale's Metrics library
Other
3.43k stars 493 forks source link

Increment and Decrement methods on Gauge #193

Closed a-robinson closed 6 years ago

a-robinson commented 7 years ago

How would you feel about supporting Inc() and Dec() methods on the gauge type? A motivating use case that's come up for me is wanting to track the number of things that are currently in some undesirable state but may leave that state later.

For example, to track the number of requests that have been waiting on a response for more than a minute -- we'd increment the gauge once a request had been waiting for at least a minute, and decrement the gauge once the request completes. Doing this with a gauge gives more information than with a counter, since we can decrement it once the request is done.

Adding these methods would add incredibly little code to the repo. If this sounds alright, I'll send a quick PR for it. What do you think?

@mihasya

a-robinson commented 7 years ago

For what it's worth, the prometheus instrumentation library supports these methods: https://github.com/prometheus/client_golang/blob/v0.8.0/prometheus/gauge.go#L24

a-robinson commented 7 years ago

Put together #195 as an example implementation

mihasya commented 7 years ago

Thanks for the ping, and sorry for the delay. This feels like something that should be solved with an FunctionalGauge where the function returns some internal value that can go up or down, rather than by changing the interface of a gauge. Is there a reason that won't work for your case? A gauge is meant to read out a value at a time, whereas this feels like using it to maintain some internal application state.

(edited because I got the name of FunctionalGauge wrong)

a-robinson commented 7 years ago

That could certainly work, it's just not quite as useful of an interface since you then have to manage a separate variable and make sure it's clear to the reader that it's only there for the sake of the metric. For what it's worth, I've just reimplemented Gauge in the relevant code base instead. I mainly brought this up because I think it's a useful method for gauges and that you may want to include it, not because it's a problem that can't be solved in another way.

In case you're curious, here's an example use of this pattern. We detect that a request has been taking a long time to complete, increment the counter to indicate to the monitoring system that there is one more slow request currently in the system, and defer a decrement such that once we're done waiting on the slow request we'll update the metric appropriately. The gauge is indeed used as a way of tracking the value at a given point in time, and isn't used for any application logic, just for monitoring.

Feel free to close this if you think the problem is best left to clients :)

sandipb commented 6 years ago

Using a FunctionalGauge() seems an overkill for this feature. Many accounting use cases need to add/decrement a value to keep track of, say the number of currently executing jobs.

The Counter type, which is supposed to be a monotonically increasing value, having the Dec() function, and not the Gauge, seems pretty odd.