Concurrent performance improvements; add performance settings field; …

…support `IgnoreCollisions` (#384)
lightstep · Feb 15, 2023 · d6beb6a · d6beb6a
1 parent 1fb5140
commit d6beb6a
Show file tree

Hide file tree

Showing 15 changed files with 320 additions and 71 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,14 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ## Unreleased
 
+## [1.13.1](https://github.com/lightstep/otel-launcher-go/releases/tag/v1.13.1) - 2022-02-15)
+
+- Adds performance improvements for concurrent use of synchronous
+  instruments. [#384](https://github.com/lightstep/otel-launcher-go/pull/384)
+- Adds `WithPerformance()` and `IgnoreCollisions` setting which offers
+  around 10% faster operations in exchange for safety and correctness.  This
+  setting is off by default. [#384](https://github.com/lightstep/otel-launcher-go/pull/384)
+
 ## [1.13.0](https://github.com/lightstep/otel-launcher-go/releases/tag/v1.13.0) - 2022-02-15)
 
 - Updates OTel-Go version dependencies to `trace@v1.12.0`, `metrics@v0.35.0`, 

diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-1.13.0
+1.13.1
diff --git 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.13.0
-	github.com/lightstep/otel-launcher-go/pipelines v1.13.0
+	github.com/lightstep/otel-launcher-go/lightstep/sdk/metric v1.13.1
+	github.com/lightstep/otel-launcher-go/pipelines v1.13.1
 	github.com/sethvargo/go-envconfig v0.8.3
 	github.com/stretchr/testify v1.8.1
 	go.opentelemetry.io/otel v1.12.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.13.0 // indirect
+	github.com/lightstep/otel-launcher-go/lightstep/instrumentation v1.13.1 // 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
@@ -14,4 +14,4 @@
 
 package launcher
 
-const version = "1.13.0"
+const version = "1.13.1"
diff --git a/lightstep/sdk/metric/README.md b/lightstep/sdk/metric/README.md
@@ -122,3 +122,15 @@ For example, to configure a synchronous Gauge:
 	)
 ```
 
+
+### Performance settings
+
+The `WithPerformance()` option supports control over performance
+settings.
+
+#### IgnoreCollisions
+
+With `IgnoreCollisions` set to true, the SDK will ignore fingerprint
+collisions and bypass a safety mechanism that ensures correctness in
+spite of fingerprint collisions in the fast path for synchronous
+instruments.
diff --git a/lightstep/sdk/metric/benchmark_test.go b/lightstep/sdk/metric/benchmark_test.go
@@ -19,10 +19,15 @@ import (
 	"testing"
 
 	"github.com/lightstep/otel-launcher-go/lightstep/sdk/metric/data"
+	"github.com/lightstep/otel-launcher-go/lightstep/sdk/metric/sdkinstrument"
 	"github.com/lightstep/otel-launcher-go/lightstep/sdk/metric/view"
 	"go.opentelemetry.io/otel/attribute"
 )
 
+var unsafePerf = WithPerformance(sdkinstrument.Performance{
+	IgnoreCollisions: true,
+})
+
 // Tested prior to 0.11.0 release
 // goos: darwin
 // goarch: arm64
@@ -64,6 +69,19 @@ func BenchmarkCounterAddOneAttr(b *testing.B) {
 	}
 }
 
+func BenchmarkCounterAddOneAttrUnsafe(b *testing.B) {
+	ctx := context.Background()
+	rdr := NewManualReader("bench")
+	provider := NewMeterProvider(WithReader(rdr), unsafePerf)
+	b.ReportAllocs()
+
+	cntr, _ := provider.Meter("test").Int64Counter("hello")
+
+	for i := 0; i < b.N; i++ {
+		cntr.Add(ctx, 1, attribute.String("K", "V"))
+	}
+}
+
 func BenchmarkCounterAddOneInvalidAttr(b *testing.B) {
 	ctx := context.Background()
 	rdr := NewManualReader("bench")
@@ -90,6 +108,19 @@ func BenchmarkCounterAddManyAttrs(b *testing.B) {
 	}
 }
 
+func BenchmarkCounterAddManyAttrsUnsafe(b *testing.B) {
+	ctx := context.Background()
+	rdr := NewManualReader("bench")
+	provider := NewMeterProvider(WithReader(rdr), unsafePerf)
+	b.ReportAllocs()
+
+	cntr, _ := provider.Meter("test").Int64Counter("hello")
+
+	for i := 0; i < b.N; i++ {
+		cntr.Add(ctx, 1, attribute.Int("K", i))
+	}
+}
+
 func BenchmarkCounterAddManyInvalidAttrs(b *testing.B) {
 	ctx := context.Background()
 	rdr := NewManualReader("bench")
@@ -103,6 +134,19 @@ func BenchmarkCounterAddManyInvalidAttrs(b *testing.B) {
 	}
 }
 
+func BenchmarkCounterAddManyInvalidAttrsUnsafe(b *testing.B) {
+	ctx := context.Background()
+	rdr := NewManualReader("bench")
+	provider := NewMeterProvider(WithReader(rdr), unsafePerf)
+	b.ReportAllocs()
+
+	cntr, _ := provider.Meter("test").Int64Counter("hello")
+
+	for i := 0; i < b.N; i++ {
+		cntr.Add(ctx, 1, attribute.Int("", i), attribute.Int("K", i))
+	}
+}
+
 func BenchmarkCounterAddManyFilteredAttrs(b *testing.B) {
 	ctx := context.Background()
 	rdr := NewManualReader("bench")

diff --git a/lightstep/sdk/metric/config.go b/lightstep/sdk/metric/config.go
@@ -15,6 +15,7 @@
 package metric // import "github.com/lightstep/otel-launcher-go/lightstep/sdk/metric"
 
 import (
+	"github.com/lightstep/otel-launcher-go/lightstep/sdk/metric/sdkinstrument"
 	"github.com/lightstep/otel-launcher-go/lightstep/sdk/metric/view"
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/sdk/resource"
@@ -32,6 +33,9 @@ type config struct {
 	// views is a slice of *Views instances corresponding with readers.
 	// the i'th views applies to the i'th reader.
 	views []*view.Views
+
+	// performance settings
+	performance sdkinstrument.Performance
 }
 
 // Option applies a configuration option value to a MeterProvider.
@@ -68,3 +72,11 @@ func WithReader(r Reader, opts ...view.Option) Option {
 		return cfg
 	})
 }
+
+// WithPerformance supports modifying performance settings.
+func WithPerformance(perf sdkinstrument.Performance) Option {
+	return optionFunction(func(cfg config) config {
+		cfg.performance = perf
+		return cfg
+	})
+}
diff --git 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.13.0
+	github.com/lightstep/otel-launcher-go/lightstep/sdk/metric v1.13.1
 	github.com/lightstep/otel-launcher-go/pipelines v1.8.0
 	go.opentelemetry.io/proto/otlp v0.19.0
 )

diff --git a/lightstep/sdk/metric/internal/asyncstate/async.go b/lightstep/sdk/metric/internal/asyncstate/async.go
@@ -79,12 +79,16 @@ func NewState(pipe int) *State {
 	}
 }
 
-// New returns a new Instrument; this compiles individual
+// New returns a new Observer; this compiles individual
 // instruments for each reader.
-func New(desc sdkinstrument.Descriptor, opaque interface{}, compiled pipeline.Register[viewstate.Instrument]) *Observer {
+func New(desc sdkinstrument.Descriptor, _ sdkinstrument.Performance, opaque interface{}, compiled pipeline.Register[viewstate.Instrument]) *Observer {
 	// Note: we return a non-nil instrument even when all readers
 	// disabled the instrument. This ensures that certain error
 	// checks still work (wrong meter, wrong callback, etc).
+	//
+	// Note: performance settings are not used because async
+	// instruments do not use fingerprinting so IgnoreCollisions is
+	// meaningless.
 	return &Observer{
 		opaque:     opaque,
 		descriptor: desc,

diff --git a/lightstep/sdk/metric/internal/asyncstate/async_test.go b/lightstep/sdk/metric/internal/asyncstate/async_test.go
@@ -53,6 +53,10 @@ var (
 		Last:  middleTime,
 		Now:   endTime,
 	}
+
+	ignorePerf = sdkinstrument.Performance{
+		IgnoreCollisions: false,
+	}
 )
 
 type testSDK struct {
@@ -106,12 +110,12 @@ type floatObserver struct {
 
 func testIntObserver(tsdk *testSDK, name string, ik sdkinstrument.Kind) intObserver {
 	desc := test.Descriptor(name, ik, number.Int64Kind)
-	return intObserver{Observer: New(desc, tsdk, tsdk.compile(desc))}
+	return intObserver{Observer: New(desc, ignorePerf, tsdk, tsdk.compile(desc))}
 }
 
 func testFloatObserver(tsdk *testSDK, name string, ik sdkinstrument.Kind) floatObserver {
 	desc := test.Descriptor(name, ik, number.Float64Kind)
-	return floatObserver{Observer: New(desc, tsdk, tsdk.compile(desc))}
+	return floatObserver{Observer: New(desc, ignorePerf, tsdk, tsdk.compile(desc))}
 }
 
 func nopCB(context.Context, metric.Observer) error {