Skip to content

Add updateAttributes as Span protocol requirement #203

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.

Sign up for GitHub

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

Open
adam-fowler wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add updateAttributes as Span protocol requirement #203

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7965574
Add updateAttributes as Span protocol requirement
adam-fowler Dec 18, 2025
33defd7
Merge branch 'main' into update-attributes-span-protocol
ktoso Dec 19, 2025
ca14bde
fix formatting
ktoso Dec 19, 2025
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
6 changes: 6 additions & 0 deletions Sources/Tracing/NoOpTracer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,12 @@ public struct NoOpTracer: LegacyTracer {
}
}

/// Update the span attributes in a block instead of individually.
///
/// The NoOpSpan implementation does not call the closure as setting attributes
/// on a NoOpSpan does nothing.
public func updateAttributes(_ update: (inout SpanAttributes) -> Void) {}
Copy link
Contributor

@czechboy0 czechboy0 Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm see my other comment, I still think we should call the closure, just with local empty span attributes that get discarded immediately, not saved on the instance.

Copy link
Contributor Author

@adam-fowler adam-fowler Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We then allocate space for attributes ie the ones we setup inside the closure and then throw them away immediately. It appears on godbolt the compiler isn't clever enough to compile out the creation of the attributes.

Copy link
Contributor

@czechboy0 czechboy0 Dec 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My lean would be that we just live with that cost and file an issue on the compiler to improve this.

But when correctness and performance are in tension, and we're locking this decision in a public API, I think we should choose correctness here.


/// Finishes the span.
/// - Parameter instant: the time instant the span completed.
public func end<Instant: TracerInstant>(at instant: @autoclosure () -> Instant) {
Expand Down
11 changes: 11 additions & 0 deletions Sources/Tracing/SpanProtocol.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,17 @@ public protocol Span: Sendable {
nonmutating set
}

/// Update the span attributes in a block instead of individually.
///
/// Updating a span attribute involves some type of thread synchronisation
/// primitive to avoid multiple threads updating the attributes at the same
/// time. If you update each attribute individually, this can cause slowdown.
/// This function updates the attributes in one call to avoid hitting the
/// thread synchronisation code multiple times.
///
/// - Parameter update: closure used to update span attributes
func updateAttributes(_ update: (inout SpanAttributes) -> Void)

/// A Boolean value that indicates whether the span is recording information such as events, attributes, status, and so on.
var isRecording: Bool { get }

Expand Down
Loading