-
Notifications
You must be signed in to change notification settings - Fork 37
Incomplete idea how to solve the Clock trouble #91
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
Conversation
public protocol Span: AnyObject, _SwiftTracingSendableSpan { | ||
@available(macOS 13.0, iOS 15.0, *) | ||
public protocol SpanProtocol<Clock>: AnyObject, _SwiftTracingSendableSpan { | ||
associatedtype Clock: _Concurrency.Clock where Clock.Duration == Swift.Duration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"The change"™
@@ -41,7 +44,7 @@ public protocol Tracer: Instrument { | |||
function: String, | |||
file fileID: String, | |||
line: UInt | |||
) -> Span | |||
) -> Self.Span |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll be returning concrete spans then, as requested in another issue
//===----------------------------------------------------------------------===// | ||
|
||
@available(macOS 13.0, iOS 15.0, *) | ||
public struct TracingClock: _Concurrency.Clock { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have to implement a TracingClock that gives us unit timestamps for "now", since there is no clock implementation in Swift which does this.
If we get one, we'd deprecate this type immediately
|
||
public func sleep(until deadline: Instant, tolerance: Duration?) async throws { | ||
fatalError("Not implemented for TracingClock") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potentially, we could not implement sleeping on it at all tbh
@@ -67,7 +70,7 @@ public protocol Span: AnyObject, _SwiftTracingSendableSpan { | |||
/// - Parameter time: The `DispatchWallTime` at which the span ended. | |||
/// | |||
/// - SeeAlso: `Span.end()` which automatically uses the "current" time. | |||
func end(at time: DispatchWallTime) | |||
func end(at time: Clock.Instant) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And this way we got rid of the Dispatch types in API - which would have stayed with us "forever"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have loved this change if it didn’t require Swift 5.7+…
It is elegant and really solves the problem. Swift 5.7 requirement is not too bad for server applications since NIO only preserves ~1year support for older Swift versions, but it may affect the adoption on client side (especially with iOS 15 requirement).
replaced by #98 |
This aims to showcase one way to solve: #71
We'd have to drop anything older than Swift 5.7, but we'd be able to express things over a generic Clock and use the Tracing clock for now as an impl detail