Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the Exponential Histogram Aggregator. #4245

Merged
merged 34 commits into from
Aug 4, 2023
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
6862bf7
Adds Exponential Histograms aggregator
MadVikingGod May 31, 2023
46f1825
Added aggregation to the pipeline.
MadVikingGod Jun 8, 2023
bd8ac1b
Add no allocation if cap is available.
MadVikingGod Jun 16, 2023
bdd0602
Expand tests
MadVikingGod Jun 20, 2023
354b1fa
Fix lint
MadVikingGod Jun 20, 2023
181d036
Fix 64 bit math on 386 platform.
MadVikingGod Jun 21, 2023
c92b2b7
Fix tests to work in go 1.19.
MadVikingGod Jun 21, 2023
2d514e0
Merge branch 'main' into mvg/exp-hist-agg
MadVikingGod Jun 21, 2023
54f540c
Merge branch 'main' into mvg/exp-hist-agg
pellared Jun 22, 2023
9b1b8d0
fix codespell
MadVikingGod Jun 21, 2023
b41b1e4
Add example
MadVikingGod Jun 22, 2023
978afbb
Update sdk/metric/aggregation/aggregation.go
MadVikingGod Jun 23, 2023
8b6f044
Merge branch 'main' into mvg/exp-hist-agg
hanyuancheung Jun 26, 2023
89a65d5
Update sdk/metric/aggregation/aggregation.go
hanyuancheung Jun 26, 2023
41757d1
Update sdk/metric/aggregation/aggregation.go
hanyuancheung Jun 26, 2023
9536e35
Changelog
MadVikingGod Jun 26, 2023
d0139f3
Merge branch 'main' into mvg/exp-hist-agg
hanyuancheung Jun 27, 2023
734f2ac
Merge branch 'main' into mvg/exp-hist-agg
MadVikingGod Jul 5, 2023
07b9cc7
Fix move
MadVikingGod Jul 5, 2023
6a68057
Merge branch 'main' into mvg/exp-hist-agg
hanyuancheung Jul 11, 2023
a4ad0b0
Address feedback from the PR.
MadVikingGod Jul 7, 2023
c0f282b
Merge remote-tracking branch 'upstream/main' into mvg/exp-hist-agg
MadVikingGod Jul 17, 2023
8ca255e
Update expo histo to new aggregator format.
MadVikingGod Jul 17, 2023
18b5f91
Fix lint
MadVikingGod Jul 17, 2023
cb3693f
Remove Zero Threshold from config of expo histograms
MadVikingGod Jul 19, 2023
9b55b27
Merge remote-tracking branch 'upstream/main' into mvg/exp-hist-agg
MadVikingGod Jul 19, 2023
66c44df
Merge remote-tracking branch 'upstream/main' into mvg/exp-hist-agg
MadVikingGod Jul 20, 2023
581c7d6
Remove DefaultExponentialHistogram()
MadVikingGod Jul 20, 2023
14aee40
Refactor GetBin, and address PR Feedback
MadVikingGod Aug 2, 2023
4244780
Merge remote-tracking branch 'upstream/main' into mvg/exp-hist-agg
MadVikingGod Aug 4, 2023
dfc13fd
Address PR feedback
MadVikingGod Aug 4, 2023
abdcaa4
Fix comment in wrong location
MadVikingGod Aug 4, 2023
4bb46fa
Fix misapplied PR feedback
MadVikingGod Aug 4, 2023
e626be7
Fix codespell
MadVikingGod Aug 4, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

