Harden Sentry lifecycle ownership and performance - #5
Merged
Conversation
Install copied fork context before running after-created callbacks so infrastructure hooks observe the captured snapshot rather than stale parent state. Keep ordinary coroutine creation on an empty initial context and preserve the existing callback and coroutine failure boundaries.\n\nAdd regression coverage for callback visibility and the distinction between create and fork semantics.
Guard the framework worker-exit callback before dispatch so repeated native invocations cannot replay application cleanup or coordinator resumption. Set the guard before listener execution to keep a throwing listener from reopening the lifecycle boundary.\n\nCover repeated callbacks and throwing listeners while preserving the existing server wiring and nonblocking exit behavior.
Introduce a small InvalidatesPool contract and expose it through object-pool and pooled-filesystem proxies. This lets operational purge paths invalidate the actual pool owner without coupling managers or decorators to concrete wrapper classes.\n\nClarify fingerprint conflicts around genuinely equivalent construction identities and cover contract forwarding, successful invalidation, and missing-pool behavior.
Carry nullable logical disk names through on-demand, configured, custom, and scoped construction so signed routes, telemetry labels, and whole-driver pool fingerprints remain accurate. Purge pooled disks through the invalidation capability, including uncached configured disks, without reconstructing pool identity in the manager.\n\nRegister signed serving routes from the explicit serve capability rather than a concrete local-driver classification. Document the additive construction seams and pool convergence rules, and cover named, anonymous, scoped, custom, served, and purge paths.
Add exact retrieval-failure events for single and many-key reads, and emit the existing write and forget failure terminals from their owning repository boundaries before rethrowing. Normalize complete many-key results before success fan-out so one handler failure cannot leave a partial terminal sequence.\n\nKeep construction and dispatch behind listener checks, type resolved key lists at the event owner, and make empty aggregate writes true no-ops across repository and Redis tag modes. Add focused lifecycle, exception, empty-batch, and tagged-store coverage plus public event documentation.
Dispatch NotificationSkipped when shouldSend or NotificationSending rejects delivery, and dispatch NotificationDelivered immediately after the channel returns. Preserve the existing afterSending and NotificationSent order while keeping pre-delivery failure deduplication and allowing post-delivery callback failures to propagate without false relabeling.\n\nGuard both new events with listener checks and cover veto, success, transport failure, post-delivery failure, ordering, and coroutine-local failure ownership.
Emit JobQueueingFailed from the exact enqueue attempt boundary and expose whether worker termination must return immediately. Preserve Database bulk inserts while partitioning immediate and after-commit groups, reacquiring deferred queues, computing delay at the actual attempt, and emitting one exact terminal per attempted job.\n\nMake SQS batch lifecycle events follow attempted chunks and correlate AWS responses by entry id, including explicit rejects and ambiguous request failures. Add regression coverage for single, bulk, rollback, deferred, partial, overflow, and forced-stop behavior.
Replace the synthetic HTTP client with the SDK client and give each detached send exclusive ownership of one borrowed pooled transport. Balance one captured WaitGroup generation across release, discard, and coroutine-spawn failure so bounded drains never wait on later sends.\n\nKeep ordinary flushes nonblocking, flush buffered logs and metrics before transport capture, reserve pool shutdown for worker exit, and preserve fail-fast backpressure. Cover event ids, status and rate-limit handling, exhaustion, generation swaps, child failures, and shutdown.
Centralize endpoint, tracing, and breadcrumb capability decisions over merged configuration, then avoid registering aspects, listeners, middleware work, and decorators that cannot produce output. Keep propagation independent from local recording and retain per-feature memoization outside request paths.\n\nPort Storage tracing through one outer decorator while preserving logical names, pools, stream leases, temporary URL capabilities, fluent chains, and scoped configuration. Add middleware continuation control, truthful feature-phase diagnostics, dynamic SDK identity, and regression coverage for inactive boot, Spotlight, capability gating, adapter delegation, purge, routes, and storage operations.
Clone inherited mutable Hub layers and requests for child coroutines while preserving client and span pointers, retain placeholder scope state during binding, and keep one authoritative root layer. Give response spans, connection-specific database transactions, Guzzle children, and feature-local spans exact owners without installing operation-local children as Hub current.\n\nAdd bounded coroutine-exit orphan cleanup, catch Throwable at the real event boundaries, and handle failed view-origin reads and null query duration honestly. Cover parent-child-sibling isolation, nested and interleaved connections, local unwind, falsey auth context, and exact span restoration.
Consume the framework's resolved single and many-key success and failure events without a pending-operation registry or Laravel's associative-key normalizer. Finish each sampled span from its exact terminal while retaining breadcrumb behavior and skipping all listener registration when neither output is possible.\n\nShare guarded session-key resolution for cache and Redis instrumentation, preferring already-resolved state and request cookies before a reentry-protected fallback. Cover read, write, forget, batch, failure, no-output, and session-resolution paths.
Omit Redis command parameters unless default PII is enabled, then redact only exact string matches for the active session key while preserving falsey keys and non-string arguments. Resolve session state through the shared bounded concern and retain existing pool metrics and nullable duration semantics.\n\nRemove duplicated success and failure recording without adding request-time resolution or context state on the ordinary fast paths. Cover disabled PII, redaction, key zero, cookies, resolved stores, guarded fallback, and resolution failures.
Close notification delivery spans on delivered, failed, or skipped events while retaining the existing NotificationSent breadcrumb. This gives the span the real channel-delivery boundary and avoids relabeling post-delivery callback failures as transport failures.\n\nKeep local orphan cleanup as a bounded final safety net and add regression coverage for success, veto, failure, event order, and exact span status.
Start sampled publication spans only when an enqueue attempt begins, carry the resolved destination through the payload hook, and correlate out-of-order success and failure terminals by the unchanged payload. Keep propagation metadata when local recording is disabled and never install publication children as Hub current.\n\nAdd per-job sampling middleware and use bounded drains only for graceful console and queue-worker lifecycles, skipping immediate and max-memory termination. Cover resolved default queues, mixed terminals, orphan cleanup, propagation-only mode, sampling, and flush ordering.
Report dynamic Hypervel SDK metadata and profile sampler state, publish environment values through the framework writer with overwrite semantics, and restore process-global error reporting after the Sentry test command. Port scheduled monitor expression overrides and normalize model-violation callables without narrowing supported callback forms.\n\nInitialize the log batch formatter explicitly and cover command output, publication, global-state restoration, schedule configuration, callable objects, and log-channel construction.
Declare the direct runtime dependencies used by the distributed Sentry package and raise the SDK floor only to the version required for profiles sampler support. Keep optional Sanctum interoperability out of requirements and suggestions because installing it unlocks no Sentry capability.\n\nAdd Laravel-style guidance for setup, DSN precedence, tracing, metrics, queues, monitors, storage, PII, Spotlight, pooled transport backpressure, and bounded shutdown. Keep the package README limited to documentation, its intentional asynchronous transport difference, and upstream provenance.
Clarify that ext requirements belong only to extensions not guaranteed by Hypervel's minimum PHP version. Limit suggest entries to installable packages that unlock a concrete documented capability, excluding incidental interoperability, class strings, tests, and metadata symmetry.\n\nThis prevents redundant core-extension requirements and misleading optional-package suggestions from being added for completeness alone.
Add the signed-off implementation plan with the verified Swoole lifecycle facts, exact ownership model, accepted and rejected designs, testing strategy, performance constraints, and compatibility result.\n\nClose Sentry in the core package checklist and update the audit ledger with every durable Sentry and shared-owner finding, dependency edge, validation gate, final review amendment, and the conclusion that no accepted defect or deferred implementation remains.
# Conflicts: # docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.md # docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md
|
Important Review skippedToo many files! This PR contains 129 files, which is 29 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (129)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR substantially hardens Sentry behavior for Hypervel’s long-lived coroutine runtime and adds framework lifecycle events needed by its instrumentation.
Confidence Score: 5/5The PR appears safe to merge, with no concrete changed-code defect remaining after tracing the principal transport, queue, storage, and coroutine lifecycle paths. Accepted sends retain exact transport and generation ownership, deferred queue groups preserve their lifecycle boundaries, and filesystem decorators retain the capabilities of their wrapped implementations without exposing a reachable regression.
|
| Filename | Overview |
|---|---|
| src/sentry/src/Transport/HttpPoolTransport.php | Transfers each borrowed SDK transport to one detached coroutine and balances transport disposal and generation accounting across completion and coroutine-creation failures. |
| src/sentry/src/Hub.php | Hardens coroutine-local Hub layer and scope ownership while preserving the intended client and span references. |
| src/sentry/src/SentryServiceProvider.php | Derives instrumentation capabilities during registration and avoids installing inactive hooks and listeners. |
| src/sentry/src/Features/Storage/Integration.php | Registers capability-aware Sentry filesystem decoration while retaining the original disk implementation and logical identity. |
| src/filesystem/src/FilesystemManager.php | Propagates logical disk names through configured, custom, scoped, on-demand, and pooled construction paths. |
| src/queue/src/DatabaseQueue.php | Preserves bulk insertion while partitioning immediate and after-commit jobs and emitting complete enqueue lifecycle events. |
| src/queue/src/SqsQueue.php | Correlates batch results by AWS entry IDs and emits terminal events only for entries actually attempted. |
| src/cache/src/Repository.php | Adds guarded retrieval and aggregate-operation lifecycle events, including explicit failure terminals. |
| src/notifications/src/NotificationSender.php | Separates skipped delivery from completed channel delivery and prevents callback failures from being mislabeled as transport failures. |
| src/coroutine/src/Coroutine.php | Copies mutable coroutine context safely and cleans up partially initialized child state on creation failures. |
Reviews (1): Last reviewed commit: "Merge branch '0.4' into audit/sentry-cor..." | Re-trigger Greptile
Collaborator
Author
|
@coderabbitai review |
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
This change hardens the Sentry integration for Hypervel's long-lived Swoole runtime while reducing its application footprint.
Motivation
The Sentry package already used pooled transports and coroutine-local state, but several lifecycle boundaries still relied on assumptions from request-per-process PHP.
Detached sends could not be drained by a closed generation, feature-local spans could outlive their real owner, queue publication spans did not have exact failure terminals, and optional instrumentation could still be installed when the SDK could not emit output. Some shared framework operations also lacked the terminal events needed by any observer, not only Sentry.
These failures become more important in a long-lived worker because stale state and unfinished ownership can cross requests or remain resident for the worker lifetime.
Transport and shutdown
The transport now borrows a real SDK transport from the existing pool and transfers it to one child coroutine. The child releases or discards that exact object and balances the captured WaitGroup generation on every exit path.
A positive drain swaps generations before waiting. Sends accepted afterward enter the replacement generation and cannot extend the caller's wait. Ordinary request and job flushes remain nonblocking. Graceful command and queue-worker boundaries may perform a bounded drain. Native worker exit closes new acquisitions and returns immediately, leaving final delivery to Swoole's existing reactor shutdown window.
The synthetic HTTP client has been removed. The SDK now receives real response codes and rate-limit headers while pooled connections remain hot.
Coroutine and tracing ownership
Child coroutines receive cloned mutable Hub layers and request objects while retaining the intended client and span pointers. Placeholder scope state survives client binding, and the final root layer cannot be popped.
Guzzle propagation remains available independently from local span sampling. HTTP client spans, response spans, database transaction spans, queue publication spans, and feature-local spans are owned directly rather than being temporarily installed as the Hub's current span. Coroutine-exit cleanup only unwinds state that its exact feature still owns.
Framework lifecycle boundaries
Cache now exposes exact retrieval failures and uses existing write and forget failure terminals consistently. Empty aggregate writes remain no-ops.
Notifications now distinguishes skipped delivery from actual channel delivery. Delivery is recorded when the channel returns, before post-delivery callbacks, without relabeling later callback failures as transport failures.
Queue now exposes enqueue failures and immediate termination intent. Database bulk enqueue preserves bulk inserts while handling immediate and after-commit groups correctly. SQS emits terminals only for attempted entries and correlates results by the ids returned by AWS.
All optional event construction remains guarded by listener checks.
Storage
Storage tracing stays lazy and preserves existing pool and stream ownership. Logical disk names now survive configured, on-demand, custom, and scoped construction so telemetry labels, signed routes, and whole-driver pool identities remain accurate.
Pool invalidation is capability-based instead of depending on concrete wrappers. Signed serving routes use the explicit serve capability, including decorated and custom disks. Test-only drift guards cover both base and S3 adapter surfaces; no runtime reflection was added.
Performance
Inactive Sentry installations register no Guzzle aspect, Hub propagation hook, tracing middleware, or unusable feature listeners. Span and breadcrumb capabilities are derived from merged configuration during boot or construction and memoized by features.
Request and ordinary job paths add no waits, retries, polling, locks, or synchronous Sentry network I/O. Pool exhaustion drops telemetry instead of blocking application work. Mutable operation state is coroutine-local and bounded by live work.
Compatibility
Supported Laravel-facing Sentry APIs and configuration remain compatible. Shared framework additions are lifecycle events and capability surfaces needed by their owning components.
The documented Hypervel differences are the bounded asynchronous pooled transport and the filesystem construction and serving adaptations required for pooled, decorated disks.
Validation
The changes pass focused Sentry and shared-owner coverage, formatting, both PHPStan configurations, the complete parallel suite, Testbench package mode, and dogfood package tests. The final review also checked caller and callee behavior, coroutine ownership, retained worker state, hot paths, dead code, metadata, and overengineering.
For the complete design and test matrix, see
docs/plans/2026-08-08-1711-sentry-correctness-coroutine-ownership-and-performance.md.