From 0f7a022657ed5ee0dbb51c2e8bac38114b532e52 Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Mon, 9 Oct 2023 20:26:09 +0000 Subject: [PATCH 1/8] add summary datatype --- sdk/metric/metricdata/data.go | 49 +++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/sdk/metric/metricdata/data.go b/sdk/metric/metricdata/data.go index 49bbc0414a2..84e8d52d6d7 100644 --- a/sdk/metric/metricdata/data.go +++ b/sdk/metric/metricdata/data.go @@ -240,3 +240,52 @@ type Exemplar[N int64 | float64] struct { // be empty. TraceID []byte `json:",omitempty"` } + +// Summary metric data are used to convey quantile summaries, +// a Prometheus (see: https://prometheus.io/docs/concepts/metric_types/#summary) +// and OpenMetrics (see: https://github.com/OpenObservability/OpenMetrics/blob/4dbf6075567ab43296eed941037c12951faafb92/protos/prometheus.proto#L45) +// data type. These data points cannot always be merged in a meaningful way. +// While they can be useful in some applications, histogram data points are +// recommended for new applications. +type Summary struct { + // DataPoints are the individual aggregated measurements with unique + // attributes. + DataPoints []SummaryDataPoint +} + +func (Summary) privateAggregation() {} + +// SummaryDataPoint is a single data point in a timeseries that describes the +// time-varying values of a Summary metric. +type SummaryDataPoint struct { + // Attributes is the set of key value pairs that uniquely identify the + // timeseries. + Attributes attribute.Set + + // StartTime is when the timeseries was started. + StartTime time.Time + // Time is the time when the timeseries was recorded. + Time time.Time + + // Count is the number of updates this histogram has been calculated with. + Count uint64 + + // Sum is the sum of the values recorded. + Sum *float64 + + // (Optional) list of values at different quantiles of the distribution calculated + // from the current snapshot. The quantiles must be strictly increasing. + QuantileValues []ValueAtQuantile +} + +// ValueAtQuantile the value at a given quantile of a distribution. +type ValueAtQuantile struct { + // The quantile of a distribution. Must be in the interval + // [0.0, 1.0]. + Quantile float64 + + // The value at the given quantile of a distribution. + // + // Quantile values must NOT be negative. + Value float64 +} From d5c5cad69d08b468d836ac925c207366d6a015c4 Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Mon, 9 Oct 2023 20:26:20 +0000 Subject: [PATCH 2/8] support comparing summaries --- .../metricdata/metricdatatest/assertion.go | 17 ++- .../metricdata/metricdatatest/comparisons.go | 101 ++++++++++++++++++ 2 files changed, 117 insertions(+), 1 deletion(-) diff --git a/sdk/metric/metricdata/metricdatatest/assertion.go b/sdk/metric/metricdata/metricdatatest/assertion.go index dc8e0d1efaf..56fed69c077 100644 --- a/sdk/metric/metricdata/metricdatatest/assertion.go +++ b/sdk/metric/metricdata/metricdatatest/assertion.go @@ -46,7 +46,10 @@ type Datatypes interface { metricdata.ExponentialHistogram[int64] | metricdata.ExponentialHistogramDataPoint[float64] | metricdata.ExponentialHistogramDataPoint[int64] | - metricdata.ExponentialBucket + metricdata.ExponentialBucket | + metricdata.Summary | + metricdata.SummaryDataPoint | + metricdata.ValueAtQuantile // Interface types are not allowed in union types, therefore the // Aggregation and Value type from metricdata are not included here. @@ -177,6 +180,12 @@ func AssertEqual[T Datatypes](t TestingT, expected, actual T, opts ...Option) bo r = equalExponentialHistogramDataPoints(e, aIface.(metricdata.ExponentialHistogramDataPoint[int64]), cfg) case metricdata.ExponentialBucket: r = equalExponentialBuckets(e, aIface.(metricdata.ExponentialBucket), cfg) + case metricdata.Summary: + r = equalSummary(e, aIface.(metricdata.Summary), cfg) + case metricdata.SummaryDataPoint: + r = equalSummaryDataPoint(e, aIface.(metricdata.SummaryDataPoint), cfg) + case metricdata.ValueAtQuantile: + r = equalValueAtQuantile(e, aIface.(metricdata.ValueAtQuantile), cfg) default: // We control all types passed to this, panic to signal developers // early they changed things in an incompatible way. @@ -251,6 +260,12 @@ func AssertHasAttributes[T Datatypes](t TestingT, actual T, attrs ...attribute.K reasons = hasAttributesExponentialHistogramDataPoints(e, attrs...) case metricdata.ExponentialBucket: // Nothing to check. + case metricdata.Summary: + reasons = hasAttributesSummary(e, attrs...) + case metricdata.SummaryDataPoint: + reasons = hasAttributesSummaryDataPoint(e, attrs...) + case metricdata.ValueAtQuantile: + // Nothing to check. default: // We control all types passed to this, panic to signal developers // early they changed things in an incompatible way. diff --git a/sdk/metric/metricdata/metricdatatest/comparisons.go b/sdk/metric/metricdata/metricdatatest/comparisons.go index bb74c4a1217..8d6d409cc7d 100644 --- a/sdk/metric/metricdata/metricdatatest/comparisons.go +++ b/sdk/metric/metricdata/metricdatatest/comparisons.go @@ -155,6 +155,12 @@ func equalAggregations(a, b metricdata.Aggregation, cfg config) (reasons []strin reasons = append(reasons, "ExponentialHistogram not equal:") reasons = append(reasons, r...) } + case metricdata.Summary: + r := equalSummary(v, b.(metricdata.Summary), cfg) + if len(r) > 0 { + reasons = append(reasons, "Summary not equal:") + reasons = append(reasons, r...) + } default: reasons = append(reasons, fmt.Sprintf("Aggregation of unknown types %T", a)) } @@ -426,6 +432,74 @@ func equalExponentialBuckets(a, b metricdata.ExponentialBucket, _ config) (reaso return reasons } +func equalSummary(a, b metricdata.Summary, cfg config) (reasons []string) { + r := compareDiff(diffSlices( + a.DataPoints, + b.DataPoints, + func(a, b metricdata.SummaryDataPoint) bool { + r := equalSummaryDataPoint(a, b, cfg) + return len(r) == 0 + }, + )) + if r != "" { + reasons = append(reasons, fmt.Sprintf("Summary DataPoints not equal:\n%s", r)) + } + return reasons +} + +func equalSummaryDataPoint(a, b metricdata.SummaryDataPoint, cfg config) (reasons []string) { + if !a.Attributes.Equals(&b.Attributes) { + reasons = append(reasons, notEqualStr( + "Attributes", + a.Attributes.Encoded(attribute.DefaultEncoder()), + b.Attributes.Encoded(attribute.DefaultEncoder()), + )) + } + if !cfg.ignoreTimestamp { + if !a.StartTime.Equal(b.StartTime) { + reasons = append(reasons, notEqualStr("StartTime", a.StartTime.UnixNano(), b.StartTime.UnixNano())) + } + if !a.Time.Equal(b.Time) { + reasons = append(reasons, notEqualStr("Time", a.Time.UnixNano(), b.Time.UnixNano())) + } + } + if !cfg.ignoreValue { + if a.Count != b.Count { + reasons = append(reasons, notEqualStr("Count", a.Count, b.Count)) + } + reasons = append(reasons, equalSummarySum(a.Sum, b.Sum, cfg)...) + r := compareDiff(diffSlices( + a.QuantileValues, + a.QuantileValues, + func(a, b metricdata.ValueAtQuantile) bool { + r := equalValueAtQuantile(a, b, cfg) + return len(r) == 0 + }, + )) + if r != "" { + reasons = append(reasons, r) + } + } + return reasons +} + +func equalValueAtQuantile(a, b metricdata.ValueAtQuantile, _ config) (reasons []string) { + if a.Quantile != b.Quantile { + reasons = append(reasons, notEqualStr("Quantile", a.Quantile, b.Quantile)) + } + if a.Value != b.Value { + reasons = append(reasons, notEqualStr("Value", a.Value, b.Value)) + } + return reasons +} + +func equalSummarySum(a, b *float64, _ config) (reasons []string) { + if a != b && ((a == nil || b == nil) || *a != *b) { + reasons = append(reasons, notEqualStr("Sum", a, b)) + } + return reasons +} + func notEqualStr(prefix string, expected, actual interface{}) string { return fmt.Sprintf("%s not equal:\nexpected: %v\nactual: %v", prefix, expected, actual) } @@ -716,6 +790,8 @@ func hasAttributesAggregation(agg metricdata.Aggregation, attrs ...attribute.Key reasons = hasAttributesExponentialHistogram(agg, attrs...) case metricdata.ExponentialHistogram[float64]: reasons = hasAttributesExponentialHistogram(agg, attrs...) + case metricdata.Summary: + reasons = hasAttributesSummary(agg, attrs...) default: reasons = []string{fmt.Sprintf("unknown aggregation %T", agg)} } @@ -752,3 +828,28 @@ func hasAttributesResourceMetrics(rm metricdata.ResourceMetrics, attrs ...attrib } return reasons } + +func hasAttributesSummary(summary metricdata.Summary, attrs ...attribute.KeyValue) (reasons []string) { + for n, dp := range summary.DataPoints { + reas := hasAttributesSummaryDataPoint(dp, attrs...) + if len(reas) > 0 { + reasons = append(reasons, fmt.Sprintf("summary datapoint %d attributes:\n", n)) + reasons = append(reasons, reas...) + } + } + return reasons + +} +func hasAttributesSummaryDataPoint(dp metricdata.SummaryDataPoint, attrs ...attribute.KeyValue) (reasons []string) { + for _, attr := range attrs { + val, ok := dp.Attributes.Value(attr.Key) + if !ok { + reasons = append(reasons, missingAttrStr(string(attr.Key))) + continue + } + if val != attr.Value { + reasons = append(reasons, notEqualStr(string(attr.Key), attr.Value.Emit(), val.Emit())) + } + } + return reasons +} From 023302badc951222c5698cda93a4f0e026248a84 Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Fri, 13 Oct 2023 16:18:53 +0000 Subject: [PATCH 3/8] update wording based on SIG meeting feedback --- sdk/metric/metricdata/data.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/sdk/metric/metricdata/data.go b/sdk/metric/metricdata/data.go index 84e8d52d6d7..897e408c659 100644 --- a/sdk/metric/metricdata/data.go +++ b/sdk/metric/metricdata/data.go @@ -243,10 +243,11 @@ type Exemplar[N int64 | float64] struct { // Summary metric data are used to convey quantile summaries, // a Prometheus (see: https://prometheus.io/docs/concepts/metric_types/#summary) -// and OpenMetrics (see: https://github.com/OpenObservability/OpenMetrics/blob/4dbf6075567ab43296eed941037c12951faafb92/protos/prometheus.proto#L45) -// data type. These data points cannot always be merged in a meaningful way. -// While they can be useful in some applications, histogram data points are -// recommended for new applications. +// data type. +// +// These data points cannot always be merged in a meaningful way. The Summary +// type is only used by bridges from other metrics libraries, and cannot be +// produced using OpenTelemetry instrumentation. type Summary struct { // DataPoints are the individual aggregated measurements with unique // attributes. @@ -267,7 +268,7 @@ type SummaryDataPoint struct { // Time is the time when the timeseries was recorded. Time time.Time - // Count is the number of updates this histogram has been calculated with. + // Count is the number of updates this summary has been calculated with. Count uint64 // Sum is the sum of the values recorded. @@ -280,8 +281,9 @@ type SummaryDataPoint struct { // ValueAtQuantile the value at a given quantile of a distribution. type ValueAtQuantile struct { - // The quantile of a distribution. Must be in the interval - // [0.0, 1.0]. + // The quantile of a distribution. + // + // Must be in the interval [0.0, 1.0]. Quantile float64 // The value at the given quantile of a distribution. From 89ff05bae127eab8c6408fc0bce1631c332c5cec Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Fri, 13 Oct 2023 16:47:33 +0000 Subject: [PATCH 4/8] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bb7a96733e..2e127b6d83e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add the `go.opentelemetry.io/otel/trace/embedded` package to be embedded in the exported trace API interfaces. (#4620) - Add the `go.opentelemetry.io/otel/trace/noop` package as a default no-op implementation of the trace API. (#4620) - Add context propagation in `go.opentelemetry.io/otel/example/dice`. (#4644) +- Add Summary, SummaryDataPoint, and ValueAtQuantile to `go.opentelemetry.io/sdk/metric/metricdata`. (#4622) ### Deprecated From 0b48fa2c8d855bfadf10d8cb852817079d7c069d Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Fri, 13 Oct 2023 16:48:30 +0000 Subject: [PATCH 5/8] lint --- sdk/metric/metricdata/metricdatatest/comparisons.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metric/metricdata/metricdatatest/comparisons.go b/sdk/metric/metricdata/metricdatatest/comparisons.go index 8d6d409cc7d..362d4ae03c2 100644 --- a/sdk/metric/metricdata/metricdatatest/comparisons.go +++ b/sdk/metric/metricdata/metricdatatest/comparisons.go @@ -838,8 +838,8 @@ func hasAttributesSummary(summary metricdata.Summary, attrs ...attribute.KeyValu } } return reasons - } + func hasAttributesSummaryDataPoint(dp metricdata.SummaryDataPoint, attrs ...attribute.KeyValue) (reasons []string) { for _, attr := range attrs { val, ok := dp.Attributes.Value(attr.Key) From 7e79a75d58c8c14ed5fb3688175cec3580816809 Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Wed, 18 Oct 2023 13:56:07 +0000 Subject: [PATCH 6/8] unit test --- .../metricdatatest/assertion_test.go | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/sdk/metric/metricdata/metricdatatest/assertion_test.go b/sdk/metric/metricdata/metricdatatest/assertion_test.go index 514f5c03267..c9af783255a 100644 --- a/sdk/metric/metricdata/metricdatatest/assertion_test.go +++ b/sdk/metric/metricdata/metricdatatest/assertion_test.go @@ -257,6 +257,46 @@ var ( Exemplars: []metricdata.Exemplar[float64]{exemplarFloat64A}, } + sum = 3.0 + quantileValueA = metricdata.ValueAtQuantile{ + Quantile: 0.0, + Value: 0.1, + } + quantileValueB = metricdata.ValueAtQuantile{ + Quantile: 0.1, + Value: 0.2, + } + summaryDataPointA = metricdata.SummaryDataPoint{ + Attributes: attrA, + StartTime: startA, + Time: endA, + Count: 2, + Sum: &sum, + QuantileValues: []metricdata.ValueAtQuantile{quantileValueA}, + } + summaryDataPointB = metricdata.SummaryDataPoint{ + Attributes: attrB, + StartTime: startB, + Time: endB, + Count: 3, + QuantileValues: []metricdata.ValueAtQuantile{quantileValueB}, + } + summaryDataPointC = metricdata.SummaryDataPoint{ + Attributes: attrA, + StartTime: startB, + Time: endB, + Count: 2, + Sum: &sum, + QuantileValues: []metricdata.ValueAtQuantile{quantileValueA}, + } + summaryDataPointD = metricdata.SummaryDataPoint{ + Attributes: attrA, + StartTime: startA, + Time: endA, + Count: 3, + QuantileValues: []metricdata.ValueAtQuantile{quantileValueB}, + } + exponentialBucket2 = metricdata.ExponentialBucket{ Offset: 2, Counts: []uint64{1, 1}, @@ -514,6 +554,22 @@ var ( DataPoints: []metricdata.ExponentialHistogramDataPoint[float64]{exponentialHistogramDataPointFloat64D}, } + summaryA = metricdata.Summary{ + DataPoints: []metricdata.SummaryDataPoint{summaryDataPointA}, + } + + summaryB = metricdata.Summary{ + DataPoints: []metricdata.SummaryDataPoint{summaryDataPointB}, + } + + summaryC = metricdata.Summary{ + DataPoints: []metricdata.SummaryDataPoint{summaryDataPointC}, + } + + summaryD = metricdata.Summary{ + DataPoints: []metricdata.SummaryDataPoint{summaryDataPointD}, + } + metricsA = metricdata.Metrics{ Name: "A", Description: "A desc", @@ -646,6 +702,9 @@ func TestAssertEqual(t *testing.T) { t.Run("ExponentialHistogramDataPointInt64", testDatatype(exponentialHistogramDataPointInt64A, exponentialHistogramDataPointInt64B, equalExponentialHistogramDataPoints[int64])) t.Run("ExponentialHistogramDataPointFloat64", testDatatype(exponentialHistogramDataPointFloat64A, exponentialHistogramDataPointFloat64B, equalExponentialHistogramDataPoints[float64])) t.Run("ExponentialBuckets", testDatatype(exponentialBucket2, exponentialBucket3, equalExponentialBuckets)) + t.Run("Summary", testDatatype(summaryA, summaryB, equalSummary)) + t.Run("SummaryDataPoint", testDatatype(summaryDataPointA, summaryDataPointB, equalSummaryDataPoint)) + t.Run("QuantileValues", testDatatype(quantileValueA, quantileValueB, equalValueAtQuantile)) } func TestAssertEqualIgnoreTime(t *testing.T) { @@ -670,6 +729,8 @@ func TestAssertEqualIgnoreTime(t *testing.T) { t.Run("ExponentialHistogramFloat64", testDatatypeIgnoreTime(exponentialHistogramFloat64A, exponentialHistogramFloat64C, equalExponentialHistograms[float64])) t.Run("ExponentialHistogramDataPointInt64", testDatatypeIgnoreTime(exponentialHistogramDataPointInt64A, exponentialHistogramDataPointInt64C, equalExponentialHistogramDataPoints[int64])) t.Run("ExponentialHistogramDataPointFloat64", testDatatypeIgnoreTime(exponentialHistogramDataPointFloat64A, exponentialHistogramDataPointFloat64C, equalExponentialHistogramDataPoints[float64])) + t.Run("Summary", testDatatypeIgnoreTime(summaryA, summaryC, equalSummary)) + t.Run("SummaryDataPoint", testDatatypeIgnoreTime(summaryDataPointA, summaryDataPointC, equalSummaryDataPoint)) } func TestAssertEqualIgnoreExemplars(t *testing.T) { @@ -718,6 +779,8 @@ func TestAssertEqualIgnoreValue(t *testing.T) { t.Run("ExponentialHistogramFloat64", testDatatypeIgnoreValue(exponentialHistogramFloat64A, exponentialHistogramFloat64D, equalExponentialHistograms[float64])) t.Run("ExponentialHistogramDataPointInt64", testDatatypeIgnoreValue(exponentialHistogramDataPointInt64A, exponentialHistogramDataPointInt64D, equalExponentialHistogramDataPoints[int64])) t.Run("ExponentialHistogramDataPointFloat64", testDatatypeIgnoreValue(exponentialHistogramDataPointFloat64A, exponentialHistogramDataPointFloat64D, equalExponentialHistogramDataPoints[float64])) + t.Run("Summary", testDatatypeIgnoreValue(summaryA, summaryD, equalSummary)) + t.Run("SummaryDataPoint", testDatatypeIgnoreValue(summaryDataPointA, summaryDataPointD, equalSummaryDataPoint)) } type unknownAggregation struct { @@ -734,6 +797,7 @@ func TestAssertAggregationsEqual(t *testing.T) { AssertAggregationsEqual(t, histogramFloat64A, histogramFloat64A) AssertAggregationsEqual(t, exponentialHistogramInt64A, exponentialHistogramInt64A) AssertAggregationsEqual(t, exponentialHistogramFloat64A, exponentialHistogramFloat64A) + AssertAggregationsEqual(t, summaryA, summaryA) r := equalAggregations(sumInt64A, nil, config{}) assert.Len(t, r, 1, "should return nil comparison mismatch only") @@ -815,6 +879,15 @@ func TestAssertAggregationsEqual(t *testing.T) { r = equalAggregations(exponentialHistogramFloat64A, exponentialHistogramFloat64D, config{ignoreValue: true}) assert.Len(t, r, 0, "value should be ignored: %v == %v", exponentialHistogramFloat64A, exponentialHistogramFloat64D) + + r = equalAggregations(summaryA, summaryB, config{}) + assert.Greaterf(t, len(r), 0, "summaries should not be equal: %v == %v", summaryA, summaryB) + + r = equalAggregations(summaryA, summaryC, config{ignoreTimestamp: true}) + assert.Len(t, r, 0, "summaries should be equal: %v", r) + + r = equalAggregations(summaryA, summaryD, config{ignoreValue: true}) + assert.Len(t, r, 0, "value should be ignored: %v == %v", summaryA, summaryD) } func TestAssertAttributes(t *testing.T) { @@ -839,6 +912,9 @@ func TestAssertAttributes(t *testing.T) { AssertHasAttributes(t, exponentialHistogramInt64A, attribute.Bool("A", true)) AssertHasAttributes(t, exponentialHistogramFloat64A, attribute.Bool("A", true)) AssertHasAttributes(t, exponentialBucket2, attribute.Bool("A", true)) // No-op, always pass. + AssertHasAttributes(t, summaryDataPointA, attribute.Bool("A", true)) + AssertHasAttributes(t, summaryA, attribute.Bool("A", true)) + AssertHasAttributes(t, quantileValueA, attribute.Bool("A", true)) // No-op, always pass. r := hasAttributesAggregation(gaugeInt64A, attribute.Bool("A", true)) assert.Equal(t, len(r), 0, "gaugeInt64A has A=True") @@ -856,6 +932,8 @@ func TestAssertAttributes(t *testing.T) { assert.Equal(t, len(r), 0, "exponentialHistogramInt64A has A=True") r = hasAttributesAggregation(exponentialHistogramFloat64A, attribute.Bool("A", true)) assert.Equal(t, len(r), 0, "exponentialHistogramFloat64A has A=True") + r = hasAttributesAggregation(summaryA, attribute.Bool("A", true)) + assert.Equal(t, len(r), 0, "summaryA has A=True") r = hasAttributesAggregation(gaugeInt64A, attribute.Bool("A", false)) assert.Greater(t, len(r), 0, "gaugeInt64A does not have A=False") @@ -873,6 +951,8 @@ func TestAssertAttributes(t *testing.T) { assert.Greater(t, len(r), 0, "exponentialHistogramInt64A does not have A=False") r = hasAttributesAggregation(exponentialHistogramFloat64A, attribute.Bool("A", false)) assert.Greater(t, len(r), 0, "exponentialHistogramFloat64A does not have A=False") + r = hasAttributesAggregation(summaryA, attribute.Bool("A", false)) + assert.Greater(t, len(r), 0, "summaryA does not have A=False") r = hasAttributesAggregation(gaugeInt64A, attribute.Bool("B", true)) assert.Greater(t, len(r), 0, "gaugeInt64A does not have Attribute B") @@ -890,6 +970,8 @@ func TestAssertAttributes(t *testing.T) { assert.Greater(t, len(r), 0, "exponentialHistogramIntA does not have Attribute B") r = hasAttributesAggregation(exponentialHistogramFloat64A, attribute.Bool("B", true)) assert.Greater(t, len(r), 0, "exponentialHistogramFloatA does not have Attribute B") + r = hasAttributesAggregation(summaryA, attribute.Bool("B", true)) + assert.Greater(t, len(r), 0, "summaryA does not have Attribute B") } func TestAssertAttributesFail(t *testing.T) { @@ -914,6 +996,10 @@ func TestAssertAttributesFail(t *testing.T) { assert.False(t, AssertHasAttributes(fakeT, exponentialHistogramDataPointFloat64A, attribute.Bool("B", true))) assert.False(t, AssertHasAttributes(fakeT, exponentialHistogramInt64A, attribute.Bool("A", false))) assert.False(t, AssertHasAttributes(fakeT, exponentialHistogramFloat64A, attribute.Bool("B", true))) + assert.False(t, AssertHasAttributes(fakeT, summaryDataPointA, attribute.Bool("A", false))) + assert.False(t, AssertHasAttributes(fakeT, summaryDataPointA, attribute.Bool("B", true))) + assert.False(t, AssertHasAttributes(fakeT, summaryA, attribute.Bool("A", false))) + assert.False(t, AssertHasAttributes(fakeT, summaryA, attribute.Bool("B", true))) sum := metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, From 28e98d5d442d2615bc00939b31a3e69a625b6ded Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Fri, 20 Oct 2023 17:36:33 +0000 Subject: [PATCH 7/8] address comments --- sdk/metric/metricdata/data.go | 12 ++++++------ .../metricdata/metricdatatest/assertion.go | 8 ++++---- .../metricdatatest/assertion_test.go | 19 +++++++++---------- .../metricdata/metricdatatest/comparisons.go | 17 ++++++----------- 4 files changed, 25 insertions(+), 31 deletions(-) diff --git a/sdk/metric/metricdata/data.go b/sdk/metric/metricdata/data.go index 897e408c659..995d42b38f1 100644 --- a/sdk/metric/metricdata/data.go +++ b/sdk/metric/metricdata/data.go @@ -272,21 +272,21 @@ type SummaryDataPoint struct { Count uint64 // Sum is the sum of the values recorded. - Sum *float64 + Sum float64 // (Optional) list of values at different quantiles of the distribution calculated // from the current snapshot. The quantiles must be strictly increasing. - QuantileValues []ValueAtQuantile + QuantileValues []QuantileValue } -// ValueAtQuantile the value at a given quantile of a distribution. -type ValueAtQuantile struct { - // The quantile of a distribution. +// QuantileValue is the value at a given quantile of a summary. +type QuantileValue struct { + // Quantile is the quantile of this value. // // Must be in the interval [0.0, 1.0]. Quantile float64 - // The value at the given quantile of a distribution. + // Value is the value at the given quantile of a summary. // // Quantile values must NOT be negative. Value float64 diff --git a/sdk/metric/metricdata/metricdatatest/assertion.go b/sdk/metric/metricdata/metricdatatest/assertion.go index 56fed69c077..a65fd99f482 100644 --- a/sdk/metric/metricdata/metricdatatest/assertion.go +++ b/sdk/metric/metricdata/metricdatatest/assertion.go @@ -49,7 +49,7 @@ type Datatypes interface { metricdata.ExponentialBucket | metricdata.Summary | metricdata.SummaryDataPoint | - metricdata.ValueAtQuantile + metricdata.QuantileValue // Interface types are not allowed in union types, therefore the // Aggregation and Value type from metricdata are not included here. @@ -184,8 +184,8 @@ func AssertEqual[T Datatypes](t TestingT, expected, actual T, opts ...Option) bo r = equalSummary(e, aIface.(metricdata.Summary), cfg) case metricdata.SummaryDataPoint: r = equalSummaryDataPoint(e, aIface.(metricdata.SummaryDataPoint), cfg) - case metricdata.ValueAtQuantile: - r = equalValueAtQuantile(e, aIface.(metricdata.ValueAtQuantile), cfg) + case metricdata.QuantileValue: + r = equalQuantileValue(e, aIface.(metricdata.QuantileValue), cfg) default: // We control all types passed to this, panic to signal developers // early they changed things in an incompatible way. @@ -264,7 +264,7 @@ func AssertHasAttributes[T Datatypes](t TestingT, actual T, attrs ...attribute.K reasons = hasAttributesSummary(e, attrs...) case metricdata.SummaryDataPoint: reasons = hasAttributesSummaryDataPoint(e, attrs...) - case metricdata.ValueAtQuantile: + case metricdata.QuantileValue: // Nothing to check. default: // We control all types passed to this, panic to signal developers diff --git a/sdk/metric/metricdata/metricdatatest/assertion_test.go b/sdk/metric/metricdata/metricdatatest/assertion_test.go index c9af783255a..9d647a54004 100644 --- a/sdk/metric/metricdata/metricdatatest/assertion_test.go +++ b/sdk/metric/metricdata/metricdatatest/assertion_test.go @@ -257,12 +257,11 @@ var ( Exemplars: []metricdata.Exemplar[float64]{exemplarFloat64A}, } - sum = 3.0 - quantileValueA = metricdata.ValueAtQuantile{ + quantileValueA = metricdata.QuantileValue{ Quantile: 0.0, Value: 0.1, } - quantileValueB = metricdata.ValueAtQuantile{ + quantileValueB = metricdata.QuantileValue{ Quantile: 0.1, Value: 0.2, } @@ -271,30 +270,30 @@ var ( StartTime: startA, Time: endA, Count: 2, - Sum: &sum, - QuantileValues: []metricdata.ValueAtQuantile{quantileValueA}, + Sum: 3, + QuantileValues: []metricdata.QuantileValue{quantileValueA}, } summaryDataPointB = metricdata.SummaryDataPoint{ Attributes: attrB, StartTime: startB, Time: endB, Count: 3, - QuantileValues: []metricdata.ValueAtQuantile{quantileValueB}, + QuantileValues: []metricdata.QuantileValue{quantileValueB}, } summaryDataPointC = metricdata.SummaryDataPoint{ Attributes: attrA, StartTime: startB, Time: endB, Count: 2, - Sum: &sum, - QuantileValues: []metricdata.ValueAtQuantile{quantileValueA}, + Sum: 3, + QuantileValues: []metricdata.QuantileValue{quantileValueA}, } summaryDataPointD = metricdata.SummaryDataPoint{ Attributes: attrA, StartTime: startA, Time: endA, Count: 3, - QuantileValues: []metricdata.ValueAtQuantile{quantileValueB}, + QuantileValues: []metricdata.QuantileValue{quantileValueB}, } exponentialBucket2 = metricdata.ExponentialBucket{ @@ -704,7 +703,7 @@ func TestAssertEqual(t *testing.T) { t.Run("ExponentialBuckets", testDatatype(exponentialBucket2, exponentialBucket3, equalExponentialBuckets)) t.Run("Summary", testDatatype(summaryA, summaryB, equalSummary)) t.Run("SummaryDataPoint", testDatatype(summaryDataPointA, summaryDataPointB, equalSummaryDataPoint)) - t.Run("QuantileValues", testDatatype(quantileValueA, quantileValueB, equalValueAtQuantile)) + t.Run("QuantileValues", testDatatype(quantileValueA, quantileValueB, equalQuantileValue)) } func TestAssertEqualIgnoreTime(t *testing.T) { diff --git a/sdk/metric/metricdata/metricdatatest/comparisons.go b/sdk/metric/metricdata/metricdatatest/comparisons.go index 362d4ae03c2..c6407aca8f8 100644 --- a/sdk/metric/metricdata/metricdatatest/comparisons.go +++ b/sdk/metric/metricdata/metricdatatest/comparisons.go @@ -467,12 +467,14 @@ func equalSummaryDataPoint(a, b metricdata.SummaryDataPoint, cfg config) (reason if a.Count != b.Count { reasons = append(reasons, notEqualStr("Count", a.Count, b.Count)) } - reasons = append(reasons, equalSummarySum(a.Sum, b.Sum, cfg)...) + if a.Sum != b.Sum { + reasons = append(reasons, notEqualStr("Sum", a.Sum, b.Sum)) + } r := compareDiff(diffSlices( a.QuantileValues, a.QuantileValues, - func(a, b metricdata.ValueAtQuantile) bool { - r := equalValueAtQuantile(a, b, cfg) + func(a, b metricdata.QuantileValue) bool { + r := equalQuantileValue(a, b, cfg) return len(r) == 0 }, )) @@ -483,7 +485,7 @@ func equalSummaryDataPoint(a, b metricdata.SummaryDataPoint, cfg config) (reason return reasons } -func equalValueAtQuantile(a, b metricdata.ValueAtQuantile, _ config) (reasons []string) { +func equalQuantileValue(a, b metricdata.QuantileValue, _ config) (reasons []string) { if a.Quantile != b.Quantile { reasons = append(reasons, notEqualStr("Quantile", a.Quantile, b.Quantile)) } @@ -493,13 +495,6 @@ func equalValueAtQuantile(a, b metricdata.ValueAtQuantile, _ config) (reasons [] return reasons } -func equalSummarySum(a, b *float64, _ config) (reasons []string) { - if a != b && ((a == nil || b == nil) || *a != *b) { - reasons = append(reasons, notEqualStr("Sum", a, b)) - } - return reasons -} - func notEqualStr(prefix string, expected, actual interface{}) string { return fmt.Sprintf("%s not equal:\nexpected: %v\nactual: %v", prefix, expected, actual) } From 6453b02450566a9fa6b6d8ed94698ea70ff60048 Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Mon, 23 Oct 2023 15:00:17 -0400 Subject: [PATCH 8/8] Update CHANGELOG.md Co-authored-by: Tyler Yahn --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52a2f1f229c..a6a5e238ef8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add the `go.opentelemetry.io/otel/trace/noop` package as a default no-op implementation of the trace API. (#4620) - Add context propagation in `go.opentelemetry.io/otel/example/dice`. (#4644) - Add view configuration to `go.opentelemetry.io/otel/example/prometheus`. (#4649) -- Add Summary, SummaryDataPoint, and ValueAtQuantile to `go.opentelemetry.io/sdk/metric/metricdata`. (#4622) +- Add Summary, SummaryDataPoint, and QuantileValue to `go.opentelemetry.io/sdk/metric/metricdata`. (#4622) ### Deprecated