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

Clarify preferred aggregation temporality; give Views a selection strategy #2314

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ release.
[#2032](https://github.com/open-telemetry/opentelemetry-specification/pull/2061)
- Changed default Prometheus Exporter host from `0.0.0.0` to `localhost`.
([#2282](https://github.com/open-telemetry/opentelemetry-specification/pull/2282))
- Clarify what is meant by "preferred" aggregation temporality.
([#2314](https://github.com/open-telemetry/opentelemetry-specification/pull/2314))

### Logs

Expand Down
8 changes: 4 additions & 4 deletions specification/metrics/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -404,9 +404,9 @@ observed. [OpenTelemetry API](../overview.md#api) authors SHOULD define whether
this callback function needs to be reentrant safe / thread safe or not.

Note: Unlike [Counter.Add()](#add) which takes the increment/delta value, the
callback function reports the absolute value of the counter. To determine the
reported rate the counter is changing, the difference between successive
measurements is used.
callback function reports the cumulative value of the counter. To determine the
jmacd marked this conversation as resolved.
Show resolved Hide resolved
reported rate at which the counter is changing, the difference between successive
measurements should be used.

The callback function SHOULD NOT take indefinite amount of time. If multiple
independent SDKs coexist in a running process, they MUST invoke the callback
Expand Down Expand Up @@ -893,7 +893,7 @@ observed. [OpenTelemetry API](../overview.md#api) authors SHOULD define whether
this callback function needs to be reentrant safe / thread safe or not.

Note: Unlike [UpDownCounter.Add()](#add-1) which takes the increment/delta value,
the callback function reports the absolute value of the Asynchronous
the callback function reports the cumulative value of the Asynchronous
jmacd marked this conversation as resolved.
Show resolved Hide resolved
UpDownCounter. To determine the reported rate the Asynchronous UpDownCounter is
changing, the difference between successive measurements is used.

Expand Down
80 changes: 61 additions & 19 deletions specification/metrics/sdk.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ are the inputs:
apply a [default aggregation](#default-aggregation). If the aggregation
outputs metric points that use aggregation temporality (e.g. Histogram,
Sum), the SDK SHOULD handle the aggregation temporality based on the
temporality of each [MetricReader](#metricreader) instance.
[preferred aggregation temporality](#preferred-aggregation-temporality)
of each [MetricReader](#metricreader) instance.
* The `exemplar_reservoir` (optional) to use for storing exemplars.
This should be a factory or callback similar to aggregation which allows
different reservoirs to be chosen by the aggregation.
Expand Down Expand Up @@ -622,25 +623,66 @@ The SDK SHOULD provide a way to allow `MetricReader` to respond to
idiomatic approach, for example, as `OnForceFlush` and `OnShutdown` callback
functions.

### Preferred aggregation temporality
Copy link
Member

Choose a reason for hiding this comment

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

I think this is too complicated for our users. I would like to think of a simpler solution.

Can we have our MetricReader support a configuration option that says exactly for each type of instrument what to produce?

I feel that we complicate things here because we have this "notion" of preferred, because of these we are trying to say "you may prefer delta, but you don't know what you want, I will give you cumulative for this".

I think this is too complicated, have we consider that every "exporter" instead of having a "preferred" to actually come with a concrete temporality <type, temporality> list so they have full control, and we don't try to be smarter like "Preferred".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we have our MetricReader support a configuration option that says exactly for each type of instrument what to produce?

That was one of the initial proposals, the "5-way" function as it has been called in the review thread above. I think that's fine as a means of configuring the exporters that support both options. If we accept the 5-way function, which seems easy for the in-memory exporter, and fix the output temporality for the console and OTLP exporters to always cumulative, then:

(1) vendors that want always-delta or sometimes-delta will provide implementations of the 5-way function they prefer
(2) Does this mean we can erase the concept of preferred temporality entirely?


The SDK SHOULD provide a way to allow the preferred [Aggregation
Temporality](./datamodel.md#temporality) to be specified for a `MetricReader`
instance during the setup (e.g. initialization, registration, etc.) time. If the
preferred temporality is explicitly specified then the SDK SHOULD respect that,
otherwise use Cumulative.

[OpenTelemetry SDK](../overview.md#sdk)
authors MAY choose the best idiomatic design for their language:

* Whether to treat the temporality settings as recommendation or enforcement.
For example, if the temporality is set to Delta, would the SDK want to perform
Cumulative->Delta conversion for an [Asynchronous
Counter](./api.md#asynchronous-counter), or downgrade it to a
[Gauge](./datamodel.md#gauge), or keep consuming it as Cumulative due to the
consideration of [memory
efficiency](./supplementary-guidelines.md#memory-management)?
* Refer to the [supplementary
guidelines](./supplementary-guidelines.md#aggregation-temporality), which have
more context and suggestions.
Temporality](./datamodel.md#temporality) to be specified for a
`MetricReader` instance during the setup (e.g. initialization,
registration, etc.) time. This preference gives the user control over
the amount of long-term memory dedicated to reading metrics.

The synchronous instruments generally define data points having
aggregation temporality (e.g., `Sum`, `Histogram`). For these points,
when a `MetricReader` is configured with Cumulative aggregation
temporality, there is an implied long-term memory cost.

The asynchronous `Counter` and `UpDownCounter` instruments are defined
by observations that are cumulative values to begin with. For these
jmacd marked this conversation as resolved.
Show resolved Hide resolved
points, a change of aggregation temporality implies conversion from
Cumulative into Delta aggregation temporality.

Because of these differences, synchronous and asynchronous instruments
are given separate treatment. When configuring the preferred
aggregation temporality, the implementation MUST provide at least the
following named preferences:

- "Cumulative": All data points that define the concept are exported
jmacd marked this conversation as resolved.
Show resolved Hide resolved
with Cumulative aggregation temporality (i.e., all data point kinds
except for `Gauge`). This implies maintaining long-term Cumulative
state for metric streams deriving from synchronous instruments.
- "Stateless": Data points having aggregation temporality deriving from
synchonous instruments are exported with Delta aggregation
temporality (e.g., Delta `Sum` and Delta `Histogram` points), while
data points having aggregation temporality deriving from asynchronous
instruments are exported with Cumulative aggregation temporality.
This preference is so named because it avoids long-term Cumulative
state in the metrics SDK.

If the preferred temporality is explicitly specified then the SDK
SHOULD respect that, otherwise use Cumulative.

A configured `MetricReader` instance MUST support reading the
preferred aggregation temporality for synchronous instruments.

An optional aggregation temporality preference "Delta" MAY be
jmacd marked this conversation as resolved.
Show resolved Hide resolved
supported by implementations, making it possible for users to convert
data points from asynchronous `UpDownCounter` and `Counter`
instruments into Delta aggregation temporality inside their SDK.

SDK support for converting Cumulative to Delta aggregation is optional
because it is not considered widely useful; in metrics data formats
jmacd marked this conversation as resolved.
Show resolved Hide resolved
that emphasize Delta aggregation temporality such as StatsD, it is
conventional to convert cumulative values into `Gauge` data points,
jmacd marked this conversation as resolved.
Show resolved Hide resolved
for example, and this is not typically encountered for `Histogram`
measurements.

Users that wish to convert Cumulative into Delta aggregation
jmacd marked this conversation as resolved.
Show resolved Hide resolved
temporality may consider using an [OpenTelemetry Collector metrics
processor meant for this purpose](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/processor/cumulativetodeltaprocessor).

Refer to the [supplementary
guidelines](./supplementary-guidelines.md#aggregation-temporality),
which have more context and suggestions.

### MetricReader operations

Expand Down