apple / swift-distributed-tracing

Instrumentation library for Swift server applications
https://swiftpackageindex.com/apple/swift-distributed-tracing/main/documentation/tracing
Apache License 2.0
224 stars 34 forks source link

Add guidance in README on how to set span attributes #123

Open adam-fowler opened 1 year ago

adam-fowler commented 1 year ago

Accessing span attributes is most likely accessed behind a lock. To avoid locking and unlocking continuously it is preferable span attributes are built up separate from the span and then set once on the span.

Eg replace

span.attributes["attr1"] = value1
span.attributes["attr2"] = value2
...

with

var attributes = span.attributes
attributes["attr1"] = value1
attributes["attr2"] = value2
...
span.attributes = attributes
adam-fowler commented 1 year ago

The alternative would be to replace the API such that you can't set single attributes.

stevapple commented 1 year ago
var attributes = span.attributes
attributes["attr1"] = value1
attributes["attr2"] = value2
...
span.attributes = attributes

I suspect if this is the best practice, since it’s risk breaking data integrity by overwriting attributes set by other actors. For most users, setting one by one should be the easiest and safest. A possibility addition for bulk access to span attributes might be

span.withAttributes { attributes in
    attributes["attr1"] = value1
    attributes["attr2"] = value2
}
ktoso commented 1 year ago

I like the bulk change idea, that's viable -- thank you @stevapple.

I think we can add these after the 1.0 which we're about to cut

slashmo commented 1 year ago

This change was added via #133, but we might want to highlight it in the documentation as well.