Add recordingSpan and currentSpan methods to TracerProtocol

[slashmo]

Motivation:

Currently, it is only possible to implicitly work with the current span by transparently creating a child span (using ServiceContext.current) under the hood. This is sufficient for almost all use-cases, but does not work in case a piece of code wants to add an event to the current span without having a handle on said span, e.g. if the span was created by a library.

The concrete use-case I'm facing is building a tracing "hook" for my OpenFeature client. OpenFeature hooks are similar to middleware, but instead of wrapping a piece of logic they are called before and after a feature flag was evaluated. Therefore, OpenFeature implementations in other ecosystems have opted to only add an event instead of creating a new span for the flag evaluation itself:

• OpenFeature Go SDK: Tracing Hook
• OpenFeature JS SDK: Tracing Hook

Modifications:

I added a recordingSpan(identifiedBy context: ServiceContext) -> Span? requirement to TracerProtocol. This way, Tracer implementations may look up and return a span identified by the data they stored in the provided ServiceContext. It's worth noting that this method is only intended for obtaining spans which are still recording as opposed to already ended ones that may not even be in memory anymore. I also added a default implementation of this method to avoid introducing a breaking change. On top of this new protocol requirement, I added an extension to TracerProtocol with sugar to obtain the current span based on the task-local ServiceContext.

Result:

Library authors and application developers are now able to look up the current recording span to interact with it by e.g. adding events and attributes:

func spanCreatingMethod() async {
    await withSpan("operation") {
        await methodWithoutSpanArgument()
    }
}

func methodWithoutSpanArgument() async {
    if let span = InstrumentationSystem.tracer.currentSpan() {
        span.addEvent("example")
    }
}

[slashmo]

Here's a draft implementation of the ServiceContext-based recording span lookup in Swift OTel: swift-otel/swift-otel#167

[czechboy0]

This looks very useful, thanks @slashmo!

Do I understand correctly that this method needs to be considered "best effort", since existing implementations might not be implementing it (yet, or ever) - so whether new events attached this way will show up depends on the concrete backend? It's fine IMO, we might just want to call this out explicitly. (It'll be the case with any new-and-defaulted requirements like this one, but it might not be obvious to new adopters of swift-distributed-tracing.)

[slashmo]

Do I understand correctly that this method needs to be considered "best effort".

Yes, it would have to be. Thanks for bringing it up. This reminds me a bit of when we introduced metadata providers in Logging, which also provides a default no-op implementation for backwards-compatibility but also includes a one-off debug log to indicate that the default is being used if the bootstrapped handler does not support metadata providers (yet):

https://github.com/apple/swift-log/blob/6efb781a2900883e1c702ad8b782b14dd1c75c13/Sources/Logging/LogHandler.swift#L181

extension LogHandler {
    /// Default implementation for `metadataProvider` which defaults to `nil`.
    /// This default exists in order to facilitate source-compatible introduction of the `metadataProvider` protocol requirement.
    public var metadataProvider: Logger.MetadataProvider? {
        get {
            nil
        }
        set {
            #if DEBUG
            if LoggingSystem.warnOnceLogHandlerNotSupportedMetadataProvider(Self.self) {
                self.log(
                    level: .warning,
                    message:
                        "Attempted to set metadataProvider on \(Self.self) that did not implement support for them. Please contact the log handler maintainer to implement metadata provider support.",
                    metadata: nil,
                    source: "Logging",
                    file: #file,
                    function: #function,
                    line: #line
                )
            }
            #endif
        }
    }
}

Perhaps we could borrow this approach here as well? Either way, I agree that we should highlight it a bit more in documentation as well.

[slashmo]

Hm, just realized that here we wouldn't be able to use Logging for the one-off warning because swift-distributed-tracing doesn't depend on swift-log. So either we'd have to print the debug log or find an alternative approach.

[czechboy0]

I'm fine with us calling it out in the doc comments only. We can reopen the discussion of whether tracing should depend on logging separately, I don't think it needs to block this.

[slashmo]

Sounds good. I'll update the docs then to provide some more clarity.