Skip to content

Commit

Permalink
fix: Symfony Messenger propagation to non-instrumented clients (#2956)
Browse files Browse the repository at this point in the history 
* Try using SerializerStamp

* tests: Update Symfony v6.2 snapshots

* tests: Update Symfony v7.0 snapshots

* tests: Update Symfony v5.2 snapshots

* tests: Update Symfony v4.4 snapshots

* style: Remove debug params
  • Loading branch information
@PROFeNoM
PROFeNoM authored Nov 15, 2024
1 parent b9cdb0a commit 08b033e
Show file tree
Hide file tree
Showing 29 changed files with 1,850 additions and 243 deletions.
20 changes: 0 additions & 20 deletions src/DDTrace/Integrations/SymfonyMessenger/DDTraceStamp.php
View file

This file was deleted.

33 changes: 17 additions & 16 deletions src/DDTrace/Integrations/SymfonyMessenger/SymfonyMessengerIntegration.php
View file
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
View file
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
View file
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
37 changes: 21 additions & 16 deletions tests/Frameworks/Symfony/Version_7_0/config/packages/messenger.yaml
View file
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
View file
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
View file
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
View file
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
View file
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

0 comments on commit 08b033e

Please sign in to comment.