Skip to content

Commit

Permalink
move time.Now call into exemplar reservoir
Browse files Browse the repository at this point in the history
  • Loading branch information
dashpole committed Jun 25, 2024
1 parent 6d45f28 commit 6de7c08
Show file tree
Hide file tree
Showing 17 changed files with 134 additions and 137 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to `go.opentelemetry.io/otel/semconv/v1.26.0` in `go.opentelemetry.io/otel/sdk/resource`. (#5490)
- Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to `go.opentelemetry.io/otel/semconv/v1.26.0` in `go.opentelemetry.io/otel/sdk/trace`. (#5490)
- Use non-generic functions in the `Start` method of `"go.opentelemetry.io/otel/sdk/trace".Trace` to reduce memory allocation. (#5497)
- Improve performance of metric instruments in `go.opentelemetry.io/otel/sdk/metric` by removing unnecessary calls to time.Now. (#5545)

### Fixed

Expand Down
4 changes: 1 addition & 3 deletions sdk/metric/internal/aggregate/exponential_histogram.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,8 +319,6 @@ func (e *expoHistogram[N]) measure(ctx context.Context, value N, fltrAttr attrib
return
}

t := now()

e.valuesMu.Lock()
defer e.valuesMu.Unlock()

Expand All @@ -333,7 +331,7 @@ func (e *expoHistogram[N]) measure(ctx context.Context, value N, fltrAttr attrib
e.values[attr.Equivalent()] = v
}
v.record(value)
v.res.Offer(ctx, t, exemplar.NewValue(value), droppedAttr)
v.res.Offer(ctx, exemplar.NewValue(value), droppedAttr)
}

func (e *expoHistogram[N]) delta(dest *metricdata.Aggregation) int {
Expand Down
20 changes: 10 additions & 10 deletions sdk/metric/internal/aggregate/exponential_histogram_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ func testDeltaExpoHist[N int64 | float64]() func(t *testing.T) {
{
Attributes: fltrAlice,
StartTime: y2kPlus(1),
Time: y2kPlus(9),
Time: y2kPlus(2),
Count: 7,
Min: metricdata.NewExtrema[N](-1),
Max: metricdata.NewExtrema[N](16),
Expand Down Expand Up @@ -832,8 +832,8 @@ func testDeltaExpoHist[N int64 | float64]() func(t *testing.T) {
DataPoints: []metricdata.ExponentialHistogramDataPoint[N]{
{
Attributes: fltrAlice,
StartTime: y2kPlus(10),
Time: y2kPlus(24),
StartTime: y2kPlus(3),
Time: y2kPlus(4),
Count: 7,
Min: metricdata.NewExtrema[N](-1),
Max: metricdata.NewExtrema[N](16),
Expand All @@ -850,8 +850,8 @@ func testDeltaExpoHist[N int64 | float64]() func(t *testing.T) {
},
{
Attributes: overflowSet,
StartTime: y2kPlus(10),
Time: y2kPlus(24),
StartTime: y2kPlus(3),
Time: y2kPlus(4),
Count: 6,
Min: metricdata.NewExtrema[N](1),
Max: metricdata.NewExtrema[N](16),
Expand Down Expand Up @@ -905,7 +905,7 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) {
{
Attributes: fltrAlice,
StartTime: y2kPlus(0),
Time: y2kPlus(9),
Time: y2kPlus(2),
Count: 7,
Min: metricdata.NewExtrema[N](-1),
Max: metricdata.NewExtrema[N](16),
Expand Down Expand Up @@ -938,7 +938,7 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) {
{
Attributes: fltrAlice,
StartTime: y2kPlus(0),
Time: y2kPlus(13),
Time: y2kPlus(3),
Count: 10,
Min: metricdata.NewExtrema[N](-1),
Max: metricdata.NewExtrema[N](16),
Expand Down Expand Up @@ -967,7 +967,7 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) {
{
Attributes: fltrAlice,
StartTime: y2kPlus(0),
Time: y2kPlus(14),
Time: y2kPlus(4),
Count: 10,
Min: metricdata.NewExtrema[N](-1),
Max: metricdata.NewExtrema[N](16),
Expand Down Expand Up @@ -1004,7 +1004,7 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) {
{
Attributes: fltrAlice,
StartTime: y2kPlus(0),
Time: y2kPlus(21),
Time: y2kPlus(5),
Count: 10,
Min: metricdata.NewExtrema[N](-1),
Max: metricdata.NewExtrema[N](16),
Expand All @@ -1022,7 +1022,7 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) {
{
Attributes: overflowSet,
StartTime: y2kPlus(0),
Time: y2kPlus(21),
Time: y2kPlus(5),
Count: 6,
Min: metricdata.NewExtrema[N](1),
Max: metricdata.NewExtrema[N](16),
Expand Down
4 changes: 1 addition & 3 deletions sdk/metric/internal/aggregate/histogram.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@ func (s *histValues[N]) measure(ctx context.Context, value N, fltrAttr attribute
// (s.bounds[len(s.bounds)-1], +∞).
idx := sort.SearchFloat64s(s.bounds, float64(value))

t := now()

s.valuesMu.Lock()
defer s.valuesMu.Unlock()

Expand All @@ -106,7 +104,7 @@ func (s *histValues[N]) measure(ctx context.Context, value N, fltrAttr attribute
if !s.noSum {
b.sum(value)
}
b.res.Offer(ctx, t, exemplar.NewValue(value), droppedAttr)
b.res.Offer(ctx, exemplar.NewValue(value), droppedAttr)
}

// newHistogram returns an Aggregator that summarizes a set of measurements as
Expand Down
32 changes: 16 additions & 16 deletions sdk/metric/internal/aggregate/histogram_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ func testDeltaHist[N int64 | float64](c conf[N]) func(t *testing.T) {
agg: metricdata.Histogram[N]{
Temporality: metricdata.DeltaTemporality,
DataPoints: []metricdata.HistogramDataPoint[N]{
c.hPt(fltrAlice, 2, 3, y2kPlus(1), y2kPlus(7)),
c.hPt(fltrBob, 10, 2, y2kPlus(1), y2kPlus(7)),
c.hPt(fltrAlice, 2, 3, y2kPlus(1), y2kPlus(2)),
c.hPt(fltrBob, 10, 2, y2kPlus(1), y2kPlus(2)),
},
},
},
Expand All @@ -96,8 +96,8 @@ func testDeltaHist[N int64 | float64](c conf[N]) func(t *testing.T) {
agg: metricdata.Histogram[N]{
Temporality: metricdata.DeltaTemporality,
DataPoints: []metricdata.HistogramDataPoint[N]{
c.hPt(fltrAlice, 10, 1, y2kPlus(7), y2kPlus(10)),
c.hPt(fltrBob, 3, 1, y2kPlus(7), y2kPlus(10)),
c.hPt(fltrAlice, 10, 1, y2kPlus(2), y2kPlus(3)),
c.hPt(fltrBob, 3, 1, y2kPlus(2), y2kPlus(3)),
},
},
},
Expand Down Expand Up @@ -126,9 +126,9 @@ func testDeltaHist[N int64 | float64](c conf[N]) func(t *testing.T) {
agg: metricdata.Histogram[N]{
Temporality: metricdata.DeltaTemporality,
DataPoints: []metricdata.HistogramDataPoint[N]{
c.hPt(fltrAlice, 1, 1, y2kPlus(11), y2kPlus(16)),
c.hPt(fltrBob, 1, 1, y2kPlus(11), y2kPlus(16)),
c.hPt(overflowSet, 1, 2, y2kPlus(11), y2kPlus(16)),
c.hPt(fltrAlice, 1, 1, y2kPlus(4), y2kPlus(5)),
c.hPt(fltrBob, 1, 1, y2kPlus(4), y2kPlus(5)),
c.hPt(overflowSet, 1, 2, y2kPlus(4), y2kPlus(5)),
},
},
},
Expand Down Expand Up @@ -167,8 +167,8 @@ func testCumulativeHist[N int64 | float64](c conf[N]) func(t *testing.T) {
agg: metricdata.Histogram[N]{
Temporality: metricdata.CumulativeTemporality,
DataPoints: []metricdata.HistogramDataPoint[N]{
c.hPt(fltrAlice, 2, 3, y2kPlus(0), y2kPlus(7)),
c.hPt(fltrBob, 10, 2, y2kPlus(0), y2kPlus(7)),
c.hPt(fltrAlice, 2, 3, y2kPlus(0), y2kPlus(2)),
c.hPt(fltrBob, 10, 2, y2kPlus(0), y2kPlus(2)),
},
},
},
Expand All @@ -183,8 +183,8 @@ func testCumulativeHist[N int64 | float64](c conf[N]) func(t *testing.T) {
agg: metricdata.Histogram[N]{
Temporality: metricdata.CumulativeTemporality,
DataPoints: []metricdata.HistogramDataPoint[N]{
c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(10)),
c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(10)),
c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(3)),
c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(3)),
},
},
},
Expand All @@ -196,8 +196,8 @@ func testCumulativeHist[N int64 | float64](c conf[N]) func(t *testing.T) {
agg: metricdata.Histogram[N]{
Temporality: metricdata.CumulativeTemporality,
DataPoints: []metricdata.HistogramDataPoint[N]{
c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(11)),
c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(11)),
c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(4)),
c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(4)),
},
},
},
Expand All @@ -213,9 +213,9 @@ func testCumulativeHist[N int64 | float64](c conf[N]) func(t *testing.T) {
agg: metricdata.Histogram[N]{
Temporality: metricdata.CumulativeTemporality,
DataPoints: []metricdata.HistogramDataPoint[N]{
c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(14)),
c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(14)),
c.hPt(overflowSet, 1, 2, y2kPlus(0), y2kPlus(14)),
c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(5)),
c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(5)),
c.hPt(overflowSet, 1, 2, y2kPlus(0), y2kPlus(5)),
},
},
},
Expand Down
4 changes: 1 addition & 3 deletions sdk/metric/internal/aggregate/lastvalue.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ type lastValue[N int64 | float64] struct {
}

func (s *lastValue[N]) measure(ctx context.Context, value N, fltrAttr attribute.Set, droppedAttr []attribute.KeyValue) {
t := now()

s.Lock()
defer s.Unlock()

Expand All @@ -53,7 +51,7 @@ func (s *lastValue[N]) measure(ctx context.Context, value N, fltrAttr attribute.

d.attrs = attr
d.value = value
d.res.Offer(ctx, t, exemplar.NewValue(value), droppedAttr)
d.res.Offer(ctx, exemplar.NewValue(value), droppedAttr)

s.values[attr.Equivalent()] = d
}
Expand Down
Loading

0 comments on commit 6de7c08

Please sign in to comment.