Skip to content

feat(scope): remove Hub from Transaction#2122

Open
Litarnus wants to merge 1 commit into
use-transaction-samplerfrom
remove-hub-from-transaction
Open

feat(scope): remove Hub from Transaction#2122
Litarnus wants to merge 1 commit into
use-transaction-samplerfrom
remove-hub-from-transaction

Conversation

@Litarnus

Copy link
Copy Markdown
Contributor

This PR removes the Hub from the transaction and instead replaces it with ClientInterface in some instances.

@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 db6e223. Configure here.

Comment thread src/Tracing/Transaction.php
}

$samplingContext = DynamicSamplingContext::fromTransaction($this->transaction, $this->hub);
$samplingContext = DynamicSamplingContext::fromTransaction($this->transaction, SentrySdk::getClient());

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: getDynamicSamplingContext now uses SentrySdk::getClient(), which may return NoOpClient if a custom hub is used without updating the global client, breaking trace propagation.
Severity: HIGH

Suggested Fix

To build the Dynamic Sampling Context, use the client from the current hub instead of the one from SentrySdk::getClient(). This ensures the correct client configuration is used, especially in multi-hub scenarios where the global client may not be the one associated with the current transaction.

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/Tracing/Transaction.php#L93

Potential issue: The function `getDynamicSamplingContext()` was changed to use
`SentrySdk::getClient()` instead of the hub's client. `SentrySdk::getClient()` resolves
the client from the isolation or global scope, not the current hub. In scenarios where a
custom hub is set via `SentrySdk::setCurrentHub()` without also updating the global
client (e.g., in multi-tenant apps), `SentrySdk::getClient()` will return a
`NoOpClient`. This results in an incomplete Dynamic Sampling Context (DSC) missing
`public_key`, `release`, and `environment`, which breaks trace propagation between
services.

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

@Litarnus Litarnus force-pushed the use-transaction-sampler branch from 5c69016 to 2e630c8 Compare June 22, 2026 22:12
@Litarnus Litarnus force-pushed the remove-hub-from-transaction branch from db6e223 to 14d6804 Compare June 22, 2026 22:12
@Litarnus Litarnus force-pushed the use-transaction-sampler branch from 2e630c8 to 012018d Compare June 22, 2026 22:46
@Litarnus Litarnus force-pushed the remove-hub-from-transaction branch from 14d6804 to 0352e56 Compare June 22, 2026 22:46
@Litarnus Litarnus force-pushed the use-transaction-sampler branch from 012018d to 3b8b293 Compare June 22, 2026 22:54
@Litarnus Litarnus force-pushed the remove-hub-from-transaction branch from 0352e56 to 3a5d496 Compare June 22, 2026 22:54
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