Skip to content

feat(scope): update aggregator flush to use a client #2125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Litarnus wants to merge 1 commit into from
+94 −17
Open

feat(scope): update aggregator flush to use a client #2125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 9 additions & 6 deletions src/Logs/LogsAggregator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Sentry\Attributes\Attribute;
use Sentry\Client;
use Sentry\ClientInterface;
use Sentry\Event;
use Sentry\EventId;
use Sentry\SentrySdk;
Expand Down Expand Up @@ -159,20 +160,22 @@ public function add(
$logs->push($log);

if ($logFlushThreshold !== null && \count($logs) >= $logFlushThreshold) {
$this->flush($hub);
$this->flush($client);
}
}

public function flush(?HubInterface $hub = null): ?EventId
public function flush(?ClientInterface $client = null): ?EventId
{
if ($this->logs === null || $this->logs->isEmpty()) {
$logs = $this->logs;

if ($logs === null || $logs->isEmpty()) {
return null;
}

$hub = $hub ?? SentrySdk::getCurrentHub();
$event = Event::createLogs()->setLogs($this->logs->drain());
$client = $client ?? SentrySdk::getCurrentHub()->getClient();
$event = Event::createLogs()->setLogs($logs->drain());

return $hub->captureEvent($event);
return $client->captureEvent($event);

@cursor cursor Bot Jun 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Flush skips hub scope

Medium Severity

Aggregator flush now sends batched logs and metrics via Client::captureEvent without a Scope. Previously Hub::captureEvent always passed the hub scope, so scope event processors and scope enrichment no longer run on these flush events.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8d1c6f0. Configure here.

@sentry sentry Bot Jun 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The direct call to client->captureEvent($event) during flush operations bypasses the hub, causing the event to be sent with a null scope and losing all contextual data.
Severity: HIGH

Suggested Fix

Instead of passing the Client to the flush methods in LogsAggregator and MetricsAggregator, pass the Hub instance. Then, inside the flush method, call hub->captureEvent($event) instead of $client->captureEvent($event). This will ensure that the event is processed with the correct scope from the hub before being sent.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/Logs/LogsAggregator.php#L178

Potential issue: The refactoring in `LogsAggregator::flush()` and
`MetricsAggregator::flush()` now calls `$client->captureEvent($event)` directly, instead
of going through the hub. This bypasses the mechanism that attaches the current scope to
the event. As a result, when logs or metrics are flushed, all scope-level context—such
as user data, tags, breadcrumbs, and trace information—is lost. The events are sent
without this critical enrichment, which severely degrades their value for debugging and
analysis. The previous implementation correctly used `$hub->captureEvent()`, which
ensured the scope was applied.

Also affects:

  • src/Metrics/MetricsAggregator.php:143~143

Did we get this right? 👍 / 👎 to inform future reviews.

}

/**
Expand Down
15 changes: 9 additions & 6 deletions src/Metrics/MetricsAggregator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Sentry\Metrics;

use Sentry\Client;
use Sentry\ClientInterface;
use Sentry\Event;
use Sentry\EventId;
use Sentry\Metrics\Types\CounterMetric;
Expand Down Expand Up @@ -124,20 +125,22 @@ public function add(
$metrics->push($metric);

if ($metricFlushThreshold !== null && \count($metrics) >= $metricFlushThreshold) {
$this->flush($hub);
$this->flush($client);
}
}

public function flush(?HubInterface $hub = null): ?EventId
public function flush(?ClientInterface $client = null): ?EventId
{
if ($this->metrics === null || $this->metrics->isEmpty()) {
$metrics = $this->metrics;

if ($metrics === null || $metrics->isEmpty()) {
return null;
}

$hub = $hub ?? SentrySdk::getCurrentHub();
$event = Event::createMetrics()->setMetrics($this->metrics->drain());
$client = $client ?? SentrySdk::getCurrentHub()->getClient();
$event = Event::createMetrics()->setMetrics($metrics->drain());

return $hub->captureEvent($event);
return $client->captureEvent($event);
}

/**
Expand Down
8 changes: 3 additions & 5 deletions src/State/RuntimeContextManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,12 @@ private function removeContextById(string $runtimeContextId, ?int $timeout = nul

private function flushRuntimeContextResources(RuntimeContext $runtimeContext, ?int $timeout, LoggerInterface $logger): void
{
$hub = $runtimeContext->getHub();
$client = $runtimeContext->getHub()->getClient();

// captureEvent can throw before transport send (for example from scope event processors
// or before_send callbacks), so we isolate failures and continue flushing other resources.
try {
$runtimeContext->getLogsAggregator()->flush($hub);
$runtimeContext->getLogsAggregator()->flush($client);
} catch (\Throwable $exception) {
$logger->error('Failed to flush logs while ending a runtime context.', [
'exception' => $exception,
Expand All @@ -176,16 +176,14 @@ private function flushRuntimeContextResources(RuntimeContext $runtimeContext, ?i

// Keep metrics flush independent from logs flush so one bad callback does not block the rest.
try {
$runtimeContext->getMetricsAggregator()->flush($hub);
$runtimeContext->getMetricsAggregator()->flush($client);
} catch (\Throwable $exception) {
$logger->error('Failed to flush trace metrics while ending a runtime context.', [
'exception' => $exception,
'runtime_context_id' => $runtimeContext->getId(),
]);
}

$client = $hub->getClient();

// Custom transports may throw from close(); endContext must stay best-effort and non-fatal.
try {
$client->flush($timeout);
Expand Down
36 changes: 36 additions & 0 deletions tests/Logs/LogsAggregatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@
use PHPUnit\Framework\TestCase;
use Sentry\Client;
use Sentry\ClientBuilder;
use Sentry\ClientInterface;
use Sentry\Event;
use Sentry\Logs\LogLevel;
use Sentry\Logs\LogsAggregator;
use Sentry\Options;
use Sentry\SentrySdk;
use Sentry\State\Hub;
use Sentry\State\Scope;
Expand Down Expand Up @@ -271,6 +274,39 @@ public function testFlushesImmediatelyWhenThresholdIsReached(): void
$this->assertSame('Second message', StubTransport::$events[0]->getLogs()[1]->getBody());
}

public function testFlushCapturesLogsWithProvidedClient(): void
{
$client = $this->createMock(ClientInterface::class);
$client->method('getOptions')
->willReturn(new Options([
'enable_logs' => true,
]));

$fallbackClient = $this->createMock(ClientInterface::class);
$fallbackClient->method('getOptions')
->willReturn(new Options([
'enable_logs' => true,
]));
$fallbackClient->expects($this->never())
->method('captureEvent');
SentrySdk::setCurrentHub(new Hub($fallbackClient));

$aggregator = new LogsAggregator();
$aggregator->add(LogLevel::info(), 'Test message');

$client->expects($this->once())
->method('captureEvent')
->with(
$this->callback(function (Event $event): bool {
$this->assertCount(1, $event->getLogs());

return true;
})
);

$aggregator->flush($client);
}

public function testDoesNotFlushImmediatelyWhenThresholdIsNull(): void
{
StubTransport::$events = [];
Expand Down
37 changes: 37 additions & 0 deletions tests/Metrics/TraceMetricsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,16 @@

use PHPUnit\Framework\TestCase;
use Sentry\Client;
use Sentry\ClientInterface;
use Sentry\Event;
use Sentry\Metrics\MetricsAggregator;
use Sentry\Metrics\Types\CounterMetric;
use Sentry\Metrics\Types\DistributionMetric;
use Sentry\Metrics\Types\GaugeMetric;
use Sentry\Metrics\Types\Metric;
use Sentry\Options;
use Sentry\SentrySdk;
use Sentry\State\Hub;
use Sentry\State\HubAdapter;
use Sentry\State\Scope;

Expand Down Expand Up @@ -110,6 +114,39 @@ public function testDoesNotFlushImmediatelyWhenMetricFlushThresholdIsNull(): voi
$this->assertCount(2, StubTransport::$events[0]->getMetrics());
}

public function testFlushCapturesMetricsWithProvidedClient(): void
{
$client = $this->createMock(ClientInterface::class);
$client->method('getOptions')
->willReturn(new Options([
'enable_metrics' => true,
]));

$fallbackClient = $this->createMock(ClientInterface::class);
$fallbackClient->method('getOptions')
->willReturn(new Options([
'enable_metrics' => true,
]));
$fallbackClient->expects($this->never())
->method('captureEvent');
SentrySdk::setCurrentHub(new Hub($fallbackClient));

$aggregator = new MetricsAggregator();
$aggregator->add(CounterMetric::TYPE, 'test-count', 2, ['foo' => 'bar'], null);

$client->expects($this->once())
->method('captureEvent')
->with(
$this->callback(function (Event $event): bool {
$this->assertCount(1, $event->getMetrics());

return true;
})
);

$aggregator->flush($client);
}

public function testMetricsBufferFullWhenMetricFlushThresholdIsNull(): void
{
HubAdapter::getInstance()->bindClient(new Client(new Options([
Expand Down