sentry-cocoa icon indicating copy to clipboard operation
sentry-cocoa copied to clipboard

SentryMetricsAPI.timing incompatible with Structured Concurrency

Open lhunath opened this issue 1 year ago • 4 comments

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

lhunath avatar May 01 '24 14:05 lhunath

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

philipphofmann avatar May 06 '24 09:05 philipphofmann

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.

philipphofmann avatar May 23 '24 08:05 philipphofmann

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.

lhunath avatar May 23 '24 11:05 lhunath

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 👍 .

philipphofmann avatar May 23 '24 14:05 philipphofmann

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

kahest avatar Jul 31 '24 12:07 kahest