From 0010eaec0242170bb4604f02c9a418bf0f156c05 Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Thu, 14 Jul 2022 23:14:53 +0200 Subject: [PATCH] Fix `Scope::getTransaction()` so that it returns also unsampled transactions --- CHANGELOG.md | 2 ++ src/State/Scope.php | 8 ++----- src/Tracing/Span.php | 28 ++++++++++++++++------ src/Tracing/Transaction.php | 1 + tests/State/HubTest.php | 47 +++++++++++++++++++++++++++++++++++++ 5 files changed, 73 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6e57b1f6..e065bc66e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Fix `Scope::getTransaction()` so that it returns also unsampled transactions (#1334) + ## 3.6.1 (2022-06-27) - Set the `sentry-trace` header when using the tracing middleware (#1331) diff --git a/src/State/Scope.php b/src/State/Scope.php index 78cd452a4..a708f8fdc 100644 --- a/src/State/Scope.php +++ b/src/State/Scope.php @@ -408,12 +408,8 @@ public function setSpan(?Span $span): self */ public function getTransaction(): ?Transaction { - $span = $this->span; - - if (null !== $span && null !== $span->getSpanRecorder() && !empty($span->getSpanRecorder()->getSpans())) { - // The first span in the recorder is considered to be a Transaction - /** @var Transaction */ - return $span->getSpanRecorder()->getSpans()[0]; + if (null !== $this->span) { + return $this->span->getTransaction(); } return null; diff --git a/src/Tracing/Span.php b/src/Tracing/Span.php index a198361c2..2095596df 100644 --- a/src/Tracing/Span.php +++ b/src/Tracing/Span.php @@ -7,7 +7,7 @@ use Sentry\EventId; /** - * This class stores all the information about a Span. + * This class stores all the information about a span. */ class Span { @@ -22,22 +22,22 @@ class Span protected $traceId; /** - * @var string|null Description of the Span + * @var string|null Description of the span */ protected $description; /** - * @var string|null Operation of the Span + * @var string|null Operation of the span */ protected $op; /** - * @var SpanStatus|null Completion status of the Span + * @var SpanStatus|null Completion status of the span */ protected $status; /** - * @var SpanId|null ID of the parent Span + * @var SpanId|null ID of the parent span */ protected $parentSpanId; @@ -47,7 +47,7 @@ class Span protected $sampled; /** - * @var array A List of tags associated to this Span + * @var array A List of tags associated to this span */ protected $tags = []; @@ -67,10 +67,15 @@ class Span protected $endTimestamp; /** - * @var SpanRecorder|null Reference instance to the SpanRecorder + * @var SpanRecorder|null Reference instance to the {@see SpanRecorder} */ protected $spanRecorder; + /** + * @var Transaction|null The transaction containing this span + */ + protected $transaction; + /** * Constructor. * @@ -390,6 +395,7 @@ public function startChild(SpanContext $context): self $context->setTraceId($this->traceId); $span = new self($context); + $span->transaction = $this->transaction; $span->spanRecorder = $this->spanRecorder; if (null != $span->spanRecorder) { @@ -417,6 +423,14 @@ public function detachSpanRecorder(): void $this->spanRecorder = null; } + /** + * Returns the transaction containing this span. + */ + public function getTransaction(): ?Transaction + { + return $this->transaction; + } + /** * Returns a string that can be used for the `sentry-trace` header. */ diff --git a/src/Tracing/Transaction.php b/src/Tracing/Transaction.php index 222361f19..d3cfb0013 100644 --- a/src/Tracing/Transaction.php +++ b/src/Tracing/Transaction.php @@ -38,6 +38,7 @@ public function __construct(TransactionContext $context, ?HubInterface $hub = nu $this->hub = $hub ?? SentrySdk::getCurrentHub(); $this->name = $context->getName(); + $this->transaction = $this; } /** diff --git a/tests/State/HubTest.php b/tests/State/HubTest.php index 85b4d58fc..5a47f5755 100644 --- a/tests/State/HubTest.php +++ b/tests/State/HubTest.php @@ -731,4 +731,51 @@ public function testStartTransactionWithCustomSamplingContext(): void $hub = new Hub($client); $hub->startTransaction(new TransactionContext(), $customSamplingContext); } + + public function testGetTransactionReturnsInstanceSetOnTheScopeIfTransactionIsNotSampled(): void + { + $client = $this->createMock(ClientInterface::class); + $client->expects($this->once()) + ->method('getOptions') + ->willReturn(new Options(['traces_sample_rate' => 1])); + + $hub = new Hub($client); + $transaction = $hub->startTransaction(new TransactionContext(TransactionContext::DEFAULT_NAME, false)); + + $hub->configureScope(static function (Scope $scope) use ($transaction): void { + $scope->setSpan($transaction); + }); + + $this->assertSame($transaction, $hub->getTransaction()); + } + + public function testGetTransactionReturnsInstanceSetOnTheScopeIfTransactionIsSampled(): void + { + $client = $this->createMock(ClientInterface::class); + $client->expects($this->once()) + ->method('getOptions') + ->willReturn(new Options(['traces_sample_rate' => 1])); + + $hub = new Hub($client); + $transaction = $hub->startTransaction(new TransactionContext(TransactionContext::DEFAULT_NAME, true)); + + $hub->configureScope(static function (Scope $scope) use ($transaction): void { + $scope->setSpan($transaction); + }); + + $this->assertSame($transaction, $hub->getTransaction()); + } + + public function testGetTransactionReturnsNullIfNoTransactionIsSetOnTheScope(): void + { + $client = $this->createMock(ClientInterface::class); + $client->expects($this->once()) + ->method('getOptions') + ->willReturn(new Options(['traces_sample_rate' => 1])); + + $hub = new Hub($client); + $hub->startTransaction(new TransactionContext(TransactionContext::DEFAULT_NAME, true)); + + $this->assertNull($hub->getTransaction()); + } }