Skip to content

Commit

Permalink
Simplify the histogram implementation (#4370)
Browse files Browse the repository at this point in the history
  • Loading branch information
MrAlias committed Jul 27, 2023
1 parent 4f0d73c commit 859a870
Show file tree
Hide file tree
Showing 6 changed files with 277 additions and 425 deletions.
31 changes: 8 additions & 23 deletions sdk/metric/internal/aggregate/aggregate.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,17 @@ package aggregate // import "go.opentelemetry.io/otel/sdk/metric/internal/aggreg

import (
"context"
"time"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/sdk/metric/aggregation"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
)

// now is used to return the current local time while allowing tests to
// override the default time.Now function.
var now = time.Now

// Measure receives measurements to be aggregated.
type Measure[N int64 | float64] func(context.Context, N, attribute.Set)

Expand Down Expand Up @@ -53,19 +58,6 @@ func (b Builder[N]) filter(f Measure[N]) Measure[N] {
return f
}

func (b Builder[N]) input(agg aggregator[N]) Measure[N] {
if b.Filter != nil {
fltr := b.Filter // Copy to make it immutable after assignment.
return func(_ context.Context, n N, a attribute.Set) {
fAttr, _ := a.Filter(fltr)
agg.Aggregate(n, fAttr)
}
}
return func(_ context.Context, n N, a attribute.Set) {
agg.Aggregate(n, a)
}
}

// LastValue returns a last-value aggregate function input and output.
//
// The Builder.Temporality is ignored and delta is use always.
Expand Down Expand Up @@ -111,19 +103,12 @@ func (b Builder[N]) Sum(monotonic bool) (Measure[N], ComputeAggregation) {
// ExplicitBucketHistogram returns a histogram aggregate function input and
// output.
func (b Builder[N]) ExplicitBucketHistogram(cfg aggregation.ExplicitBucketHistogram, noSum bool) (Measure[N], ComputeAggregation) {
var h aggregator[N]
h := newHistogram[N](cfg, noSum)
switch b.Temporality {
case metricdata.DeltaTemporality:
h = newDeltaHistogram[N](cfg, noSum)
return b.filter(h.measure), h.delta
default:
h = newCumulativeHistogram[N](cfg, noSum)
}
return b.input(h), func(dest *metricdata.Aggregation) int {
// TODO (#4220): optimize memory reuse here.
*dest = h.Aggregation()

hData, _ := (*dest).(metricdata.Histogram[N])
return len(hData.DataPoints)
return b.filter(h.measure), h.cumulative
}
}

Expand Down
26 changes: 8 additions & 18 deletions sdk/metric/internal/aggregate/aggregate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,21 +55,12 @@ var (
}
)

type inputTester[N int64 | float64] struct {
aggregator[N]

value N
attr attribute.Set
func TestBuilderFilter(t *testing.T) {
t.Run("Int64", testBuilderFilter[int64]())
t.Run("Float64", testBuilderFilter[float64]())
}

func (it *inputTester[N]) Aggregate(v N, a attribute.Set) { it.value, it.attr = v, a }

func TestBuilderInput(t *testing.T) {
t.Run("Int64", testBuilderInput[int64]())
t.Run("Float64", testBuilderInput[float64]())
}

func testBuilderInput[N int64 | float64]() func(t *testing.T) {
func testBuilderFilter[N int64 | float64]() func(t *testing.T) {
return func(t *testing.T) {
t.Helper()

Expand All @@ -78,12 +69,11 @@ func testBuilderInput[N int64 | float64]() func(t *testing.T) {
return func(t *testing.T) {
t.Helper()

it := &inputTester[N]{}
meas := b.input(it)
meas := b.filter(func(_ context.Context, v N, a attribute.Set) {
assert.Equal(t, value, v, "measured incorrect value")
assert.Equal(t, wantA, a, "measured incorrect attributes")
})
meas(context.Background(), value, attr)

assert.Equal(t, value, it.value, "measured incorrect value")
assert.Equal(t, wantA, it.attr, "measured incorrect attributes")
}
}

Expand Down
40 changes: 0 additions & 40 deletions sdk/metric/internal/aggregate/aggregator.go

This file was deleted.

148 changes: 0 additions & 148 deletions sdk/metric/internal/aggregate/aggregator_test.go

This file was deleted.

Loading

0 comments on commit 859a870

Please sign in to comment.