From 746dd565d0654dff500d82d406ecb27c075dc160 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 13 Mar 2020 13:43:34 -0700 Subject: [PATCH 1/2] Update SummaryDataPoint percentile comment Include information on the way we are fitting the `MinMaxSumCount` aggregation into the `Summary` metric kind. --- gen/go/metrics/v1/metrics.pb.go | 12 ++++++++++++ opentelemetry/proto/metrics/v1/metrics.proto | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/gen/go/metrics/v1/metrics.pb.go b/gen/go/metrics/v1/metrics.pb.go index 7a73370d3..33d021759 100644 --- a/gen/go/metrics/v1/metrics.pb.go +++ b/gen/go/metrics/v1/metrics.pb.go @@ -853,6 +853,18 @@ func (m *SummaryDataPoint) GetPercentileValues() []*SummaryDataPoint_ValueAtPerc } // Represents the value at a given percentile of a distribution. +// +// To support the Min and Max values of a MinMaxSumCount aggregation the +// following conventions are used: +// - The 100th percentile is equivalent to the maximum value observed +// so can be used as a stand in for the Max value. +// - Set the 0th percentile to the Min value. This is mathematically +// incorrect, but the most suitable way to transmit this data with the +// existing metric kinds. +// +// For more information about work underway to better support the +// MinMaxSumCount aggregation refer to: +// https://github.com/open-telemetry/opentelemetry-proto/issues/125 type SummaryDataPoint_ValueAtPercentile struct { // The percentile of a distribution. Must be in the interval // [0.0, 100.0]. diff --git a/opentelemetry/proto/metrics/v1/metrics.proto b/opentelemetry/proto/metrics/v1/metrics.proto index a15a7b6c2..2e7b72e49 100644 --- a/opentelemetry/proto/metrics/v1/metrics.proto +++ b/opentelemetry/proto/metrics/v1/metrics.proto @@ -340,6 +340,18 @@ message SummaryDataPoint { double sum = 5; // Represents the value at a given percentile of a distribution. + // + // To support the Min and Max values of a MinMaxSumCount aggregation the + // following conventions are used: + // - The 100th percentile is equivalent to the maximum value observed + // so can be used as a stand in for the Max value. + // - Set the 0th percentile to the Min value. This is mathematically + // incorrect, but the most suitable way to transmit this data with the + // existing metric kinds. + // + // For more information about work underway to better support the + // MinMaxSumCount aggregation refer to: + // https://github.com/open-telemetry/opentelemetry-proto/issues/125 message ValueAtPercentile { // The percentile of a distribution. Must be in the interval // [0.0, 100.0]. From 13c8eba492c0790a466c57d9151f37afb746bec4 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 13 Mar 2020 17:45:51 -0700 Subject: [PATCH 2/2] Apply feedback --- gen/go/metrics/v1/metrics.pb.go | 13 ++++--------- opentelemetry/proto/metrics/v1/metrics.proto | 13 ++++--------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/gen/go/metrics/v1/metrics.pb.go b/gen/go/metrics/v1/metrics.pb.go index dc91eb062..79f737b8b 100644 --- a/gen/go/metrics/v1/metrics.pb.go +++ b/gen/go/metrics/v1/metrics.pb.go @@ -854,16 +854,11 @@ func (m *SummaryDataPoint) GetPercentileValues() []*SummaryDataPoint_ValueAtPerc // Represents the value at a given percentile of a distribution. // -// To support the Min and Max values of a MinMaxSumCount aggregation the -// following conventions are used: -// - The 100th percentile is equivalent to the maximum value observed -// so can be used as a stand in for the Max value. -// - Set the 0th percentile to the Min value. This is mathematically -// incorrect, but the most suitable way to transmit this data with the -// existing metric kinds. +// To record Min and Max values following conventions are used: +// - The 100th percentile is equivalent to the maximum value observed. +// - The 0th percentile is equivalent to the minimum value observed. // -// For more information about work underway to better support the -// MinMaxSumCount aggregation refer to: +// See the following issue for more context: // https://github.com/open-telemetry/opentelemetry-proto/issues/125 type SummaryDataPoint_ValueAtPercentile struct { // The percentile of a distribution. Must be in the interval diff --git a/opentelemetry/proto/metrics/v1/metrics.proto b/opentelemetry/proto/metrics/v1/metrics.proto index 215e0d370..1ca10385f 100644 --- a/opentelemetry/proto/metrics/v1/metrics.proto +++ b/opentelemetry/proto/metrics/v1/metrics.proto @@ -341,16 +341,11 @@ message SummaryDataPoint { // Represents the value at a given percentile of a distribution. // - // To support the Min and Max values of a MinMaxSumCount aggregation the - // following conventions are used: - // - The 100th percentile is equivalent to the maximum value observed - // so can be used as a stand in for the Max value. - // - Set the 0th percentile to the Min value. This is mathematically - // incorrect, but the most suitable way to transmit this data with the - // existing metric kinds. + // To record Min and Max values following conventions are used: + // - The 100th percentile is equivalent to the maximum value observed. + // - The 0th percentile is equivalent to the minimum value observed. // - // For more information about work underway to better support the - // MinMaxSumCount aggregation refer to: + // See the following issue for more context: // https://github.com/open-telemetry/opentelemetry-proto/issues/125 message ValueAtPercentile { // The percentile of a distribution. Must be in the interval