Enhanced Gauge API (#106)

[mgkanani]

Added below functions for Gauge Metric:

• Add(float64)

• Sub(float64)

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[coveralls]

Coverage increased (+0.04%) to 94.317% when pulling 65711ac on mgkanani:enhancements/gauge-api into f266f90 on uber-go:master.

[coveralls]

Coverage increased (+0.04%) to 94.317% when pulling 65711ac on mgkanani:enhancements/gauge-api into f266f90 on uber-go:master.

[mgkanani]

@robskillington, can you please review this PR?

[mgkanani]

@robskillington & @mway, will you please review or tag the persons who can review?
My apologies for bothering you.

[mway]

@robskillington & @mway, will you please review or tag the persons who can review?
My apologies for bothering you.

No problem, thank you for the PR!

At a high level, I have two concerns with this change:

1. It's a breaking change. Interface additions change the contract that must be satisfied by implementors, thus this could cause downstream builds to begin failing until devs added/mocked/etc the new methods. Not to say that we're not able to do major revision bumps, but at a minimum, we do tend to batch them together.
2. This significantly increases the implicit complexity of gauges. Because Tally is largely predicated on its underlying configured reporters (i.e., Tally is push-based, versus other pull-based systems), adding increase/decrease methods to tally.Gauge means that either (a) they must be stateful or (b) the user now must be aware of underlying reporting intervals and where they are in time relative to the boundary of the current reporting cycle. In other words: if you update a gauge to 10.0, add 5.0, and then add 5.0 again, should you (as a user) understand whether or not your adds fall within a given reporting interval (and thus potentially an underlying aggregation bucket)? What happens if the first add happens at 11s, and the second add happens at 21s? The user may think they're adding to the original value, but gauge values are not sustained and re-reported. This means that either (a) the user or (b) Tally must keep state to ensure that these methods function according to least surprise - the increased complexity would be better off left to the user to manage the value themselves and call Gauge.Update() as appropriate.

It might help if you give a little more info about your use case and the problem you're trying to solve - there may be another solution, or we may be able to incorporate this into our design considerations for the redesign we're planning right now. Let us know!

[mgkanani]

Thanks, @mway for the reply.

It might help if you give a little more info about your use case and the problem you're trying to solve - there may be another solution, or we may be able to incorporate this into our design considerations for the redesign we're planning right now. Let us know!

Let's say we want to produce the metric for total in-process requests. On every request arrival increase it by 1 and on response decrement by 1.

There are multiple ways to solve it:

A1. By using Counter with tags/labels i.e. type=request and type=response : To graph it, we query like {type=request} - {type=response}

A2. Gauge with existing API, user will maintain the value and calls Gauge.Update method for every value-update.

A3. Using gauge(provided it has Add and Sub functions): use Add(1) on every request arrival and Sub(1) on every reply

Now, the ask is: "We want to produce total active requests per client/region". We need to add the tag "client" and it's value will be known during runtime. For every client value, library will allocate and maintain the metric value.

Now let's discuss dis-advantages:

A1. To maintain time series data at server side, the space needed is twice. Application is also allocating 2 counters(uint64) per client-type.

A2. User have to dynamically create and maintain the variables for every client type. On every request, it will first update it's variable value and then call Gauge.Update to update the underlying gauge metric value. This leads to 2 ops. Similar to A1, Application is still allocating/maintaining 2 float64 type. Though time-series server will not need extra space compared to A1.

Apart from metrics code, services also have logging and tracing related code. These increases monitoring related code foot-print as well. Also, the focus should be more towards writing business logic. Here, they also have to take care of these variables and updating them appropriately.

About redesign proposal, can you please send me the reference to participate in it? i have some suggestions which can be discussed in that forum.

1. It's a breaking change. Interface additions change the contract that must be satisfied by implementors, thus this could cause downstream builds to begin failing until devs added/mocked/etc the new methods. Not to say that we're not able to do major revision bumps, but at a minimum, we do tend to batch them together.

I will change the destination branch for this PR to future-release(Can you please refer the branch name?). I raised against master as I couldn't find any 4.x.x tag and assumed that it will be cut out from master.

2. This significantly increases the implicit complexity of gauges.
   ...

Based on the implementation, on every specified interval it publishes all the registered metrics-values to the reporter. From user's perspective, these changes don't force the deviation from existing. In Add/Sub functions, it invokes Update method(which just updates value of float64 atomically).

[mgkanani]

@mway @robskillington ,
Is there any future release planned? I will rebase the PR to target for the next release branch.