Sampling and entry point by rubenvanassche · Pull Request #69 · spatie/flare-client-php

rubenvanassche · 2026-04-29T12:54:16Z

No description provided.

Centralize entry point detection (web/cli/queue) into EntryPoint and EntryPointResolver, replacing scattered attribute assignments in providers and recorders. Add JobRecorder and QueueRecorder for job span tracking. Change Sampler::shouldSample to accept an EntryPoint instead of an untyped array, enabling future per-entry-point sampling.

Introduce DynamicSampler with SamplingRule support for fine-grained sampling control, PatternMatcher utility for matching rules, and AddJobInformation middleware. Expand recorder tests for command, job, and queue recorders.

…gation Introduce subtask-aware job recorder paths (traceparent resume, AddJobInformation entry point + tracking uuid hand-off), extract a PausableRecorder trait shared by command/job recorders, simplify Tracer pause/resume to a single bool, and harden the git hash check.

…variants

…ache

Splits the legacy request/response attribute providers into Php and Symfony implementations behind RequestAttributesProvider, ResponseAttributesProvider, and RouteAttributesProvider contracts. Adds PhpRouteAttributesProvider and the EntryPointHandlerProvider contract. RequestRecorder and RoutingRecorder now accept providers; recordRoutingEnd requires a RouteAttributesProvider and recordForcedRoutingEnd handles cleanup paths where no matched route is available.

recordRouting drops its required route argument and falls back to recordForcedRoutingEnd, since the route is only known at end time. recordRoutingEnd now sources the handler identifier from the provider's entryPointHandlerIdentifier(), with an 'unknown' fallback when the provider doesn't supply one. FlareTest's censor-request test unsets FLARE_FAKE_WEB_REQUEST and the request globals it sets in a try/finally so the env doesn't leak into later tests, which was producing order-dependent entry point detection in the ReportTest snapshots.

