From 86df763d90b8aee4154196ec72a9f68b4121f9c7 Mon Sep 17 00:00:00 2001 From: ttoad Date: Wed, 5 Jun 2024 02:14:10 +0800 Subject: [PATCH] trace: Span in noop.Start is no longer allocated (#5457) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit trace/noop/noop.Span should be instance. This will reduce memory waste. Benchmark results with changes: new.txt ``` goos: darwin goarch: arm64 pkg: go.opentelemetry.io/otel/trace/noop BenchmarkNoopInstance-10 29894822 39.94 ns/op 48 B/op 1 allocs/op BenchmarkNoopInstance-10 29675424 39.99 ns/op 48 B/op 1 allocs/op BenchmarkNoopInstance-10 30064700 39.98 ns/op 48 B/op 1 allocs/op BenchmarkNoopInstance-10 29962016 40.03 ns/op 48 B/op 1 allocs/op BenchmarkNoopInstance-10 30060465 40.02 ns/op 48 B/op 1 allocs/op BenchmarkNoopInstance-10 29916855 40.04 ns/op 48 B/op 1 allocs/op BenchmarkNoopInstance-10 29829998 40.28 ns/op 48 B/op 1 allocs/op BenchmarkNoopInstance-10 30084706 39.99 ns/op 48 B/op 1 allocs/op BenchmarkNoopInstance-10 30087441 40.02 ns/op 48 B/op 1 allocs/op BenchmarkNoopInstance-10 29864365 40.14 ns/op 48 B/op 1 allocs/op ``` without changes on `main`: old.txt ``` goos: darwin goarch: arm64 pkg: go.opentelemetry.io/otel/trace/noop BenchmarkNoopInstance-10 14813442 67.64 ns/op 128 B/op 2 allocs/op BenchmarkNoopInstance-10 17714486 68.17 ns/op 128 B/op 2 allocs/op BenchmarkNoopInstance-10 17701257 67.66 ns/op 128 B/op 2 allocs/op BenchmarkNoopInstance-10 17805859 67.69 ns/op 128 B/op 2 allocs/op BenchmarkNoopInstance-10 17912841 67.43 ns/op 128 B/op 2 allocs/op BenchmarkNoopInstance-10 17864120 67.58 ns/op 128 B/op 2 allocs/op BenchmarkNoopInstance-10 17663130 68.41 ns/op 128 B/op 2 allocs/op BenchmarkNoopInstance-10 17740423 67.57 ns/op 128 B/op 2 allocs/op BenchmarkNoopInstance-10 17751040 67.56 ns/op 128 B/op 2 allocs/op BenchmarkNoopInstance-10 17738064 67.91 ns/op 128 B/op 2 allocs/op ``` benchstat: ``` goos: darwin goarch: arm64 pkg: go.opentelemetry.io/otel/trace/noop │ old.txt │ new.txt │ │ sec/op │ sec/op vs base │ NoopInstance-10 67.65n ± 1% 40.02n ± 0% -40.84% (p=0.000 n=10) │ old.txt │ new.txt │ │ B/op │ B/op vs base │ NoopInstance-10 128.00 ± 0% 48.00 ± 0% -62.50% (p=0.000 n=10) │ old.txt │ new.txt │ │ allocs/op │ allocs/op vs base │ NoopInstance-10 2.000 ± 0% 1.000 ± 0% -50.00% (p=0.000 n=10) ``` Co-authored-by: Tyler Yahn --- CHANGELOG.md | 4 ++++ trace/noop/noop.go | 4 +++- trace/noop/noop_test.go | 13 +++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9021fa522..5bc75bb01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `IsEmpty` method is added to the `Instrument` type in `go.opentelemetry.io/otel/sdk/metric`. This method is used to check if an `Instrument` instance is a zero-value. (#5431) +### Changed + +- `Tracer.Start` in `go.opentelemetry.io/otel/trace/noop` no longer allocates a span for empty span context. (#5457) + ### Fixed - Log a warning to the OpenTelemetry internal logger when a `Record` in `go.opentelemetry.io/otel/sdk/log` drops an attribute due to a limit being reached. (#5376) diff --git a/trace/noop/noop.go b/trace/noop/noop.go index 1dfa52c52..64a4f1b36 100644 --- a/trace/noop/noop.go +++ b/trace/noop/noop.go @@ -67,11 +67,13 @@ func (t Tracer) Start(ctx context.Context, _ string, _ ...trace.SpanStartOption) span = Span{sc: sc} } else { // No parent, return a No-Op span with an empty span context. - span = Span{} + span = noopSpanInstance } return trace.ContextWithSpan(ctx, span), span } +var noopSpanInstance trace.Span = Span{} + // Span is an OpenTelemetry No-Op Span. type Span struct { embedded.Span diff --git a/trace/noop/noop_test.go b/trace/noop/noop_test.go index c228ad099..a8eedb777 100644 --- a/trace/noop/noop_test.go +++ b/trace/noop/noop_test.go @@ -101,6 +101,19 @@ func TestTracerStartPropagatesSpanContext(t *testing.T) { assert.False(t, span.IsRecording(), "recording span returned") } +func BenchmarkNoopInstance(b *testing.B) { + tracer := NewTracerProvider().Tracer("") + ctx := trace.ContextWithSpanContext(context.Background(), trace.SpanContext{}) + + b.ReportAllocs() + b.ResetTimer() + + for i := 0; i < b.N; i++ { + _, span := tracer.Start(ctx, "") + span.End() + } +} + type recordingSpan struct{ Span } func (recordingSpan) IsRecording() bool { return true }