From 23746fbddc51d8820810c3e1edf69819a80a69cc Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Tue, 12 Oct 2021 13:11:17 -0400 Subject: [PATCH 1/2] Fixes monotonicity issues we have implementing the specificaiton. - Prevent negative measurements in Histogram Instruments so we can include monotonic sum in the data and export to prometheus. - Remove explicit monotonicity argument to aggregators and always infer from instrument type. - Expand list of supported instruments for Sum aggregation with details on default aggregation. - Add note on handling `sum` in histogram for instruments that allow negative measurements. --- specification/metrics/api.md | 2 +- specification/metrics/sdk.md | 28 +++++++++++++--------------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/specification/metrics/api.md b/specification/metrics/api.md index d0305200467..04ca9d149b6 100644 --- a/specification/metrics/api.md +++ b/specification/metrics/api.md @@ -542,7 +542,7 @@ certain programming languages or systems, for example `null`, `undefined`). Parameters: -* The amount of the `Measurement`. +* The amount of the `Measurement`, which MUST be a non-negative numeric value. * Optional [attributes](../common/common.md#attributes). [OpenTelemetry API](../overview.md#api) authors MAY decide to allow flexible diff --git a/specification/metrics/sdk.md b/specification/metrics/sdk.md index b67a478517b..d17cb08fdda 100644 --- a/specification/metrics/sdk.md +++ b/specification/metrics/sdk.md @@ -331,24 +331,22 @@ This Aggregation does not have any configuration parameters. The Sum Aggregation informs the SDK to collect data for the [Sum Metric Point](./datamodel.md#sums). -The default values for the configuration parameters will be set based on -the Instrument Kind (e.g. at View registration OR at first seen measurement). - -| Instrument Kind | Default `SumType` | Default `Temporality` | -| --- | --- | --- | -| [Counter](./api.md#counter) | Monotonic | Cumulative | -| [Asynchronous Counter](./api.md#asynchronous-counter) | Monotonic | Cumulative | -| [UpDownCounter](./api.md#updowncounter) | Non-Monotonic | Cumulative | -| [Asynchrounous UpDownCounter](./api.md#asynchronous-updowncounter) | Non-Monotonic | Cumulative | - This Aggregation honors the following configuration parameters: | Key | Value | Default Value | Description | | --- | --- | --- | --- | -| SumType | Monotonic, Non-Monotonic, Other | See 1 | See [SumType in PR](https://github.com/open-telemetry/opentelemetry-proto/pull/320). | -| Temporality | Delta, Cumulative | See 1 | See [Temporality](./datamodel.md#temporality). | +| Temporality | Delta, Cumulative | Cumulative | | -\[1\]: See Default values based on Instrument Kind above. +The monotonicity of the aggregation is determined by the instrument type: + +| Instrument Kind | `SumType` | +| --- | --- | --- | +| [Counter](./api.md#counter) | Monotonic | +| [UpDownCounter](./api.md#updowncounter) | Non-Monotonic | +| [Histogram](./api.md#histogram) | Monotonic | +| [Asynchronous Gauge](./api.md#asynchronous-gauge) | Non-Monotonic | +| [Asynchronous Counter](./api.md#asynchronous-counter) | Monotonic | +| [Asynchrounous UpDownCounter](./api.md#asynchronous-updowncounter) | Non-Monotonic | This Aggregation informs the SDK to collect: @@ -384,11 +382,11 @@ This Aggregation honors the following configuration parameters: | Key | Value | Default Value | Description | | --- | --- | --- | --- | -| Monotonic | boolean | true | if true, non-positive values are treated as errors1. | | Temporality | Delta, Cumulative | Cumulative | See [Temporality](./datamodel.md#temporality). | | Boundaries | double\[\] | [ 0, 5, 10, 25, 50, 75, 100, 250, 500, 1000 ] | Array of increasing values representing explicit bucket boundary values.

The Default Value represents the following buckets:
(-∞, 0], (0, 5.0], (5.0, 10.0], (10.0, 25.0], (25.0, 50.0], (50.0, 75.0], (75.0, 100.0], (100.0, 250.0], (250.0, 500.0], (500.0, 1000.0], (1000.0, +∞) | -\[1\]: Language implementations may choose the best strategy for handling errors. (i.e. Log, Discard, etc...) +Note: This aggregator should not fill out `sum` when used with instruments +that record negative measurements, e.g. `UpDownCounter` or `ObservableGauge`. This Aggregation informs the SDK to collect: From 9ca10391d6a3ebd23a793b01f7bc0cd281f9ab95 Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Wed, 13 Oct 2021 09:28:10 -0400 Subject: [PATCH 2/2] Update specification/metrics/sdk.md Co-authored-by: Aaron Abbott --- specification/metrics/sdk.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specification/metrics/sdk.md b/specification/metrics/sdk.md index d17cb08fdda..8a7ca197da4 100644 --- a/specification/metrics/sdk.md +++ b/specification/metrics/sdk.md @@ -340,7 +340,7 @@ This Aggregation honors the following configuration parameters: The monotonicity of the aggregation is determined by the instrument type: | Instrument Kind | `SumType` | -| --- | --- | --- | +| --- | --- | | [Counter](./api.md#counter) | Monotonic | | [UpDownCounter](./api.md#updowncounter) | Non-Monotonic | | [Histogram](./api.md#histogram) | Monotonic |