Add the Exponential Histogram and other aggregators for review LS-29757

lightstep/sdk/metric/aggregator/aggregation/aggregation.go

@@ -51,10 +51,10 @@ type (
 		Gauge() number.Number
 	}
 
-	// ExponentialHistogram returns the count of events in
-	// exponential-scale buckets defined as a function of a
-	// scale parameter.  See a detailed explanation in the
-	// OpenTelemetry metrics data model:
+	// Histogram returns the count of events in exponential-scale
+	// buckets defined as a function of a scale parameter.  See a
+	// detailed explanation in the OpenTelemetry metrics data
+	// model:
 	// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/datamodel.md#exponentialhistogram
 	Histogram interface {
 		Aggregation
@@ -66,9 +66,9 @@ type (
 		Negative() Buckets
 	}
 
-	// ExponentialBuckets describes a range of consecutive
-	// buckets, starting at Offset().  This type is used to encode
-	// either the positive or negative ranges of an ExponentialHistogram.
+	// Buckets describes a range of consecutive buckets, starting
+	// at Offset().  This type is used to encode either the
+	// positive or negative ranges of an Histogram.
 	Buckets interface {
 		Offset() int32
 		Len() uint32

lightstep/sdk/metric/aggregator/aggregator.go

@@ -34,7 +34,7 @@ var (
 // This rejects NaN and Inf values.  This rejects negative values when the
 // aggregation does not support negative values, including
 // monotonic counter metrics and Histogram metrics.
-func RangeTest[N number.Any, Traits number.Traits[N]](num N, kind aggregation.Category) bool {
+func RangeTest[N number.Any, Traits number.Traits[N]](num N, kind sdkinstrument.Kind) bool {
 	var traits Traits
 
 	if traits.IsInf(num) {
@@ -49,8 +49,8 @@ func RangeTest[N number.Any, Traits number.Traits[N]](num N, kind aggregation.Ca
 
 	// Check for negative values
 	switch kind {
-	case aggregation.MonotonicSumCategory,
-		aggregation.HistogramCategory:
+	case sdkinstrument.CounterKind,
+		sdkinstrument.HistogramKind:
 		if num < 0 {
 			otel.Handle(ErrNegativeInput)
 			return false
@@ -68,29 +68,37 @@ type Config struct {
 }
 
 // Methods implements a specific aggregation behavior.  Methods
-// are parameterized by the type of the number (int64, flot64),
-// the Storage (generally an `Storage` struct in the same package),
-// and the Config (generally a `Config` struct in the same package).
+// are parameterized by the type of the number (int64, float64),
+// the Storage (generally an `Storage` struct in the same package).
 type Methods[N number.Any, Storage any] interface {
-	// Init initializes the storage with its configuration.
+	// Init initializes the storage.
 	Init(ptr *Storage, cfg Config)
 
 	// Update modifies the aggregator concurrently with respect to
-	// SynchronizedMove() for single new measurement.
+	// Move() or Copy()
 	Update(ptr *Storage, number N)
 
-	// SynchronizedMove concurrently copies and resets the
-	// `inputIsReset` aggregator.
-	SynchronizedMove(inputIsReset, output *Storage)
+	// Move atomically copies `input` to `output` and resets the
+	// `input` to the zero state.  The change to `input` is
+	// synchronized with `Update()`.  The change to `output` is
+	// synchronized with the accessor methods in ./aggregation.
+	Move(input, output *Storage)
 
-	// Simply reset the storage.
-	Reset(ptr *Storage)
+	// Merge adds the contents of `input` to `output`.  The read
+	// of `input` is unsynchronized.  The write to `output` is
+	// synchronized with concurrent `Merge()` calls (writing) and
+	// concurrent `Copy()` calls (reading).
+	Merge(input, output *Storage)
 
-	// Merge adds the contents of `input` to `output`.
-	Merge(output, input *Storage)
+	// Copy replaces the contents of `output` with `input`.  The
+	// read from `input` is synchronized with `Merge()` calls.
+	Copy(input, output *Storage)
 
-	// SubtractSwap removes the contents of `operand` from `valueToModify`
-	SubtractSwap(valueToModify, operandToModify *Storage)
+	// SubtractSwap performs `*operand = *value - *operand`
+	// without synchronization.  We are not concerned with
+	// synchronization because this is only used for asynchronous
+	// instruments.
+	SubtractSwap(value, operand *Storage)
 
 	// ToAggregation returns an exporter-ready value.
 	ToAggregation(ptr *Storage) aggregation.Aggregation
@@ -105,7 +113,7 @@ type Methods[N number.Any, Storage any] interface {
 	// Updates.  This tests whether an aggregation has zero sum,
 	// zero count, or zero difference, depending on the
 	// aggregation.  If the instrument is asynchronous, this will
-	// be used after subtraction.
+	// be called after subtraction.
 	HasChange(ptr *Storage) bool
 }
 

lightstep/sdk/metric/aggregator/doc.go

@@ -0,0 +1,29 @@
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// package aggregator defines the interface used between the SDK and
+// various kinds of aggregation.
+//
+// Note, about the use of Go-1.18 generics!  This code makes use of a
+// pattern that gives the SDK fine control over memory layout via the
+// use of a `Storage any` type constraint.  The SDK is expected to
+// pair the allocated `Storage any` value with corresponding `Methods`
+// type, which provides pointer-receiver methods that operate on one
+// or two `Storage` objects.
+//
+// This means there are places where a Storage object appears as an
+// generic type with no constraints.  Where that happens without the
+// corresponding Methods, it means the Storage is opaque at that level
+// in the code.
+package aggregator // import "github.com/lightstep/otel-launcher-go/lightstep/sdk/metric/aggregator"

lightstep/sdk/metric/aggregator/gauge/gauge.go

@@ -64,29 +64,26 @@ func (Methods[N, Traits, Storage]) Init(state *State[N, Traits], _ aggregator.Co
 	// Note: storage is zero to start
 }
 
-func (Methods[N, Traits, Storage]) Reset(ptr *State[N, Traits]) {
-	var t Traits
-	t.SetAtomic(&ptr.value, 0)
-}
-
 func (Methods[N, Traits, Storage]) HasChange(ptr *State[N, Traits]) bool {
 	return ptr.value != 0
 }
 
-func (Methods[N, Traits, Storage]) SynchronizedMove(resetSrc, dest *State[N, Traits]) {
+func (Methods[N, Traits, Storage]) Move(src, dest *State[N, Traits]) {
 	var t Traits
-	dest.value = t.SwapAtomic(&resetSrc.value, 0)
+	dest.value = t.SwapAtomic(&src.value, 0)
+}
+
+func (Methods[N, Traits, Storage]) Copy(src, dest *State[N, Traits]) {
+	var t Traits
+	dest.value = t.GetAtomic(&src.value)
 }
 
 func (Methods[N, Traits, Storage]) Update(state *State[N, Traits], number N) {
-	if !aggregator.RangeTest[N, Traits](number, aggregation.GaugeCategory) {
-		return
-	}
 	var t Traits
 	t.SetAtomic(&state.value, number)
 }
 
-func (Methods[N, Traits, Storage]) Merge(to, from *State[N, Traits]) {
+func (Methods[N, Traits, Storage]) Merge(from, to *State[N, Traits]) {
 	to.value = from.value
 }
 

lightstep/sdk/metric/aggregator/histogram/README.md

@@ -73,7 +73,7 @@ maximum bucket indices for the current scale.
 TODO: Apply Go 1.18 generics treatment to this `interface{}`.
 
 The backing array is circular.  When the first observation is added to
-a set of (positive or negative) buckets, the initial conditition is
+a set of (positive or negative) buckets, the initial condition is
 `indexBase == indexStart == indexEnd`.  When new observations are
 added at indices lower than `indexStart` and while capacity is greater
 than `indexEnd - indexBase`, new values are filled in by adjusting
@@ -146,7 +146,7 @@ The `Scale` function returns the current scale of the histogram.
 
 If the scale is variable and there are no non-zero values in the
 histogram, the scale is zero by definition; when there is only a
-single value in this case, it's scale is MinScale (20) by definition.
+single value in this case, its scale is MinScale (20) by definition.
 
 If the scale is fixed because of range limits, the fixed scale will be
 returned even for any size histogram.