search

apple / swift-distributed-tracing

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

consider TracerSpan assoc type -> Span #111

Closed ktoso closed 1 year ago

ktoso commented 1 year ago

Something @fabianfett requested we give another look at.

Resolves https://github.com/apple/swift-distributed-tracing/issues/110 but we should discuss this more -- it comes with a confusing tradeoff for implementers of spans I suppose. Please discuss. I'm on the fence, it is nice to not have Tracer.TracerSpan but the tradeoff is some potential confusion hmmm

Motivation:

Users who use generic Tracer end up having to spell Tracer.TracerSpan.

We could help those situations by renaming the associated type. However at the cost of causing name conflicts where the Span implementation must qualify that it is implementing Tracing.Span.

This may or may not be worth it -- discuss.

Modifications:

associatedtype TracerSpan: Span -> associatedtype Span: Tracing.Span

fabianfett commented 1 year ago

@ktoso can you give an example for this?

We could help those situations by renaming the associated type. However at the cost of causing name conflicts where the Span implementation must qualify that it is implementing Tracing.Span.

ktoso commented 1 year ago

Yeah I think this will be fine. I'll merge tomorrow probably tho

ktoso commented 1 year ago

Let's give this a shot -- thank you for the discussion @stevapple and @fabianfett, I think Fabian made a good point and we can try this out in the next beta. I think it'll work out fine.