diff --git a/CHANGELOG.md b/CHANGELOG.md index b35384ca..8e6ff0d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## Unreleased +## [1.16.0](https://github.com/lightstep/otel-launcher-go/releases/tag/v1.16.0) - 2023-04-27) + +- LS Metrics SDK supports an API hint for controlling Temporality on a per-instrument basis. [#426](https://github.com/lightstep/otel-launcher-go/pull/426) + ## [1.15.1](https://github.com/lightstep/otel-launcher-go/releases/tag/v1.15.1) - 2023-04-03) - Correct race conditions related to sharing of the input attributes slice. [#422](https://github.com/lightstep/otel-launcher-go/pull/422) diff --git a/VERSION b/VERSION index ace44233..15b989e3 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.15.1 +1.16.0 diff --git a/go.mod b/go.mod index 0659b7f6..825c068e 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,8 @@ module github.com/lightstep/otel-launcher-go go 1.18 require ( - github.com/lightstep/otel-launcher-go/lightstep/sdk/metric v1.15.1 - github.com/lightstep/otel-launcher-go/pipelines v1.15.1 + github.com/lightstep/otel-launcher-go/lightstep/sdk/metric v1.16.0 + github.com/lightstep/otel-launcher-go/pipelines v1.16.0 github.com/sethvargo/go-envconfig v0.8.3 github.com/stretchr/testify v1.8.2 go.opentelemetry.io/otel v1.14.0 @@ -23,7 +23,7 @@ require ( github.com/golang/protobuf v1.5.2 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.7.0 // indirect github.com/lightstep/go-expohisto v1.0.0 // indirect - github.com/lightstep/otel-launcher-go/lightstep/instrumentation v1.15.1 // indirect + github.com/lightstep/otel-launcher-go/lightstep/instrumentation v1.16.0 // indirect github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c // indirect diff --git a/launcher/version.go b/launcher/version.go index f469536d..1b989789 100644 --- a/launcher/version.go +++ b/launcher/version.go @@ -14,4 +14,4 @@ package launcher -const version = "1.15.1" +const version = "1.16.0" diff --git a/lightstep/sdk/metric/README.md b/lightstep/sdk/metric/README.md index 1fe6759e..30b4ef16 100644 --- a/lightstep/sdk/metric/README.md +++ b/lightstep/sdk/metric/README.md @@ -88,6 +88,15 @@ To set the MinMaxSumCount aggregation for a specific histogram instrument: } ``` +To override the Temporality selected for a specific instrument: + +``` +{ + "description": "measurement of ...", + "temporality": "delta" +} +``` + ### Synchronous Gauge instrument [OpenTelemetry metrics API does not support a synchronous Gauge @@ -122,6 +131,10 @@ For example, to configure a synchronous Gauge: ) ``` +Note that the API hint for Gauge aggregation can be combined with the +API hint for temporality, allowing control over Gauge behavior +independent of the default Temporality choice for UpDownCounter +instruments. ### Performance settings diff --git a/lightstep/sdk/metric/aggregator/aggregation/temporality.go b/lightstep/sdk/metric/aggregator/aggregation/temporality.go index add70a9c..a8cd86d9 100644 --- a/lightstep/sdk/metric/aggregator/aggregation/temporality.go +++ b/lightstep/sdk/metric/aggregator/aggregation/temporality.go @@ -12,11 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:generate stringer -type=Temporality - package aggregation // import "github.com/lightstep/otel-launcher-go/lightstep/sdk/metric/aggregator/aggregation" -import "github.com/lightstep/otel-launcher-go/lightstep/sdk/metric/sdkinstrument" +import ( + "strings" + + "github.com/lightstep/otel-launcher-go/lightstep/sdk/metric/sdkinstrument" +) type Temporality uint8 @@ -57,3 +59,28 @@ func (t Temporality) Valid() bool { } return false } + +const ( + NameCumulative = "cumulative" + NameDelta = "delta" +) + +func (t Temporality) String() string { + switch t { + case DeltaTemporality: + return NameDelta + case CumulativeTemporality: + return NameCumulative + } + return "undefined" +} + +func ParseTemporality(str string) (Temporality, bool) { + switch strings.ToLower(str) { + case NameCumulative: + return CumulativeTemporality, true + case NameDelta: + return DeltaTemporality, true + } + return UndefinedTemporality, false +} diff --git a/lightstep/sdk/metric/aggregator/aggregation/temporality_string.go b/lightstep/sdk/metric/aggregator/aggregation/temporality_string.go deleted file mode 100644 index 36cc5e8c..00000000 --- a/lightstep/sdk/metric/aggregator/aggregation/temporality_string.go +++ /dev/null @@ -1,25 +0,0 @@ -// Code generated by "stringer -type=Temporality"; DO NOT EDIT. - -package aggregation - -import "strconv" - -func _() { - // An "invalid array index" compiler error signifies that the constant values have changed. - // Re-run the stringer command to generate them again. - var x [1]struct{} - _ = x[UndefinedTemporality-0] - _ = x[CumulativeTemporality-1] - _ = x[DeltaTemporality-2] -} - -const _Temporality_name = "UndefinedTemporalityCumulativeTemporalityDeltaTemporality" - -var _Temporality_index = [...]uint8{0, 20, 41, 57} - -func (i Temporality) String() string { - if i >= Temporality(len(_Temporality_index)-1) { - return "Temporality(" + strconv.FormatInt(int64(i), 10) + ")" - } - return _Temporality_name[_Temporality_index[i]:_Temporality_index[i+1]] -} diff --git a/lightstep/sdk/metric/aggregator/aggregation/temporality_test.go b/lightstep/sdk/metric/aggregator/aggregation/temporality_test.go new file mode 100644 index 00000000..35455100 --- /dev/null +++ b/lightstep/sdk/metric/aggregator/aggregation/temporality_test.go @@ -0,0 +1,40 @@ +// 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 aggregation + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestParseTemporality(t *testing.T) { + for _, test := range []struct { + input string + tempo Temporality + ok bool + }{ + {"cumulative", CumulativeTemporality, true}, + {"Cumulative", CumulativeTemporality, true}, + {"delta", DeltaTemporality, true}, + {"DELTA", DeltaTemporality, true}, + {"other", UndefinedTemporality, false}, + {"", UndefinedTemporality, false}, + } { + tempo, ok := ParseTemporality(test.input) + require.Equal(t, test.tempo, tempo) + require.Equal(t, test.ok, ok) + } +} diff --git a/lightstep/sdk/metric/example/go.mod b/lightstep/sdk/metric/example/go.mod index 27d8bd0e..6b57d174 100644 --- a/lightstep/sdk/metric/example/go.mod +++ b/lightstep/sdk/metric/example/go.mod @@ -3,7 +3,7 @@ module github.com/lightstep/otel-launcher-go/lightstep/sdk/metric/example go 1.18 require ( - github.com/lightstep/otel-launcher-go/lightstep/sdk/metric v1.15.1 + github.com/lightstep/otel-launcher-go/lightstep/sdk/metric v1.16.0 github.com/lightstep/otel-launcher-go/pipelines v1.8.0 go.opentelemetry.io/proto/otlp v0.19.0 ) diff --git a/lightstep/sdk/metric/exporters/otlp/internal/retry/retry_test.go b/lightstep/sdk/metric/exporters/otlp/internal/retry/retry_test.go index 2b2f92e6..d890fde9 100644 --- a/lightstep/sdk/metric/exporters/otlp/internal/retry/retry_test.go +++ b/lightstep/sdk/metric/exporters/otlp/internal/retry/retry_test.go @@ -17,11 +17,9 @@ package retry import ( "context" "errors" - "math" "testing" "time" - "github.com/cenkalti/backoff/v4" "github.com/stretchr/testify/assert" ) @@ -133,7 +131,9 @@ func TestBackoffRetry(t *testing.T) { origWait := waitFunc var done bool waitFunc = func(_ context.Context, d time.Duration) error { - delta := math.Ceil(float64(delay)*backoff.DefaultRandomizationFactor) - float64(delay) + // N.B. the old formula was flaky, this code is EOL + // and we won't wonder how this ever passed or changed. + delta := float64(time.Nanosecond) assert.InDelta(t, delay, d, delta, "retry not backoffed") // Try twice to ensure call is attempted again after delay. if done { diff --git a/lightstep/sdk/metric/internal/viewstate/viewstate.go b/lightstep/sdk/metric/internal/viewstate/viewstate.go index f4e9ab2c..c3bd89cc 100644 --- a/lightstep/sdk/metric/internal/viewstate/viewstate.go +++ b/lightstep/sdk/metric/internal/viewstate/viewstate.go @@ -195,9 +195,10 @@ func (v *Compiler) Collectors() []data.Collector { // tryToApplyHint looks for a Lightstep-specified hint structure // encoded as JSON in the description. If valid, returns the modified // configuration, otherwise returns the default for the instrument. -func (v *Compiler) tryToApplyHint(instrument sdkinstrument.Descriptor) (_ sdkinstrument.Descriptor, akind aggregation.Kind, acfg, defCfg aggregator.Config, hinted bool) { +func (v *Compiler) tryToApplyHint(instrument sdkinstrument.Descriptor) (_ sdkinstrument.Descriptor, akind aggregation.Kind, tempo aggregation.Temporality, acfg, defCfg aggregator.Config, hinted bool) { // These are the default behaviors, we'll use them unless there's a valid hint. akind = v.views.Defaults.Aggregation(instrument.Kind) + tempo = v.views.Defaults.Temporality(instrument.Kind) defCfg = v.views.Defaults.AggregationConfig( instrument.Kind, instrument.NumberKind, @@ -206,14 +207,14 @@ func (v *Compiler) tryToApplyHint(instrument sdkinstrument.Descriptor) (_ sdkins // Check for required JSON symbols, empty strings, ... if !strings.Contains(instrument.Description, "{") { - return instrument, akind, acfg, defCfg, hinted + return instrument, akind, tempo, acfg, defCfg, hinted } var hint view.Hint if err := json.Unmarshal([]byte(instrument.Description), &hint); err != nil { // This could be noisy if valid descriptions contain spurious '{' chars. otel.Handle(fmt.Errorf("hint parse error: %w", err)) - return instrument, akind, acfg, defCfg, hinted + return instrument, akind, tempo, acfg, defCfg, hinted } // Replace the hint input with its embedded description. @@ -232,6 +233,15 @@ func (v *Compiler) tryToApplyHint(instrument sdkinstrument.Descriptor) (_ sdkins } } + if hint.Temporality != "" { + parseTempo, ok := aggregation.ParseTemporality(hint.Temporality) + if !ok { + otel.Handle(fmt.Errorf("hint invalid temporality: %v", hint.Temporality)) + } else if parseTempo != aggregation.UndefinedTemporality { + tempo = parseTempo + } + } + if hint.Config.Histogram.MaxSize != 0 { cfg := histostruct.NewConfig(histostruct.WithMaxSize(hint.Config.Histogram.MaxSize)) cfg, err := cfg.Validate() @@ -244,7 +254,7 @@ func (v *Compiler) tryToApplyHint(instrument sdkinstrument.Descriptor) (_ sdkins acfg.CardinalityLimit = hint.Config.CardinalityLimit } - return instrument, akind, acfg, defCfg, hinted + return instrument, akind, tempo, acfg, defCfg, hinted } // Compile is called during NewInstrument by the Meter @@ -267,7 +277,7 @@ func (v *Compiler) Compile(instrument sdkinstrument.Descriptor) (Instrument, Vie continue } - modified, hintAkind, hintAcfg, defCfg, hinted := v.tryToApplyHint(instrument) + modified, hintAkind, tempo, hintAcfg, defCfg, hinted := v.tryToApplyHint(instrument) instrument = modified // the hint erases itself from the description if akind == aggregation.UndefinedKind { @@ -279,7 +289,7 @@ func (v *Compiler) Compile(instrument sdkinstrument.Descriptor) (Instrument, Vie desc: viewDescriptor(instrument, view), kind: akind, acfg: pickAggConfig(hintAcfg, defCfg, view.AggregatorConfig()), - tempo: v.views.Defaults.Temporality(instrument.Kind), + tempo: tempo, hinted: hinted, } @@ -293,7 +303,7 @@ func (v *Compiler) Compile(instrument sdkinstrument.Descriptor) (Instrument, Vie // If there were no matching views, set the default aggregation. if len(matches) == 0 { - modified, akind, acfg, _, hinted := v.tryToApplyHint(instrument) + modified, akind, tempo, acfg, _, hinted := v.tryToApplyHint(instrument) instrument = modified // the hint erases itself from the description if akind != aggregation.DropKind { @@ -302,7 +312,7 @@ func (v *Compiler) Compile(instrument sdkinstrument.Descriptor) (Instrument, Vie desc: instrument, kind: akind, acfg: acfg, - tempo: v.views.Defaults.Temporality(instrument.Kind), + tempo: tempo, hinted: hinted, }) } diff --git a/lightstep/sdk/metric/internal/viewstate/viewstate_test.go b/lightstep/sdk/metric/internal/viewstate/viewstate_test.go index 0538629c..1cfa307e 100644 --- a/lightstep/sdk/metric/internal/viewstate/viewstate_test.go +++ b/lightstep/sdk/metric/internal/viewstate/viewstate_test.go @@ -1554,8 +1554,8 @@ func TestViewHints(t *testing.T) { gg, err := testCompileDescUnit( vc, - "gauge", - sdkinstrument.SyncUpDownCounter, // updowncounter->gauge + "default_gauge", + sdkinstrument.SyncUpDownCounter, // updowncounter->gauge(default is cumulative) number.Float64Kind, `{ "description": "check it", @@ -1565,6 +1565,19 @@ func TestViewHints(t *testing.T) { ) require.NoError(t, err) + dg, err := testCompileDescUnit( + vc, + "delta_gauge", + sdkinstrument.SyncUpDownCounter, // updowncounter->gauge(hinted as delta) + number.Float64Kind, + `{ + "aggregation": "gauge", + "temporality": "delta" +}`, + "", + ) + require.NoError(t, err) + set := attribute.NewSet(attribute.String("test", "attr")) seq := testSequence inputs := []float64{1, 2, 3, 4, 5, 6, 7, 8} @@ -1578,6 +1591,7 @@ func TestViewHints(t *testing.T) { histo.NewAccumulator(set), mmsc.NewAccumulator(set), gg.NewAccumulator(set), + dg.NewAccumulator(set), } { for _, inp := range inputs { acc.(Updater[float64]).Update(inp) @@ -1595,9 +1609,13 @@ func TestViewHints(t *testing.T) { test.Point(seq.Start, seq.Now, minmaxsumcount.NewFloat64(inputs...), cumulative, set.ToSlice()...), ), test.Instrument( - test.DescriptorDescUnit("gauge", sdkinstrument.SyncUpDownCounter, number.Float64Kind, "check it", ""), + test.DescriptorDescUnit("default_gauge", sdkinstrument.SyncUpDownCounter, number.Float64Kind, "check it", ""), test.Point(seq.Start, seq.Now, gauge.NewFloat64(inputs[numInputs-1]), cumulative, set.ToSlice()...), ), + test.Instrument( + test.DescriptorDescUnit("delta_gauge", sdkinstrument.SyncUpDownCounter, number.Float64Kind, "", ""), + test.Point(seq.Last, seq.Now, gauge.NewFloat64(inputs[numInputs-1]), delta, set.ToSlice()...), + ), ) require.Nil(t, *otelErrs) diff --git a/lightstep/sdk/metric/view/hint.go b/lightstep/sdk/metric/view/hint.go index 940f096f..a073721d 100644 --- a/lightstep/sdk/metric/view/hint.go +++ b/lightstep/sdk/metric/view/hint.go @@ -33,4 +33,7 @@ type Hint struct { // Config configures the aggregator selected in the // Aggregation field. Config aggregator.JSONConfig `json:"config"` + + // Temporality overrides the default temporality choice. + Temporality string `json:"temporality"` } diff --git a/pipelines/go.mod b/pipelines/go.mod index 211655a0..318a8429 100644 --- a/pipelines/go.mod +++ b/pipelines/go.mod @@ -51,8 +51,8 @@ require ( ) require ( - github.com/lightstep/otel-launcher-go/lightstep/instrumentation v1.15.1 - github.com/lightstep/otel-launcher-go/lightstep/sdk/metric v1.15.1 + github.com/lightstep/otel-launcher-go/lightstep/instrumentation v1.16.0 + github.com/lightstep/otel-launcher-go/lightstep/sdk/metric v1.16.0 go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v0.37.0 )