feat: support concurrent runtime contexts - #2190
Conversation
Allow hosts to provide execution-local runtime context storage during SDK initialization while keeping the existing process-local behavior as the default. Replace the fixed process key and two lookup maps with a direct nullable context for the normal path. Concurrent hosts delegate selection and release through one small storage contract, and withContext performs only one storage lookup when entering an execution. Expose RuntimeContext only as the opaque value required by the public interface; its constructor and members remain internal. Preserve native Hub cloning, independent best-effort resource flushing, and the shared client transport contract.
Add a runtime-neutral storage stub with independently selectable execution slots so the isolation contract can be tested without an async extension. Cover distinct contexts, Hubs, scopes, log and metric aggregators, switching between overlapping executions, abandoned execution release, repeated teardown, and failure-isolated resource settlement. Also verify that the global init helper forwards the configured storage to the SDK.
Avoid reallocating the process-wide fatal error memory reservation whenever a new runtime context starts. Re-arm the buffer only after the previous reservation has actually been released while continuing to reset the fatal-handler flags on every call. Extend the existing PHPT to cover both released and live reservations, and remove the PHPStan and Mago suppressions made obsolete by reading the reservation state.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit db21a63. Configure here.
Remove any context held for the current execution before replacing the runtime context manager. This prevents reinitialization with the same storage from selecting a context owned by the previous manager and binding the new client to a Hub that will be discarded. Document that initialization discards the current stored context without flushing and that concurrent runtimes must not reinitialize while other executions are active. Add a regression covering the end/start transition that previously left the fresh baseline without a client.
Litarnus
left a comment
Litarnus
left a comment
There was a problem hiding this comment.
Thanks for the PR @binaryfire, it looks good in general. Just one thing about init.
Can you also tell me a little bit how you intend to use it in Hypervel? I'm wondering if we can add the support for it directly in the SDK so it works out of the box for users running Hypervel
| public static function init(?RuntimeContextStorageInterface $runtimeContextStorage = null): HubInterface | ||
| { | ||
| if ($runtimeContextStorage !== null) { | ||
| // The new manager must not select a context the previous one left in host storage. | ||
| // The removed context is discarded unflushed, matching how reinitialization has | ||
| // always dropped active manager state. | ||
| $runtimeContextStorage->remove(); | ||
| } | ||
|
|
||
| self::$currentHub = new Hub(); | ||
| self::$runtimeContextManager = new RuntimeContextManager(self::$currentHub); | ||
| self::$runtimeContextManager = new RuntimeContextManager(self::$currentHub, $runtimeContextStorage); |
There was a problem hiding this comment.
I think it would be better if we keep it detached from init and instead have a setter for that somewhere. When calling init, the storage itself can remain and we can just drop the telemetry. Frameworks/libraries can then just register the storage and a user can use init without having to care about the storage itself
There was a problem hiding this comment.
Done. Storage is now registered separately via SentrySdk::setRuntimeContextStorage() and stays registered across init() calls, so Sentry\init([...]) is back to its original signature.
I also fixed a bug in my earlier version. init() only cleared the storage passed in, so switching or removing storage could leave a stale context behind in the old one. Now init() and setRuntimeContextStorage() both discard the current execution's context through the manager that owns it. I've added tests added for both.
Register runtime context storage separately so frameworks can install it once while applications keep using the existing init API. Discard the active context through its owning manager before SDK initialization or storage changes to avoid retaining stale execution state.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
No problem @Litarnus. I've pushed the
The storage is a small wrapper around Swoole's per-coroutine context. Hypervel registers it at boot and calls There's nothing Hypervel-specific in that storage, so it could live in the SDK and Hypervel, Hyperf and other Swoole coroutine frameworks would just register it instead of each writing their own. They'd still wire their own lifecycle boundaries, so it's a building block rather than full automatic support, and it's dormant unless registered. It does mean one more runtime-specific class for you to maintain though so I understand if you'd rather it stay framework-side. But from our perspective it'd certainly be nice to have in the SDK. Either way, would you be OK keeping this PR to the contract and fix, so it can make the next release? Happy to open the Swoole storage as a follow-up if you want it. |
|
Thanks for the changes and your work, looks great now!
I'm up for having it in the SDK, even more so if other Swoole based frameworks can just reuse it. It seems like it's a very isolated piece which makes it easy to maintain and it sounds like a good opportunity to learn a bit more about Swoole internals. Happy to review the storage PR once it lands |
|
Oh and feel free to rename it to |
2 participants
Hi! I’m the co-creator of Hypervel and a contributor to Swoole. I’d like to include a first-party Sentry integration as part of the framework, but the SDK currently makes that difficult to do safely.
Sentry currently keeps one active runtime context for the whole PHP process. That works when requests are handled one at a time. In a server where several requests or tasks can overlap, a second request can reuse the first request’s context. The requests can then share Sentry state, logs, and metrics, and one request can flush or remove data that belongs to another.
This PR lets a runtime provide the storage used for the current request’s context. Sentry still creates the context and handles flushing; the runtime only stores and returns the context for whichever request or task is currently running.
The storage is passed when Sentry is initialized because it controls how work inside the process is kept separate. It isn’t client configuration and shouldn’t change when the client is replaced.
Nothing changes for applications that don’t provide custom storage. Their context stays process-local, and the default implementation becomes simpler: one nullable property replaces the fixed
processkey, two lookup maps, and their cleanup logic.When custom storage is used, the SDK manager doesn’t retain active contexts itself. If a request ends unexpectedly, its runtime can release that context without leaving its Hub, logs, and metrics in memory for the rest of the worker’s lifetime.
I ran into this while working on Hypervel, but the problem isn’t specific to Swoole. The same change can be used by any async PHP runtime, including Amp and ReactPHP.
One thing I wasn’t sure about was the naming for the new public type. The SDK already has
Sentry\Context\RuntimeContextfor information attached to events. This PR exposesSentry\State\RuntimeContextas the value stored for a running request, although its constructor and methods stay internal. I kept the existing name, butExecutionContextmight be clearer.