- Add `ManualReader` struct in `go.opentelemetry.io/otel/sdk/metric`. (#4244)
- Add `PeriodicReader` struct in `go.opentelemetry.io/otel/sdk/metric`. (#4244)
- Add support for Exponential Histogram Aggregations. A Histogram can be configured as an Exponential Histogram using a view with the `go.opentelemetry.io/otel/sdk/metric/aggregation.ExponentialHistogram` as the aggregation. (#4245)
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
- Add `Exporter` struct in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc`. (#4272)
- Add `Exporter` struct in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#4272)
- OTLP Metrics Exporter now supports the `OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE` environment variable. (#4287)
Expand Down
51 changes: 51 additions & 0 deletions sdk/metric/aggregation/aggregation.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,3 +164,54 @@
NoMinMax: h.NoMinMax,
}
}

// ExponentialHistogram is an aggregation that summarizes a set of
// measurements as an histogram with bucket widths that grow exponentially.
type ExponentialHistogram struct {
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
// MaxSize is the maximum number of buckets to use for the histogram.
MaxSize int
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
// MaxScale is the maximum resolution scale to use for the histogram.
MaxScale int
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
MrAlias marked this conversation as resolved.
Show resolved Hide resolved

// NoMinMax indicates whether to not record the min and max of the
// distribution. By default, these extrema are recorded.
//
// Recording these extrema for cumulative data is expected to have little
// value, they will represent the entire life of the instrument instead of
// just the current collection cycle. It is recommended to set this to true
// for that type of data to avoid computing the low-value extrema.
NoMinMax bool
}

var _ Aggregation = ExponentialHistogram{}

// private attempts to ensure no user-defined Aggregation is allowed. The
// OTel specification does not allow user-defined Aggregation currently.
func (e ExponentialHistogram) private() {}

Check warning on line 190 in sdk/metric/aggregation/aggregation.go

View check run for this annotation

Codecov / codecov/patch

sdk/metric/aggregation/aggregation.go#L190

Added line #L190 was not covered by tests

// Copy returns a deep copy of the Aggregation.
func (e ExponentialHistogram) Copy() Aggregation {
return e

Check warning on line 194 in sdk/metric/aggregation/aggregation.go

View check run for this annotation

Codecov / codecov/patch

sdk/metric/aggregation/aggregation.go#L193-L194

Added lines #L193 - L194 were not covered by tests
}

const (
expoMaxScale = 20
expoMinScale = -10
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
)

// errExpoHist is returned by misconfigured ExplicitBucketHistograms.
var errExpoHist = fmt.Errorf("%w: explicit bucket histogram", errAgg)
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved

// Err returns an error for any misconfigured Aggregation.
func (e ExponentialHistogram) Err() error {
if e.MaxScale < expoMinScale {
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
return fmt.Errorf("%w: max size %d is less than minimum scale %d", errExpoHist, e.MaxSize, expoMinScale)
}
if e.MaxScale > expoMaxScale {
return fmt.Errorf("%w: max size %d is greater than maximum scale %d", errExpoHist, e.MaxSize, expoMaxScale)
}
if e.MaxSize <= 0 {
return fmt.Errorf("%w: max size %d is less than or equal to zero", errExpoHist, e.MaxSize)
}
return nil
}
36 changes: 36 additions & 0 deletions sdk/metric/aggregation/aggregation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ import (
"github.com/stretchr/testify/assert"
)

func defaultExponentialHistogram() ExponentialHistogram {
return ExponentialHistogram{
MaxSize: 160,
MaxScale: 20,
}
}

func TestAggregationErr(t *testing.T) {
t.Run("DropOperation", func(t *testing.T) {
assert.NoError(t, Drop{}.Err())
Expand Down Expand Up @@ -55,6 +62,35 @@ func TestAggregationErr(t *testing.T) {
Boundaries: []float64{0, 1, 2, 1, 3, 4},
}.Err(), errAgg)
})

t.Run("ExponentialHistogramOperation", func(t *testing.T) {
assert.NoError(t, defaultExponentialHistogram().Err())
MrAlias marked this conversation as resolved.
Show resolved Hide resolved

assert.NoError(t, ExponentialHistogram{
MaxSize: 1,
NoMinMax: true,
}.Err())

assert.NoError(t, ExponentialHistogram{
MaxSize: 1024,
MaxScale: -3,
}.Err())
})

t.Run("InvalidExponentialHistogramOperation", func(t *testing.T) {
// MazSize must be greater than 0
assert.ErrorIs(t, ExponentialHistogram{}.Err(), errAgg)

// MaxScale Must be <=20 and >= -10
assert.ErrorIs(t, ExponentialHistogram{
MaxSize: 1,
MaxScale: 30,
}.Err(), errAgg)
assert.ErrorIs(t, ExponentialHistogram{
MaxSize: 1,
MaxScale: -20,
}.Err(), errAgg)
})
}

func TestExplicitBucketHistogramDeepCopy(t *testing.T) {
Expand Down
19 changes: 19 additions & 0 deletions sdk/metric/internal/aggregate/aggregate.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,22 @@
return len(hData.DataPoints)
}
}

// ExponentialBucketHistogram returns a histogram aggregate function input and
// output.
func (b Builder[N]) ExponentialBucketHistogram(cfg aggregation.ExponentialHistogram, noSum bool) (Measure[N], ComputeAggregation) {
var h aggregator[N]
switch b.Temporality {
case metricdata.DeltaTemporality:
h = newDeltaExponentialHistogram[N](cfg, noSum)
default:
h = newCumulativeExponentialHistogram[N](cfg, noSum)

Check warning on line 137 in sdk/metric/internal/aggregate/aggregate.go

View check run for this annotation

Codecov / codecov/patch

sdk/metric/internal/aggregate/aggregate.go#L131-L137

Added lines #L131 - L137 were not covered by tests
}
return b.input(h), func(dest *metricdata.Aggregation) int {
// TODO (#4220): optimize memory reuse here.
*dest = h.Aggregation()

hData, _ := (*dest).(metricdata.ExponentialHistogram[N])
return len(hData.DataPoints)
}

Check warning on line 145 in sdk/metric/internal/aggregate/aggregate.go

View check run for this annotation

Codecov / codecov/patch

sdk/metric/internal/aggregate/aggregate.go#L139-L145

Added lines #L139 - L145 were not covered by tests
}
Loading