Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/sentry/publish/sentry.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@
// @see: https://docs.sentry.io/platforms/php/guides/laravel/configuration/options/#enable_metrics
'enable_metrics' => env('SENTRY_ENABLE_METRICS', false),

// Metrics are buffered in memory and are no longer flushed on every request
// or annotation call. They are reported when:
// - the SDK `metric_flush_threshold` is reached (automatic flush), or
// - the request runtime context ends (endContext flush), or
// - the periodic flush kicks in as a fallback (`metrics_interval`).
Comment on lines +59 to +63

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Synchronize the new metrics lifecycle documentation

This new buffering and reporting behavior is documented only in the publishable configuration; neither component README nor any of the four localized Sentry pages was updated. The repository specifically requires all four Sentry pages and both READMEs to remain semantically synchronized for metrics changes, so mirror the verified lifecycle guidance across those sources.

AGENTS.md reference: AGENTS.md:L129-L130

Useful? React with 👍 / 👎.

// Forcing a flush per request/call amplifies transport channel pressure and
// increases the risk of memory exhaustion under high traffic.

// @see: https://docs.sentry.io/platforms/php/configuration/options/#before_send_metric
// 'before_send_metric' => function (Sentry\Metrics\Types\Metric $metric): ?Sentry\Metrics\Types\Metric {
// return $metric;
Expand Down
2 changes: 0 additions & 2 deletions src/sentry/src/Metrics/Aspect/CounterAspect.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint): mixed
'class' => $proceedingJoinPoint->className,
'method' => $proceedingJoinPoint->methodName,
]);
Comment on lines 49 to 51

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve flushing for short-lived annotated commands

When #[Counter] runs in a top-level, short-lived command with the published defaults (enable_command_metrics and enable_default_metrics are false), OnBeforeHandle::process does not start the periodic metrics timer and the command itself never enters the coroutine startContext/endContext lifecycle. If the SDK threshold is not reached before exit, this change leaves the counter buffered and silently loses it; #[Histogram] has the same problem. Add a command/shutdown flush or an unconditional periodic fallback before removing these per-annotation flushes.

AGENTS.md reference: AGENTS.md:L122-L125

Useful? React with 👍 / 👎.


metrics()->flush();
}

return $proceedingJoinPoint->process();
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/src/Metrics/Aspect/HistogramAspect.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint): mixed
]);

return tap($proceedingJoinPoint->process(), function () use ($timer) {
defer(fn () => $timer->end(true));
defer(fn () => $timer->end());
});
}

Expand Down
2 changes: 1 addition & 1 deletion src/sentry/src/Metrics/Listener/RequestWatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public function process(object $event): void
++$this->stats->response_count;
--$this->stats->connection_num;

$timer->end(true);
$timer->end();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep a flush path for request-only metrics

When metrics are enabled but enable_default_metrics, enable_pool_metrics, and enable_queue_metrics are all false, no listener starts the metrics_interval timer. For a top-level HTTP/RPC request that was not created through the intercepted Hyperf\Coroutine\Coroutine::create, RequestWatcher also has no active runtime context whose endContext() can flush this timer, so low-volume request metrics remain in the global aggregator and can be lost when the worker exits before metric_flush_threshold is reached. Retain a flush for this case or start an unconditional periodic flusher whenever metrics are enabled.

Useful? React with 👍 / 👎.


unset($timer);
});
Expand Down
75 changes: 75 additions & 0 deletions tests/Sentry/Metrics/RequestWatcherTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?php

declare(strict_types=1);
/**
* This file is part of friendsofhyperf/components.
*
* @link https://github.com/friendsofhyperf/components
* @document https://github.com/friendsofhyperf/components/blob/main/README.md
* @contact huangdijia@gmail.com
*/

namespace FriendsOfHyperf\Tests\Sentry\Metrics;

use FriendsOfHyperf\Sentry\Feature;
use FriendsOfHyperf\Sentry\Metrics\CoroutineServerStats;
use FriendsOfHyperf\Sentry\Metrics\Listener\RequestWatcher;
use Hyperf\HttpMessage\Server\Request;
use Hyperf\HttpServer\Event\RequestReceived;
use Mockery as m;

function waitForCoroutine(int $cid): void
{
if (\Swoole\Coroutine::getCid() === -1) {
return; // Top level: the created coroutine already finished synchronously.
}

while (\Swoole\Coroutine::exists($cid)) {
\Swoole\Coroutine::sleep(0.001);
}
}

test('process request received increments counters without throwing', function () {
$stats = new CoroutineServerStats();
$feature = m::mock(Feature::class);
$feature->shouldReceive('isMetricsEnabled')->andReturn(true);

$watcher = new RequestWatcher($stats, $feature);
$request = new Request('GET', 'http://127.0.0.1:9501/health');

$snapshot = null;
$cid = \Swoole\Coroutine::create(function () use ($watcher, $request, $stats, &$snapshot) {
$watcher->process(new RequestReceived($request, null));
$snapshot = [
'accept_count' => $stats->accept_count,
'request_count' => $stats->request_count,
'connection_num' => $stats->connection_num,
];
});

waitForCoroutine($cid);

expect(\Swoole\Coroutine::exists($cid))->toBeFalse()
->and($snapshot['accept_count'])->toBe(1)
->and($snapshot['request_count'])->toBe(1)
->and($snapshot['connection_num'])->toBe(1);
});

test('defer closes the request counters after the coroutine ends', function () {
$stats = new CoroutineServerStats();
$feature = m::mock(Feature::class);
$feature->shouldReceive('isMetricsEnabled')->andReturn(true);

$watcher = new RequestWatcher($stats, $feature);
$request = new Request('GET', 'http://127.0.0.1:9501/health');

$cid = \Swoole\Coroutine::create(function () use ($watcher, $request) {
$watcher->process(new RequestReceived($request, null));
});

waitForCoroutine($cid);

expect($stats->close_count)->toBe(1)
->and($stats->response_count)->toBe(1)
->and($stats->connection_num)->toBe(0);
});
Loading