Skip to content
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

fix: Symfony Messenger propagation to non-instrumented clients #2956

Merged
merged 6 commits into from
Nov 15, 2024
Merged
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
20 changes: 0 additions & 20 deletions src/DDTrace/Integrations/SymfonyMessenger/DDTraceStamp.php

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use Symfony\Component\Messenger\Stamp\ReceivedStamp;
use Symfony\Component\Messenger\Stamp\RedeliveryStamp;
use Symfony\Component\Messenger\Stamp\SentStamp;
use Symfony\Component\Messenger\Stamp\SerializerStamp;
use Symfony\Component\Messenger\Stamp\TransportMessageIdStamp;
use Symfony\Component\Messenger\Transport\Receiver\ReceiverInterface;
use function DDTrace\hook_method;
Expand Down Expand Up @@ -58,15 +59,11 @@ function (HookData $hook) {
$envelope = $hook->args[0];

if (\ddtrace_config_distributed_tracing_enabled()) {
$ddTraceStamp = $envelope->last(DDTraceStamp::class);

// Add distributed tracing stamp only if not already on the envelope
if ($ddTraceStamp === null) {
$tracingHeaders = \DDTrace\generate_distributed_tracing_headers();
$hook->overrideArguments([
$envelope->with(new DDTraceStamp($tracingHeaders))
]);
}
$tracingHeaders = \DDTrace\generate_distributed_tracing_headers();
$datadogHeaders = ['datadog' => $tracingHeaders];
$hook->overrideArguments([
$envelope->with(new SerializerStamp($datadogHeaders))
]);
}
}
);
Expand Down Expand Up @@ -101,13 +98,17 @@ function (SpanData $span, array $args, $envelope) use ($integration) {
'receive'
);

