Skip to content

Commit

Permalink
Add trace to sentry propagration context instead of tags (#26)
Browse files Browse the repository at this point in the history
  • Loading branch information
frankdekker authored Feb 26, 2024
1 parent db92c7b commit 7854aca
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 20 deletions.
77 changes: 67 additions & 10 deletions src/Sentry/SentryAwareTraceStorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@

use DR\SymfonyTraceBundle\TraceContext;
use DR\SymfonyTraceBundle\TraceStorageInterface;
use InvalidArgumentException;
use Sentry\State\HubInterface;
use Sentry\State\Scope;
use Sentry\Tracing\SpanId;
use Sentry\Tracing\TraceId;

/**
* @internal
Expand Down Expand Up @@ -48,29 +51,83 @@ public function getTrace(): TraceContext
public function setTrace(TraceContext $trace): void
{
$this->storage->setTrace($trace);
$this->hub->configureScope(
static function (Scope $scope) use ($trace) {
self::updateTraceId($scope, $trace->getTraceId());
self::updateTransactionId($scope, $trace->getTransactionId());
}
);
$this->hub->configureScope(static fn(Scope $scope) => self::updatePropagationContext($scope, $trace));
}

private static function updateTraceId(Scope $scope, ?string $traceId): void
{
$sentryTraceId = self::tryCreateTraceId($traceId);
if ($sentryTraceId !== null) {
$scope->getPropagationContext()->setTraceId($sentryTraceId);
} elseif ($traceId !== null) {
$scope->setTag('trace_id', $traceId);
}

if ($traceId === null) {
$scope->removeTag('trace_id');
} else {
$scope->setTag('trace_id', $traceId);
}
}

private static function updateTransactionId(Scope $scope, ?string $transactionId): void
{
$sentryTransactionId = self::tryCreateSpanId($transactionId);
if ($sentryTransactionId !== null) {
$scope->getPropagationContext()->setSpanId($sentryTransactionId);
} elseif ($transactionId !== null) {
$scope->setTag('transaction_id', $transactionId);
}

if ($transactionId === null) {
$scope->removeTag('transaction_id');
} else {
$scope->setTag('transaction_id', $transactionId);
}
}

private static function updatePropagationContext(Scope $scope, TraceContext $traceContext): void
{
// set trace id
self::updateTraceId($scope, $traceContext->getTraceId());

// set transaction id
self::updateTransactionId($scope, $traceContext->getTransactionId());

// set parent transaction id
$parentTransactionId = self::tryCreateSpanId($traceContext->getParentTransactionId());
if ($parentTransactionId !== null) {
$scope->getPropagationContext()->setParentSpanId($parentTransactionId);
} elseif ($traceContext->getParentTransactionId() !== null) {
$scope->setTag('parent_transaction_id', $traceContext->getParentTransactionId());
}

if ($traceContext->getParentTransactionId() === null) {
$scope->removeTag('parent_transaction_id');
}
}

private static function tryCreateSpanId(?string $spanId): ?SpanId
{
if ($spanId === null) {
return null;
}

try {
return new SpanId($spanId);
} catch (InvalidArgumentException) {
// the span id format is not supported by Sentry
return null;
}
}

private static function tryCreateTraceId(?string $traceId): ?TraceId
{
if ($traceId === null) {
return null;
}

try {
return new TraceId($traceId);
} catch (InvalidArgumentException) {
// the trace id format is not supported by Sentry
return null;
}
}
}
40 changes: 30 additions & 10 deletions tests/Unit/Sentry/SentryAwareTraceStorageTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
use ReflectionClass;
use Sentry\State\HubInterface;
use Sentry\State\Scope;
use Sentry\Tracing\SpanId;
use Sentry\Tracing\TraceId;

#[CoversClass(SentryAwareTraceStorage::class)]
class SentryAwareTraceStorageTest extends TestCase
Expand Down Expand Up @@ -78,6 +80,7 @@ public function testSetTraceShouldSetValues(): void
$traceContext = new TraceContext();
$traceContext->setTraceId('trace-id-a');
$traceContext->setTransactionId('transaction-id-b');
$traceContext->setParentTransactionId('parent-transaction-id-c');

$this->traceStorage->expects(self::once())->method('setTrace')->with($traceContext);
$this->hub->expects(self::once())->method('configureScope')
Expand All @@ -86,16 +89,19 @@ public function testSetTraceShouldSetValues(): void
$this->storage->setTrace($traceContext);
self::assertScopeHasTag($scope, 'trace_id', 'trace-id-a');
self::assertScopeHasTag($scope, 'transaction_id', 'transaction-id-b');
self::assertScopeHasTag($scope, 'parent_transaction_id', 'parent-transaction-id-c');
}

public function testSetTraceShouldRemoveValues(): void
{
$scope = new Scope();
$scope->setTag('trace_id', 'trace-id-a');
$scope->setTag('transaction_id', 'transaction-id-b');
$scope->setTag('parent_transaction_id', 'parent-transaction-id-c');
$traceContext = new TraceContext();
$traceContext->setTraceId(null);
$traceContext->setTransactionId(null);
$traceContext->setParentTransactionId(null);

$this->traceStorage->expects(self::once())->method('setTrace')->with($traceContext);
$this->hub->expects(self::once())->method('configureScope')
Expand All @@ -104,27 +110,41 @@ public function testSetTraceShouldRemoveValues(): void
$this->storage->setTrace($traceContext);
self::assertScopeDoesNotHaveTag($scope, 'trace_id');
self::assertScopeDoesNotHaveTag($scope, 'transaction_id');
self::assertScopeDoesNotHaveTag($scope, 'parent_transaction_id');
}

private static function assertScopeHasTag(Scope $scope, string $key, string $value): void
public function testSetTraceShouldSetSentryPropagationContext(): void
{
$class = new ReflectionClass($scope);
$property = $class->getProperty('tags');
$property->setAccessible(true);
$scope = new Scope();
$traceContext = new TraceContext();
$traceContext->setTraceId((string)TraceId::generate());
$traceContext->setTransactionId((string)SpanId::generate());
$traceContext->setParentTransactionId((string)SpanId::generate());

$this->traceStorage->expects(self::once())->method('setTrace')->with($traceContext);
$this->hub->expects(self::once())->method('configureScope')
->willReturnCallback(static fn(callable $callback) => $callback($scope));

$tags = $property->getValue($scope);
$this->storage->setTrace($traceContext);
static::assertSame($traceContext->getTraceId(), (string)$scope->getPropagationContext()->getTraceId());
static::assertSame($traceContext->getTransactionId(), (string)$scope->getPropagationContext()->getSpanId());
static::assertSame($traceContext->getParentTransactionId(), (string)$scope->getPropagationContext()->getParentSpanId());
self::assertScopeDoesNotHaveTag($scope, 'trace_id');
self::assertScopeDoesNotHaveTag($scope, 'transaction_id');
self::assertScopeDoesNotHaveTag($scope, 'parent_transaction_id');
}

private static function assertScopeHasTag(Scope $scope, string $key, string $value): void
{
$tags = (new ReflectionClass($scope))->getProperty('tags')->getValue($scope);
static::assertIsArray($tags);
static::assertArrayHasKey($key, $tags);
static::assertSame($value, $tags[$key]);
}

private static function assertScopeDoesNotHaveTag(Scope $scope, string $key): void
{
$class = new ReflectionClass($scope);
$property = $class->getProperty('tags');
$property->setAccessible(true);

$tags = $property->getValue($scope);
$tags = (new ReflectionClass($scope))->getProperty('tags')->getValue($scope);
static::assertIsArray($tags);
static::assertArrayNotHasKey($key, $tags);
}
Expand Down

0 comments on commit 7854aca

Please sign in to comment.