-
Notifications
You must be signed in to change notification settings - Fork 164
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
New metric instruments from OTEP 88 #93
Conversation
After this discussion, I'm seriously considering that |
I've responded to all the feedback and left two conversations open. |
Thanks Josh for this. It greatly helped me clarify my understanding of the various use cases that metrics needs to cover. How would these translate into actual work that needs to be done? If there are no API changes being made, how can these refinement concepts be reflected through what the user does? |
OTEP 88 didn't make API changes. This PR (#93) proposes new instruments be added, but I don't want to start on that until we get it merged and into the specification. I think beginning to prototype a Views API is a good plan. In order to get Cumulative export from Delta instruments and Delta export from Cumulative instruments, we'll need some new structure in the SDK. Since I haven't prototyped this myself, I haven't much guidance, but this is a project I'd like to be doing now. |
1. **Counter** remains unchanged. It uses Sum aggregation by default. | ||
2. **Distribution** is an an unrefined Measure instrument. Distribution accepts positive or negative values and uses MinMaxSumCount aggregation by default. | ||
3. **UpDownCounter** is a Sum-only instrument with no other refinements. It supports capturing positive and negative changes to a sum (deltas). UpDownCounter uses Sum aggregation by default. | ||
4. **Timing** is a Non-negative instrument specialized for the native clock duration measured on the platform. It ensures that duration values are always captured with correct units, that ensures exporters can convert duration measurements correctly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I 100% think we should include this as an instrument, but having a unit based refinement wasn't specified at the start of the OTEP. Can we add something there about this?
|
||
The four instrument refinements discussed in OTEP 88 are: | ||
|
||
* Sum-only: When computing only a sum is the instrument's primary purpose |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency, should this explicitly say that only a sum aggregation is to be used for instruments of this kind of refinement.
|
||
## Explanation | ||
|
||
The four instrument refinements discussed in OTEP 88 are: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like this document has identified 2 different categories of refinements:
- Restrictions on the input values (non-negative, precomputed-sum, and non-negative-rate). This is a refinement that acts as a contract between the instrumenter and the exporter. The former agrees to only send a type of data the latter agrees to handle/interpret the data as such.
I imagine the unit refinement (i.e. theTimer
related restriction) to fit in here as well, right? - Restrictions on meaningful aggregations (sum-only). This is a refinement limiting what processing make sense to be done on the data (though I imagine you could define a view that would go against this?).
Are there any other that people see?
Did we want to explicitly include these categories in this OTEP?
instruments with various refinements and ended with a [sample | ||
proposal](https://github.com/open-telemetry/oteps/pull/88#sample-proposal). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instruments with various refinements and ended with a [sample | |
proposal](https://github.com/open-telemetry/oteps/pull/88#sample-proposal). | |
instruments with various refinements and ended with a [sample | |
proposal](https://github.com/open-telemetry/oteps/blob/master/text/0088-metric-instrument-optional-refinements.md#sample-proposal). | |
This will be replaced by #98. |
This PR follows OTEP 88 with a concrete proposal for 4 new instruments.