Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trace: SpanFromContext and SpanContextFromContext make no allocs #5049

Merged
merged 6 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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]

### Changed

- `SpanFromContext` and `SpanContextFromContext` in `go.opentelemetry.io/otel/trace` no longer make a heap allocation when the passed context has no span. (#5049)

### Fixed

- Clarify the documentation about equivalence guarantees for the `Set` and `Distinct` types in `go.opentelemetry.io/otel/attribute`. (#5027)
Expand Down
4 changes: 2 additions & 2 deletions trace/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ func ContextWithRemoteSpanContext(parent context.Context, rsc SpanContext) conte
// performs no operations is returned.
func SpanFromContext(ctx context.Context) Span {
if ctx == nil {
return noopSpan{}
return noopSpanInstance
}
if span, ok := ctx.Value(currentSpanKey).(Span); ok {
return span
}
return noopSpan{}
return noopSpanInstance
}

// SpanContextFromContext returns the current Span's SpanContext.
Expand Down
10 changes: 10 additions & 0 deletions trace/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,16 @@ func TestSpanFromContext(t *testing.T) {
// Ensure SpanContextFromContext is just
// SpanFromContext(…).SpanContext().
assert.Equal(t, tc.expectedSpan.SpanContext(), SpanContextFromContext(tc.context))

// Check that SpanFromContext does not produce any heap allocation.
assert.Equal(t, 0.0, testing.AllocsPerRun(5, func() {
SpanFromContext(tc.context)
}), "SpanFromContext allocs")

// Check that SpanContextFromContext does not produce any heap allocation.
assert.Equal(t, 0.0, testing.AllocsPerRun(5, func() {
SpanContextFromContext(tc.context)
}), "SpanContextFromContext allocs")
})
}
}
4 changes: 2 additions & 2 deletions trace/noop.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ func (t noopTracer) Start(ctx context.Context, name string, _ ...SpanStartOption
span := SpanFromContext(ctx)
if _, ok := span.(nonRecordingSpan); !ok {
// span is likely already a noopSpan, but let's be sure
span = noopSpan{}
span = noopSpanInstance
}
return ContextWithSpan(ctx, span), span
}

// noopSpan is an implementation of Span that performs no operations.
type noopSpan struct{ embedded.Span }

var _ Span = noopSpan{}
var noopSpanInstance Span = noopSpan{}

// SpanContext returns an empty span context.
func (noopSpan) SpanContext() SpanContext { return SpanContext{} }
Expand Down