-
-
Notifications
You must be signed in to change notification settings - Fork 28
refactor(sentry): replace manual flush with context lifecycle management #1070
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
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
eee9285
refactor(sentry): replace manual flush with context lifecycle management
huangdijia 9724b53
chore(sentry): bump sentry/sentry minimum version to ^4.28.0
huangdijia e78ed94
chore(sentry): sync sentry/sentry version in root composer.json
huangdijia 9348304
♻️ refactor(sentry): remove unused SingletonAspect and clean up comm…
huangdijia 3947e48
🐛 fix(sentry): move startContext into coroutine after context restore
huangdijia 449e87d
✨ feat(sentry): add coroutine-safe RuntimeContextManager via class_map
huangdijia 32760b8
📝 docs(sentry): add comment clarifying ContextArrayObject usage in c…
huangdijia 59f842d
🐛 fix(sentry): ensure baseHub has a valid client in RuntimeContextMa…
huangdijia 6978a6c
♻️ refactor(sentry): move context lifecycle into tracing aspect
huangdijia f7b5fce
📝 docs(sentry): correct type hints and comment for ContextArrayObject
huangdijia fd289ca
🐛 fix(sentry): run metrics collection within a coroutine context
huangdijia bced65b
🐛 fix(sentry): disable coordinator tracing span by default
huangdijia 62ed785
♻️ refactor(sentry): remove coordinator tracing aspect
huangdijia 4b159ae
Revert "🐛 fix(sentry): run metrics collection within a coroutine cont…
huangdijia d66bd09
🐛 fix(sentry): isolate sentry context per coroutine
huangdijia 6e612c2
🐛 fix(sentry): treat sentry metrics/transport as coroutine backtrace …
huangdijia e3ddd4b
📦 build(sentry): require hyperf/coordinator
huangdijia File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,299 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
| /** | ||
| * This file is part of friendsofhyperf/components. | ||
| * | ||
| * @link https://github.com/friendsofhyperf/components | ||
| * @document https://github.com/friendsofhyperf/components/blob/main/README.md | ||
| * @contact huangdijia@gmail.com | ||
| */ | ||
|
|
||
| namespace Sentry\State; | ||
|
|
||
| use FriendsOfHyperf\Sentry\Util\ContextArrayObject; | ||
| use Psr\Log\LoggerInterface; | ||
| use Psr\Log\NullLogger; | ||
| use Sentry\Tracing\PropagationContext; | ||
| use Throwable; | ||
|
|
||
| use function Hyperf\Support\make; | ||
|
|
||
| /** | ||
| * Manages runtime-local SDK state across different execution models. | ||
| * | ||
| * Lifecycle model: | ||
| * - The manager keeps a lazily initialized global context as fallback. | ||
| * - startContext() creates an isolated runtime context for the current | ||
| * execution key when no context is active yet. | ||
| * - endContext() flushes context resources and removes that context. | ||
| * | ||
| * @internal | ||
| */ | ||
| final class RuntimeContextManager | ||
| { | ||
| private const PROCESS_EXECUTION_CONTEXT_KEY = 'sentry.process.execution_context'; | ||
|
|
||
| /** | ||
| * @var HubInterface | ||
| */ | ||
| private $baseHub; | ||
|
|
||
| /** | ||
| * @var null|RuntimeContext | ||
| */ | ||
| private $globalContext; | ||
|
|
||
| /** | ||
| * @var ContextArrayObject{string: RuntimeContext} map of active runtime contexts by their internal ID | ||
| */ | ||
| private $activeContexts; | ||
|
|
||
| /** | ||
| * @var ContextArrayObject{string: string} map of execution context keys to active runtime context IDs | ||
| */ | ||
| private $executionContextToRuntimeContext; | ||
|
|
||
| public function __construct(HubInterface $baseHub) | ||
| { | ||
| if (! $baseHub->getClient()) { | ||
| $baseHub = make(HubInterface::class); | ||
| } | ||
| $this->baseHub = $baseHub; | ||
| $this->globalContext = null; | ||
| // Using ContextArrayObject here since the manager is designed to be used in a single-threaded execution environment and does not require the overhead of thread-safe structures. | ||
| $context = new ContextArrayObject(); | ||
| $this->activeContexts = $context; | ||
| $this->executionContextToRuntimeContext = $context; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the current hub with context-aware behavior. | ||
| * | ||
| * If a runtime context is active for the current execution key, the hub is | ||
| * updated only for that active context. Otherwise, the baseline/global hub | ||
| * template is updated. | ||
| * | ||
| * @return bool Whether the hub was set on an active runtime context | ||
| */ | ||
| public function setCurrentHub(HubInterface $hub): bool | ||
| { | ||
| $executionContextKey = $this->getExecutionContextKey(); | ||
|
|
||
| if ($this->hasActiveContextForExecutionContextKey($executionContextKey)) { | ||
| $runtimeContextId = $this->executionContextToRuntimeContext[$executionContextKey]; | ||
| $this->activeContexts[$runtimeContextId]->setHub($hub); | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| $this->baseHub = $hub; | ||
|
|
||
| if ($this->globalContext !== null) { | ||
| $this->globalContext->setHub($hub); | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| public function getCurrentHub(): HubInterface | ||
| { | ||
| return $this->getCurrentContext()->getHub(); | ||
| } | ||
|
|
||
| public function getCurrentContext(): RuntimeContext | ||
| { | ||
| $executionContextKey = $this->getExecutionContextKey(); | ||
|
|
||
| if ($this->hasActiveContextForExecutionContextKey($executionContextKey)) { | ||
| $runtimeContextId = $this->executionContextToRuntimeContext[$executionContextKey]; | ||
|
|
||
| return $this->activeContexts[$runtimeContextId]; | ||
| } | ||
|
|
||
| return $this->getGlobalContext(); | ||
| } | ||
|
|
||
| public function hasActiveContext(): bool | ||
| { | ||
| return $this->hasActiveContextForExecutionContextKey($this->getExecutionContextKey()); | ||
| } | ||
|
|
||
| /** | ||
| * Starts an isolated context for the current execution key. | ||
| */ | ||
| public function startContext(): void | ||
| { | ||
| $executionContextKey = $this->getExecutionContextKey(); | ||
|
|
||
| if ($this->hasActiveContextForExecutionContextKey($executionContextKey)) { | ||
| // Nested start calls for the same execution key should be a no-op. | ||
| return; | ||
| } | ||
|
|
||
| $this->createContextForExecutionContextKey($executionContextKey); | ||
| } | ||
|
|
||
| /** | ||
| * Ends and flushes the active context for the current execution key. | ||
| * | ||
| * When no context is active for the key this is a no-op. | ||
| */ | ||
| public function endContext(?int $timeout = null): void | ||
| { | ||
| $executionContextKey = $this->getExecutionContextKey(); | ||
|
|
||
| if (! $this->hasActiveContextForExecutionContextKey($executionContextKey)) { | ||
| return; | ||
| } | ||
|
|
||
| $runtimeContextId = $this->executionContextToRuntimeContext[$executionContextKey]; | ||
| unset($this->executionContextToRuntimeContext[$executionContextKey]); | ||
|
|
||
| $this->removeContextById($runtimeContextId, $timeout); | ||
| } | ||
|
|
||
| private function createContextForExecutionContextKey(string $executionContextKey): void | ||
| { | ||
| $runtimeContextId = $this->generateRuntimeContextId(); | ||
| $runtimeContext = new RuntimeContext($runtimeContextId, $this->createHubFromBaseHub()); | ||
|
|
||
| $this->activeContexts[$runtimeContextId] = $runtimeContext; | ||
| $this->executionContextToRuntimeContext[$executionContextKey] = $runtimeContextId; | ||
| } | ||
|
|
||
| private function removeContextById(string $runtimeContextId, ?int $timeout = null): void | ||
| { | ||
| if (! isset($this->activeContexts[$runtimeContextId])) { | ||
| return; | ||
| } | ||
|
|
||
| $runtimeContext = $this->activeContexts[$runtimeContextId]; | ||
| unset($this->activeContexts[$runtimeContextId]); | ||
| // Remove any key mappings that may still reference this context. | ||
| $this->removeExecutionContextMappingsForRuntimeContext($runtimeContextId); | ||
|
|
||
| $logger = $this->getLoggerFromHub($runtimeContext->getHub()); | ||
|
|
||
| $this->flushRuntimeContextResources($runtimeContext, $timeout, $logger); | ||
| } | ||
|
|
||
| private function flushRuntimeContextResources(RuntimeContext $runtimeContext, ?int $timeout, LoggerInterface $logger): void | ||
| { | ||
| $hub = $runtimeContext->getHub(); | ||
|
|
||
| // captureEvent can throw before transport send (for example from scope event processors | ||
| // or before_send callbacks), so we isolate failures and continue flushing other resources. | ||
| try { | ||
| $runtimeContext->getLogsAggregator()->flush($hub); | ||
| } catch (Throwable $exception) { | ||
| $logger->error('Failed to flush logs while ending a runtime context.', [ | ||
| 'exception' => $exception, | ||
| 'runtime_context_id' => $runtimeContext->getId(), | ||
| ]); | ||
| } | ||
|
|
||
| // Keep metrics flush independent from logs flush so one bad callback does not block the rest. | ||
| try { | ||
| $runtimeContext->getMetricsAggregator()->flush($hub); | ||
| } catch (Throwable $exception) { | ||
| $logger->error('Failed to flush trace metrics while ending a runtime context.', [ | ||
| 'exception' => $exception, | ||
| 'runtime_context_id' => $runtimeContext->getId(), | ||
| ]); | ||
| } | ||
|
|
||
| $client = $hub->getClient(); | ||
|
|
||
| if ($client === null) { | ||
| return; | ||
| } | ||
|
|
||
| // Custom transports may throw from close(); endContext must stay best-effort and non-fatal. | ||
| try { | ||
| $client->flush($timeout); | ||
| } catch (Throwable $exception) { | ||
| $logger->error('Failed to flush the client transport while ending a runtime context.', [ | ||
| 'exception' => $exception, | ||
| 'runtime_context_id' => $runtimeContext->getId(), | ||
| ]); | ||
| } | ||
| } | ||
|
|
||
| private function removeExecutionContextMappingsForRuntimeContext(string $runtimeContextId): void | ||
| { | ||
| foreach ($this->executionContextToRuntimeContext as $executionContextKey => $mappedRuntimeContextId) { | ||
| if ($mappedRuntimeContextId === $runtimeContextId) { | ||
| unset($this->executionContextToRuntimeContext[$executionContextKey]); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private function hasActiveContextForExecutionContextKey(string $executionContextKey): bool | ||
| { | ||
| if (! isset($this->executionContextToRuntimeContext[$executionContextKey])) { | ||
| return false; | ||
| } | ||
|
|
||
| $runtimeContextId = $this->executionContextToRuntimeContext[$executionContextKey]; | ||
|
|
||
| if (! isset($this->activeContexts[$runtimeContextId])) { | ||
| // Mapping points to a context that was already evicted/ended; drop the stale index entry. | ||
| unset($this->executionContextToRuntimeContext[$executionContextKey]); | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| private function createHubFromBaseHub(): HubInterface | ||
| { | ||
| if (! $this->baseHub instanceof Hub) { | ||
| return new Hub($this->baseHub->getClient()); | ||
| } | ||
|
|
||
| $clonedScope = null; | ||
|
|
||
| $this->baseHub->configureScope(static function (Scope $scope) use (&$clonedScope): void { | ||
| $clonedScope = clone $scope; | ||
| // Do not inherit active traces into a new runtime context. | ||
| $clonedScope->setSpan(null); | ||
| $clonedScope->setPropagationContext(PropagationContext::fromDefaults()); | ||
| }); | ||
|
|
||
| return new Hub($this->baseHub->getClient(), $clonedScope ?? new Scope()); | ||
| } | ||
|
|
||
| private function getLoggerFromHub(HubInterface $hub): LoggerInterface | ||
| { | ||
| $client = $hub->getClient(); | ||
|
|
||
| if ($client === null) { | ||
| return new NullLogger(); | ||
| } | ||
|
|
||
| return $client->getOptions()->getLoggerOrNullLogger(); | ||
| } | ||
|
|
||
| private function generateRuntimeContextId(): string | ||
| { | ||
| return \sprintf('%s-%d', str_replace('.', '', uniqid('', true)), mt_rand()); | ||
| } | ||
|
|
||
| private function getExecutionContextKey(): string | ||
| { | ||
| // All supported runtime modes currently use a process-local execution key. | ||
| return self::PROCESS_EXECUTION_CONTEXT_KEY; | ||
| } | ||
|
|
||
| private function getGlobalContext(): RuntimeContext | ||
| { | ||
| if ($this->globalContext === null) { | ||
| // Lazy fallback keeps baseline behavior when users do not opt into explicit context lifecycle. | ||
| $this->globalContext = new RuntimeContext('global', $this->baseHub); | ||
| } | ||
|
|
||
| return $this->globalContext; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
When a request/RPC/command is handled without going through
Hyperf\Coroutine\Coroutine::create, it now falls back to this singleglobalRuntimeContext, soTracing\Listener\EventHandleListener::handleRequestReceived()sets spans/scopes on a hub shared by concurrent requests. The removedSentrySdkAspectpreviously kept the runtime manager in Hyperf's coroutineContext, but the only newstartContext()calls are in the coroutine aspects, leaving normal server lifecycle events unisolated and allowing breadcrumbs/spans/metrics to bleed across requests until some later flush.Useful? React with 👍 / 👎.