Skip to content
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

SentryMetricsAPI.timing incompatible with Structured Concurrency #3930

Open
lhunath opened this issue May 1, 2024 · 4 comments
Open

SentryMetricsAPI.timing incompatible with Structured Concurrency #3930

lhunath opened this issue May 1, 2024 · 4 comments

Comments

@lhunath
Copy link
Contributor

lhunath commented May 1, 2024

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
Copy link
Member

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

@philipphofmann
Copy link
Member

Swift async is only available since Swift 5.5 but we still use Swift 5.3 for SPM

// swift-tools-version:5.3

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
Copy link
Contributor Author

lhunath commented May 23, 2024

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Waiting for: Product Owner
Status: No status
Status: Backlog
Development

No branches or pull requests

3 participants