fix: no faulty host resolving (#4)

contributors: @mmross @cHeeSaW
auxmoney · Feb 6, 2020 · 68d3d08 · 68d3d08
1 parent 3891bb1
commit 68d3d08
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 33 deletions.
diff --git a/Factory/JaegerTracerFactory.php b/Factory/JaegerTracerFactory.php
@@ -12,13 +12,16 @@
 final class JaegerTracerFactory implements TracerFactory
 {
     private $jaegerConfigFactory;
+    private $agentHostResolver;
     private $logger;
 
     public function __construct(
         JaegerConfigFactory $jaegerConfigFactory,
+        AgentHostResolver $agentHostResolver,
         LoggerInterface $logger
     ) {
         $this->jaegerConfigFactory = $jaegerConfigFactory;
+        $this->agentHostResolver = $agentHostResolver;
         $this->logger = $logger;
     }
 
@@ -28,12 +31,8 @@ public function create(string $projectName, string $agentHost, string $agentPort
 
         $config = $this->jaegerConfigFactory->create();
 
-        if (!dns_get_record($agentHost) && !filter_var($agentHost, FILTER_VALIDATE_IP)) {
-            $this->logger->warning(self::class . ': could not resolve agent host "' . $agentHost . '"');
-            return $tracer;
-        }
-
         try {
+            $this->agentHostResolver->ensureAgentHostIsResolvable($agentHost);
             $config->gen128bit();
             $configuredTracer = $config->initTracer($projectName, $agentHost . ':' . $agentPort);
             if ($configuredTracer) {

diff --git a/Tests/Factory/JaegerTracerFactoryTest.php b/Tests/Factory/JaegerTracerFactoryTest.php
@@ -4,6 +4,7 @@
 
 namespace Auxmoney\OpentracingBundle\Tests\Factory;
 
+use Auxmoney\OpentracingBundle\Factory\AgentHostResolver;
 use Auxmoney\OpentracingBundle\Factory\JaegerConfigFactory;
 use Auxmoney\OpentracingBundle\Factory\JaegerTracerFactory;
 use Exception;
@@ -13,9 +14,11 @@
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Psr\Log\LoggerInterface;
+use RuntimeException;
 
 class JaegerTracerFactoryTest extends TestCase
 {
+    private $agentHostResolver;
     private $jaegerConfigFactory;
     private $logger;
     private $projectName;
@@ -30,60 +33,41 @@ public function setUp()
         $this->projectName = 'project name';
         $this->agentHost = 'localhost';
         $this->agentPort = '6831';
+        $this->agentHostResolver = $this->prophesize(AgentHostResolver::class);
         $this->jaegerConfigFactory = $this->prophesize(JaegerConfigFactory::class);
 
         $this->subject = new JaegerTracerFactory(
             $this->jaegerConfigFactory->reveal(),
+            $this->agentHostResolver->reveal(),
             $this->logger->reveal()
         );
     }
 
-    public function testCreateSuccessDns(): void
+    public function testCreateSuccess(): void
     {
         $tracer = $this->prophesize(Jaeger::class);
         $config = $this->prophesize(Config::class);
         $config->initTracer('project name', Argument::type('string'))->willReturn($tracer->reveal());
         $config->gen128bit()->shouldBeCalled();
         $this->jaegerConfigFactory->create()->willReturn($config->reveal());
 
+        $this->agentHostResolver->ensureAgentHostIsResolvable('localhost')->shouldBeCalled();
         $this->logger->warning(Argument::type('string'))->shouldNotBeCalled();
 
         self::assertSame($tracer->reveal(), $this->subject->create($this->projectName, $this->agentHost, $this->agentPort));
     }
 
-    public function testCreateSuccessIp(): void
+    public function testCreateResolvingFailed(): void
     {
-        $this->subject = new JaegerTracerFactory(
-            $this->jaegerConfigFactory->reveal(),
-            $this->logger->reveal()
-        );
-
-        $tracer = $this->prophesize(Jaeger::class);
-        $config = $this->prophesize(Config::class);
-        $config->initTracer('project name', Argument::type('string'))->willReturn($tracer->reveal());
-        $config->gen128bit()->shouldBeCalled();
-        $this->jaegerConfigFactory->create()->willReturn($config->reveal());
-
-        $this->logger->warning(Argument::type('string'))->shouldNotBeCalled();
-
-        self::assertSame($tracer->reveal(), $this->subject->create($this->projectName, '127.0.0.1', $this->agentPort));
-    }
-
-    public function testCreateNoDnsOrIp(): void
-    {
-        $this->subject = new JaegerTracerFactory(
-            $this->jaegerConfigFactory->reveal(),
-            $this->logger->reveal()
-        );
-
         $config = $this->prophesize(Config::class);
         $this->jaegerConfigFactory->create()->willReturn($config->reveal());
 
-        $this->logger->warning(Argument::containingString('could not resolve agent host'))->shouldBeCalledOnce();
+        $this->agentHostResolver->ensureAgentHostIsResolvable('localhost')->shouldBeCalled()->willThrow(new RuntimeException('resolving failed'));
+        $this->logger->warning(Argument::containingString('resolving failed'))->shouldBeCalledOnce();
 
         self::assertInstanceOf(
             NoopTracer::class,
-            $this->subject->create($this->projectName, 'älsakfdkaofkeäkvaäsooäaegölsgälkfdvpaoskvä.cöm', $this->agentPort)
+            $this->subject->create($this->projectName, $this->agentHost, $this->agentPort)
         );
     }
 

diff --git a/composer.json b/composer.json
@@ -18,7 +18,7 @@
   },
   "require": {
     "php": "^7.1.33",
-    "auxmoney/opentracing-bundle-core": "^0.3.3",
+    "auxmoney/opentracing-bundle-core": "^0.3.5",
     "opentracing/opentracing": "1.0.0-beta5@beta",
     "jukylin/jaeger-php": "dev-master@dev"
   },