-
Notifications
You must be signed in to change notification settings - Fork 896
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Clarify reader / exporter relationship for temporality preference and…
… default aggregation (#4142) Reviewing @dashpole's [comments](#4124 (review)) on #4124, I realize that a key clarification is missing for the contract between readers and exporters, and for the configuration of built-in exporters. We [state that](/~https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#metricreader) MetricReaders should be constructed with: - "The default output aggregation (optional), a function of instrument kind. If not configured, the [default aggregation](/~https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#default-aggregation) SHOULD be used." - "The default output temporality (optional), a function of instrument kind. If not configured, the Cumulative temporality SHOULD be used." And for MetricExporters, we [state that](/~https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#metricexporter) "The aggregation and temporality properties used by the OpenTelemetry Metric SDK are determined when registering Metric Exporters through their associated MetricReader." But we don't connect the dots that the `aggregation` and `temporality` options for the MetricReader should be obtained from a MetricExporter. I think this is understood, since the whole reason these are configurable at the MetricReader level is that MetricExporters and their associated backends have different requirements around temporality and default aggregation (especially in the case of base2 exponential histogram). This PR clarifies that the MetricReader `aggregation` and `temporality` options SHOULD come from the associated MetricExporter, NOT from the caller. Furthermore, because these are now clearly spelled out as preferences of MetricExporter, I've updated all the built-in metric exporter definitions to make expectations clear around what defaults and configuration of `aggregation` and `temporality` should be supported. This achieves the goals of #4124 in a more generic way. | Exporter | default `aggregation` | `temporality` | |---|---|---| | Prometheus | configurable | always cumulative | | OTLP | configurable | configurable | | Standard Output | configurable | configurable | | InMemory | configurable | configurable |
- Loading branch information
Showing
7 changed files
with
112 additions
and
75 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters