Skip to content

Commit bad3e98

Browse files
committed
fix(flow-php/telemetry): bound profiler signal store to prevent worker OO
- add opt-in per-signal cap to MemoryExporter (drops oldest) - cap profiler store and reset it per messenger worker cycle
1 parent 48c659e commit bad3e98

7 files changed

Lines changed: 203 additions & 7 deletions

File tree

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Flow\Bridge\Symfony\TelemetryBundle\Profiler;
6+
7+
use Flow\Telemetry\Provider\Memory\MemoryExporter;
8+
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
9+
use Symfony\Component\Messenger\Event\WorkerRunningEvent;
10+
11+
/**
12+
* The profiler store accumulates every signal teed into it and is normally emptied by the data collector
13+
* between HTTP requests. A messenger worker is a single long-lived process with no request cycle, so the
14+
* collector never resets the store and it grows until the process runs out of memory. Resetting it after
15+
* each worker cycle bounds it to a single message, mirroring how Symfony resets data collectors per message.
16+
*
17+
* The event only fires when symfony/messenger is installed; otherwise this subscriber is inert.
18+
*/
19+
final readonly class ProfilerStoreWorkerResetSubscriber implements EventSubscriberInterface
20+
{
21+
private const RESET_PRIORITY = 512;
22+
23+
public function __construct(
24+
private MemoryExporter $store,
25+
) {}
26+
27+
public static function getSubscribedEvents(): array
28+
{
29+
return [
30+
WorkerRunningEvent::class => ['onWorkerRunning', self::RESET_PRIORITY],
31+
];
32+
}
33+
34+
public function onWorkerRunning(WorkerRunningEvent $event): void
35+
{
36+
$this->store->reset();
37+
}
38+
}

src/bridge/symfony/telemetry-bundle/src/Flow/Bridge/Symfony/TelemetryBundle/Resources/config/profiler.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
declare(strict_types=1);
44

55
use Flow\Bridge\Symfony\TelemetryBundle\Profiler\FlowTelemetryDataCollector;
6+
use Flow\Bridge\Symfony\TelemetryBundle\Profiler\ProfilerStoreWorkerResetSubscriber;
67
use Flow\Telemetry\Provider\Memory\MemoryExporter;
78
use Flow\Telemetry\Telemetry;
89
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
@@ -12,7 +13,12 @@
1213
return static function (ContainerConfigurator $container): void {
1314
$services = $container->services();
1415

15-
$services->set('flow.telemetry.profiler.store', MemoryExporter::class)->public();
16+
$services->set('flow.telemetry.profiler.store', MemoryExporter::class)->args([10000])->public();
17+
18+
$services
19+
->set('flow.telemetry.profiler.store_worker_reset_subscriber', ProfilerStoreWorkerResetSubscriber::class)
20+
->args([service('flow.telemetry.profiler.store')])
21+
->tag('kernel.event_subscriber');
1622

1723
$services->set('flow.telemetry.profiler.collector', FlowTelemetryDataCollector::class)->args([
1824
service(Telemetry::class),
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Flow\Bridge\Symfony\TelemetryBundle\Tests\Unit\Profiler;
6+
7+
use Flow\Bridge\Symfony\TelemetryBundle\Profiler\ProfilerStoreWorkerResetSubscriber;
8+
use Flow\Telemetry\Provider\Memory\MemoryExporter;
9+
use Flow\Telemetry\Signal\Signals;
10+
use Flow\Telemetry\Tests\Mother\SpanMother;
11+
use PHPUnit\Framework\Attributes\CoversClass;
12+
use PHPUnit\Framework\TestCase;
13+
use Symfony\Component\Messenger\Event\WorkerRunningEvent;
14+
use Symfony\Component\Messenger\MessageBus;
15+
use Symfony\Component\Messenger\Worker;
16+
17+
use function class_exists;
18+
19+
#[CoversClass(ProfilerStoreWorkerResetSubscriber::class)]
20+
final class ProfilerStoreWorkerResetSubscriberTest extends TestCase
21+
{
22+
protected function setUp(): void
23+
{
24+
if (!class_exists(Worker::class)) {
25+
self::markTestSkipped('symfony/messenger is not installed');
26+
}
27+
}
28+
29+
public function test_it_empties_the_store_after_a_worker_cycle(): void
30+
{
31+
$store = new MemoryExporter();
32+
$store->export(Signals::traces([SpanMother::withName('span')]));
33+
static::assertCount(1, $store->spans());
34+
35+
$subscriber = new ProfilerStoreWorkerResetSubscriber($store);
36+
$subscriber->onWorkerRunning(new WorkerRunningEvent(new Worker([], new MessageBus()), false));
37+
38+
static::assertSame([], $store->spans());
39+
}
40+
41+
public function test_it_subscribes_to_the_worker_running_event_after_the_cycle_flush(): void
42+
{
43+
// WorkerReceiveCycleSubscriber flushes the cycle into the store at priority 1024; resetting must run
44+
// afterwards (priority 512 < 1024) so the just-flushed signals are cleared and never accumulate.
45+
static::assertSame(
46+
[WorkerRunningEvent::class => ['onWorkerRunning', 512]],
47+
ProfilerStoreWorkerResetSubscriber::getSubscribedEvents(),
48+
);
49+
}
50+
}

src/lib/telemetry/src/Flow/Telemetry/DSL/functions.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -341,11 +341,13 @@ function void_exporter(): VoidExporter
341341
*
342342
* Unified exporter that stores logs, metrics, and spans in memory for direct access.
343343
* Useful for testing and inspection without serialization.
344+
*
345+
* @param null|int $maxEntriesPerSignal maximum entries retained per signal type; null keeps everything
344346
*/
345347
#[DocumentationDSL(module: Module::TELEMETRY, type: DSLType::HELPER)]
346-
function memory_exporter(): MemoryExporter
348+
function memory_exporter(?int $maxEntriesPerSignal = null): MemoryExporter
347349
{
348-
return new MemoryExporter();
350+
return new MemoryExporter($maxEntriesPerSignal);
349351
}
350352

351353
/**

src/lib/telemetry/src/Flow/Telemetry/Provider/Memory/MemoryExporter.php

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@
1010
use Flow\Telemetry\Signal\Signals;
1111
use Flow\Telemetry\Signal\SignalType;
1212
use Flow\Telemetry\Tracer\Span;
13+
use InvalidArgumentException;
14+
15+
use function array_slice;
16+
use function count;
17+
use function sprintf;
1318

1419
/**
1520
* Exporter that stores telemetry batches in memory for direct access.
@@ -34,12 +39,28 @@ final class MemoryExporter implements Exporter
3439
*/
3540
private array $spans = [];
3641

42+
/**
43+
* @param null|int $maxEntriesPerSignal maximum number of entries retained per signal type; once exceeded the
44+
* oldest entries are dropped. Null (default) keeps everything, which is
45+
* unbounded and unsafe in long-running processes that never reset().
46+
*/
47+
public function __construct(
48+
private readonly ?int $maxEntriesPerSignal = null,
49+
) {
50+
if ($maxEntriesPerSignal !== null && $maxEntriesPerSignal < 1) {
51+
throw new InvalidArgumentException(sprintf(
52+
'MemoryExporter maxEntriesPerSignal must be a positive integer, got %d',
53+
$maxEntriesPerSignal,
54+
));
55+
}
56+
}
57+
3758
public function export(Signals $signal): bool
3859
{
3960
match ($signal->type) {
40-
SignalType::LOGS => $this->logs = [...$this->logs, ...$signal->allLogs()],
41-
SignalType::METRICS => $this->metrics = [...$this->metrics, ...$signal->allMetrics()],
42-
SignalType::TRACES => $this->spans = [...$this->spans, ...$signal->allSpans()],
61+
SignalType::LOGS => $this->logs = $this->cap([...$this->logs, ...$signal->allLogs()]),
62+
SignalType::METRICS => $this->metrics = $this->cap([...$this->metrics, ...$signal->allMetrics()]),
63+
SignalType::TRACES => $this->spans = $this->cap([...$this->spans, ...$signal->allSpans()]),
4364
};
4465

4566
return true;
@@ -77,4 +98,20 @@ public function spans(): array
7798
{
7899
return $this->spans;
79100
}
101+
102+
/**
103+
* @template T
104+
*
105+
* @param array<T> $entries
106+
*
107+
* @return array<T>
108+
*/
109+
private function cap(array $entries): array
110+
{
111+
if ($this->maxEntriesPerSignal === null || count($entries) <= $this->maxEntriesPerSignal) {
112+
return $entries;
113+
}
114+
115+
return array_slice($entries, -$this->maxEntriesPerSignal);
116+
}
80117
}

src/lib/telemetry/tests/Flow/Telemetry/Tests/Unit/Provider/Memory/MemoryExporterTest.php

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,75 @@
1414
use Flow\Telemetry\Signal\Signals;
1515
use Flow\Telemetry\Tests\Mother\InstrumentationScopeMother;
1616
use Flow\Telemetry\Tests\Mother\LogEntryMother;
17+
use Flow\Telemetry\Tests\Mother\MetricMother;
1718
use Flow\Telemetry\Tests\Mother\ResourceMother;
1819
use Flow\Telemetry\Tests\Mother\SpanMother;
20+
use InvalidArgumentException;
1921
use PHPUnit\Framework\TestCase;
2022

2123
final class MemoryExporterTest extends TestCase
2224
{
25+
public function test_constructor_rejects_non_positive_max_entries(): void
26+
{
27+
$this->expectException(InvalidArgumentException::class);
28+
$this->expectExceptionMessage('MemoryExporter maxEntriesPerSignal must be a positive integer, got 0');
29+
30+
new MemoryExporter(0);
31+
}
32+
33+
public function test_export_caps_logs_to_max_entries_dropping_oldest(): void
34+
{
35+
$exporter = new MemoryExporter(2);
36+
$entry1 = LogEntryMother::create('Log 1', Severity::INFO);
37+
$entry2 = LogEntryMother::create('Log 2', Severity::WARN);
38+
$entry3 = LogEntryMother::create('Log 3', Severity::ERROR);
39+
40+
$exporter->export(Signals::logs([$entry1, $entry2]));
41+
$exporter->export(Signals::logs([$entry3]));
42+
43+
static::assertCount(2, $exporter->logs());
44+
static::assertSame($entry2, $exporter->logs()[0]);
45+
static::assertSame($entry3, $exporter->logs()[1]);
46+
}
47+
48+
public function test_export_caps_metrics_to_max_entries_dropping_oldest(): void
49+
{
50+
$exporter = new MemoryExporter(1);
51+
$first = MetricMother::counter('first', 1);
52+
$second = MetricMother::counter('second', 2);
53+
54+
$exporter->export(Signals::metrics([$first]));
55+
$exporter->export(Signals::metrics([$second]));
56+
57+
static::assertCount(1, $exporter->metrics());
58+
static::assertSame($second, $exporter->metrics()[0]);
59+
}
60+
61+
public function test_export_caps_spans_to_max_entries_dropping_oldest(): void
62+
{
63+
$exporter = new MemoryExporter(2);
64+
$span1 = SpanMother::withName('span-1');
65+
$span2 = SpanMother::withName('span-2');
66+
$span3 = SpanMother::withName('span-3');
67+
68+
$exporter->export(Signals::traces([$span1, $span2, $span3]));
69+
70+
static::assertCount(2, $exporter->spans());
71+
static::assertSame($span2, $exporter->spans()[0]);
72+
static::assertSame($span3, $exporter->spans()[1]);
73+
}
74+
75+
public function test_export_without_cap_keeps_every_entry(): void
76+
{
77+
$exporter = new MemoryExporter();
78+
79+
for ($i = 0; $i < 100; $i++) {
80+
$exporter->export(Signals::traces([SpanMother::withName('span-' . $i)]));
81+
}
82+
83+
static::assertCount(100, $exporter->spans());
84+
}
85+
2386
public function test_export_empty_logs_does_not_modify_state(): void
2487
{
2588
$exporter = new MemoryExporter();

web/landing/resources/dsl.json

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)