diff --git a/src/sentry/src/Metrics/Listener/OnBeforeHandle.php b/src/sentry/src/Metrics/Listener/OnBeforeHandle.php index f75d48f57..34b4e2a4a 100644 --- a/src/sentry/src/Metrics/Listener/OnBeforeHandle.php +++ b/src/sentry/src/Metrics/Listener/OnBeforeHandle.php @@ -31,11 +31,14 @@ class OnBeforeHandle implements ListenerInterface protected Timer $timer; + private bool $ticking = false; + public function __construct( protected ContainerInterface $container, - protected Feature $feature + protected Feature $feature, + ?Timer $timer = null, ) { - $this->timer = new Timer(); + $this->timer = $timer ?? new Timer(); } public function listen(): array @@ -95,6 +98,12 @@ public function process(object $event): void 'ru_stime_tv_sec', ]; + if ($this->ticking) { + return; + } + + $this->ticking = true; + $this->timer->tick( $this->feature->getMetricsInterval(), function () use ($metrics) { diff --git a/src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php b/src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php index e34c294a2..f4a00c3f1 100644 --- a/src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php +++ b/src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php @@ -32,11 +32,14 @@ class OnMetricFactoryReady implements ListenerInterface private Timer $timer; + private bool $ticking = false; + public function __construct( protected ContainerInterface $container, protected Feature $feature, + ?Timer $timer = null, ) { - $this->timer = new Timer(); + $this->timer = $timer ?? new Timer(); } public function listen(): array @@ -81,6 +84,12 @@ public function process(object $event): void 'metric_process_memory_peak_usage', ]; + if ($this->ticking) { + return; + } + + $this->ticking = true; + $serverStatsFactory = null; if (! SentryConstants::$runningInCommand) { diff --git a/src/sentry/src/Metrics/Listener/QueueWatcher.php b/src/sentry/src/Metrics/Listener/QueueWatcher.php index d58d64762..38efb239a 100644 --- a/src/sentry/src/Metrics/Listener/QueueWatcher.php +++ b/src/sentry/src/Metrics/Listener/QueueWatcher.php @@ -25,11 +25,14 @@ class QueueWatcher implements ListenerInterface { private Timer $timer; + private bool $ticking = false; + public function __construct( protected ContainerInterface $container, protected Feature $feature, + ?Timer $timer = null, ) { - $this->timer = new Timer(); + $this->timer = $timer ?? new Timer(); } /** @@ -51,6 +54,12 @@ public function process(object $event): void return; } + if ($this->ticking) { + return; + } + + $this->ticking = true; + $this->timer->tick( $this->feature->getMetricsInterval(), function () { diff --git a/tests/Sentry/Metrics/Listener/FakeTimer.php b/tests/Sentry/Metrics/Listener/FakeTimer.php new file mode 100644 index 000000000..7a6beda28 --- /dev/null +++ b/tests/Sentry/Metrics/Listener/FakeTimer.php @@ -0,0 +1,35 @@ + + */ + public array $ticks = []; + + public function tick(float $timeout, callable $closure, string $identifier = Constants::WORKER_EXIT): int + { + $this->ticks[] = [$timeout, $closure, $identifier]; + + return count($this->ticks); + } +} diff --git a/tests/Sentry/Metrics/Listener/OnBeforeHandleTest.php b/tests/Sentry/Metrics/Listener/OnBeforeHandleTest.php new file mode 100644 index 000000000..6ba3672d6 --- /dev/null +++ b/tests/Sentry/Metrics/Listener/OnBeforeHandleTest.php @@ -0,0 +1,67 @@ +timer = new FakeTimer(); + $this->container = $this->createMock(ContainerInterface::class); + $this->feature = $this->createMock(Feature::class); + $this->application = $this->createMock(Application::class); + $this->command = $this->createMock(Command::class); + $this->command->method('getApplication')->willReturn($this->application); + + $this->event = new BeforeHandle($this->command); +}); + +test('ticks only once when process is called twice', function () { + $this->feature->method('isCommandMetricsEnabled')->willReturn(true); + $this->feature->method('isDefaultMetricsEnabled')->willReturn(true); + $this->feature->method('getMetricsInterval')->willReturn(10); + $this->container->method('has')->willReturn(false); + $this->application->method('isAutoExitEnabled')->willReturn(true); + + $listener = new OnBeforeHandle($this->container, $this->feature, $this->timer); + $listener->process($this->event); + $listener->process($this->event); + + expect($this->timer->ticks)->toHaveCount(1); +}); + +test('does not tick when metrics are disabled', function () { + $this->feature->method('isCommandMetricsEnabled')->willReturn(false); + $this->feature->method('isDefaultMetricsEnabled')->willReturn(false); + $this->application->method('isAutoExitEnabled')->willReturn(true); + + $listener = new OnBeforeHandle($this->container, $this->feature, $this->timer); + $listener->process($this->event); + + expect($this->timer->ticks)->toHaveCount(0); +}); + +test('does not tick when auto exit is disabled', function () { + $this->application->method('isAutoExitEnabled')->willReturn(false); + + $listener = new OnBeforeHandle($this->container, $this->feature, $this->timer); + $listener->process($this->event); + + expect($this->timer->ticks)->toHaveCount(0); +}); diff --git a/tests/Sentry/Metrics/Listener/OnMetricFactoryReadyTest.php b/tests/Sentry/Metrics/Listener/OnMetricFactoryReadyTest.php new file mode 100644 index 000000000..15ba46db7 --- /dev/null +++ b/tests/Sentry/Metrics/Listener/OnMetricFactoryReadyTest.php @@ -0,0 +1,49 @@ +timer = new FakeTimer(); + $this->container = $this->createMock(ContainerInterface::class); + $this->feature = $this->createMock(Feature::class); + + $this->event = new MetricFactoryReady(); +}); + +test('ticks only once when process is called twice', function () { + $this->feature->method('isDefaultMetricsEnabled')->willReturn(true); + $this->feature->method('getMetricsInterval')->willReturn(10); + $this->container->method('has')->willReturn(false); + + $listener = new OnMetricFactoryReady($this->container, $this->feature, $this->timer); + $listener->process($this->event); + $listener->process($this->event); + + expect($this->timer->ticks)->toHaveCount(1); +}); + +test('does not tick when default metrics are disabled', function () { + $this->feature->method('isDefaultMetricsEnabled')->willReturn(false); + + $listener = new OnMetricFactoryReady($this->container, $this->feature, $this->timer); + $listener->process($this->event); + + expect($this->timer->ticks)->toHaveCount(0); +}); diff --git a/tests/Sentry/Metrics/Listener/QueueWatcherTest.php b/tests/Sentry/Metrics/Listener/QueueWatcherTest.php new file mode 100644 index 000000000..41989b0ab --- /dev/null +++ b/tests/Sentry/Metrics/Listener/QueueWatcherTest.php @@ -0,0 +1,45 @@ +timer = new FakeTimer(); + $this->container = $this->createMock(ContainerInterface::class); + $this->feature = $this->createMock(Feature::class); + + $this->event = new MetricFactoryReady(); +}); + +test('ticks only once when process is called twice', function () { + $this->feature->method('isQueueMetricsEnabled')->willReturn(true); + $this->feature->method('getMetricsInterval')->willReturn(10); + + $listener = new QueueWatcher($this->container, $this->feature, $this->timer); + $listener->process($this->event); + $listener->process($this->event); + + expect($this->timer->ticks)->toHaveCount(1); +}); + +test('does not tick when queue metrics are disabled', function () { + $this->feature->method('isQueueMetricsEnabled')->willReturn(false); + + $listener = new QueueWatcher($this->container, $this->feature, $this->timer); + $listener->process($this->event); + + expect($this->timer->ticks)->toHaveCount(0); +});