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

Add AbsoluteValueRecorder instrument #608

Closed
bogdandrutu opened this issue May 15, 2020 · 11 comments
Closed

Add AbsoluteValueRecorder instrument #608

bogdandrutu opened this issue May 15, 2020 · 11 comments
Labels
area:api Cross language API specification issue enhancement New feature or request spec:metrics Related to the specification/metrics directory

Comments

@bogdandrutu
Copy link
Member

AbsoluteValueRecorder - use case is the following: user wants to report bytes sent (always positive) naturally will want a distribution so will recommend to use ValueRecorder - but if someone is concerned about the cost and they want to just see the Sum will configure a View to produce only Sum. Now the problem is that it will be good to see a monotonic Sum which we cannot guarantee unless we know the values are absolute (non negative).

Proposal 1
New instrument “AbsoluteValueRecorder”:

  • Using the “Absolute*” has the chance of being misunderstood as an absolute function over the values, but it should mean “you will only hand this instrument positive values”.
  • “Less is more” -- would be ideal to keep things minimal.

Proposal 2
Have the view support restricting this in a view:

  • This might not be feasible. The Operator might want to ensure this, but what was instrumented may not support this. Therefore, the view might not support this.
@arminru arminru added area:api Cross language API specification issue enhancement New feature or request spec:metrics Related to the specification/metrics directory labels May 15, 2020
@jmacd
Copy link
Contributor

jmacd commented May 16, 2020

(I feel like I already asked you this, but I can't remember your answer.)

Can the user instead use a SumObserver and configure it to compute a distribution including the sum? In your example, something is being configured. Why not configure an existing instrument that has the semantics you want, with an aggregator that does what you want?

Edit: This question was meant to be about Counter, since ValueRecorder is synchronous. Same question though.

@jmacd
Copy link
Contributor

jmacd commented May 18, 2020

Can the user instead use a Counter and configure it to compute a distribution including the sum? This makes the user change the default behavior, but in your example a user "will configure a View".

@bogdandrutu
Copy link
Member Author

@jmacd sorry I had the response pending for this:
There is a good question why user should not use Counter for this instead of ValueRecorder.

I think it is a matter of what is the natural way to report that metric, if the default way is to see that as a distribution of values then asking users to use Counter just to ensure that the information about monotonicity/absolute value is passed from the instrumentation to the library feels a bit hacky and unnatural.

@jmacd
Copy link
Contributor

jmacd commented May 27, 2020

I wonder about the following names:

ValueRecorder supports positive and negative values, defaults to last-value aggregation (Prometheus: Gauge)
MeasureRecorder supports non-negative values, defaults to distribution aggregation (Prometheus: Histogram)

The term "Measure" implies a size, which is generally non-negative.

@jmacd
Copy link
Contributor

jmacd commented May 28, 2020

Another possibility:
ValueRecorder (arbitrary values, gauge)
CountRecorder (non-negative values, histogram)

I think this emphasizes the use-case given above ("bytes sent"); this is an instrument you use when you have a count and want to record individual values and expose the sum as a rate. It's less technical jargon too.

@jmacd
Copy link
Contributor

jmacd commented May 28, 2020

Summary of the above four responses of mine:

In #625 I've proposed calling this new instrument a Collective instrument, and I favor the term CountRecorder thus far.

Today's specification has six instruments, CountRecorder would be the 7th. CountRecorder is a Collective instrument that collects Additive measurements, which has the desired property of allowing an automatic rate calculation derived from the sum of measurements.

We could discuss at length the mathematical and/or statistical rationalization for having this instrument, but for me the primary motivation is to answer the question about Prometheus/Statsd Gauges. The reason that we need more than two Collective instruments (ValueRecorder and ValueObserver) is that users want two different defaults. This proposal is pragmatic: by adding CountRecorder we have a way to select a relatively less-expensive Collective instrument (ValueRecorder => Gauge) or a relatively more-expensive Collective instrument (CountObserver => Histogram, etc).

As in OTEP 88, we could enumerate more instruments than seven. Why not UpDownCountRecorder? Probably the answer is that in most cases, a UpDownCounter should be used instead, and reconfigured with a Collective aggregator.

For the asynchronous instruments, is there a need for a Collective Observer instrument with a default Gauge interpretation? This is a different case because Observer instruments are specified to report only one value per collection interval, so the pragmatic rationalization used above to support a (synchronous) CountRecorder does not apply.

@jkwatson
Copy link
Contributor

The term "Measure" implies a size, which is generally non-negative.

I think this is a bit over-thought. Non-mathematicians measure negative things all the time: voltage and temperature spring to mind immediately.

@jmacd
Copy link
Contributor

jmacd commented May 29, 2020

I think this is a bit over-thought.

Measure was being used as a noun, not a verb. :-) In any case, I agree it's not a good term for us, regardless of whether it's accurate. I still favor "CountRecorder".

@jkwatson
Copy link
Contributor

I think this is a bit over-thought.

Measure was being used as a noun, not a verb. :-) In any case, I agree it's not a good term for us, regardless of whether it's accurate. I still favor "CountRecorder".

So, "the best kind of correct". :)
I'm fine with CountRecorder, FWIW, although I do think this is going to be a rare case, and could easily be solved with the appropriate view on a normal Counter.

@jmacd
Copy link
Contributor

jmacd commented May 29, 2020

@bogdandrutu There's not much support here. The number of people paying attention is also very small. I'm inclined to close this and not add a new instrument, given that no one has added any support.

@jmacd
Copy link
Contributor

jmacd commented Jun 11, 2020

I'm closing this because there hasn't been much support. I was only in favor of this because I was thinking that users want a way to select an instrument with either Histogram or LastValue aggregation by default. After some feedback, I'm starting to think that the default (for ValueRecorder) should be LastValue.

To support the use-case in question, I think we should focus on making the SDK configurable. A programmatic Views API would let users configure a Counter to expose a histogram that translates into a meaningful rate, for example.

@jmacd jmacd closed this as completed Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api Cross language API specification issue enhancement New feature or request spec:metrics Related to the specification/metrics directory
Projects
None yet
Development

No branches or pull requests

4 participants