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

New metric instruments from OTEP 88 #93

Closed
wants to merge 8 commits into from

Conversation

jmacd
Copy link
Contributor

@jmacd jmacd commented Apr 8, 2020

This PR follows OTEP 88 with a concrete proposal for 4 new instruments.

text/0093-metric-instruments.md Outdated Show resolved Hide resolved
text/0093-metric-instruments.md Outdated Show resolved Hide resolved
text/0093-metric-instruments.md Outdated Show resolved Hide resolved
text/0093-metric-instruments.md Outdated Show resolved Hide resolved
text/0093-metric-instruments.md Outdated Show resolved Hide resolved
text/0093-metric-instruments.md Outdated Show resolved Hide resolved
text/0093-metric-instruments.md Outdated Show resolved Hide resolved
text/0093-metric-instruments.md Outdated Show resolved Hide resolved
text/0093-metric-instruments.md Show resolved Hide resolved
text/0093-metric-instruments.md Outdated Show resolved Hide resolved
@jmacd
Copy link
Contributor Author

jmacd commented Apr 10, 2020

After this discussion, I'm seriously considering that Recorder is a good name, better than either Measure or Distribution.

@jmacd
Copy link
Contributor Author

jmacd commented Apr 10, 2020

I've responded to all the feedback and left two conversations open.

@lzchen
Copy link
Contributor

lzchen commented Apr 13, 2020

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?

@jmacd
Copy link
Contributor Author

jmacd commented Apr 14, 2020

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.
Copy link
Contributor

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
Copy link
Contributor

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:
Copy link
Contributor

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. the Timer 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?

Comment on lines +8 to +9
instruments with various refinements and ended with a [sample
proposal](https://github.com/open-telemetry/oteps/pull/88#sample-proposal).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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).

@jmacd
Copy link
Contributor Author

jmacd commented Apr 18, 2020

This will be replaced by #98.

@jmacd jmacd closed this Apr 18, 2020
@jmacd jmacd deleted the jmacd/new_instruments branch May 6, 2020 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
metrics Relates to the Metrics API/SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants