Skip to content

feat(scope): remove Hub from Transaction #2122

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
Open

feat(scope): remove Hub from Transaction #2122

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
6 changes: 2 additions & 4 deletions src/Tracing/DynamicSamplingContext.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

namespace Sentry\Tracing;

use Sentry\ClientInterface;
use Sentry\Options;
use Sentry\State\HubInterface;
use Sentry\State\Scope;

/**
Expand Down Expand Up @@ -149,7 +149,7 @@ public static function fromHeader(string $header): self
*
* @see https://develop.sentry.dev/sdk/performance/dynamic-sampling-context/#baggage-header
*/
public static function fromTransaction(Transaction $transaction, HubInterface $hub): self
public static function fromTransaction(Transaction $transaction, ClientInterface $client): self
{
$samplingContext = new self();
$samplingContext->set('trace_id', (string) $transaction->getTraceId());
Expand All @@ -164,8 +164,6 @@ public static function fromTransaction(Transaction $transaction, HubInterface $h
$samplingContext->set('transaction', $transaction->getName());
}

$client = $hub->getClient();

self::setOrgOptions($client->getOptions(), $samplingContext);

if ($transaction->getSampled() !== null) {
Expand Down
16 changes: 4 additions & 12 deletions src/Tracing/Transaction.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,12 @@
use Sentry\Options;
use Sentry\Profiling\Profiler;
use Sentry\SentrySdk;
use Sentry\State\HubInterface;

/**
* This class stores all the information about a Transaction.
*/
final class Transaction extends Span
{
/**
* @var HubInterface The hub instance
*/
private $hub;

/**
* @var string Name of the transaction
*/
Expand All @@ -45,15 +39,13 @@ final class Transaction extends Span
* Span constructor.
*
* @param TransactionContext $context The context to create the transaction with
* @param HubInterface|null $hub Instance of a hub to flush the transaction
*
* @internal
*/
public function __construct(TransactionContext $context, ?HubInterface $hub = null)
public function __construct(TransactionContext $context)
{
parent::__construct($context);

$this->hub = $hub ?? SentrySdk::getCurrentHub();

@stayallive stayallive Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does it now make sense that this transaction is able to "switch clients" I feel like the AI comments try to point this out with a lot of words but that seems odd that if the global client changes for whatever reason a transaction will be finished on it where it wasn't started. Client or scope. Maybe it should get the scope instead of the hub and hold a ref to it?

@Litarnus Litarnus Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

True that's a good point, I will update the signature to allow a (isolation) Scope

$this->name = $context->getName();
$this->metadata = $context->getMetadata();
$this->transaction = $this;
Expand Down Expand Up @@ -98,7 +90,7 @@ public function getDynamicSamplingContext(): DynamicSamplingContext
return $this->metadata->getDynamicSamplingContext();
}

$samplingContext = DynamicSamplingContext::fromTransaction($this->transaction, $this->hub);
$samplingContext = DynamicSamplingContext::fromTransaction($this->transaction, SentrySdk::getClient());
Comment thread
cursor[bot] marked this conversation as resolved.

@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: 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.

@cursor cursor Bot Jun 24, 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.

Wrong client for sampling context

Medium Severity

getDynamicSamplingContext() and initProfiler() now resolve options via SentrySdk::getClient(), which reads the isolation/global scope, while finish() still sends through \Sentry\captureEvent() on the current hub’s client. When the real client is only on the hub (e.g. bindClient() or setCurrentHub(new Hub($client)) without updating scope clients), DSC and profiler use NoOpClient options but the transaction event is captured with the hub client—missing or wrong public_key, release, and related baggage fields.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0f25f5f. Configure here.

$this->getMetadata()->setDynamicSamplingContext($samplingContext);

return $samplingContext;
Expand All @@ -123,7 +115,7 @@ public function initSpanRecorder(int $maxSpans = 1000): self
public function initProfiler(?Options $options = null): Profiler
{
if ($this->profiler === null) {
$this->profiler = new Profiler($options ?? $this->hub->getClient()->getOptions());
$this->profiler = new Profiler($options ?? SentrySdk::getClient()->getOptions());
}

return $this->profiler;
Expand Down Expand Up @@ -188,6 +180,6 @@ public function finish(?float $endTimestamp = null): ?EventId
}
}

return $this->hub->captureEvent($event);
return \Sentry\captureEvent($event);
}
}
1 change: 1 addition & 0 deletions tests/FunctionsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,7 @@ public function testBaggageWithTracingEnabled(): void

$hub = new Hub($client);

SentrySdk::getGlobalScope()->setClient($client);
SentrySdk::setCurrentHub($hub);

$transactionContext = new TransactionContext();
Expand Down
23 changes: 8 additions & 15 deletions tests/Tracing/DynamicSamplingContextTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
use Sentry\ClientInterface;
use Sentry\NoOpClient;
use Sentry\Options;
use Sentry\State\Hub;
use Sentry\State\Scope;
use Sentry\Tracing\DynamicSamplingContext;
use Sentry\Tracing\PropagationContext;
Expand Down Expand Up @@ -91,16 +90,14 @@ public function testFromTransaction(): void
'environment' => 'test',
]));

$hub = new Hub($client);

$transactionContext = new TransactionContext();
$transactionContext->setName('foo');

$transaction = new Transaction($transactionContext, $hub);
$transaction = new Transaction($transactionContext);
$transaction->getMetadata()->setSamplingRate(1.0);
$transaction->getMetadata()->setSampleRand(0.5);

$samplingContext = DynamicSamplingContext::fromTransaction($transaction, $hub);
$samplingContext = DynamicSamplingContext::fromTransaction($transaction, $client);

$this->assertSame((string) $transaction->getTraceId(), $samplingContext->get('trace_id'));
$this->assertSame((string) $transaction->getMetaData()->getSamplingRate(), $samplingContext->get('sample_rate'));
Expand All @@ -114,15 +111,13 @@ public function testFromTransaction(): void

public function testFromTransactionSourceUrl(): void
{
$hub = new Hub(new NoOpClient());

$transactionContext = new TransactionContext();
$transactionContext->setName('/foo/bar/123');
$transactionContext->setSource(TransactionSource::url());

$transaction = new Transaction($transactionContext, $hub);
$transaction = new Transaction($transactionContext);

$samplingContext = DynamicSamplingContext::fromTransaction($transaction, $hub);
$samplingContext = DynamicSamplingContext::fromTransaction($transaction, new NoOpClient());

$this->assertNull($samplingContext->get('transaction'));
}
Expand Down Expand Up @@ -189,9 +184,8 @@ public function testFromTransactionUsesConfiguredOrgIdOverDsnOrgId(): void
'org_id' => 2,
]));

$hub = new Hub($client);
$transaction = new Transaction(new TransactionContext(), $hub);
$samplingContext = DynamicSamplingContext::fromTransaction($transaction, $hub);
$transaction = new Transaction(new TransactionContext());
$samplingContext = DynamicSamplingContext::fromTransaction($transaction, $client);

$this->assertSame('2', $samplingContext->get('org_id'));
}
Expand All @@ -205,9 +199,8 @@ public function testFromTransactionFallsBackToDsnOrgId(): void
'dsn' => 'http://public@o1.example.com/1',
]));

$hub = new Hub($client);
$transaction = new Transaction(new TransactionContext(), $hub);
$samplingContext = DynamicSamplingContext::fromTransaction($transaction, $hub);
$transaction = new Transaction(new TransactionContext());
$samplingContext = DynamicSamplingContext::fromTransaction($transaction, $client);

$this->assertSame('1', $samplingContext->get('org_id'));
}
Expand Down
22 changes: 11 additions & 11 deletions tests/Tracing/TransactionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@
use Sentry\EventId;
use Sentry\EventType;
use Sentry\Options;
use Sentry\SentrySdk;
use Sentry\State\Hub;
use Sentry\State\HubInterface;
use Sentry\State\Scope;
use Sentry\Tests\TestUtil\ClockMock;
use Sentry\Tracing\SpanContext;
use Sentry\Tracing\Transaction;
Expand All @@ -37,19 +38,16 @@ public function testFinish(): void
->method('getOptions')
->willReturn(new Options());

$hub = $this->createMock(HubInterface::class);
$hub->expects($this->once())
->method('getClient')
->willReturn($client);
SentrySdk::init($client);

$transaction = new Transaction($transactionContext, $hub);
$transaction = new Transaction($transactionContext);
$transaction->initSpanRecorder();

$span1 = $transaction->startChild(new SpanContext());
$span2 = $transaction->startChild(new SpanContext());
$span3 = $transaction->startChild(new SpanContext()); // This span isn't finished, so it should not be included in the event

$hub->expects($this->once())
$client->expects($this->once())
->method('captureEvent')
->with($this->callback(function (Event $eventArg) use ($transactionContext, $span1, $span2): bool {
$this->assertSame(EventType::transaction(), $eventArg->getType());
Expand All @@ -60,7 +58,7 @@ public function testFinish(): void
$this->assertSame([$span1, $span2], $eventArg->getSpans());

return true;
}))
}), null, $this->isInstanceOf(Scope::class))
->willReturnCallback(static function (Event $eventArg) use (&$expectedEventId): EventId {
$expectedEventId = $eventArg->getId();

Expand All @@ -77,11 +75,13 @@ public function testFinish(): void

public function testFinishDoesNothingIfSampledFlagIsNotTrue(): void
{
$hub = $this->createMock(HubInterface::class);
$hub->expects($this->never())
$client = $this->createMock(ClientInterface::class);
$client->expects($this->never())
->method('captureEvent');

$transaction = new Transaction(new TransactionContext(), $hub);
SentrySdk::init($client);

$transaction = new Transaction(new TransactionContext());
$transaction->finish();
}

Expand Down
Loading