From 2a4a8dd8b734f9f6c339c7bf7bf85a4e937a1eca Mon Sep 17 00:00:00 2001 From: Ruben Van Assche Date: Tue, 19 May 2026 16:57:02 +0200 Subject: [PATCH] Bake pause/resume into SpansRecorder with a depth counter The PausableRecorder trait could only handle a single ignored event with nothing nested under it. Anything trickier (a normal recordStart/recordEnd under a pause, or a second ignored event) silently desynced the resume. Pause/resume is now a feature of SpansRecorder itself. Static $pauseOwner and $pauseDepth track ownership and call depth across the recorder hierarchy. startSpan and endSpan carry the counter math: increments while owner-paused do not produce real spans, and resume fires the moment depth returns to zero. Lifecycle::flush() clears the static state at every subtask/request boundary so long-running runtimes cannot leak pause state across executions. --- src/Concerns/Recorders/PausableRecorder.php | 70 ------- .../CommandRecorder/CommandRecorder.php | 9 - src/Recorders/JobRecorder/JobRecorder.php | 15 -- src/Recorders/QueueRecorder/QueueRecorder.php | 9 - src/Recorders/SpansRecorder.php | 65 ++++++- src/Support/Lifecycle.php | 3 + tests/Pest.php | 2 + tests/Recorders/SpansRecorderTest.php | 177 ++++++++++++++++++ tests/TestClasses/ConcreteSpansRecorder.php | 15 ++ 9 files changed, 258 insertions(+), 107 deletions(-) delete mode 100644 src/Concerns/Recorders/PausableRecorder.php diff --git a/src/Concerns/Recorders/PausableRecorder.php b/src/Concerns/Recorders/PausableRecorder.php deleted file mode 100644 index 99631d60..00000000 --- a/src/Concerns/Recorders/PausableRecorder.php +++ /dev/null @@ -1,70 +0,0 @@ -pausedTrace = true; - - if ($this->tracer->isSamplingPaused()) { - return; - } - - $this->tracer->pauseSampling(); - $this->ourTracePause = true; - } - - protected function resumeTrace(): void - { - $this->pausedTrace = false; - - if (! $this->ourTracePause) { - return; - } - - $this->tracer->resumeSampling(); - $this->ourTracePause = false; - } - - protected function pausedTrace(): bool - { - return $this->pausedTrace; - } -} diff --git a/src/Recorders/CommandRecorder/CommandRecorder.php b/src/Recorders/CommandRecorder/CommandRecorder.php index b4ef52e9..3a32c32d 100644 --- a/src/Recorders/CommandRecorder/CommandRecorder.php +++ b/src/Recorders/CommandRecorder/CommandRecorder.php @@ -4,7 +4,6 @@ use Spatie\FlareClient\AttributesProviders\PhpCommandAttributesProvider; use Spatie\FlareClient\AttributesProviders\SymfonyInputCommandAttributesProvider; -use Spatie\FlareClient\Concerns\Recorders\PausableRecorder; use Spatie\FlareClient\Contracts\CommandAttributesProvider; use Spatie\FlareClient\Contracts\EntryPointHandlerProvider; use Spatie\FlareClient\EntryPoint\EntryPointResolver; @@ -19,8 +18,6 @@ class CommandRecorder extends SpansRecorder { - use PausableRecorder; - /** @var array */ protected array $ignoredCommands = []; @@ -152,12 +149,6 @@ public function recordEnd( int $exitCode = 0, array $attributes = [] ): ?Span { - if ($this->pausedTrace()) { - $this->resumeTrace(); - - return null; - } - return $this->endSpan(additionalAttributes: [ 'process.exit_code' => $exitCode, ...$attributes, diff --git a/src/Recorders/JobRecorder/JobRecorder.php b/src/Recorders/JobRecorder/JobRecorder.php index b89a5364..0aeee289 100644 --- a/src/Recorders/JobRecorder/JobRecorder.php +++ b/src/Recorders/JobRecorder/JobRecorder.php @@ -3,7 +3,6 @@ namespace Spatie\FlareClient\Recorders\JobRecorder; use Spatie\FlareClient\AttributesProviders\PhpJobAttributesProvider; -use Spatie\FlareClient\Concerns\Recorders\PausableRecorder; use Spatie\FlareClient\Contracts\EntryPointHandlerProvider; use Spatie\FlareClient\Contracts\JobAttributesProvider; use Spatie\FlareClient\EntryPoint\EntryPoint; @@ -25,8 +24,6 @@ class JobRecorder extends SpansRecorder { - use PausableRecorder; - /** @var array */ protected array $ignoredClasses = []; @@ -125,12 +122,6 @@ public function recordStartFromJob( public function recordEnd(array $attributes = []): ?Span { - if ($this->pausedTrace()) { - $this->resumeTrace(); - - return null; - } - $span = $this->endSpan( additionalAttributes: $attributes, includeMemoryUsage: true, @@ -147,12 +138,6 @@ public function recordFailed( Throwable $exception, array $attributes = [], ): ?Span { - if ($this->pausedTrace()) { - $this->resumeTrace(); - - return null; - } - $throwableClass = $exception::class; $trackingUuid = $this->tracer->ids->uuid(); diff --git a/src/Recorders/QueueRecorder/QueueRecorder.php b/src/Recorders/QueueRecorder/QueueRecorder.php index d327d996..29dec715 100644 --- a/src/Recorders/QueueRecorder/QueueRecorder.php +++ b/src/Recorders/QueueRecorder/QueueRecorder.php @@ -3,7 +3,6 @@ namespace Spatie\FlareClient\Recorders\QueueRecorder; use Spatie\FlareClient\AttributesProviders\PhpJobAttributesProvider; -use Spatie\FlareClient\Concerns\Recorders\PausableRecorder; use Spatie\FlareClient\Contracts\JobAttributesProvider; use Spatie\FlareClient\Enums\RecorderType; use Spatie\FlareClient\Enums\SpanType; @@ -15,8 +14,6 @@ class QueueRecorder extends SpansRecorder { - use PausableRecorder; - /** @var array */ protected array $ignoredClasses = []; @@ -74,12 +71,6 @@ public function recordStartFromQueuedJob( public function recordEnd(array $attributes = []): ?Span { - if ($this->pausedTrace()) { - $this->resumeTrace(); - - return null; - } - return $this->endSpan(additionalAttributes: $attributes); } diff --git a/src/Recorders/SpansRecorder.php b/src/Recorders/SpansRecorder.php index 43f955f4..a19caf0b 100644 --- a/src/Recorders/SpansRecorder.php +++ b/src/Recorders/SpansRecorder.php @@ -25,6 +25,10 @@ abstract class SpansRecorder extends Recorder implements SpansRecorderContract /** @var array */ protected array $stack = []; + protected static ?SpansRecorder $pauseOwner = null; + + protected static int $pauseDepth = 0; + public function __construct( protected Tracer $tracer, protected BackTracer $backTracer, @@ -69,6 +73,12 @@ final protected function startSpan( throw new InvalidArgumentException('Either $nameAndAttributes must be set, or both $name and $attributes must be set.'); } + if (self::$pauseOwner === $this) { + self::$pauseDepth++; + + return null; + } + $shouldTrace = $this->withTraces && $this->tracer->sampling; $shouldReport = $this->shouldReport(); @@ -120,6 +130,10 @@ final protected function endSpan( ?Closure $spanCallback = null, bool $includeMemoryUsage = false, ): ?Span { + if ($this->consumePauseEnd()) { + return null; + } + $span = array_pop($this->stack); if ($span === null) { @@ -183,10 +197,6 @@ final protected function span( time: $start, ); - if ($span === null) { - return null; - } - $this->endSpan( time: $end, additionalAttributes: $additionalAttributes === null ? [] : $additionalAttributes, @@ -194,6 +204,10 @@ final protected function span( includeMemoryUsage: $includeMemoryUsage, ); + if ($span === null) { + return null; + } + if ($this->withTraces && $this->tracer->sampling === true) { $this->backtraceEntry($span); } @@ -205,4 +219,47 @@ final public function getSpans(): array { return $this->entries; } + + public static function resetPauseState(): void + { + self::$pauseOwner = null; + self::$pauseDepth = 0; + } + + protected function pauseTrace(): void + { + if (self::$pauseOwner === $this) { + self::$pauseDepth++; + + return; + } + + if (self::$pauseOwner !== null) { + return; + } + + self::$pauseOwner = $this; + self::$pauseDepth = 1; + + $this->tracer->pauseSampling(); + } + + private function consumePauseEnd(): bool + { + if (self::$pauseOwner !== $this) { + return false; + } + + self::$pauseDepth--; + + if (self::$pauseDepth > 0) { + return true; + } + + self::$pauseOwner = null; + + $this->tracer->resumeSampling(); + + return true; + } } diff --git a/src/Support/Lifecycle.php b/src/Support/Lifecycle.php index 9ecdc348..be699835 100644 --- a/src/Support/Lifecycle.php +++ b/src/Support/Lifecycle.php @@ -10,6 +10,7 @@ use Spatie\FlareClient\Enums\SpanType; use Spatie\FlareClient\Logger; use Spatie\FlareClient\Memory\Memory; +use Spatie\FlareClient\Recorders\SpansRecorder; use Spatie\FlareClient\Resources\Resource; use Spatie\FlareClient\Spans\Span; use Spatie\FlareClient\Time\Time; @@ -391,6 +392,8 @@ public function flush(): void $this->sentReports->clear(); $this->recorders->reset(); + SpansRecorder::resetPauseState(); + $this->applicationSpan = null; $this->registeringSpan = null; $this->bootingSpan = null; diff --git a/tests/Pest.php b/tests/Pest.php index 44947360..d831def6 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -5,6 +5,7 @@ use Spatie\FlareClient\Flare; use Spatie\FlareClient\FlareConfig; use Spatie\FlareClient\FlareProvider; +use Spatie\FlareClient\Recorders\SpansRecorder; use Spatie\FlareClient\Support\Container; use Spatie\FlareClient\Tests\Shared\FakeApi; use Spatie\FlareClient\Tests\Shared\FakeIds; @@ -21,6 +22,7 @@ FakeIds::reset(); FakeMemory::reset(); FakeSampler::reset(); + SpansRecorder::resetPauseState(); })->in(__DIR__); function makePathsRelative(string $text): string diff --git a/tests/Recorders/SpansRecorderTest.php b/tests/Recorders/SpansRecorderTest.php index 91ab3db8..bc0ab7f6 100644 --- a/tests/Recorders/SpansRecorderTest.php +++ b/tests/Recorders/SpansRecorderTest.php @@ -181,6 +181,183 @@ public function recordMessage(string $message): ?Span ); }); +it('pauses and resumes tracing for a single ignored event', function () { + $flare = setupFlare(alwaysSampleTraces: true); + + $recorder = new ConcreteSpansRecorder($flare->tracer, $flare->backTracer, config: [ + 'with_traces' => true, + ]); + + $flare->tracer->startTrace(); + + $recorder->pause(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(1); + expect(ConcreteSpansRecorder::pauseOwnedBy($recorder))->toBeTrue(); + expect($flare->tracer->isSamplingPaused())->toBeTrue(); + + expect($recorder->popSpan())->toBeNull(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(0); + expect(ConcreteSpansRecorder::pauseOwnedBy(null))->toBeTrue(); + expect($flare->tracer->isSamplingPaused())->toBeFalse(); +}); + +it('tracks depth through a normal span nested under a pause', function () { + $flare = setupFlare(alwaysSampleTraces: true); + + $recorder = new ConcreteSpansRecorder($flare->tracer, $flare->backTracer, config: [ + 'with_traces' => true, + ]); + + $flare->tracer->startTrace(); + + $recorder->pause(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(1); + + expect($recorder->pushSpan('Inner'))->toBeNull(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(2); + + expect($recorder->popSpan())->toBeNull(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(1); + expect($flare->tracer->isSamplingPaused())->toBeTrue(); + + expect($recorder->popSpan())->toBeNull(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(0); + expect($flare->tracer->isSamplingPaused())->toBeFalse(); +}); + +it('tracks depth when pauseTrace is called while already the owner', function () { + $flare = setupFlare(alwaysSampleTraces: true); + + $recorder = new ConcreteSpansRecorder($flare->tracer, $flare->backTracer, config: [ + 'with_traces' => true, + ]); + + $flare->tracer->startTrace(); + + $recorder->pause(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(1); + + $recorder->pause(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(2); + + $recorder->popSpan(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(1); + expect($flare->tracer->isSamplingPaused())->toBeTrue(); + + $recorder->popSpan(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(0); + expect($flare->tracer->isSamplingPaused())->toBeFalse(); +}); + +it('handles the three-span scenario: ignored under normal under ignored', function () { + $flare = setupFlare(alwaysSampleTraces: true); + + $recorder = new ConcreteSpansRecorder($flare->tracer, $flare->backTracer, config: [ + 'with_traces' => true, + ]); + + $flare->tracer->startTrace(); + + // A (ignored) + $recorder->pause(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(1); + + // B (normal, nested) + expect($recorder->pushSpan('B'))->toBeNull(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(2); + + // C (ignored, nested) + $recorder->pause(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(3); + + // end C + $recorder->popSpan(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(2); + expect($flare->tracer->isSamplingPaused())->toBeTrue(); + + // end B + $recorder->popSpan(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(1); + expect($flare->tracer->isSamplingPaused())->toBeTrue(); + + // end A + $recorder->popSpan(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(0); + expect(ConcreteSpansRecorder::pauseOwnedBy(null))->toBeTrue(); + expect($flare->tracer->isSamplingPaused())->toBeFalse(); +}); + +it('treats a pause from a non-owner recorder as a no-op', function () { + $flare = setupFlare(alwaysSampleTraces: true); + + $recorderA = new ConcreteSpansRecorder($flare->tracer, $flare->backTracer, config: [ + 'with_traces' => true, + ]); + + $recorderB = new ConcreteSpansRecorder($flare->tracer, $flare->backTracer, config: [ + 'with_traces' => true, + ]); + + $flare->tracer->startTrace(); + + $recorderA->pause(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(1); + expect(ConcreteSpansRecorder::pauseOwnedBy($recorderA))->toBeTrue(); + + $recorderB->pause(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(1); + expect(ConcreteSpansRecorder::pauseOwnedBy($recorderA))->toBeTrue(); + + expect($recorderB->popSpan())->toBeNull(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(1); + expect(ConcreteSpansRecorder::pauseOwnedBy($recorderA))->toBeTrue(); + + $recorderA->popSpan(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(0); + expect(ConcreteSpansRecorder::pauseOwnedBy(null))->toBeTrue(); + expect($flare->tracer->isSamplingPaused())->toBeFalse(); +}); + +it('clears pause state via resetPauseState', function () { + $flare = setupFlare(alwaysSampleTraces: true); + + $recorder = new ConcreteSpansRecorder($flare->tracer, $flare->backTracer, config: [ + 'with_traces' => true, + ]); + + $flare->tracer->startTrace(); + + $recorder->pause(); + $recorder->pause(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(2); + + ConcreteSpansRecorder::resetPauseState(); + + expect(ConcreteSpansRecorder::currentPauseDepth())->toBe(0); + expect(ConcreteSpansRecorder::pauseOwnedBy(null))->toBeTrue(); +}); + it('will correctly nest spans', function () { $flare = setupFlare(alwaysSampleTraces: true); diff --git a/tests/TestClasses/ConcreteSpansRecorder.php b/tests/TestClasses/ConcreteSpansRecorder.php index 1ac98c93..dc243b2f 100644 --- a/tests/TestClasses/ConcreteSpansRecorder.php +++ b/tests/TestClasses/ConcreteSpansRecorder.php @@ -32,4 +32,19 @@ public function record( duration: $duration, ); } + + public function pause(): void + { + $this->pauseTrace(); + } + + public static function currentPauseDepth(): int + { + return self::$pauseDepth; + } + + public static function pauseOwnedBy(?ConcreteSpansRecorder $recorder): bool + { + return self::$pauseOwner === $recorder; + } }