From 5d9e23ebcd718855c970308c262b5c2422201123 Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Sat, 8 May 2021 10:29:26 -0400 Subject: [PATCH 1/5] Refine descriptions of Histogram+Summary for explicit OpenMetrics compatibility Fixes #187. - Summary data points must record non-negative values - Histogram sum can only be present if underlying measurements are non-negative. (Opening a bug to expand our API to allow this to present in more scenarios later) --- opentelemetry/proto/metrics/v1/metrics.proto | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/opentelemetry/proto/metrics/v1/metrics.proto b/opentelemetry/proto/metrics/v1/metrics.proto index df7b0d985..4569c44a6 100644 --- a/opentelemetry/proto/metrics/v1/metrics.proto +++ b/opentelemetry/proto/metrics/v1/metrics.proto @@ -220,8 +220,8 @@ message IntHistogram { AggregationTemporality aggregation_temporality = 2; } -// Histogram represents the type of a metric that is calculated by aggregating as a -// Histogram of all reported double measurements over a time interval. +// Histogram represents the type of a metric that is calculated by aggregating +// as a Histogram of all reported double measurements over a time interval. message Histogram { repeated HistogramDataPoint data_points = 1; @@ -527,6 +527,11 @@ message HistogramDataPoint { // sum of the values in the population. If count is zero then this field // must be zero. This value must be equal to the sum of the "sum" fields in // buckets if a histogram is provided. + // + // Note: Sum should only be filled out when measuring non-negative discrete + // events, and is assumed to be monotonic over the values of these events. + // Negative events *can* be recorded, but sum should not be filled out when + // doing so. double sum = 5; // bucket_counts is an optional field contains the count values of histogram @@ -617,6 +622,8 @@ message SummaryDataPoint { double quantile = 1; // The value at the given quantile of a distribution. + // + // Quantile values must NOT be negative. double value = 2; } From 12a95edd6e9a815371c9670fc3542955ac289ce5 Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Sat, 8 May 2021 10:31:47 -0400 Subject: [PATCH 2/5] Update CHANGELOG.md --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 435f6f78f..8d4904860 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,9 @@ Full list of differences found in [this compare.](https://github.com/open-teleme * Remove if no changes for this section before release. -### Changed +### Changed: MEtrics -* Remove if no changes for this section before release. +* :stop_sign: [DATA MODEL CHANGE] Histogram/Summary sums must be monotonic counters of events (#302) ### Added From 3a29cf0b427614dc255b4ce5ae14279dd0b3cde1 Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Mon, 10 May 2021 15:11:13 -0700 Subject: [PATCH 3/5] Update CHANGELOG.md Co-authored-by: Joshua MacDonald --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d4904860..2438f92a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ Full list of differences found in [this compare.](https://github.com/open-teleme * Remove if no changes for this section before release. -### Changed: MEtrics +### Changed: Metrics * :stop_sign: [DATA MODEL CHANGE] Histogram/Summary sums must be monotonic counters of events (#302) From 087ab7d7fce7eaf51e950939c170f4fef436c785 Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Mon, 10 May 2021 19:27:04 -0400 Subject: [PATCH 4/5] Call out openmetrics specification --- opentelemetry/proto/metrics/v1/metrics.proto | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/opentelemetry/proto/metrics/v1/metrics.proto b/opentelemetry/proto/metrics/v1/metrics.proto index 4569c44a6..83b8fef6b 100644 --- a/opentelemetry/proto/metrics/v1/metrics.proto +++ b/opentelemetry/proto/metrics/v1/metrics.proto @@ -531,7 +531,8 @@ message HistogramDataPoint { // Note: Sum should only be filled out when measuring non-negative discrete // events, and is assumed to be monotonic over the values of these events. // Negative events *can* be recorded, but sum should not be filled out when - // doing so. + // doing so. This is specifically to enforce compatibility w/ OpenMetrics, + // see: https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#histogram double sum = 5; // bucket_counts is an optional field contains the count values of histogram From 123179cc7b0b8b2cf29f399492f924d390a46960 Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Tue, 11 May 2021 16:40:07 -0400 Subject: [PATCH 5/5] Update metrics.proto --- opentelemetry/proto/metrics/v1/metrics.proto | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/opentelemetry/proto/metrics/v1/metrics.proto b/opentelemetry/proto/metrics/v1/metrics.proto index 99a16c1b8..fc2e4ebcb 100644 --- a/opentelemetry/proto/metrics/v1/metrics.proto +++ b/opentelemetry/proto/metrics/v1/metrics.proto @@ -599,6 +599,12 @@ message SummaryDataPoint { // sum of the values in the population. If count is zero then this field // must be zero. + // + // Note: Sum should only be filled out when measuring non-negative discrete + // events, and is assumed to be monotonic over the values of these events. + // Negative events *can* be recorded, but sum should not be filled out when + // doing so. This is specifically to enforce compatibility w/ OpenMetrics, + // see: https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#summary double sum = 5; // Represents the value at a given quantile of a distribution.