From 22be3be0982dfc79e28a352739cfa556e831c9b9 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Delhommeau Date: Mon, 30 Mar 2026 16:09:41 +0200 Subject: [PATCH 01/10] feat(redis): declare DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED config --- ext/configuration.h | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/configuration.h b/ext/configuration.h index 3b015d0501..d2fc18427e 100644 --- a/ext/configuration.h +++ b/ext/configuration.h @@ -140,6 +140,7 @@ enum ddtrace_sidecar_connection_mode { CONFIG(BOOL, DD_TRACE_DB_CLIENT_SPLIT_BY_INSTANCE, "false") \ CONFIG(BOOL, DD_TRACE_HTTP_CLIENT_SPLIT_BY_DOMAIN, "false") \ CONFIG(BOOL, DD_TRACE_REDIS_CLIENT_SPLIT_BY_HOST, "false") \ + CONFIG(BOOL, DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED, "true") \ CONFIG(BOOL, DD_EXCEPTION_REPLAY_ENABLED, "false", .ini_change = ddtrace_alter_DD_EXCEPTION_REPLAY_ENABLED) \ CONFIG(INT, DD_EXCEPTION_REPLAY_CAPTURE_MAX_FRAMES, "-1") \ CONFIG(INT, DD_EXCEPTION_REPLAY_CAPTURE_INTERVAL_SECONDS, "3600") \ From 05a4428f6f241333ad1b5961f51c1b8c4e47670f Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Delhommeau Date: Mon, 30 Mar 2026 16:09:58 +0200 Subject: [PATCH 02/10] docs(redis): document DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED --- metadata/supported-configurations.json | 7 +++++++ tests/randomized/config/envs.php | 1 + 2 files changed, 8 insertions(+) diff --git a/metadata/supported-configurations.json b/metadata/supported-configurations.json index f4c50c9287..194983879e 100644 --- a/metadata/supported-configurations.json +++ b/metadata/supported-configurations.json @@ -2056,6 +2056,13 @@ "default": "false" } ], + "DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED": [ + { + "implementation": "A", + "type": "boolean", + "default": "true" + } + ], "DD_TRACE_REMOVE_AUTOINSTRUMENTATION_ORPHANS": [ { "implementation": "A", diff --git a/tests/randomized/config/envs.php b/tests/randomized/config/envs.php index e79466fdd7..c774d76ba3 100644 --- a/tests/randomized/config/envs.php +++ b/tests/randomized/config/envs.php @@ -31,6 +31,7 @@ 'DD_TRACE_DB_CLIENT_SPLIT_BY_INSTANCE' => ['true'], 'DD_TRACE_HTTP_CLIENT_SPLIT_BY_DOMAIN' => ['true'], 'DD_TRACE_REDIS_CLIENT_SPLIT_BY_HOST' => ['true'], + 'DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED' => ['false'], 'DD_TRACE_MEASURE_COMPILE_TIME' => ['false'], 'DD_TRACE_NO_AUTOLOADER' => ['true'], 'DD_TRACE_RESOURCE_URI_FRAGMENT_REGEX' => ['^aaabbbccc$'], From 40be844c29cfc8f6e2c025ed67d5b43a3dd25b67 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Delhommeau Date: Mon, 30 Mar 2026 16:11:17 +0200 Subject: [PATCH 03/10] test(phpredis): add tests for DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED Co-Authored-By: Claude Sonnet 4.6 --- .../Integrations/PHPRedis/V5/PHPRedisTest.php | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/tests/Integrations/PHPRedis/V5/PHPRedisTest.php b/tests/Integrations/PHPRedis/V5/PHPRedisTest.php index 55e0eb5ae2..5a2d2e5aef 100644 --- a/tests/Integrations/PHPRedis/V5/PHPRedisTest.php +++ b/tests/Integrations/PHPRedis/V5/PHPRedisTest.php @@ -54,6 +54,7 @@ protected function envsToCleanUpAtTearDown() 'DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED', 'DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED', 'DD_SERVICE', + 'DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED', ]; } @@ -2491,4 +2492,84 @@ public function testOrphansRemoval64bit() $span = $traces[0][0]; $this->assertEquals(0, $span['metrics']['_sampling_priority_v1']); } + + public function testLifecycleCommandsDisabledPhpRedis() + { + $this->putEnvAndReloadConfig(['DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED=false']); + + $redis = new \Redis(); + $traces = $this->isolateTracer(function () use ($redis) { + $redis->connect($this->host, $this->port); + $redis->set('lc_key', 'lc_value'); + $redis->get('lc_key'); + $redis->del('lc_key'); + $redis->close(); + }); + + $this->assertFlameGraph($traces, [ + SpanAssertion::build( + 'Redis.set', + 'phpredis', + 'redis', + 'Redis.set' + )->withExactTags([ + 'redis.raw_command' => 'set lc_key lc_value', + Tag::SPAN_KIND => 'client', + Tag::COMPONENT => 'phpredis', + Tag::DB_SYSTEM => 'redis', + Tag::TARGET_HOST => $this->host, + ]), + SpanAssertion::build( + 'Redis.get', + 'phpredis', + 'redis', + 'Redis.get' + )->withExactTags([ + 'redis.raw_command' => 'get lc_key', + Tag::SPAN_KIND => 'client', + Tag::COMPONENT => 'phpredis', + Tag::DB_SYSTEM => 'redis', + Tag::TARGET_HOST => $this->host, + ]), + SpanAssertion::build( + 'Redis.del', + 'phpredis', + 'redis', + 'Redis.del' + )->withExactTags([ + 'redis.raw_command' => 'del lc_key', + Tag::SPAN_KIND => 'client', + Tag::COMPONENT => 'phpredis', + Tag::DB_SYSTEM => 'redis', + Tag::TARGET_HOST => $this->host, + ]), + ]); + } + + public function testLifecycleCommandsDisabledPreservesHostMetadata() + { + $this->putEnvAndReloadConfig(['DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED=false']); + + $redis = new \Redis(); + $traces = $this->isolateTracer(function () use ($redis) { + $redis->connect($this->host, $this->port); + $redis->set('meta_key', 'meta_value'); + $redis->close(); + }); + + $this->assertFlameGraph($traces, [ + SpanAssertion::build( + 'Redis.set', + 'phpredis', + 'redis', + 'Redis.set' + )->withExactTags([ + 'redis.raw_command' => 'set meta_key meta_value', + Tag::SPAN_KIND => 'client', + Tag::COMPONENT => 'phpredis', + Tag::DB_SYSTEM => 'redis', + Tag::TARGET_HOST => $this->host, + ]), + ]); + } } From b6954aa0a822df86fdb589721202fdf5085ce57f Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Delhommeau Date: Mon, 30 Mar 2026 16:11:21 +0200 Subject: [PATCH 04/10] test(predis): add tests for DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED Co-Authored-By: Claude Sonnet 4.6 --- .../Integrations/Predis/Latest/PredisTest.php | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/Integrations/Predis/Latest/PredisTest.php b/tests/Integrations/Predis/Latest/PredisTest.php index 756ecffe48..946257fc9e 100644 --- a/tests/Integrations/Predis/Latest/PredisTest.php +++ b/tests/Integrations/Predis/Latest/PredisTest.php @@ -25,6 +25,7 @@ protected function ddSetUp() 'DD_TRACE_REDIS_CLIENT_SPLIT_BY_HOST', 'DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED', 'DD_SERVICE', + 'DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED', ]); parent::ddSetUp(); } @@ -35,6 +36,7 @@ protected function ddTearDown() 'DD_TRACE_REDIS_CLIENT_SPLIT_BY_HOST', 'DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED', 'DD_SERVICE', + 'DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED', ]); parent::ddTearDown(); } @@ -384,6 +386,48 @@ public function testNoFakeServices() ]); } + public function testLifecycleCommandsDisabledPredis() + { + $this->putEnvAndReloadConfig(['DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED=false']); + + $traces = $this->isolateTracer(function () { + $client = new \Predis\Client(["host" => $this->host]); + $client->set('lc_key', 'lc_value'); + $this->assertSame('lc_value', $client->get('lc_key')); + }); + + $this->assertFlameGraph($traces, [ + SpanAssertion::build('Predis.Client.executeCommand', 'redis', 'redis', 'SET lc_key lc_value') + ->withExactTags(array_merge([], $this->baseTags(), [ + 'redis.raw_command' => 'SET lc_key lc_value', + 'redis.args_length' => '3', + ])), + SpanAssertion::build('Predis.Client.executeCommand', 'redis', 'redis', 'GET lc_key') + ->withExactTags(array_merge([], $this->baseTags(), [ + 'redis.raw_command' => 'GET lc_key', + 'redis.args_length' => '2', + ])), + ]); + } + + public function testLifecycleCommandsDisabledPreservesHostMetadataPredis() + { + $this->putEnvAndReloadConfig(['DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED=false']); + + $traces = $this->isolateTracer(function () { + $client = new \Predis\Client(["host" => $this->host]); + $client->set('meta_key', 'meta_value'); + }); + + $this->assertFlameGraph($traces, [ + SpanAssertion::build('Predis.Client.executeCommand', 'redis', 'redis', 'SET meta_key meta_value') + ->withExactTags(array_merge([], $this->baseTags(), [ + 'redis.raw_command' => 'SET meta_key meta_value', + 'redis.args_length' => '3', + ])), + ]); + } + private function baseTags() { return [ From a4c27519e36b5be8dfe1a1967e9609f254b0fe20 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Delhommeau Date: Mon, 30 Mar 2026 16:16:49 +0200 Subject: [PATCH 05/10] feat(predis): skip lifecycle spans when DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED=false --- .../Integrations/Predis/PredisIntegration.php | 83 +++++++++++-------- 1 file changed, 48 insertions(+), 35 deletions(-) diff --git a/src/DDTrace/Integrations/Predis/PredisIntegration.php b/src/DDTrace/Integrations/Predis/PredisIntegration.php index b971b57ca7..7fe4f9c2d5 100644 --- a/src/DDTrace/Integrations/Predis/PredisIntegration.php +++ b/src/DDTrace/Integrations/Predis/PredisIntegration.php @@ -2,6 +2,7 @@ namespace DDTrace\Integrations\Predis; +use DDTrace\HookData; use DDTrace\Integrations\Integration; use DDTrace\SpanData; use DDTrace\Tag; @@ -27,24 +28,34 @@ class PredisIntegration extends Integration */ public static function init(): int { - \DDTrace\trace_method('Predis\Client', '__construct', function (SpanData $span, $args) { - Integration::handleOrphan($span); + $lifecycleEnabled = \dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED"); + + if ($lifecycleEnabled) { + \DDTrace\trace_method('Predis\Client', '__construct', function (SpanData $span, $args) { + Integration::handleOrphan($span); + + $span->name = 'Predis.Client.__construct'; + $span->type = Type::REDIS; + $span->resource = 'Predis.Client.__construct'; + PredisIntegration::storeConnectionMetaAndService($this, $args); + PredisIntegration::setMetaAndServiceFromConnection($this, $span); + }); + } else { + \DDTrace\install_hook('Predis\Client::__construct', null, static function (HookData $hook) { + PredisIntegration::storeConnectionMetaAndService($hook->instance, $hook->args); + }); + } - $span->name = 'Predis.Client.__construct'; - $span->type = Type::REDIS; - $span->resource = 'Predis.Client.__construct'; - PredisIntegration::storeConnectionMetaAndService($this, $args); - PredisIntegration::setMetaAndServiceFromConnection($this, $span); - }); + if ($lifecycleEnabled) { + \DDTrace\trace_method('Predis\Client', 'connect', function (SpanData $span, $args) { + Integration::handleOrphan($span); - \DDTrace\trace_method('Predis\Client', 'connect', function (SpanData $span, $args) { - Integration::handleOrphan($span); - - $span->name = 'Predis.Client.connect'; - $span->type = Type::REDIS; - $span->resource = 'Predis.Client.connect'; - PredisIntegration::setMetaAndServiceFromConnection($this, $span); - }); + $span->name = 'Predis.Client.connect'; + $span->type = Type::REDIS; + $span->resource = 'Predis.Client.connect'; + PredisIntegration::setMetaAndServiceFromConnection($this, $span); + }); + } \DDTrace\trace_method('Predis\Client', 'executeCommand', function (SpanData $span, $args) { Integration::handleOrphan($span); @@ -93,25 +104,27 @@ public static function init(): int $span->meta['redis.raw_command'] = $query; }); - \DDTrace\trace_method( - 'Predis\Pipeline\Pipeline', - 'executePipeline', - [ - 'prehook' => function (SpanData $span, $args) { - Integration::handleOrphan($span); - - $span->name = 'Predis.Pipeline.executePipeline'; - $span->resource = $span->name; - $span->type = Type::REDIS; - PredisIntegration::setMetaAndServiceFromConnection($this->getClient(), $span); - if (\count($args) < 2) { - return; - } - $commands = $args[1]; - $span->meta['redis.pipeline_length'] = count($commands); - }, - ] - ); + if ($lifecycleEnabled) { + \DDTrace\trace_method( + 'Predis\Pipeline\Pipeline', + 'executePipeline', + [ + 'prehook' => function (SpanData $span, $args) { + Integration::handleOrphan($span); + + $span->name = 'Predis.Pipeline.executePipeline'; + $span->resource = $span->name; + $span->type = Type::REDIS; + PredisIntegration::setMetaAndServiceFromConnection($this->getClient(), $span); + if (\count($args) < 2) { + return; + } + $commands = $args[1]; + $span->meta['redis.pipeline_length'] = count($commands); + }, + ] + ); + } return Integration::LOADED; } From d99468f799e967d8ab411b598c46a44ca2a0e190 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Delhommeau Date: Mon, 30 Mar 2026 16:16:52 +0200 Subject: [PATCH 06/10] feat(redis): skip lifecycle spans when DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED=false --- .../PHPRedis/PHPRedisIntegration.php | 238 +++++++++++------- 1 file changed, 148 insertions(+), 90 deletions(-) diff --git a/src/DDTrace/Integrations/PHPRedis/PHPRedisIntegration.php b/src/DDTrace/Integrations/PHPRedis/PHPRedisIntegration.php index e993c3078e..a3dac2470d 100644 --- a/src/DDTrace/Integrations/PHPRedis/PHPRedisIntegration.php +++ b/src/DDTrace/Integrations/PHPRedis/PHPRedisIntegration.php @@ -2,6 +2,7 @@ namespace DDTrace\Integrations\PHPRedis; +use DDTrace\HookData; use DDTrace\Integrations\DatabaseIntegrationHelper; use DDTrace\Integrations\Integration; use DDTrace\SpanData; @@ -35,94 +36,147 @@ class PHPRedisIntegration extends Integration public static function init(): int { - $traceConnectOpen = function (SpanData $span, $args) { - Integration::handleOrphan($span); + $lifecycleEnabled = \dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED"); + + if ($lifecycleEnabled) { + $traceConnectOpen = function (SpanData $span, $args) { + Integration::handleOrphan($span); + + $hostOrUDS = (isset($args[0]) && \is_string($args[0])) ? $args[0] : PHPRedisIntegration::DEFAULT_HOST; + $span->meta[Tag::TARGET_HOST] = $hostOrUDS; + $span->meta[Tag::TARGET_PORT] = isset($args[1]) && \is_numeric($args[1]) ? + $args[1] : + PHPRedisIntegration::DEFAULT_PORT; + $span->meta[Tag::SPAN_KIND] = 'client'; + + // While we would have access to Redis::getHost() from the instance to retrieve it later, we compute the + // service name here and store in the instance for later because of the following reasons: + // - we would do over and over the same operation, involving a regex, for each method invocation. + // - in case of connection error, the Redis::host value is not set and we would not have access to it + // during callbacks, meaning that we would have to use two different ways to extract the name: args or + // Redis::getHost() depending on when we are interested in such information. + ObjectKVStore::put($this, PHPRedisIntegration::KEY_HOST, $hostOrUDS); + + PHPRedisIntegration::enrichSpan($span, $this, 'Redis'); + }; + \DDTrace\trace_method('Redis', 'connect', $traceConnectOpen); + \DDTrace\trace_method('Redis', 'pconnect', $traceConnectOpen); + \DDTrace\trace_method('Redis', 'open', $traceConnectOpen); + \DDTrace\trace_method('Redis', 'popen', $traceConnectOpen); + } else { + $storeHostOnly = static function (HookData $hook) { + $args = $hook->args; + $hostOrUDS = (isset($args[0]) && \is_string($args[0])) + ? $args[0] + : PHPRedisIntegration::DEFAULT_HOST; + ObjectKVStore::put($hook->instance, PHPRedisIntegration::KEY_HOST, $hostOrUDS); + }; + \DDTrace\install_hook('Redis::connect', null, $storeHostOnly); + \DDTrace\install_hook('Redis::pconnect', null, $storeHostOnly); + \DDTrace\install_hook('Redis::open', null, $storeHostOnly); + \DDTrace\install_hook('Redis::popen', null, $storeHostOnly); + } - $hostOrUDS = (isset($args[0]) && \is_string($args[0])) ? $args[0] : PHPRedisIntegration::DEFAULT_HOST; - $span->meta[Tag::TARGET_HOST] = $hostOrUDS; - $span->meta[Tag::TARGET_PORT] = isset($args[1]) && \is_numeric($args[1]) ? - $args[1] : - PHPRedisIntegration::DEFAULT_PORT; - $span->meta[Tag::SPAN_KIND] = 'client'; - - // While we would have access to Redis::getHost() from the instance to retrieve it later, we compute the - // service name here and store in the instance for later because of the following reasons: - // - we would do over and over the same operation, involving a regex, for each method invocation. - // - in case of connection error, the Redis::host value is not set and we would not have access to it - // during callbacks, meaning that we would have to use two different ways to extract the name: args or - // Redis::getHost() depending on when we are interested in such information. - ObjectKVStore::put($this, PHPRedisIntegration::KEY_HOST, $hostOrUDS); - - PHPRedisIntegration::enrichSpan($span, $this, 'Redis'); - }; - \DDTrace\trace_method('Redis', 'connect', $traceConnectOpen); - \DDTrace\trace_method('Redis', 'pconnect', $traceConnectOpen); - \DDTrace\trace_method('Redis', 'open', $traceConnectOpen); - \DDTrace\trace_method('Redis', 'popen', $traceConnectOpen); - - $traceNewCluster = function (SpanData $span, $args) { - Integration::handleOrphan($span); + if ($lifecycleEnabled) { + $traceNewCluster = function (SpanData $span, $args) { + Integration::handleOrphan($span); + + if (isset($args[1]) && \is_array($args[1]) && !empty($args[1])) { + $firstHostOrUDS = $args[1][0]; + } else { + $seeds = \ini_get('redis.clusters.seeds'); + if (!empty($seeds)) { + $clusters = []; + parse_str($seeds, $clusters); + if (array_key_exists($args[0], $clusters) && !empty($clusters[$args[0]])) { + $firstHostOrUDS = $clusters[$args[0]][0]; + } + } + } + if (empty($firstHostOrUDS)) { + $firstHostOrUDS = PHPRedisIntegration::DEFAULT_HOST; + } - if (isset($args[1]) && \is_array($args[1]) && !empty($args[1])) { - $firstHostOrUDS = $args[1][0]; - } else { - $seeds = \ini_get('redis.clusters.seeds'); - if (!empty($seeds)) { - $clusters = []; - parse_str($seeds, $clusters); - if (array_key_exists($args[0], $clusters) && !empty($clusters[$args[0]])) { - $firstHostOrUDS = $clusters[$args[0]][0]; + $configuredClusterName = isset($args[0]) && \is_string($args[0]) ? $args[0] : null; + ObjectKVStore::put($this, PHPRedisIntegration::KEY_CLUSTER_NAME, $configuredClusterName); + + $url = parse_url($firstHostOrUDS); + $firstConfiguredHost = is_array($url) && isset($url["host"]) ? + $url["host"] : + PHPRedisIntegration::DEFAULT_HOST; + $span->meta[Tag::TARGET_HOST] = $firstConfiguredHost; + $span->meta[Tag::TARGET_PORT] = is_array($url) && isset($url["port"]) ? + $url["port"] : + PHPRedisIntegration::DEFAULT_PORT; + ObjectKVStore::put($this, PHPRedisIntegration::KEY_FIRST_HOST, $firstConfiguredHost); + ObjectKVStore::put($this, PHPRedisIntegration::KEY_FIRST_HOST_OR_UDS, $firstHostOrUDS); + + PHPRedisIntegration::enrichSpan($span, $this, 'RedisCluster'); + }; + \DDTrace\trace_method('RedisCluster', '__construct', $traceNewCluster); + } else { + \DDTrace\install_hook('RedisCluster::__construct', null, static function (HookData $hook) { + $args = $hook->args; + $instance = $hook->instance; + + if (isset($args[1]) && \is_array($args[1]) && !empty($args[1])) { + $firstHostOrUDS = $args[1][0]; + } else { + $seeds = \ini_get('redis.clusters.seeds'); + if (!empty($seeds)) { + $clusters = []; + parse_str($seeds, $clusters); + if (array_key_exists($args[0], $clusters) && !empty($clusters[$args[0]])) { + $firstHostOrUDS = $clusters[$args[0]][0]; + } } } - } - if (empty($firstHostOrUDS)) { - $firstHostOrUDS = PHPRedisIntegration::DEFAULT_HOST; - } + if (empty($firstHostOrUDS)) { + $firstHostOrUDS = PHPRedisIntegration::DEFAULT_HOST; + } - $configuredClusterName = isset($args[0]) && \is_string($args[0]) ? $args[0] : null; - ObjectKVStore::put($this, PHPRedisIntegration::KEY_CLUSTER_NAME, $configuredClusterName); - - $url = parse_url($firstHostOrUDS); - $firstConfiguredHost = is_array($url) && isset($url["host"]) ? - $url["host"] : - PHPRedisIntegration::DEFAULT_HOST; - $span->meta[Tag::TARGET_HOST] = $firstConfiguredHost; - $span->meta[Tag::TARGET_PORT] = is_array($url) && isset($url["port"]) ? - $url["port"] : - PHPRedisIntegration::DEFAULT_PORT; - ObjectKVStore::put($this, PHPRedisIntegration::KEY_FIRST_HOST, $firstConfiguredHost); - ObjectKVStore::put($this, PHPRedisIntegration::KEY_FIRST_HOST_OR_UDS, $firstHostOrUDS); - - PHPRedisIntegration::enrichSpan($span, $this, 'RedisCluster'); - }; - \DDTrace\trace_method('RedisCluster', '__construct', $traceNewCluster); - - self::traceMethodNoArgs('close'); - self::traceMethodNoArgs('auth'); - self::traceMethodNoArgs('ping'); - self::traceMethodNoArgs('echo'); - self::traceMethodNoArgs('bgRewriteAOF'); - self::traceMethodNoArgs('bgSave'); - self::traceMethodNoArgs('flushAll'); - self::traceMethodNoArgs('flushDb'); - self::traceMethodNoArgs('save'); - // We do not trace arguments of restore as they are binary - self::traceMethodNoArgs('restore'); - - \DDTrace\trace_method('Redis', 'select', function (SpanData $span, $args) { - Integration::handleOrphan($span); + $configuredClusterName = isset($args[0]) && \is_string($args[0]) ? $args[0] : null; + ObjectKVStore::put($instance, PHPRedisIntegration::KEY_CLUSTER_NAME, $configuredClusterName); - PHPRedisIntegration::enrichSpan($span, $this, 'Redis'); - if (isset($args[0]) && \is_numeric($args[0])) { - $span->meta['db.index'] = $args[0]; - } + $url = parse_url($firstHostOrUDS); + $firstConfiguredHost = is_array($url) && isset($url["host"]) + ? $url["host"] + : PHPRedisIntegration::DEFAULT_HOST; + ObjectKVStore::put($instance, PHPRedisIntegration::KEY_FIRST_HOST, $firstConfiguredHost); + ObjectKVStore::put($instance, PHPRedisIntegration::KEY_FIRST_HOST_OR_UDS, $firstHostOrUDS); + }); + } - $host = ObjectKVStore::get($this, PHPRedisIntegration::KEY_HOST); - $span->meta[Tag::TARGET_HOST] = $host; - $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES; - }); + if ($lifecycleEnabled) { + self::traceMethodNoArgs('close'); + self::traceMethodNoArgs('auth'); + self::traceMethodNoArgs('ping'); + self::traceMethodNoArgs('echo'); + self::traceMethodNoArgs('bgRewriteAOF'); + self::traceMethodNoArgs('bgSave'); + self::traceMethodNoArgs('flushAll'); + self::traceMethodNoArgs('flushDb'); + self::traceMethodNoArgs('save'); + // We do not trace arguments of restore as they are binary + self::traceMethodNoArgs('restore'); + + \DDTrace\trace_method('Redis', 'select', function (SpanData $span, $args) { + Integration::handleOrphan($span); + + PHPRedisIntegration::enrichSpan($span, $this, 'Redis'); + if (isset($args[0]) && \is_numeric($args[0])) { + $span->meta['db.index'] = $args[0]; + } + + $host = ObjectKVStore::get($this, PHPRedisIntegration::KEY_HOST); + $span->meta[Tag::TARGET_HOST] = $host; + $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES; + }); + } - self::traceMethodAsCommand('swapdb'); + if ($lifecycleEnabled) { + self::traceMethodAsCommand('swapdb'); + } self::traceMethodAsCommand('append'); self::traceMethodAsCommand('decr'); @@ -278,18 +332,22 @@ public static function init(): int self::traceMethodAsCommand('eval'); self::traceMethodAsCommand('evalSha'); self::traceMethodAsCommand('script'); - self::traceMethodAsCommand('getLastError'); - self::traceMethodAsCommand('clearLastError'); - self::traceMethodAsCommand('_unserialize'); - self::traceMethodAsCommand('_serialize'); + if ($lifecycleEnabled) { + self::traceMethodAsCommand('getLastError'); + self::traceMethodAsCommand('clearLastError'); + self::traceMethodAsCommand('_unserialize'); + self::traceMethodAsCommand('_serialize'); + } // Introspection - self::traceMethodAsCommand('isConnected'); - self::traceMethodAsCommand('getHost'); - self::traceMethodAsCommand('getPort'); - self::traceMethodAsCommand('getDbNum'); - self::traceMethodAsCommand('getTimeout'); - self::traceMethodAsCommand('getReadTimeout'); + if ($lifecycleEnabled) { + self::traceMethodAsCommand('isConnected'); + self::traceMethodAsCommand('getHost'); + self::traceMethodAsCommand('getPort'); + self::traceMethodAsCommand('getDbNum'); + self::traceMethodAsCommand('getTimeout'); + self::traceMethodAsCommand('getReadTimeout'); + } // Geocoding self::traceMethodAsCommand('geoAdd'); From 525d2a0de6e13c61d5917a80547a803543f6de31 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Delhommeau Date: Mon, 30 Mar 2026 16:20:41 +0200 Subject: [PATCH 07/10] refactor(redis): extract storeClusterMeta to eliminate duplication Co-Authored-By: Claude Sonnet 4.6 --- .../PHPRedis/PHPRedisIntegration.php | 97 ++++++++----------- 1 file changed, 41 insertions(+), 56 deletions(-) diff --git a/src/DDTrace/Integrations/PHPRedis/PHPRedisIntegration.php b/src/DDTrace/Integrations/PHPRedis/PHPRedisIntegration.php index a3dac2470d..06cb17da95 100644 --- a/src/DDTrace/Integrations/PHPRedis/PHPRedisIntegration.php +++ b/src/DDTrace/Integrations/PHPRedis/PHPRedisIntegration.php @@ -81,69 +81,25 @@ public static function init(): int $traceNewCluster = function (SpanData $span, $args) { Integration::handleOrphan($span); - if (isset($args[1]) && \is_array($args[1]) && !empty($args[1])) { - $firstHostOrUDS = $args[1][0]; - } else { - $seeds = \ini_get('redis.clusters.seeds'); - if (!empty($seeds)) { - $clusters = []; - parse_str($seeds, $clusters); - if (array_key_exists($args[0], $clusters) && !empty($clusters[$args[0]])) { - $firstHostOrUDS = $clusters[$args[0]][0]; - } - } - } - if (empty($firstHostOrUDS)) { - $firstHostOrUDS = PHPRedisIntegration::DEFAULT_HOST; - } - - $configuredClusterName = isset($args[0]) && \is_string($args[0]) ? $args[0] : null; - ObjectKVStore::put($this, PHPRedisIntegration::KEY_CLUSTER_NAME, $configuredClusterName); - - $url = parse_url($firstHostOrUDS); - $firstConfiguredHost = is_array($url) && isset($url["host"]) ? - $url["host"] : - PHPRedisIntegration::DEFAULT_HOST; + PHPRedisIntegration::storeClusterMeta($this, $args); + + $url = parse_url( + ObjectKVStore::get($this, PHPRedisIntegration::KEY_FIRST_HOST_OR_UDS) + ?? PHPRedisIntegration::DEFAULT_HOST + ); + $firstConfiguredHost = ObjectKVStore::get($this, PHPRedisIntegration::KEY_FIRST_HOST) + ?? PHPRedisIntegration::DEFAULT_HOST; $span->meta[Tag::TARGET_HOST] = $firstConfiguredHost; - $span->meta[Tag::TARGET_PORT] = is_array($url) && isset($url["port"]) ? - $url["port"] : - PHPRedisIntegration::DEFAULT_PORT; - ObjectKVStore::put($this, PHPRedisIntegration::KEY_FIRST_HOST, $firstConfiguredHost); - ObjectKVStore::put($this, PHPRedisIntegration::KEY_FIRST_HOST_OR_UDS, $firstHostOrUDS); + $span->meta[Tag::TARGET_PORT] = is_array($url) && isset($url["port"]) + ? $url["port"] + : PHPRedisIntegration::DEFAULT_PORT; PHPRedisIntegration::enrichSpan($span, $this, 'RedisCluster'); }; \DDTrace\trace_method('RedisCluster', '__construct', $traceNewCluster); } else { \DDTrace\install_hook('RedisCluster::__construct', null, static function (HookData $hook) { - $args = $hook->args; - $instance = $hook->instance; - - if (isset($args[1]) && \is_array($args[1]) && !empty($args[1])) { - $firstHostOrUDS = $args[1][0]; - } else { - $seeds = \ini_get('redis.clusters.seeds'); - if (!empty($seeds)) { - $clusters = []; - parse_str($seeds, $clusters); - if (array_key_exists($args[0], $clusters) && !empty($clusters[$args[0]])) { - $firstHostOrUDS = $clusters[$args[0]][0]; - } - } - } - if (empty($firstHostOrUDS)) { - $firstHostOrUDS = PHPRedisIntegration::DEFAULT_HOST; - } - - $configuredClusterName = isset($args[0]) && \is_string($args[0]) ? $args[0] : null; - ObjectKVStore::put($instance, PHPRedisIntegration::KEY_CLUSTER_NAME, $configuredClusterName); - - $url = parse_url($firstHostOrUDS); - $firstConfiguredHost = is_array($url) && isset($url["host"]) - ? $url["host"] - : PHPRedisIntegration::DEFAULT_HOST; - ObjectKVStore::put($instance, PHPRedisIntegration::KEY_FIRST_HOST, $firstConfiguredHost); - ObjectKVStore::put($instance, PHPRedisIntegration::KEY_FIRST_HOST_OR_UDS, $firstHostOrUDS); + PHPRedisIntegration::storeClusterMeta($hook->instance, $hook->args); }); } @@ -467,6 +423,35 @@ public static function traceMethodAsCommand($method) }); } + public static function storeClusterMeta($instance, $args) + { + if (isset($args[1]) && \is_array($args[1]) && !empty($args[1])) { + $firstHostOrUDS = $args[1][0]; + } else { + $seeds = \ini_get('redis.clusters.seeds'); + if (!empty($seeds)) { + $clusters = []; + parse_str($seeds, $clusters); + if (array_key_exists($args[0], $clusters) && !empty($clusters[$args[0]])) { + $firstHostOrUDS = $clusters[$args[0]][0]; + } + } + } + if (empty($firstHostOrUDS)) { + $firstHostOrUDS = self::DEFAULT_HOST; + } + + $configuredClusterName = isset($args[0]) && \is_string($args[0]) ? $args[0] : null; + ObjectKVStore::put($instance, self::KEY_CLUSTER_NAME, $configuredClusterName); + + $url = parse_url($firstHostOrUDS); + $firstConfiguredHost = is_array($url) && isset($url["host"]) + ? $url["host"] + : self::DEFAULT_HOST; + ObjectKVStore::put($instance, self::KEY_FIRST_HOST, $firstConfiguredHost); + ObjectKVStore::put($instance, self::KEY_FIRST_HOST_OR_UDS, $firstHostOrUDS); + } + /** * Based on logic from python tracer: * https://github.com/DataDog/dd-trace-py/blob/0d7e7cb38216acb0c8b29f0ae1318d25bc160123/ddtrace/contrib/redis/util.py#L25 From a10c0cb284b4e904d6ad846d86cbb2df8b40f776 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Delhommeau Date: Mon, 30 Mar 2026 16:36:29 +0200 Subject: [PATCH 08/10] fix(redis): check lifecycle flag per-call instead of at init time Replace init-time $lifecycleEnabled check with per-call dd_trace_env_config() inside install_hook callbacks so putEnvAndReloadConfig() in tests takes effect. Co-Authored-By: Claude Sonnet 4.6 --- .../PHPRedis/PHPRedisIntegration.php | 254 ++++++++++-------- 1 file changed, 147 insertions(+), 107 deletions(-) diff --git a/src/DDTrace/Integrations/PHPRedis/PHPRedisIntegration.php b/src/DDTrace/Integrations/PHPRedis/PHPRedisIntegration.php index 06cb17da95..11b3fa5f07 100644 --- a/src/DDTrace/Integrations/PHPRedis/PHPRedisIntegration.php +++ b/src/DDTrace/Integrations/PHPRedis/PHPRedisIntegration.php @@ -36,103 +36,85 @@ class PHPRedisIntegration extends Integration public static function init(): int { - $lifecycleEnabled = \dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED"); - - if ($lifecycleEnabled) { - $traceConnectOpen = function (SpanData $span, $args) { - Integration::handleOrphan($span); - - $hostOrUDS = (isset($args[0]) && \is_string($args[0])) ? $args[0] : PHPRedisIntegration::DEFAULT_HOST; - $span->meta[Tag::TARGET_HOST] = $hostOrUDS; - $span->meta[Tag::TARGET_PORT] = isset($args[1]) && \is_numeric($args[1]) ? - $args[1] : - PHPRedisIntegration::DEFAULT_PORT; - $span->meta[Tag::SPAN_KIND] = 'client'; - - // While we would have access to Redis::getHost() from the instance to retrieve it later, we compute the - // service name here and store in the instance for later because of the following reasons: - // - we would do over and over the same operation, involving a regex, for each method invocation. - // - in case of connection error, the Redis::host value is not set and we would not have access to it - // during callbacks, meaning that we would have to use two different ways to extract the name: args or - // Redis::getHost() depending on when we are interested in such information. - ObjectKVStore::put($this, PHPRedisIntegration::KEY_HOST, $hostOrUDS); - - PHPRedisIntegration::enrichSpan($span, $this, 'Redis'); - }; - \DDTrace\trace_method('Redis', 'connect', $traceConnectOpen); - \DDTrace\trace_method('Redis', 'pconnect', $traceConnectOpen); - \DDTrace\trace_method('Redis', 'open', $traceConnectOpen); - \DDTrace\trace_method('Redis', 'popen', $traceConnectOpen); - } else { - $storeHostOnly = static function (HookData $hook) { - $args = $hook->args; - $hostOrUDS = (isset($args[0]) && \is_string($args[0])) - ? $args[0] - : PHPRedisIntegration::DEFAULT_HOST; - ObjectKVStore::put($hook->instance, PHPRedisIntegration::KEY_HOST, $hostOrUDS); - }; - \DDTrace\install_hook('Redis::connect', null, $storeHostOnly); - \DDTrace\install_hook('Redis::pconnect', null, $storeHostOnly); - \DDTrace\install_hook('Redis::open', null, $storeHostOnly); - \DDTrace\install_hook('Redis::popen', null, $storeHostOnly); - } + $connectHook = static function (HookData $hook) { + $args = $hook->args; + $instance = $hook->instance; + $hostOrUDS = (isset($args[0]) && \is_string($args[0])) ? $args[0] : PHPRedisIntegration::DEFAULT_HOST; - if ($lifecycleEnabled) { - $traceNewCluster = function (SpanData $span, $args) { - Integration::handleOrphan($span); - - PHPRedisIntegration::storeClusterMeta($this, $args); - - $url = parse_url( - ObjectKVStore::get($this, PHPRedisIntegration::KEY_FIRST_HOST_OR_UDS) - ?? PHPRedisIntegration::DEFAULT_HOST - ); - $firstConfiguredHost = ObjectKVStore::get($this, PHPRedisIntegration::KEY_FIRST_HOST) - ?? PHPRedisIntegration::DEFAULT_HOST; - $span->meta[Tag::TARGET_HOST] = $firstConfiguredHost; - $span->meta[Tag::TARGET_PORT] = is_array($url) && isset($url["port"]) - ? $url["port"] - : PHPRedisIntegration::DEFAULT_PORT; - - PHPRedisIntegration::enrichSpan($span, $this, 'RedisCluster'); - }; - \DDTrace\trace_method('RedisCluster', '__construct', $traceNewCluster); - } else { - \DDTrace\install_hook('RedisCluster::__construct', null, static function (HookData $hook) { - PHPRedisIntegration::storeClusterMeta($hook->instance, $hook->args); - }); - } + // Always store metadata so data command spans have out.host + ObjectKVStore::put($instance, PHPRedisIntegration::KEY_HOST, $hostOrUDS); - if ($lifecycleEnabled) { - self::traceMethodNoArgs('close'); - self::traceMethodNoArgs('auth'); - self::traceMethodNoArgs('ping'); - self::traceMethodNoArgs('echo'); - self::traceMethodNoArgs('bgRewriteAOF'); - self::traceMethodNoArgs('bgSave'); - self::traceMethodNoArgs('flushAll'); - self::traceMethodNoArgs('flushDb'); - self::traceMethodNoArgs('save'); - // We do not trace arguments of restore as they are binary - self::traceMethodNoArgs('restore'); - - \DDTrace\trace_method('Redis', 'select', function (SpanData $span, $args) { - Integration::handleOrphan($span); - - PHPRedisIntegration::enrichSpan($span, $this, 'Redis'); - if (isset($args[0]) && \is_numeric($args[0])) { - $span->meta['db.index'] = $args[0]; - } + if (!\dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED")) { + return; + } - $host = ObjectKVStore::get($this, PHPRedisIntegration::KEY_HOST); - $span->meta[Tag::TARGET_HOST] = $host; - $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES; - }); - } + $span = $hook->span(); + Integration::handleOrphan($span); + $span->meta[Tag::TARGET_HOST] = $hostOrUDS; + $span->meta[Tag::TARGET_PORT] = isset($args[1]) && \is_numeric($args[1]) + ? $args[1] + : PHPRedisIntegration::DEFAULT_PORT; + $span->meta[Tag::SPAN_KIND] = 'client'; + PHPRedisIntegration::enrichSpan($span, $instance, 'Redis'); + }; + \DDTrace\install_hook('Redis::connect', $connectHook); + \DDTrace\install_hook('Redis::pconnect', $connectHook); + \DDTrace\install_hook('Redis::open', $connectHook); + \DDTrace\install_hook('Redis::popen', $connectHook); + + \DDTrace\install_hook('RedisCluster::__construct', static function (HookData $hook) { + PHPRedisIntegration::storeClusterMeta($hook->instance, $hook->args); + + if (!\dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED")) { + return; + } - if ($lifecycleEnabled) { - self::traceMethodAsCommand('swapdb'); - } + $span = $hook->span(); + Integration::handleOrphan($span); + + $firstHostOrUDS = ObjectKVStore::get($hook->instance, PHPRedisIntegration::KEY_FIRST_HOST_OR_UDS) + ?? PHPRedisIntegration::DEFAULT_HOST; + $firstConfiguredHost = ObjectKVStore::get($hook->instance, PHPRedisIntegration::KEY_FIRST_HOST) + ?? PHPRedisIntegration::DEFAULT_HOST; + + $url = parse_url($firstHostOrUDS); + $span->meta[Tag::TARGET_HOST] = $firstConfiguredHost; + $span->meta[Tag::TARGET_PORT] = is_array($url) && isset($url["port"]) + ? $url["port"] + : PHPRedisIntegration::DEFAULT_PORT; + + PHPRedisIntegration::enrichSpan($span, $hook->instance, 'RedisCluster'); + }); + + self::installLifecycleHookNoArgs('close'); + self::installLifecycleHookNoArgs('auth'); + self::installLifecycleHookNoArgs('ping'); + self::installLifecycleHookNoArgs('echo'); + self::installLifecycleHookNoArgs('bgRewriteAOF'); + self::installLifecycleHookNoArgs('bgSave'); + self::installLifecycleHookNoArgs('flushAll'); + self::installLifecycleHookNoArgs('flushDb'); + self::installLifecycleHookNoArgs('save'); + // We do not trace arguments of restore as they are binary + self::installLifecycleHookNoArgs('restore'); + + \DDTrace\install_hook('Redis::select', static function (HookData $hook) { + if (!\dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED")) { + return; + } + $span = $hook->span(); + Integration::handleOrphan($span); + PHPRedisIntegration::enrichSpan($span, $hook->instance, 'Redis'); + $args = $hook->args; + if (isset($args[0]) && \is_numeric($args[0])) { + $span->meta['db.index'] = $args[0]; + } + $host = ObjectKVStore::get($hook->instance, PHPRedisIntegration::KEY_HOST); + $span->meta[Tag::TARGET_HOST] = $host; + $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES; + }); + + self::installLifecycleHookAsCommand('swapdb'); self::traceMethodAsCommand('append'); self::traceMethodAsCommand('decr'); @@ -288,22 +270,18 @@ public static function init(): int self::traceMethodAsCommand('eval'); self::traceMethodAsCommand('evalSha'); self::traceMethodAsCommand('script'); - if ($lifecycleEnabled) { - self::traceMethodAsCommand('getLastError'); - self::traceMethodAsCommand('clearLastError'); - self::traceMethodAsCommand('_unserialize'); - self::traceMethodAsCommand('_serialize'); - } + self::installLifecycleHookAsCommand('getLastError'); + self::installLifecycleHookAsCommand('clearLastError'); + self::installLifecycleHookAsCommand('_unserialize'); + self::installLifecycleHookAsCommand('_serialize'); // Introspection - if ($lifecycleEnabled) { - self::traceMethodAsCommand('isConnected'); - self::traceMethodAsCommand('getHost'); - self::traceMethodAsCommand('getPort'); - self::traceMethodAsCommand('getDbNum'); - self::traceMethodAsCommand('getTimeout'); - self::traceMethodAsCommand('getReadTimeout'); - } + self::installLifecycleHookAsCommand('isConnected'); + self::installLifecycleHookAsCommand('getHost'); + self::installLifecycleHookAsCommand('getPort'); + self::installLifecycleHookAsCommand('getDbNum'); + self::installLifecycleHookAsCommand('getTimeout'); + self::installLifecycleHookAsCommand('getReadTimeout'); // Geocoding self::traceMethodAsCommand('geoAdd'); @@ -367,6 +345,68 @@ public static function enrichSpan(SpanData $span, $instance, $class, $method = n } } + private static function installLifecycleHookNoArgs($method) + { + \DDTrace\install_hook('Redis::' . $method, static function (HookData $hook) use ($method) { + if (!\dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED")) { + return; + } + $span = $hook->span(); + Integration::handleOrphan($span); + PHPRedisIntegration::enrichSpan($span, $hook->instance, 'Redis', $method); + $host = ObjectKVStore::get($hook->instance, PHPRedisIntegration::KEY_HOST); + $span->meta[Tag::TARGET_HOST] = $host; + $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES; + }); + \DDTrace\install_hook('RedisCluster::' . $method, static function (HookData $hook) use ($method) { + if (!\dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED")) { + return; + } + $span = $hook->span(); + Integration::handleOrphan($span); + PHPRedisIntegration::enrichSpan($span, $hook->instance, 'RedisCluster', $method); + if ($clusterName = ObjectKVStore::get($hook->instance, PHPRedisIntegration::KEY_CLUSTER_NAME)) { + $span->meta[PHPRedisIntegration::INTERNAL_ONLY_TAG_CLUSTER_NAME] = $clusterName; + } elseif ($firstHost = ObjectKVStore::get($hook->instance, PHPRedisIntegration::KEY_FIRST_HOST)) { + $span->meta[PHPRedisIntegration::INTERNAL_ONLY_TAG_FIRST_HOST] = $firstHost; + } + $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES; + }); + } + + private static function installLifecycleHookAsCommand($method) + { + \DDTrace\install_hook('Redis::' . $method, static function (HookData $hook) use ($method) { + if (!\dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED")) { + return; + } + $span = $hook->span(); + Integration::handleOrphan($span); + PHPRedisIntegration::enrichSpan($span, $hook->instance, 'Redis', $method); + $normalizedArgs = PHPRedisIntegration::normalizeArgs($hook->args); + $span->meta[Tag::REDIS_RAW_COMMAND] = empty($normalizedArgs) ? $method : ($method . ' ' . $normalizedArgs); + $host = ObjectKVStore::get($hook->instance, PHPRedisIntegration::KEY_HOST); + $span->meta[Tag::TARGET_HOST] = $host; + $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES; + }); + \DDTrace\install_hook('RedisCluster::' . $method, static function (HookData $hook) use ($method) { + if (!\dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED")) { + return; + } + $span = $hook->span(); + Integration::handleOrphan($span); + PHPRedisIntegration::enrichSpan($span, $hook->instance, 'RedisCluster', $method); + $normalizedArgs = PHPRedisIntegration::normalizeArgs($hook->args); + $span->meta[Tag::REDIS_RAW_COMMAND] = empty($normalizedArgs) ? $method : ($method . ' ' . $normalizedArgs); + if ($clusterName = ObjectKVStore::get($hook->instance, PHPRedisIntegration::KEY_CLUSTER_NAME)) { + $span->meta[PHPRedisIntegration::INTERNAL_ONLY_TAG_CLUSTER_NAME] = $clusterName; + } elseif ($firstHost = ObjectKVStore::get($hook->instance, PHPRedisIntegration::KEY_FIRST_HOST)) { + $span->meta[PHPRedisIntegration::INTERNAL_ONLY_TAG_FIRST_HOST] = $firstHost; + } + $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES; + }); + } + public static function traceMethodNoArgs($method) { \DDTrace\trace_method('Redis', $method, function (SpanData $span, $args) use ($method) { From 30eb9e8023eb7ab13353490b73415c9a096dc6bc Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Delhommeau Date: Mon, 30 Mar 2026 16:58:54 +0200 Subject: [PATCH 09/10] fix(predis): check lifecycle flag per-call instead of at init time Co-Authored-By: Claude Sonnet 4.6 --- .../Integrations/Predis/PredisIntegration.php | 105 ++++++++++-------- 1 file changed, 59 insertions(+), 46 deletions(-) diff --git a/src/DDTrace/Integrations/Predis/PredisIntegration.php b/src/DDTrace/Integrations/Predis/PredisIntegration.php index 7fe4f9c2d5..21f22f3321 100644 --- a/src/DDTrace/Integrations/Predis/PredisIntegration.php +++ b/src/DDTrace/Integrations/Predis/PredisIntegration.php @@ -28,35 +28,49 @@ class PredisIntegration extends Integration */ public static function init(): int { - $lifecycleEnabled = \dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED"); + // __construct: always store connection metadata (needed for executeCommand tags), + // but only create a span when lifecycle commands are enabled + \DDTrace\install_hook( + 'Predis\Client::__construct', + // Prehook: create span before constructor runs (so it covers the entire execution) + static function (HookData $hook) { + if (\dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED")) { + $hook->span(); // Create span now, before constructor body runs + $hook->data = true; + } + }, + // Posthook: constructor has completed, safe to call getConnection() + static function (HookData $hook) { + PredisIntegration::storeConnectionMetaAndService($hook->instance, $hook->args); - if ($lifecycleEnabled) { - \DDTrace\trace_method('Predis\Client', '__construct', function (SpanData $span, $args) { - Integration::handleOrphan($span); + if (!isset($hook->data)) { + return; + } + $span = $hook->span(); + Integration::handleOrphan($span); $span->name = 'Predis.Client.__construct'; $span->type = Type::REDIS; $span->resource = 'Predis.Client.__construct'; - PredisIntegration::storeConnectionMetaAndService($this, $args); - PredisIntegration::setMetaAndServiceFromConnection($this, $span); - }); - } else { - \DDTrace\install_hook('Predis\Client::__construct', null, static function (HookData $hook) { - PredisIntegration::storeConnectionMetaAndService($hook->instance, $hook->args); - }); - } + PredisIntegration::setMetaAndServiceFromConnection($hook->instance, $span); + } + ); - if ($lifecycleEnabled) { - \DDTrace\trace_method('Predis\Client', 'connect', function (SpanData $span, $args) { - Integration::handleOrphan($span); + // connect: lifecycle-only span + \DDTrace\install_hook('Predis\Client::connect', static function (HookData $hook) { + if (!\dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED")) { + return; + } - $span->name = 'Predis.Client.connect'; - $span->type = Type::REDIS; - $span->resource = 'Predis.Client.connect'; - PredisIntegration::setMetaAndServiceFromConnection($this, $span); - }); - } + $span = $hook->span(); + Integration::handleOrphan($span); + $span->name = 'Predis.Client.connect'; + $span->type = Type::REDIS; + $span->resource = 'Predis.Client.connect'; + PredisIntegration::setMetaAndServiceFromConnection($hook->instance, $span); + }); + // executeCommand: always traced (data command) \DDTrace\trace_method('Predis\Client', 'executeCommand', function (SpanData $span, $args) { Integration::handleOrphan($span); @@ -65,8 +79,6 @@ public static function init(): int PredisIntegration::setMetaAndServiceFromConnection($this, $span); PredisIntegration::addTraceAnalyticsIfEnabled($span); - // We default resource name to 'Predis.Client.executeCommand', but if we are able below to extract the query - // then we replace it with the query $span->resource = 'Predis.Client.executeCommand'; if (\count($args) == 0) { @@ -82,6 +94,7 @@ public static function init(): int $span->meta['redis.raw_command'] = $query; }); + // executeRaw: always traced (data command) \DDTrace\trace_method('Predis\Client', 'executeRaw', function (SpanData $span, $args) { Integration::handleOrphan($span); @@ -90,8 +103,6 @@ public static function init(): int PredisIntegration::setMetaAndServiceFromConnection($this, $span); PredisIntegration::addTraceAnalyticsIfEnabled($span); - // We default resource name to 'Predis.Client.executeRaw', but if we are able below to extract the query - // then we replace it with the query $span->resource = 'Predis.Client.executeRaw'; if (\count($args) == 0) { @@ -104,27 +115,29 @@ public static function init(): int $span->meta['redis.raw_command'] = $query; }); - if ($lifecycleEnabled) { - \DDTrace\trace_method( - 'Predis\Pipeline\Pipeline', - 'executePipeline', - [ - 'prehook' => function (SpanData $span, $args) { - Integration::handleOrphan($span); - - $span->name = 'Predis.Pipeline.executePipeline'; - $span->resource = $span->name; - $span->type = Type::REDIS; - PredisIntegration::setMetaAndServiceFromConnection($this->getClient(), $span); - if (\count($args) < 2) { - return; - } - $commands = $args[1]; - $span->meta['redis.pipeline_length'] = count($commands); - }, - ] - ); - } + // executePipeline: lifecycle-only span + \DDTrace\install_hook( + 'Predis\Pipeline\Pipeline::executePipeline', + static function (HookData $hook) { + if (!\dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED")) { + return; + } + + $span = $hook->span(); + Integration::handleOrphan($span); + $span->name = 'Predis.Pipeline.executePipeline'; + $span->resource = $span->name; + $span->type = Type::REDIS; + // getClient() is on the Pipeline instance + PredisIntegration::setMetaAndServiceFromConnection($hook->instance->getClient(), $span); + $args = $hook->args; + if (\count($args) < 2) { + return; + } + $commands = $args[1]; + $span->meta['redis.pipeline_length'] = count($commands); + } + ); return Integration::LOADED; } From 1c7c52efd8871fcd38e32809f241f5e44679cf49 Mon Sep 17 00:00:00 2001 From: Bob Weinand Date: Thu, 16 Apr 2026 16:18:20 +0200 Subject: [PATCH 10/10] Restore comment and deduplicate code --- .../PHPRedis/PHPRedisIntegration.php | 77 ++++--------------- 1 file changed, 16 insertions(+), 61 deletions(-) diff --git a/src/DDTrace/Integrations/PHPRedis/PHPRedisIntegration.php b/src/DDTrace/Integrations/PHPRedis/PHPRedisIntegration.php index 11b3fa5f07..b51097b8e6 100644 --- a/src/DDTrace/Integrations/PHPRedis/PHPRedisIntegration.php +++ b/src/DDTrace/Integrations/PHPRedis/PHPRedisIntegration.php @@ -41,6 +41,12 @@ public static function init(): int $instance = $hook->instance; $hostOrUDS = (isset($args[0]) && \is_string($args[0])) ? $args[0] : PHPRedisIntegration::DEFAULT_HOST; + // While we would have access to Redis::getHost() from the instance to retrieve it later, we compute the + // service name here and store in the instance for later because of the following reasons: + // - we would do over and over the same operation, involving a regex, for each method invocation. + // - in case of connection error, the Redis::host value is not set and we would not have access to it + // during callbacks, meaning that we would have to use two different ways to extract the name: args or + // Redis::getHost() depending on when we are interested in such information. // Always store metadata so data command spans have out.host ObjectKVStore::put($instance, PHPRedisIntegration::KEY_HOST, $hostOrUDS); @@ -374,10 +380,15 @@ private static function installLifecycleHookNoArgs($method) }); } - private static function installLifecycleHookAsCommand($method) + public static function installLifecycleHookAsCommand($method) { - \DDTrace\install_hook('Redis::' . $method, static function (HookData $hook) use ($method) { - if (!\dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED")) { + self::traceMethodAsCommand($method, true); + } + + public static function traceMethodAsCommand($method, $isLifeCycle = false) + { + \DDTrace\install_hook('Redis::' . $method, static function (HookData $hook) use ($method, $isLifeCycle) { + if ($isLifeCycle && !\dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED")) { return; } $span = $hook->span(); @@ -389,8 +400,8 @@ private static function installLifecycleHookAsCommand($method) $span->meta[Tag::TARGET_HOST] = $host; $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES; }); - \DDTrace\install_hook('RedisCluster::' . $method, static function (HookData $hook) use ($method) { - if (!\dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED")) { + \DDTrace\install_hook('RedisCluster::' . $method, static function (HookData $hook) use ($method, $isLifeCycle) { + if ($isLifeCycle && !\dd_trace_env_config("DD_TRACE_REDIS_LIFECYCLE_COMMANDS_ENABLED")) { return; } $span = $hook->span(); @@ -407,62 +418,6 @@ private static function installLifecycleHookAsCommand($method) }); } - public static function traceMethodNoArgs($method) - { - \DDTrace\trace_method('Redis', $method, function (SpanData $span, $args) use ($method) { - Integration::handleOrphan($span); - - PHPRedisIntegration::enrichSpan($span, $this, 'Redis', $method); - - $host = ObjectKVStore::get($this, PHPRedisIntegration::KEY_HOST); - $span->meta[Tag::TARGET_HOST] = $host; - $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES; - }); - \DDTrace\trace_method('RedisCluster', $method, function (SpanData $span, $args) use ($method) { - Integration::handleOrphan($span); - - PHPRedisIntegration::enrichSpan($span, $this, 'RedisCluster', $method); - if ($clusterName = ObjectKVStore::get($this, PHPRedisIntegration::KEY_CLUSTER_NAME)) { - $span->meta[PHPRedisIntegration::INTERNAL_ONLY_TAG_CLUSTER_NAME] = $clusterName; - } elseif ($firstHost = ObjectKVStore::get($this, PHPRedisIntegration::KEY_FIRST_HOST)) { - $span->meta[PHPRedisIntegration::INTERNAL_ONLY_TAG_FIRST_HOST] = $firstHost; - } - $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES; - }); - } - - public static function traceMethodAsCommand($method) - { - \DDTrace\trace_method('Redis', $method, function (SpanData $span, $args) use ($method) { - Integration::handleOrphan($span); - - PHPRedisIntegration::enrichSpan($span, $this, 'Redis', $method); - $normalizedArgs = PHPRedisIntegration::normalizeArgs($args); - // Obfuscable methods: see https://github.com/DataDog/datadog-agent/blob/master/pkg/trace/obfuscate/redis.go - $span->meta[Tag::REDIS_RAW_COMMAND] - = empty($normalizedArgs) ? $method : ($method . ' ' . $normalizedArgs); - - $host = ObjectKVStore::get($this, PHPRedisIntegration::KEY_HOST); - $span->meta[Tag::TARGET_HOST] = $host; - $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES; - }); - \DDTrace\trace_method('RedisCluster', $method, function (SpanData $span, $args) use ($method) { - Integration::handleOrphan($span); - - PHPRedisIntegration::enrichSpan($span, $this, 'RedisCluster', $method); - $normalizedArgs = PHPRedisIntegration::normalizeArgs($args); - // Obfuscable methods: see https://github.com/DataDog/datadog-agent/blob/master/pkg/trace/obfuscate/redis.go - $span->meta[Tag::REDIS_RAW_COMMAND] - = empty($normalizedArgs) ? $method : ($method . ' ' . $normalizedArgs); - if ($clusterName = ObjectKVStore::get($this, PHPRedisIntegration::KEY_CLUSTER_NAME)) { - $span->meta[PHPRedisIntegration::INTERNAL_ONLY_TAG_CLUSTER_NAME] = $clusterName; - } elseif ($firstHost = ObjectKVStore::get($this, PHPRedisIntegration::KEY_FIRST_HOST)) { - $span->meta[PHPRedisIntegration::INTERNAL_ONLY_TAG_FIRST_HOST] = $firstHost; - } - $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES; - }); - } - public static function storeClusterMeta($instance, $args) { if (isset($args[1]) && \is_array($args[1]) && !empty($args[1])) {