Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
dashpole committed Oct 20, 2023
1 parent 7e79a75 commit 4ceffd5
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 31 deletions.
12 changes: 6 additions & 6 deletions sdk/metric/metricdata/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,21 +272,21 @@ type SummaryDataPoint struct {
Count uint64

// Sum is the sum of the values recorded.
Sum *float64
Sum Extrema[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
Expand Down
8 changes: 4 additions & 4 deletions sdk/metric/metricdata/metricdatatest/assertion.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
20 changes: 10 additions & 10 deletions sdk/metric/metricdata/metricdatatest/assertion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,12 @@ var (
Exemplars: []metricdata.Exemplar[float64]{exemplarFloat64A},
}

sum = 3.0
quantileValueA = metricdata.ValueAtQuantile{
sum = metricdata.NewExtrema[float64](3)
quantileValueA = metricdata.QuantileValue{
Quantile: 0.0,
Value: 0.1,
}
quantileValueB = metricdata.ValueAtQuantile{
quantileValueB = metricdata.QuantileValue{
Quantile: 0.1,
Value: 0.2,
}
Expand All @@ -271,30 +271,30 @@ var (
StartTime: startA,
Time: endA,
Count: 2,
Sum: &sum,
QuantileValues: []metricdata.ValueAtQuantile{quantileValueA},
Sum: sum,
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: sum,
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{
Expand Down Expand Up @@ -704,7 +704,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) {
Expand Down
15 changes: 4 additions & 11 deletions sdk/metric/metricdata/metricdatatest/comparisons.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,12 +467,12 @@ 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)...)
reasons = append(reasons, equalExtrema[float64](a.Sum, b.Sum, cfg)...)
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
},
))
Expand All @@ -483,7 +483,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))
}
Expand All @@ -493,13 +493,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)
}
Expand Down

0 comments on commit 4ceffd5

Please sign in to comment.