The pest-plugin-snapshots increment counter lands on a different value in older versions (used by CI's prefer-lowest matrix) than in newer ones, so __1.yml is read in CI while __2.yml is read locally for single-assertion tests. Copying the up-to-date content into __1.yml so both versions agree.

Extracts the random source in RateSampler into an overridable random() method. The new FakeSampler subclass lets tests preload the values random() returns, so sampling-rate decisions become deterministic. The lifecycle sampling-decision test no longer relies on mt_rand landing the right way and stops being flaky on prefer-lowest CI.

Adds path-based URL ignoring alongside the existing full-URL ignore, so consumers can match against the request path without having to encode the scheme and host into a glob. Mirrors the existing ignoredUrls plumbing including the default-list extension hook.

Move the heavy toArray() work out of recordStart so the request span can start cheaply before the route is resolved. recordStart now only calls the cheap url()/path() accessors for ignore matching and span naming. recordEnd now accepts request, response, route and user providers and runs toArray() once. The span name is updated to the resolved route via a spanCallback if a route provider is supplied. This unblocks dynamic sampling rules that depend on the route without paying the cost of header/cookie/session/body attribute collection on requests that are ultimately not sampled.

JobRecorder: collapse ignore branches into early returns and only mutate the entry-point resolver / reevaluate sampling in subtask mode. In non-subtask mode the job is a span inside the parent's trace, so the parent (request/command) remains the entry point. QueueRecorder: use the PausableRecorder trait so pause ownership is tracked. Calling Tracer::pauseSampling/resumeSampling directly clobbers another recorder's active pause. SamplingRule: validate rate bounds in the constructor so all paths (forUrl/forRoute/forCommand/forJob, fromArray) reject out-of-range rates consistently with RateSampler.

Add a "What's new in v3" intro covering Lifecycle, EntryPoint, attribute providers, dedicated logging, dynamic sampling, job/queue recorders, and subtask mode. Split the change list into typical-user changes and a separate framework-integrator section so app developers using laravel-flare can skip what doesn't apply to them. Standardize entries with before/after snippets and consolidate the recorder attribute-provider migrations under one heading.

AlexVanderbist

Difficult to review this much code without taking a day and really diving in there. Couple of questions in the review.

AlexVanderbist · 2026-05-04T15:03:28Z

+class GitAttributesProvider implements AttributesProvider
 {
-    protected array $cached;
+    protected static ?array $cached = null;


Why the switch to a static property? Doesn't this make it more difficult/risky to implement properly for Octane?

Nah since a new commit probably resets Octane on deploy

AlexVanderbist · 2026-05-04T15:06:57Z

+
+        return array_filter(
+            $payload,
+            fn ($value) => $value !== null && $value !== '',


Can url.path be an empty string that we may want to keep? E.g. https://flareapp.io/ is either '/' or ''

AlexVanderbist · 2026-05-04T15:10:35Z

+    public function url(): string
+    {
+        $scheme = $this->scheme();
+        $host = $this->serverString('HTTP_HOST') ?: $this->serverString('SERVER_NAME');
+        $uri = $this->serverString('REQUEST_URI');
+
+        if ($scheme === null || $host === null || $uri === null) {
+            return 'unknown';
+        }
+
+        return "{$scheme}://{$host}{$uri}";
+    }
+
+    public function path(): ?string
+    {
+        $uri = $this->serverString('REQUEST_URI');
+
+        if ($uri === null) {
+            return null;
+        }
+
+        $position = strpos($uri, '?');
+
+        return $position === false ? $uri : substr($uri, 0, $position);
+    }
+
+    public function method(): string
+    {
+        return strtoupper($this->serverString('REQUEST_METHOD') ?? 'GET');
+    }
+
+    protected function scheme(): ?string
+    {
+        $https = $this->serverString('HTTPS');
+
+        if ($https !== null && strtolower($https) !== 'off') {
+            return 'https';
+        }
+
+        if ((int) $this->serverString('SERVER_PORT') === 443) {
+            return 'https';
+        }
+
+        return $this->serverString('REQUEST_SCHEME') ?? 'http';
+    }
+
+    protected function serverString(string $key): ?string
+    {
+        $value = $this->server[$key] ?? null;
+
+        if ($value === null || $value === '') {
+            return null;
+        }
+
+        return (string) $value;
+    }
+
+    protected function headerValue(string $name): ?string
+    {
+        $name = strtolower($name);
+
+        foreach ($this->headers ?? [] as $key => $value) {
+            if (strtolower($key) === $name) {
+                return $value;
+            }
+        }
+
+        return null;
+    }
+
+    /** @return array<string, string> */
+    protected function resolveHeaders(): array
+    {
+        if (function_exists('getallheaders')) {
+            $headers = getallheaders();
+
+            if (! empty($headers)) {
+                return array_map(fn ($value) => (string) $value, $headers);
+            }
+        }
+
+        $headers = [];
+
+        foreach ($this->server ?? [] as $key => $value) {
+            if (! is_string($key)) {
+                continue;
+            }
+
+            if (str_starts_with($key, 'HTTP_')) {
+                $headerName = strtolower(str_replace('_', '-', substr($key, 5)));
+                $headers[$headerName] = (string) $value;
+
+                continue;
+            }
+
+            if (in_array($key, ['CONTENT_TYPE', 'CONTENT_LENGTH'], true)) {
+                $headerName = strtolower(str_replace('_', '-', $key));
+                $headers[$headerName] = (string) $value;
+            }
+        }
+
+        return $headers;
+    }


I feel like a lot of this was solved in each framework's request class (implementing PSR HTTP message interfaces). Is there a reason we don't piggyback on PSR interface and leave this logic up to the framework?

I just noticed the SymfonyRequestAttributesProvider. Is this PhpRequestAttributesProvider just to make implementation on pure vanilla PHP projects easier?

Yeah, I technically don't want to rely on Symfony for this kind of stuff, though most people will and then the SymfonyRequestAttributesProvider is the way to go. If people want something vanilla PHP this would be the bare minimum.

AlexVanderbist · 2026-05-04T15:12:54Z

+use Spatie\FlareClient\Contracts\EntryPointHandlerProvider;
+use Spatie\FlareClient\Contracts\RouteAttributesProvider;
+
+class PhpRouteAttributesProvider implements RouteAttributesProvider, EntryPointHandlerProvider


I think this class might actually benefit from a doc block documenting the consturctor parameters. Is $route the route name? $handler the controller class? $method the HTTP method or the method in the handler class?

Yeah I'll link to the protocol docs which describe all of these and how they should be used

AlexVanderbist · 2026-05-04T15:19:16Z

+            'server.address' => empty($this->request->server->get('SERVER_NAME'))
+                ? $this->request->server->get('SERVER_ADDR')
+                : $this->request->server->get('SERVER_NAME'),


Isn't there a difference in OTEL attributes for server hostname vs server IP?

Yeah that would be 'host.name' but this one is collected in the resource

AlexVanderbist · 2026-05-04T15:22:21Z

+    public bool $handlerResolved = false;
+
+    public string $handlerIdentifier;
+
+    public ?string $handlerName;
+
+    public ?string $handlerType;


Again, doc blocks would be helpful to quickly get a grasp of what we're dealing with here

Only filter null values from the request attribute payload so legitimately empty strings (e.g. empty url.query) are kept.

Adds unit tests for EntryPoint, EntryPointResolver, the four previously untested attribute providers, the Response, Filesystem, RedisCommand, View, and Context recorders, the AddEntryPoint, AddRequestInformation, and AddConsoleInformation middleware, and the Redactor and StacktraceMapper support utilities. Extends sampling, tracer, lifecycle, sender, and truncation tests with edge cases that were not covered. Splits SamplingRule URL handling: forUrl now matches against the full entry point value while a new forPath factory handles path-only matching. Test fixtures and the SamplingRuleType::Path enum case are updated accordingly. Adds a single end-to-end integration file covering a sampled web request, a console command, and a queued job in subtask mode, each asserting trace structure, log entries, and entry point attribute propagation through reports, traces, and logs.

# Conflicts: # src/FlareConfig.php # src/FlareMiddleware/AddSolutions.php # src/FlareProvider.php

rubenvanassche and others added 18 commits April 28, 2026 11:51

Add dynamic sampling, pattern matching, and job middleware

20fa161

Introduce DynamicSampler with SamplingRule support for fine-grained sampling control, PatternMatcher utility for matching rules, and AddJobInformation middleware. Expand recorder tests for command, job, and queue recorders.

Fix styling

7e97dab

Remove spec

bac7f92

Introduce CommandAttributesProvider and command-specific recordStart …

a40b8fb

…variants

Make GitAttributesProvider implement AttributesProvider with static c…

98e2c95

…ache

Replace recorder/middleware register hooks with container autowiring

de3cfd6

Drop Container::make in favour of singleton parameter overrides

5461716

Introduce JobAttributesProvider and provider-based job/queue recordStart

8b1965c

AlexVanderbist reviewed May 4, 2026

View reviewed changes

rubenvanassche and others added 7 commits May 5, 2026 10:24

Preserve empty request attribute strings in PHP request payload

3000c4d

Only filter null values from the request attribute payload so legitimately empty strings (e.g. empty url.query) are kept.

Document enabling standalone logging with the daemon in v3 upgrade guide

ca1bbb6

Fix styling

3d4f1a9

Merge remote-tracking branch 'origin/main' into sampling-and-entry-point

70d533a

# Conflicts: # src/FlareConfig.php # src/FlareMiddleware/AddSolutions.php # src/FlareProvider.php

Use Closure instanceof check instead of is_callable for closure params

59cde79

Fix PHPStan null narrowing in span recorders

0678ce4

rubenvanassche merged commit 8f83161 into main May 5, 2026
17 of 19 checks passed

Uh oh!

Uh oh!

Conversation

rubenvanassche commented Apr 29, 2026

Uh oh!

AlexVanderbist left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rubenvanassche May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rubenvanassche May 5, 2026 •

edited

Loading