Skip to content

Implement strict_trace_propagation #1838

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.

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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 27 additions & 2 deletions src/Tracing/PropagationContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,33 @@
{
$context = self::fromDefaults();
$hasSentryTrace = false;
$samplingContext = DynamicSamplingContext::fromHeader($baggage);

$client = SentrySdk::getCurrentHub()->getClient();
if ($client !== null) {
$options = $client->getOptions();
$orgId = $options->getOrgId() ?? $options->getDsn()->getOrgId();

Check failure on line 196 in src/Tracing/PropagationContext.php

View workflow job for this annotation

GitHub Actions / PHPStan

Cannot call method getOrgId() on Sentry\Dsn|null.

// Always check if the received org ID in the baggages matches
// the one configured in the SDK.
if (
$orgId !== null
&& $samplingContext->has('org_id')
&& $samplingContext->get('org_id') !== (string) $orgId
) {
// We do not continue the trace.
return $context;
}

if ($client->getOptions()->isStrictTracePropagationEnabled()) {
if (!$samplingContext->has('org_id')) {
// We did not receive any org id in the baggage,
// do not continue the trace. The none matching org ID
// case was already handled above.
return $context;
}
}
}

if (preg_match(self::SENTRY_TRACEPARENT_HEADER_REGEX, $traceparent, $matches)) {
if (!empty($matches['trace_id'])) {
Expand All @@ -206,8 +233,6 @@
}
}

$samplingContext = DynamicSamplingContext::fromHeader($baggage);

if ($hasSentryTrace && !$samplingContext->hasEntries()) {
// The request comes from an old SDK which does not support Dynamic Sampling.
// Propagate the Dynamic Sampling Context as is, but frozen, even without sentry-* entries.
Expand Down
31 changes: 29 additions & 2 deletions src/Tracing/TransactionContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

namespace Sentry\Tracing;

use Sentry\SentrySdk;

final class TransactionContext extends SpanContext
{
private const SENTRY_TRACEPARENT_HEADER_REGEX = '/^[ \\t]*(?<trace_id>[0-9a-f]{32})?-?(?<span_id>[0-9a-f]{16})?-?(?<sampled>[01])?[ \\t]*$/i';
Expand Down Expand Up @@ -148,6 +150,33 @@
{
$context = new self();
$hasSentryTrace = false;
$samplingContext = DynamicSamplingContext::fromHeader($baggage);

$client = SentrySdk::getCurrentHub()->getClient();
if ($client !== null) {
$options = $client->getOptions();
$orgId = $options->getOrgId() ?? $options->getDsn()->getOrgId();

Check failure on line 158 in src/Tracing/TransactionContext.php

View workflow job for this annotation

GitHub Actions / PHPStan

Cannot call method getOrgId() on Sentry\Dsn|null.

// Always check if the received org ID in the baggages matches
// the one configured in the SDK.
if (
$orgId !== null
&& $samplingContext->has('org_id')
&& $samplingContext->get('org_id') !== (string) $orgId
) {
// We do not continue the trace.
return $context;
}

if ($client->getOptions()->isStrictTracePropagationEnabled()) {
if (!$samplingContext->has('org_id')) {
// We did not receive any org id in the baggage,
// do not continue the trace. The none matching org ID
// case was already handled above.
return $context;
}
}
}

if (preg_match(self::SENTRY_TRACEPARENT_HEADER_REGEX, $sentryTrace, $matches)) {
if (!empty($matches['trace_id'])) {
Expand All @@ -166,8 +195,6 @@
}
}

$samplingContext = DynamicSamplingContext::fromHeader($baggage);

if ($hasSentryTrace && !$samplingContext->hasEntries()) {
// The request comes from an old SDK which does not support Dynamic Sampling.
// Propagate the Dynamic Sampling Context as is, but frozen, even without sentry-* entries.
Expand Down
43 changes: 41 additions & 2 deletions tests/FunctionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -514,13 +514,20 @@ public function testBaggageWithTracingEnabled(): void

public function testContinueTrace(): void
{
$hub = new Hub();
$client = $this->createMock(ClientInterface::class);
$client->expects($this->atLeastOnce())
->method('getOptions')
->willReturn(new Options([
'org_id' => 1,
]));

$hub = new Hub($client);

SentrySdk::setCurrentHub($hub);

$transactionContext = continueTrace(
'566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8-1',
'sentry-trace_id=566e3688a61d4bc888951642d6f14a19'
'sentry-trace_id=566e3688a61d4bc888951642d6f14a19,sentry-org_id=1'
);

$this->assertSame('566e3688a61d4bc888951642d6f14a19', (string) $transactionContext->getTraceId());
Expand All @@ -539,4 +546,36 @@ public function testContinueTrace(): void
$this->assertTrue($dynamicSamplingContext->isFrozen());
});
}

public function testContinueTraceWithNoneMatchingOrgId(): void
{
$client = $this->createMock(ClientInterface::class);
$client->expects($this->atLeastOnce())
->method('getOptions')
->willReturn(new Options([
'org_id' => 1,
]));

$hub = new Hub($client);

SentrySdk::setCurrentHub($hub);

$transactionContext = continueTrace(
'566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8-1',
'sentry-trace_id=566e3688a61d4bc888951642d6f14a19,sentry-org_id=2'
);

$this->assertNull($transactionContext->getTraceId());
$this->assertNull($transactionContext->getParentSpanId());
$this->assertNull($transactionContext->getParentSampled());

configureScope(function (Scope $scope): void {
$propagationContext = $scope->getPropagationContext();

$this->assertNotSame('566e3688a61d4bc888951642d6f14a19', (string) $propagationContext->getTraceId());
$this->assertNotSame('566e3688a61d4bc8', (string) $propagationContext->getParentSpanId());

$this->assertNull($propagationContext->getDynamicSamplingContext());
});
}
}
Loading