-
Notifications
You must be signed in to change notification settings - Fork 167
WIP: Discuss approval for three metrics RFCs 0003, 0008, and 0009. #51
Conversation
Hello @jmacd, this is my first attempt to help the OpenTelemetry Community, so take all this with a little grain of salt. I'm bringing here the discussions from the open-telemetry/opentelemetry-go/pull/100 pull request. Initially, I was confused about the need of both Now that I have read this RFC, I started to think that these could be a little simpler. Why do we need to specify that a If we agree on those, we may revisit on which method each metric kind should have. Maybe I haven't read all the accepted RFCs neither have been present on the meetings (which I'll start to attend), so if this has been already discussed or if I am missing some point, just tell me and I'll try to catch up with the project. Thanks. |
Definitely in favor of "Monotonic" over Uni/Bidirectional. The former is a high school-level well-defined math term that describes exactly what is meant, while the latter seems to be our own invention in this context. |
@Oberon00 Yes, it was definitely a slip not to use Monotonic from the start. I've updated Unidirectional->Monotonic. However, monotonic behavior is default for Counters while it is an option for Gauges, so Monotonic names the optional Gauge behavior and we need an opposite to name the Counter behavior. Non-Monotonic is potentially an answer, but I took an informal poll of some folks around the office here and "BiDirectional" was still preferred. I'm open to suggestions. |
I'm starting to prefer |
I've updated the optional behavior names. |
There are two changes in open-telemetry/opentelemetry-specification#250 relative to this PR: |
@@ -124,9 +124,17 @@ The optional properties of a metric instrument are: | |||
|----------|-------------|-------------| | |||
| Required Keys | Determines labels that are always set on metric handles | All kinds | | |||
| Disabled | Indicates a verbose metric that does not report by default | All kinds | |
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 think this is removed?
| NonNegative | Indicates a measure that is never negative, for rate calculation | Measure | | ||
| NonMonotonic | Indicates a cumulative metric instrument that goes up and down | Cumulative | | ||
| Monotonic | Indicate a gauge that expresses a monotonic cumulative value | Gauge | | ||
| Signed | Indicates a measure that is positive or negative | Measure | |
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.
Can we remove the signed as we discussed in one of the meeting?
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's a meaningful distinction to keep IMO.
Examples were provided in the referenced thread, e.g., for recording spanAsssasin scores. For recording the change in a bank account, there are just so many examples.
Why should the implementation care? What value is there to the implementation in not having to support negative values? I can think of one. For computing summary metrics, i.e., quantile estimates, we need an approximation algorithm. It is significantly easier to approximate a "one-sided" distribution that has one boundary. It's significantly easier to approximate a "two-sided" distribution that has no boundaries. If you're told the measure is non-negative you might choose to use a DDSketch. If you're told the measure is signed, you might choose to use a T-digest .
I don't see this as being expensive to keep in, but I see it as a lack of symmetry if we exclude it. We shouldn't be so confident that negative measures are not useful.
Numbers 0003, 0008, and 0009 are all related. This PR serves to discuss these RFCs and/or approve them.