Repository navigation
feat: add the Laravel runtime package - #283
marandaneto wants to merge 26 commits into
Conversation
posthog-php-fork_curl Compliance ReportDate: 2026-10-30T13:05:31.195709+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 37ms |
| Format Validation.Event Has Uuid | ✅ | 529ms |
| Format Validation.Event Has Lib Properties | ✅ | 531ms |
| Format Validation.Distinct Id Is String | ✅ | 531ms |
| Format Validation.Token Is Present | ✅ | 531ms |
| Format Validation.Custom Properties Preserved | ✅ | 531ms |
| Format Validation.Event Has Timestamp | ✅ | 531ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 532ms |
| Retry Behavior.Retries On 503 | ✅ | 5852ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2534ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2536ms |
| Retry Behavior.Respects Retry After Header | ❌ | 6349ms |
| Retry Behavior.Implements Backoff | ✅ | 16275ms |
| Retry Behavior.Retries On 500 | ✅ | 5644ms |
| Retry Behavior.Retries On 502 | ✅ | 5647ms |
| Retry Behavior.Retries On 504 | ✅ | 5649ms |
| Retry Behavior.Max Retries Respected | ✅ | 16271ms |
| Deduplication.Generates Unique Uuids | ✅ | 43ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5642ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10863ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5647ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 538ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 532ms |
| Compression.Sends Gzip When Enabled | ✅ | 533ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 531ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 519ms |
| Batch Format.Multiple Events Batched Together | ✅ | 520ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2532ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2532ms |
| Error Handling.Retries On 408 | ✅ | 5646ms |
Failures
retry_behavior.respects_retry_after_header
Retry delay too short: 810ms < 2500ms
Feature_Flags Tests
✅ 17/17 tests passed
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 524ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 522ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 522ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 523ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 523ms |
| Request Payload.Groups Round Trip | ✅ | 522ms |
| Request Payload.Groups Default To Empty Object | ✅ | 522ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 522ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ✅ | 522ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 522ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 520ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 516ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 528ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 523ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 624ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 626ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 532ms |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 3b2ad5a: 7 Should fix. This pull request is large, so the review ran in 4 parts with less depth than usual.
This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
posthog-php-socket Compliance ReportDate: 2026-10-30T13:05:39.054141+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 24ms |
| Format Validation.Event Has Uuid | ✅ | 520ms |
| Format Validation.Event Has Lib Properties | ✅ | 521ms |
| Format Validation.Distinct Id Is String | ✅ | 522ms |
| Format Validation.Token Is Present | ✅ | 521ms |
| Format Validation.Custom Properties Preserved | ✅ | 522ms |
| Format Validation.Event Has Timestamp | ✅ | 521ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 521ms |
| Retry Behavior.Retries On 503 | ✅ | 5830ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2523ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2525ms |
| Retry Behavior.Respects Retry After Header | ❌ | 6327ms |
| Retry Behavior.Implements Backoff | ✅ | 16243ms |
| Retry Behavior.Retries On 500 | ✅ | 5633ms |
| Retry Behavior.Retries On 502 | ✅ | 5629ms |
| Retry Behavior.Retries On 504 | ✅ | 5631ms |
| Retry Behavior.Max Retries Respected | ✅ | 16244ms |
| Deduplication.Generates Unique Uuids | ✅ | 526ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5630ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10836ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5632ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 526ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 523ms |
| Compression.Sends Gzip When Enabled | ✅ | 522ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 521ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 518ms |
| Batch Format.Multiple Events Batched Together | ✅ | 510ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2524ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2523ms |
| Error Handling.Retries On 408 | ✅ | 5630ms |
Failures
retry_behavior.respects_retry_after_header
Retry delay too short: 802ms < 2500ms
Feature_Flags Tests
✅ 17/17 tests passed
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 521ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 520ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 519ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 519ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 520ms |
| Request Payload.Groups Round Trip | ✅ | 520ms |
| Request Payload.Groups Default To Empty Object | ✅ | 519ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 520ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ✅ | 520ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 519ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 516ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 507ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 523ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 521ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 622ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 622ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 521ms |
posthog-php-lib_curl Compliance ReportDate: 2026-10-30T13:05:10.923424+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 29ms |
| Format Validation.Event Has Uuid | ✅ | 524ms |
| Format Validation.Event Has Lib Properties | ✅ | 524ms |
| Format Validation.Distinct Id Is String | ✅ | 525ms |
| Format Validation.Token Is Present | ✅ | 525ms |
| Format Validation.Custom Properties Preserved | ✅ | 524ms |
| Format Validation.Event Has Timestamp | ✅ | 525ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 524ms |
| Retry Behavior.Retries On 503 | ✅ | 5832ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2529ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2528ms |
| Retry Behavior.Respects Retry After Header | ❌ | 6333ms |
| Retry Behavior.Implements Backoff | ✅ | 16252ms |
| Retry Behavior.Retries On 500 | ✅ | 5636ms |
| Retry Behavior.Retries On 502 | ✅ | 5632ms |
| Retry Behavior.Retries On 504 | ✅ | 5640ms |
| Retry Behavior.Max Retries Respected | ✅ | 17049ms |
| Deduplication.Generates Unique Uuids | ✅ | 233ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5632ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10842ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5637ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 532ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 527ms |
| Compression.Sends Gzip When Enabled | ✅ | 525ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 525ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 521ms |
| Batch Format.Multiple Events Batched Together | ✅ | 512ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2528ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2527ms |
| Error Handling.Retries On 408 | ✅ | 5633ms |
Failures
retry_behavior.respects_retry_after_header
Retry delay too short: 804ms < 2500ms
Feature_Flags Tests
✅ 17/17 tests passed
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 525ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 522ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 525ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 523ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 523ms |
| Request Payload.Groups Round Trip | ✅ | 524ms |
| Request Payload.Groups Default To Empty Object | ✅ | 522ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 523ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ✅ | 524ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 522ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 518ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 509ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 526ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 524ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 624ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 625ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 525ms |
Preserve identity fallback and normalize invokable transport error handlers. Bind flag construction to immutable Laravel data; delayed exposure regression awaits the approved core host dependency.
Remove library_name, library_version and release_id from the public client options. Framework integrations override internal Client methods instead, so customers keep POSTHOG_RELEASE_ID as the only documented release id input.
…avel-package-runtime
ioannisj
left a comment
ioannisj
left a comment
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
Nothing blocking from my side. The two I'd look at before the first Laravel release are the per-scope definitions refetch when flag_cache is off and the unguarded lazy client build in packages/laravel/src/PostHog.php, the rest are smaller and could be follow-ups.
| && trim((string) $this->app['config']->get('posthog.api_key')) !== ''; | ||
| } | ||
|
|
||
| public function client(): Client |
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
[suggestion] client() builds the core client with no try/catch, and $this->client stays null when the build throws, so a mistyped posthog.sdk.consumer (undefined key, which Laravel turns into an ErrorException) or a provider that doesn't implement FlagDefinitionCacheProvider throws from the first capture() or flag read in the app, on every call, while posthog:doctor still reports healthy because it never builds the client.
Would it be worth catching here, logging once with the failing option, and falling back to the noop client? Or if fail-fast is the intent, validating the options at boot so it blows up at deploy time rather than per request. Either way doctor should probably build the client.
| public function captureException( | ||
| Throwable|string $exception, | ||
| ?string $distinctId = null, | ||
| array $properties = [], |
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
[suggestion] The third parameter here is $properties where core Client::captureException uses $additionalProperties, so captureException($e, additionalProperties: [...]) copied from core code throws Unknown named parameter on the manager and the facade. The README documents properties: so I take it that's deliberate, but testSupportedForwardersKeepTheCoreParameterSignatures skips captureException and evaluateFlags, so these two can drift from core unnoticed.
Rename to $additionalProperties before 0.1.0 freezes it, or keep properties and add both methods to the parity test with the rename allowlisted? Is there a reason not to match core?
| private function leave(): void | ||
| { | ||
| try { | ||
| $this->app->make(PostHog::class)->flush(); |
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
[suggestion] leave() flushes at every nesting depth (the finally in call() reaches it), so an Artisan::call() inside a controller sends the whole queue synchronously before the response goes out. I ran a route with one nested call against a flush-counting client and it flushed once inside the request. With QueueConsumer::flush() ignoring the retry cooldown and a 10 s default timeout, an ingestion outage holds the response for up to that per nested call.
Should nested calls skip the flush and leave events for the root boundary or HTTP termination, which already flush? Per-job and per-request flushing at scope end looks fine, it's only the nested console case.
| if (!$explicitId && $resolver) { | ||
| try { | ||
| $id = ($this->app->make($resolver))($this->app); | ||
| } catch (Throwable) { |
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
[suggestion] The three catch (Throwable) {} blocks around the distinct_id resolver, the user lookup and the groups resolver swallow everything, so when a resolver throws (db down, bad binding) events go out without identity or groups and nothing says why, on every event until the scope resolves.
Worth one rate-limited error_log or logger line per resolver per scope with the class and message? Not via report() though, that would re-enter the reporter.
| return ['handled' => false, 'mechanism' => 'runtime']; | ||
| } | ||
| } | ||
| return ['handled' => true, 'mechanism' => 'middleware']; |
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
[suggestion] The fallback here returns mechanism => 'middleware' whenever no boundary frame matched, and the reportable callback runs for every handler report(), so a manual report($e) from a job, command or scheduler task gets mechanism.type = middleware without any middleware being crossed. The exception-event-metadata spec has manual capture on generic unless a more specific category is known.
Should the fallback be generic, with middleware reserved for the HTTP pipeline frames? RuntimeTest only asserts handled on the manual path so it'd need one more assertion.
| || !empty($config['sdk']['flag_definition_cache_provider']) | ||
| || ($config['flag_cache']['enabled'] ?? false); | ||
| $interval = max(1, (int) ($config['flag_cache']['refresh_interval'] ?? 30)); | ||
| $refreshDue = $this->definitionsLoadedAt === null || time() - $this->definitionsLoadedAt >= $interval; |
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
[suggestion] $definitionsLoadedAt lives on the scoped manager, and the queue worker's forgetScopedInstances() plus Octane's per-request sandbox drop it together with the client, so with secret_key set and flag_cache.enabled at its default false every job or request that reads a flag does a blocking /flags/definitions GET (core shouldFetchFlagDefinitionsFromApi() returns true with no provider). I counted three fetches across three scopes inside one 30 s interval, so refresh_interval only throttles within a scope.
OctaneTest asserts separate clients per sandbox, so a singleton isn't it. Would it make sense to default the cache provider onto the process-local array store when a secret key is set and no provider is configured, so definitions live per worker? Or at least a doctor/README warning that a secret key without flag_cache refetches per request and job.
| $request = $this->request; | ||
| $headerIdentity = $this->headerIdentity; | ||
| $resolved = $this->resolved; | ||
| $sessionId = $data['sessionId'] ?? $data['session_id'] ?? null; |
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
[nit] The new session_id read only feeds the $session_id property sync, the alias keys stay raw in the merged context, and data(), QueueContext::normalize() and the headerIdentity reset all look at distinctId/sessionId. So with(['session_id' => 'a', 'distinct_id' => 'b']) on a header-identified request gives the event session a with the header's distinct id, and a job dispatched inside forwards the header identity.
Either promote the aliases to canonical keys like core normalizeContextData() does, or drop the alias read so Context::with() consistently ignores snake_case (the README only documents aliases for the manager bridge). Which did you intend?
| $this->lifecycle->begin($request); | ||
| try { | ||
| return $next($request); | ||
| } catch (Throwable $exception) { |
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
[nit] This catch never runs in a real request: Pipeline::carry() catches downstream throwables and Routing\Pipeline::handleException() reports and renders instead of rethrowing, so $next() returns a response. Your own /failure route test asserts laravel.reportable, and laravel.middleware is only produced by invoking the middleware directly, yet the README lists it as an emitted source (line 181).
Drop it from the README, or reword to say request failures report as laravel.reportable with mechanism middleware? The catch can stay as a safety net for custom pipelines.
💡 Motivation and Context
Add the independently versioned
posthog/posthog-laravelpackage underpackages/laravel. It depends on the already-published PHP SDK ^4.20. Core framework support landed in #282 and was released in 4.20.0. This branch now includes currentmain, so the PR no longer repeats those core implementation changes.Composer discovery registers scoped services, a facade and an injectable core client. The package adds lazy SDK construction, authenticated/header analytics context, safe route metadata, bounded queue propagation, Laravel exception reporting, request-scoped flag snapshots, an optional Laravel Cache definition provider, diagnostics and a test fake.
Public API boundary
The supported surface is the manager/facade, documented
ContextAPIs, theIsolatesCommandContexttrait, test fake, service provider registration and the Laravel cache adapter. Framework-only classes are marked@internaland are final where appropriate. Public framework callback methods are not supported customer extension points.The manager declares supported client conveniences explicitly instead of forwarding arbitrary methods through
__call(). These include the five legacy flag getters,loadFlags()andcontextFromHeaders(). Legacy getters inherit current context groups when no nonempty groups argument is supplied, consistently withevaluateFlags(). Explicit nonempty groups override the ambient groups. Other lower-level operations remain available throughclient(). The newapi/laravel-public-api.jsonsnapshot tracks the Laravel API separately from core and excludes internal classes and members. CI checks both snapshots and runs the Laravel tests.Lifecycles and exceptions
HTTP context remains active through kernel termination and after-response callbacks. Queue and sequential Octane boundaries flush and clean context. Console integration decorates the application's existing kernel and restores nested kernel calls in
finally.Direct child-command
call(),callSilent()andcallSilently()isolation is opt-in: applyPostHog\Laravel\Concerns\IsolatesCommandContextto the calling command or the application's base command. Uninstrumented callers, including closure commands, still need kernel-levelArtisan::call()or an explicit context scope. There is no global command interception, vendor patching or reflection-based wrapping.Automatic exception reporting reuses
captureException()inside a fresh saved-context scope. It preserves the original identity, groups and boundary metadata without a secondreportException()API. Automatic reports of the same exception object are deduplicated. Laravel retains ownership of PHP handlers and exception-reporting policy.Known hooks supply stable
$exception_sourcevalues:laravel.middleware,laravel.queue_job,laravel.console_lifecycleandlaravel.octane_worker_error. The registered reportable callback supplieslaravel.reportablewhen no earlier source was remembered. Sources are not inferred from stack frames. Ordinary manual captures and direct reporter calls with no known hook omit the source. Later boundaries do not overwrite an earlier known source.Safety and compatibility
Missing credentials and explicit disablement prevent SDK network paths. Defaults exclude raw URLs, query strings, bodies, cookies, email, visitor IP and implicit profile synchronization. GeoIP defaults off. PostHog tracing headers and queue metadata are untrusted analytics context, never authentication or authorization evidence.
Custom HTTP kernels without Laravel's lifecycle callback API cannot provide automatic after-response correlation. Overlapping shared-container Swoole coroutines are not supported. Do not retain scoped services in application singletons or prewarm them in Octane.
Capture, identify, alias, groups, exception metadata, feature-flag evaluation and tracing-header contracts reuse the PHP SDK. The core's pre-existing wire behavior outside the integration changes is not changed by this PR.
Core distribution boundary
The root
.gitattributesexcludespackages/laravel/from core Composer archives. Laravel source remains available in Git and is not registered by the core autoloader. An archive regression test confirms the exclusion while retaining the core manifest, runtime and root license. The test failed before the attribute was added and passes afterward.Source-copy tools must use a full Git checkout rather than a whole-repository
git archive, which now excludes Laravel. The helper in the later tooling PR #287 needs that adjustment before it lands. Core release versioning remains scoped to the root package.💚 How did you test it?
captureException(), preserve context through failures, and attach known source metadata without leaking it into manual captures.git diff --checkpass.d172504cf848badbb6d34936c61a2b0c0fd80066received a clean isolated review after its ambient-group finding was reproduced and fixed in a separate signed commit. The archive closeout at4837e6877a5847e1e3ae6ffae6642df282492c0balso received a clean isolated review.📝 Checklist
🤖 Agent context
Autonomy: Human-driven (agent-assisted).
Implemented and validated with Pi, PHP/Composer, Git and GitHub CLI in the existing dedicated runtime worktree. The user requested the API naming changes, reporting consolidation, internal API restrictions, explicit forwarding, API snapshot coverage, shared-license symlink and removal of release intents. Human review is still required before merge.
Stack
Merge bottom-up:
Laravel source lives in this repository but its Composer package is released separately.
packages/laravel/LICENSE.mdis a relative symlink to the repository's shared license. The later distribution tooling must materialize that file when exporting a standalone package.Before the first Laravel release
posthog-laravelstill needs server registration/aliases for PHP usage attribution, Python/Rust feature-flag library classification/parsing, and SDK health registries, following PostHog/posthog#93911. Its independent0.1.xpackage version must not be compared against PHP core4.x. Event ingestion is not rejected solely because the library identifier is unclassified.The distribution repository, protected release environment and Packagist setup remain administrator prerequisites. Release intents and the standalone publisher belong with the release tooling, not this runtime PR. No Laravel release has been published.