fix: no faulty host resolving (#4)

contributors: @mmross @cHeeSaW
auxmoney · Feb 6, 2020 · 6cb95c9 · 6cb95c9
1 parent 5920561
commit 6cb95c9
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 11 deletions.
diff --git a/Factory/ZipkinTracerFactory.php b/Factory/ZipkinTracerFactory.php
@@ -17,9 +17,11 @@
 final class ZipkinTracerFactory implements TracerFactory
 {
     private $logger;
+    private $agentHostResolver;
 
-    public function __construct(LoggerInterface $logger)
+    public function __construct(AgentHostResolver $agentHostResolver, LoggerInterface $logger)
     {
+        $this->agentHostResolver = $agentHostResolver;
         $this->logger = $logger;
     }
 
@@ -30,12 +32,8 @@ public function create(string $projectName, string $agentHost, string $agentPort
     {
         $tracer = new NoopTracer();
 
-        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);
             $endpoint = Endpoint::create($projectName, gethostbyname($agentHost), null, (int) $agentPort);
             $reporter = new Http();
             $sampler = BinarySampler::createAsAlwaysSample();

diff --git a/Tests/Factory/ZipkinTracerFactoryTest.php b/Tests/Factory/ZipkinTracerFactoryTest.php
@@ -4,15 +4,18 @@
 
 namespace Auxmoney\OpentracingBundle\Tests\Factory;
 
+use Auxmoney\OpentracingBundle\Factory\AgentHostResolver;
 use Auxmoney\OpentracingBundle\Factory\ZipkinTracerFactory;
 use OpenTracing\NoopTracer;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Psr\Log\LoggerInterface;
+use RuntimeException;
 use ZipkinOpenTracing\Tracer;
 
 class ZipkinTracerFactoryTest extends TestCase
 {
+    private $agentHostResolver;
     private $logger;
     private $projectName;
     private $agentHost;
@@ -23,27 +26,30 @@ public function setUp()
     {
         parent::setUp();
         $this->logger = $this->prophesize(LoggerInterface::class);
+        $this->agentHostResolver = $this->prophesize(AgentHostResolver::class);
         $this->projectName = 'project name';
         $this->agentHost = 'localhost';
         $this->agentPort = '9411';
 
-        $this->subject = new ZipkinTracerFactory($this->logger->reveal());
+        $this->subject = new ZipkinTracerFactory($this->agentHostResolver->reveal(), $this->logger->reveal());
     }
 
     public function testCreateSuccess(): void
     {
+        $this->agentHostResolver->ensureAgentHostIsResolvable('localhost')->shouldBeCalled();
         $this->logger->warning(Argument::type('string'))->shouldNotBeCalled();
 
         self::assertInstanceOf(Tracer::class, $this->subject->create($this->projectName, $this->agentHost, $this->agentPort));
     }
 
-    public function testCreateNoDnsOrIp(): void
+    public function testCreateResolvingFailed(): void
     {
-        $this->logger->warning(Argument::containingString('could not resolve agent host'))->shouldBeCalledOnce();
+        $this->agentHostResolver->ensureAgentHostIsResolvable('localhost')->shouldBeCalled()->willThrow(new RuntimeException('resolving failer'));
+        $this->logger->warning(Argument::containingString('resolving failer'))->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",
     "jcchavezs/zipkin-opentracing": "^0.1"
   },