$ddTraceStamp = $envelope->last(DDTraceStamp::class);
if ($ddTraceStamp instanceof DDTraceStamp) {
$tracingHeaders = $ddTraceStamp->getHeaders();
if (\dd_trace_env_config('DD_TRACE_SYMFONY_MESSENGER_DISTRIBUTED_TRACING')) {
\DDTrace\consume_distributed_tracing_headers($tracingHeaders);
} else {
$span->links[] = \DDTrace\SpanLink::fromHeaders($tracingHeaders);
$serializerStamps = $envelope->all(SerializerStamp::class);
foreach ($serializerStamps as $serializerStamp) {
$context = $serializerStamp->getContext();
if (isset($context['datadog'])) {
$tracingHeaders = $context['datadog'];
if (\dd_trace_env_config('DD_TRACE_SYMFONY_MESSENGER_DISTRIBUTED_TRACING')) {
\DDTrace\consume_distributed_tracing_headers($tracingHeaders);
} else {
$span->links[] = \DDTrace\SpanLink::fromHeaders($tracingHeaders);
}
break;
}
}
},
Expand Down
14 changes: 6 additions & 8 deletions tests/Common/TracerTestTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ public function inCli($scriptPath, $customEnvs = [], $customInis = [], $argument
return $out;
}

public function executeCli($scriptPath, $customEnvs = [], $customInis = [], $arguments = '', $withOutput = false, $skipSyncFlush = false)
public function executeCli($scriptPath, $customEnvs = [], $customInis = [], $arguments = '', $withOutput = false, $skipSyncFlush = false, $withExitCode = false)
{
$envs = (string) new EnvSerializer(array_merge(
[
Expand Down Expand Up @@ -267,16 +267,14 @@ public function executeCli($scriptPath, $customEnvs = [], $customInis = [], $arg
$arguments = implode(' ', array_map('escapeshellarg', $arguments));
}
$commandToExecute = "$envs " . PHP_BINARY . " $inis $script $arguments";
if ($withOutput) {
$ret = (string) `$commandToExecute 2>&1`;
} else {
`$commandToExecute`;
$ret = null;
}
$output = [];
$exitCode = 0;
exec($commandToExecute . ' 2>&1', $output, $exitCode);
$ret = $withOutput ? implode("\n", $output) : null;
if (!$skipSyncFlush && \dd_trace_env_config("DD_TRACE_SIDECAR_TRACE_SENDER")) {
\dd_trace_synchronous_flush();
}
return $ret;
return $withExitCode ? [$ret, $exitCode] : $ret;
}

/**
Expand Down
5 changes: 5 additions & 0 deletions tests/Frameworks/Symfony/Version_7_0/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
"doctrine/doctrine-bundle": "^2.11",
"doctrine/doctrine-migrations-bundle": "^3.3",
"doctrine/orm": "^2.17",
"phpdocumentor/reflection-docblock": "^5.6",
"phpstan/phpdoc-parser": "^1.0",
"symfony/console": "7.0.*",
"symfony/doctrine-messenger": "^7.1",
"symfony/dotenv": "7.0.*",
Expand All @@ -19,8 +21,11 @@
"symfony/framework-bundle": "7.0.*",
"symfony/messenger": "^7.1",
"symfony/monolog-bundle": "^3.10",
"symfony/property-access": "7.0.*",
"symfony/property-info": "7.0.*",
"symfony/runtime": "7.0.*",
"symfony/security-bundle": "7.0.*",
"symfony/serializer": "7.0.*",
"symfony/twig-bundle": "7.0.*",
"symfony/validator": "7.0.*",
"symfony/yaml": "7.0.*",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,27 @@
framework:
messenger:
# Uncomment this (and the failed transport below) to send failed messages to this transport for later handling.
failure_transport: failed
messenger:
serializer:
default_serializer: messenger.transport.symfony_serializer
symfony_serializer:
format: json
context: { }
# Uncomment this (and the failed transport below) to send failed messages to this transport for later handling.
failure_transport: failed

transports:
# https://symfony.com/doc/current/messenger.html#transport-configuration
async:
dsn: '%env(MESSENGER_TRANSPORT_DSN)%'
options:
queue_name: high
retry_strategy:
max_retries: 0
failed: 'doctrine://default?queue_name=failed'
sync: 'sync://'
transports:
# https://symfony.com/doc/current/messenger.html#transport-configuration
async:
dsn: '%env(MESSENGER_TRANSPORT_DSN)%'
options:
queue_name: high
retry_strategy:
max_retries: 0
failed: 'doctrine://default?queue_name=failed'
sync: 'sync://'

routing:
# Route your messages to the transports
'App\Message\LuckyNumberNotification': async
routing:
# Route your messages to the transports
'App\Message\LuckyNumberNotification': async


# when@test:
Expand Down
25 changes: 25 additions & 0 deletions tests/Integrations/Symfony/V4_4/MessengerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ protected static function getEnvs()
'DD_SERVICE' => 'symfony_messenger_test',
'DD_TRACE_DEBUG' => 'true',
'DD_TRACE_SYMFONY_MESSENGER_MIDDLEWARES' => 'true',
'DD_INSTRUMENTATION_TELEMETRY_ENABLED' => 'false',
'DD_TRACE_PHPREDIS_ENABLED' => 'false' // We are NOT testing the phpredis integration
]);
}
Expand All @@ -55,6 +56,7 @@ public function testAsyncSuccess()
'DD_TRACE_REMOVE_AUTOINSTRUMENTATION_ORPHANS' => 'true',
'DD_TRACE_SYMFONY_MESSENGER_MIDDLEWARES' => 'true',
'DD_TRACE_DEBUG' => 'true',
'DD_INSTRUMENTATION_TELEMETRY_ENABLED' => 'false',
], [], ['messenger:consume', 'async', '--limit=1']);

$this->snapshotFromTraces(
Expand All @@ -78,6 +80,7 @@ public function testAsyncFailure()
'DD_SERVICE' => 'symfony_messenger_test',
'DD_TRACE_REMOVE_AUTOINSTRUMENTATION_ORPHANS' => 'true',
'DD_TRACE_SYMFONY_MESSENGER_MIDDLEWARES' => 'true',
'DD_INSTRUMENTATION_TELEMETRY_ENABLED' => 'false',
], [], ['messenger:consume', 'async', '--limit=1']);

$this->snapshotFromTraces(
Expand All @@ -87,4 +90,26 @@ public function testAsyncFailure()
true
);
}

public function testAsyncWithTracerDisabledOnConsume()
{
// GH Issue: https://github.com/DataDog/dd-trace-php/pull/2749#issuecomment-2467409884

$this->tracesFromWebRequestSnapshot(function () {
$spec = GetSpec::create('Lucky number', '/lucky/number');
$this->call($spec);
}, self::FIELDS_TO_IGNORE);

list($output, $exitCode) = $this->executeCli(
self::getConsoleScript(),
[],
['ddtrace.disable' => 'true'],
['messenger:consume', 'async', '--limit=1'],
true,
true,
true
);

$this->assertEquals(0, $exitCode, "Command failed with exit code 1. Output: $output");
}
}
25 changes: 25 additions & 0 deletions tests/Integrations/Symfony/V5_2/MessengerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ protected static function getEnvs()
'DD_SERVICE' => 'symfony_messenger_test',
'DD_TRACE_DEBUG' => 'true',
'DD_TRACE_SYMFONY_MESSENGER_MIDDLEWARES' => 'true',
'DD_INSTRUMENTATION_TELEMETRY_ENABLED' => 'false',
'DD_TRACE_PHPREDIS_ENABLED' => 'false' // We are NOT testing the phpredis integration
]);
}
Expand All @@ -55,6 +56,7 @@ public function testAsyncSuccess()
'DD_TRACE_REMOVE_AUTOINSTRUMENTATION_ORPHANS' => 'true',
'DD_TRACE_SYMFONY_MESSENGER_MIDDLEWARES' => 'true',
'DD_TRACE_DEBUG' => 'true',
'DD_INSTRUMENTATION_TELEMETRY_ENABLED' => 'false',
], [], ['messenger:consume', 'async', '--limit=1']);

$this->snapshotFromTraces(
Expand All @@ -78,6 +80,7 @@ public function testAsyncFailure()
'DD_SERVICE' => 'symfony_messenger_test',
'DD_TRACE_REMOVE_AUTOINSTRUMENTATION_ORPHANS' => 'true',
'DD_TRACE_SYMFONY_MESSENGER_MIDDLEWARES' => 'true',
'DD_INSTRUMENTATION_TELEMETRY_ENABLED' => 'false',
], [], ['messenger:consume', 'async', '--limit=1']);


Expand All @@ -88,4 +91,26 @@ public function testAsyncFailure()
true
);
}

public function testAsyncWithTracerDisabledOnConsume()
{
// GH Issue: https://github.com/DataDog/dd-trace-php/pull/2749#issuecomment-2467409884

$this->tracesFromWebRequestSnapshot(function () {
$spec = GetSpec::create('Lucky number', '/lucky/number');
$this->call($spec);
}, self::FIELDS_TO_IGNORE);

list($output, $exitCode) = $this->executeCli(
self::getConsoleScript(),
[],
['ddtrace.disable' => 'true'],
['messenger:consume', 'async', '--limit=1'],
true,
true,
true
);

$this->assertEquals(0, $exitCode, "Command failed with exit code 1. Output: $output");
}
}
25 changes: 25 additions & 0 deletions tests/Integrations/Symfony/V6_2/MessengerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ protected static function getEnvs()
'DD_SERVICE' => 'symfony_messenger_test',
'DD_TRACE_DEBUG' => 'true',
'DD_TRACE_SYMFONY_MESSENGER_MIDDLEWARES' => 'true',
'DD_INSTRUMENTATION_TELEMETRY_ENABLED' => 'false',
'DD_TRACE_PHPREDIS_ENABLED' => 'false' // We are NOT testing the phpredis integration
]);
}
Expand All @@ -55,6 +56,7 @@ public function testAsyncSuccess()
'DD_TRACE_REMOVE_AUTOINSTRUMENTATION_ORPHANS' => 'true',
'DD_TRACE_SYMFONY_MESSENGER_MIDDLEWARES' => 'true',
'DD_TRACE_DEBUG' => 'true',
'DD_INSTRUMENTATION_TELEMETRY_ENABLED' => 'false',
'DD_TRACE_PHPREDIS_ENABLED' => 'false' // We are NOT testing the phpredis integration
], [], ['mess:cons', 'async', '--limit=1']);

Expand All @@ -79,6 +81,7 @@ public function testAsyncFailure()
'DD_SERVICE' => 'symfony_messenger_test',
'DD_TRACE_REMOVE_AUTOINSTRUMENTATION_ORPHANS' => 'true',
'DD_TRACE_SYMFONY_MESSENGER_MIDDLEWARES' => 'true',
'DD_INSTRUMENTATION_TELEMETRY_ENABLED' => 'false',
'DD_TRACE_PHPREDIS_ENABLED' => 'false' // We are NOT testing the phpredis integration
], [], ['messenger:consume', 'async', '--limit=1']);

Expand All @@ -89,4 +92,26 @@ public function testAsyncFailure()
true
);
}

public function testAsyncWithTracerDisabledOnConsume()
{
// GH Issue: https://github.com/DataDog/dd-trace-php/pull/2749#issuecomment-2467409884

$this->tracesFromWebRequestSnapshot(function () {
$spec = GetSpec::create('Lucky number', '/lucky/number');
$this->call($spec);
}, self::FIELDS_TO_IGNORE);

list($output, $exitCode) = $this->executeCli(
self::getConsoleScript(),
[],
['ddtrace.disable' => 'true'],
['messenger:consume', 'async', '--limit=1'],
true,
true,
true
);

$this->assertEquals(0, $exitCode, "Command failed with exit code 1. Output: $output");
}
}
27 changes: 27 additions & 0 deletions tests/Integrations/Symfony/V7_0/MessengerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

class MessengerTest extends WebFrameworkTestCase
{
public static $database = "symfony70";

const FIELDS_TO_IGNORE = [
'metrics.php.compilation.total_time_ms',
'metrics.php.memory.peak_usage_bytes',
Expand Down Expand Up @@ -37,6 +39,7 @@ protected static function getEnvs()
'DD_SERVICE' => 'symfony_messenger_test',
'DD_TRACE_DEBUG' => 'true',
'DD_TRACE_SYMFONY_MESSENGER_MIDDLEWARES' => 'true',
'DD_INSTRUMENTATION_TELEMETRY_ENABLED' => 'false',
]);
}

Expand All @@ -54,6 +57,7 @@ public function testAsyncSuccess()
'DD_TRACE_REMOVE_AUTOINSTRUMENTATION_ORPHANS' => 'true',
'DD_TRACE_SYMFONY_MESSENGER_MIDDLEWARES' => 'true',
'DD_TRACE_DEBUG' => 'true',
'DD_INSTRUMENTATION_TELEMETRY_ENABLED' => 'false',
], [], ['messenger:consume', 'async', '--limit=1']);

$this->snapshotFromTraces(
Expand All @@ -77,6 +81,7 @@ public function testAsyncFailure()
'DD_SERVICE' => 'symfony_messenger_test',
'DD_TRACE_REMOVE_AUTOINSTRUMENTATION_ORPHANS' => 'true',
'DD_TRACE_SYMFONY_MESSENGER_MIDDLEWARES' => 'true',
'DD_INSTRUMENTATION_TELEMETRY_ENABLED' => 'false',
], [], ['messenger:consume', 'async', '--limit=1']);

$this->snapshotFromTraces(
Expand All @@ -86,4 +91,26 @@ public function testAsyncFailure()
true
);
}

public function testAsyncWithTracerDisabledOnConsume()
{
// GH Issue: https://github.com/DataDog/dd-trace-php/pull/2749#issuecomment-2467409884

$this->tracesFromWebRequestSnapshot(function () {
$spec = GetSpec::create('Lucky number', '/lucky/number');
$this->call($spec);
}, self::FIELDS_TO_IGNORE);

list($output, $exitCode) = $this->executeCli(
self::getConsoleScript(),
[],
['ddtrace.disable' => 'true'],
['messenger:consume', 'async', '--limit=1'],
true,
true,
true
);

$this->assertEquals(0, $exitCode, "Command failed with exit code 1. Output: $output");
}
}
Loading
Loading