Skip to content

Commit

Permalink
Support API hint for per-instrument temporality selection (#426)
Browse files Browse the repository at this point in the history
  • Loading branch information
jmacd authored Apr 27, 2023
1 parent 1d4b504 commit 89adab8
Show file tree
Hide file tree
Showing 14 changed files with 140 additions and 50 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.15.1
1.16.0
6 changes: 3 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion launcher/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,4 @@

package launcher

const version = "1.15.1"
const version = "1.16.0"
13 changes: 13 additions & 0 deletions lightstep/sdk/metric/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
33 changes: 30 additions & 3 deletions lightstep/sdk/metric/aggregator/aggregation/temporality.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
}
25 changes: 0 additions & 25 deletions lightstep/sdk/metric/aggregator/aggregation/temporality_string.go

This file was deleted.

40 changes: 40 additions & 0 deletions lightstep/sdk/metric/aggregator/aggregation/temporality_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
2 changes: 1 addition & 1 deletion lightstep/sdk/metric/example/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@ package retry
import (
"context"
"errors"
"math"
"testing"
"time"

"github.com/cenkalti/backoff/v4"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -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 {
Expand Down
26 changes: 18 additions & 8 deletions lightstep/sdk/metric/internal/viewstate/viewstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand All @@ -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()
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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,
}

Expand All @@ -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 {
Expand All @@ -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,
})
}
Expand Down
24 changes: 21 additions & 3 deletions lightstep/sdk/metric/internal/viewstate/viewstate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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}
Expand All @@ -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)
Expand All @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions lightstep/sdk/metric/view/hint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
4 changes: 2 additions & 2 deletions pipelines/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down

0 comments on commit 89adab8

Please sign in to comment.