Skip to content

Commit

Permalink
fix: Missing Predis Service Override (#3098)
Browse files Browse the repository at this point in the history
  • Loading branch information
PROFeNoM authored Feb 20, 2025
1 parent 445f72a commit 2a55349
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 5 deletions.
10 changes: 7 additions & 3 deletions src/DDTrace/Integrations/Predis/PredisIntegration.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ public static function storeConnectionMetaAndService($predis, $args)
{
$tags = [];
$connection = $predis->getConnection();
$service = PredisIntegration::DEFAULT_SERVICE_NAME;

if ($connection instanceof NodeConnectionInterface) {
$connectionParameters = $connection->getParameters();
Expand All @@ -144,6 +143,7 @@ public static function storeConnectionMetaAndService($predis, $args)
? $connectionParameters->path
: $connectionParameters->host)
);
ObjectKVStore::put($predis->getConnection(), 'service', $service);
}
}

Expand All @@ -161,7 +161,6 @@ public static function storeConnectionMetaAndService($predis, $args)
}
}

ObjectKVStore::put($predis->getConnection(), 'service', $service);
ObjectKVStore::put($predis->getConnection(), 'connection_meta', $tags);
}

Expand All @@ -175,7 +174,12 @@ public static function storeConnectionMetaAndService($predis, $args)
*/
public static function setMetaAndServiceFromConnection($predis, SpanData $span)
{
$span->service = ObjectKVStore::get($predis->getConnection(), 'service', PredisIntegration::DEFAULT_SERVICE_NAME);
$service = ObjectKVStore::get($predis->getConnection(), 'service');
if ($service) {
$span->meta[Tag::SERVICE_NAME] = $service;
} else {
Integration::handleInternalSpanServiceName($span, PredisIntegration::DEFAULT_SERVICE_NAME);
}
$span->meta[Tag::SPAN_KIND] = 'client';
$span->meta[Tag::COMPONENT] = PredisIntegration::NAME;
$span->meta[Tag::DB_SYSTEM] = PredisIntegration::SYSTEM;
Expand Down
31 changes: 29 additions & 2 deletions tests/Integrations/Predis/Latest/PredisTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,21 @@ public static function ddSetUpBeforeClass()

protected function ddSetUp()
{
$this->putEnvAndReloadConfig(['DD_TRACE_REDIS_CLIENT_SPLIT_BY_HOST']);
$this->putEnvAndReloadConfig([
'DD_TRACE_REDIS_CLIENT_SPLIT_BY_HOST',
'DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED',
'DD_SERVICE',
]);
parent::ddSetUp();
}

protected function ddTearDown()
{
$this->putEnvAndReloadConfig(['DD_TRACE_REDIS_CLIENT_SPLIT_BY_HOST']);
$this->putEnvAndReloadConfig([
'DD_TRACE_REDIS_CLIENT_SPLIT_BY_HOST',
'DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED',
'DD_SERVICE',
]);
parent::ddTearDown();
}

Expand Down Expand Up @@ -357,6 +365,25 @@ public function testOrphansRemoval64bit()
$this->assertEquals(0, $span['metrics']['_sampling_priority_v1']);
}

public function testNoFakeServices()
{
$this->putEnvAndReloadConfig([
'DD_SERVICE=configured_service',
'DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED=true',
]);

$traces = $this->isolateTracer(function () {
$client = new \Predis\Client(["host" => $this->host]);
$client->connect();
});

$this->assertFlameGraph($traces, [
SpanAssertion::exists('Predis.Client.__construct'),
SpanAssertion::build('Predis.Client.connect', 'configured_service', 'redis', 'Predis.Client.connect')
->withExactTags($this->baseTags()),
]);
}

private function baseTags()
{
return [
Expand Down

0 comments on commit 2a55349

Please sign in to comment.