getsentry / sentry-cocoa

The official Sentry SDK for iOS, tvOS, macOS, watchOS.
https://sentry.io/for/cocoa/
MIT License
803 stars 316 forks source link

SentryMetricsAPI.timing incompatible with Structured Concurrency #3930

Closed lhunath closed 2 months ago

lhunath commented 5 months ago

Problem Statement

SentryMetricsAPI.timing's API is synchronous and needs to wrap a synchronously-executing block of code which may last a long time. This paradigm only makes sense in the traditional world where the thread will be blocked until the operation completes. This paradigm is incompatible out-of-the-box with asynchronous APIs such as the traditional callback-based mechanisms, without special handling and boilerplate. Swift is seeking to move, using the concept of Structured Concurrency, to a paradigm where all threads make forward progress. As a result, it is impossible to wrap an async operation in a synchronous block which waits for the operation to complete (the only hack would be to block the current thread until the async operation completes, using a lock or semaphore, but this violates the concept of forward progress, opening up the possibility for deadlocks). Sentry utilizes an alternative API SentrySDK.startTransactionWithName for performance monitoring, which is compatible with structured concurrency, as the span can be .finish()ed from any context and there's no need to block the current thread until the transaction competes.

Solution Brainstorm

The simple solution is to add an alternative API which supports async operations:

public func timing<T>(key: String, tags: [String : String] = [:], _ closure: () async throws -> T) async rethrows -> T

It would be fantastic if the Swift language team had made reasync available. Unfortunately, they have not yet done so (some additional encouragement wouldn't be out of place).

Are you willing to submit a PR?

Yes

philipphofmann commented 5 months ago

Thanks for reporting this, @lhunath. We will add an API to support your use case. Can't give you an ETA yet.

philipphofmann commented 4 months ago

Swift async is only available since Swift 5.5 but we still use Swift 5.3 for SPM https://github.com/getsentry/sentry-cocoa/blob/cf972098baef6f2042eee18549c33451fd983731/Package.swift#L1

We will bump the swift-tools-version to at least 5.5 in our next major, which is around the corner, and then we can tackle this.

lhunath commented 4 months ago

Fair enough! Please note that as mentioned in the OP, this applies equally to all other asynchronous APIs, such as those that work with callbacks or delegate responses.

philipphofmann commented 4 months ago

Fair enough! Please note that as mentioned in the OP, this applies equally to all other asynchronous APIs, such as those that work with callbacks or delegate responses.

Oh, yes, indeed. We need to find a solution to that. So, this doesn't have to wait until we use Swift 5.5 👍 .

kahest commented 2 months ago

We're gonna change the way Metrics work and are used from an API perspective (see our docs) so this issue is obsolete.