Skip to content

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

Open
Litarnus wants to merge 1 commit into
remove-hub-from-transactionfrom
aggregator-flush-signatures
Open

feat(scope): update aggregator flush to use a client#2125
Litarnus wants to merge 1 commit into
remove-hub-from-transactionfrom
aggregator-flush-signatures

Conversation

@Litarnus

Copy link
Copy Markdown
Contributor

This PR removes the Hub from the flush signature and replaces it with the Client

@Litarnus Litarnus marked this pull request as ready for review June 22, 2026 14:38

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

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

$event = Event::createLogs()->setLogs($logs->drain());

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

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.

$event = Event::createLogs()->setLogs($logs->drain());

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant