From 98c336baf3f1310d73c2ec93454d05c618ec25ae Mon Sep 17 00:00:00 2001 From: Enrico Zimuel Date: Thu, 19 Sep 2019 15:27:05 +0200 Subject: [PATCH 1/2] Added the HTTP port in logRequestSuccess and Fail --- src/Elasticsearch/Connections/Connection.php | 3 + .../Tests/ClientBuilder/DummyLogger.php | 50 ++++++++++++- .../Tests/ClientIntegrationTests.php | 73 +++++++++++++++---- 3 files changed, 111 insertions(+), 15 deletions(-) diff --git a/src/Elasticsearch/Connections/Connection.php b/src/Elasticsearch/Connections/Connection.php index 72dfc345c..540bcfe15 100644 --- a/src/Elasticsearch/Connections/Connection.php +++ b/src/Elasticsearch/Connections/Connection.php @@ -374,6 +374,7 @@ public function logRequestSuccess(array $request, array $response): void array( 'method' => $request['http_method'], 'uri' => $response['effective_url'], + 'port' => $response['transfer_stats']['primary_port'], 'headers' => $request['headers'], 'HTTP code' => $response['status'], 'duration' => $response['transfer_stats']['total_time'], @@ -408,11 +409,13 @@ public function logRequestSuccess(array $request, array $response): void public function logRequestFail(array $request, array $response, \Exception $exception): void { $this->log->debug('Request Body', array($request['body'])); + $this->log->warning( 'Request Failure:', array( 'method' => $request['http_method'], 'uri' => $response['effective_url'], + 'port' => $response['transfer_stats']['primary_port'], 'headers' => $request['headers'], 'HTTP code' => $response['status'], 'duration' => $response['transfer_stats']['total_time'], diff --git a/tests/Elasticsearch/Tests/ClientBuilder/DummyLogger.php b/tests/Elasticsearch/Tests/ClientBuilder/DummyLogger.php index f5c536c77..87f1e1d81 100644 --- a/tests/Elasticsearch/Tests/ClientBuilder/DummyLogger.php +++ b/tests/Elasticsearch/Tests/ClientBuilder/DummyLogger.php @@ -3,7 +3,55 @@ namespace Elasticsearch\Tests\ClientBuilder; -class DummyLogger +use Psr\Log\LoggerInterface; +use Psr\Log\LogLevel; + +class DummyLogger implements LoggerInterface { + public $output = []; + + public function emergency($message, array $context = array()) + { + $this->log(LogLevel::EMERGENCY, $message, $context); + } + + public function alert($message, array $context = array()) + { + $this->log(LogLevel::ALERT, $message, $context); + } + + public function critical($message, array $context = array()) + { + $this->log(LogLevel::CRITICAL, $message, $context); + } + + public function error($message, array $context = array()) + { + $this->log(LogLevel::ERROR, $message, $context); + } + + public function warning($message, array $context = array()) + { + $this->log(LogLevel::WARNING, $message, $context); + } + + public function notice($message, array $context = array()) + { + $this->log(LogLevel::NOTICE, $message, $context); + } + + public function info($message, array $context = array()) + { + $this->log(LogLevel::INFO, $message, $context); + } + + public function debug($message, array $context = array()) + { + $this->log(LogLevel::DEBUG, $message, $context); + } + public function log($level, $message, array $context = array()) + { + $this->output[] = sprintf("%s: %s %s", $level, $message, json_encode($context)); + } } diff --git a/tests/Elasticsearch/Tests/ClientIntegrationTests.php b/tests/Elasticsearch/Tests/ClientIntegrationTests.php index 55bc3b456..299ae7de6 100644 --- a/tests/Elasticsearch/Tests/ClientIntegrationTests.php +++ b/tests/Elasticsearch/Tests/ClientIntegrationTests.php @@ -4,7 +4,10 @@ namespace Elasticsearch\Tests; -use Elasticsearch; +use Elasticsearch\ClientBuilder; +use Elasticsearch\Common\Exceptions\Missing404Exception; +use Elasticsearch\Tests\ClientBuilder\DummyLogger; +use Psr\Log\LogLevel; /** * Class ClientTest @@ -18,22 +21,64 @@ */ class ClientIntegrationTests extends \PHPUnit\Framework\TestCase { - public function testCustomQueryParams() + public function setUp() { - $client = Elasticsearch\ClientBuilder::create() + if (empty(getenv('ES_TEST_HOST'))) { + $this->markTestSkipped('I cannot execute integration test without ES_TEST_HOST env'); + } + } + + public function testLogRequestSuccessHasInfoNotEmpty() + { + $logger = new DummyLogger(); + $client = ClientBuilder::create() + ->setHosts([getenv('ES_TEST_HOST')]) + ->setLogger($logger) + ->build(); + + $result = $client->info(); + + $this->assertNotEmpty($this->getLevelOutput(LogLevel::INFO, $logger->output)); + } + + public function testLogRequestSuccessHasPortInInfo() + { + $logger = new DummyLogger(); + $client = ClientBuilder::create() ->setHosts([getenv('ES_TEST_HOST')]) + ->setLogger($logger) ->build(); - $getParams = [ - 'index' => 'test', - 'type' => 'test', - 'id' => 1, - 'parent' => 'abc', - 'custom' => ['customToken' => 'abc', 'otherToken' => 123], - 'client' => ['ignore' => 400] - ]; - $exists = $client->exists($getParams); - - $this->assertFalse((bool) $exists); + $result = $client->info(); + + $this->assertContains('"port"', $this->getLevelOutput(LogLevel::INFO, $logger->output)); + } + + public function testLogRequestFailHasWarning() + { + $logger = new DummyLogger(); + $client = ClientBuilder::create() + ->setHosts([getenv('ES_TEST_HOST')]) + ->setLogger($logger) + ->build(); + + try { + $result = $client->get([ + 'index' => 'foo', + 'id' => 'bar' + ]); + } catch (Missing404Exception $e) { + $this->assertNotEmpty($this->getLevelOutput(LogLevel::WARNING, $logger->output)); + } + } + + private function getLevelOutput(string $level, array $output): string + { + foreach ($output as $out) { + if (false !== strpos($out, $level)) { + return $out; + } + } + return ''; } } From f22755b51af581acfa90766ff920403c8c65a4a6 Mon Sep 17 00:00:00 2001 From: Enrico Zimuel Date: Thu, 19 Sep 2019 15:56:30 +0200 Subject: [PATCH 2/2] Changed DummyLogger in favor of ArrayLogger --- .../Tests/ClientBuilder/ArrayLogger.php | 57 +++++++++++++++++++ .../Tests/ClientBuilder/DummyLogger.php | 52 +---------------- .../Tests/ClientIntegrationTests.php | 18 +++--- 3 files changed, 67 insertions(+), 60 deletions(-) create mode 100644 tests/Elasticsearch/Tests/ClientBuilder/ArrayLogger.php diff --git a/tests/Elasticsearch/Tests/ClientBuilder/ArrayLogger.php b/tests/Elasticsearch/Tests/ClientBuilder/ArrayLogger.php new file mode 100644 index 000000000..3a9010794 --- /dev/null +++ b/tests/Elasticsearch/Tests/ClientBuilder/ArrayLogger.php @@ -0,0 +1,57 @@ +log(LogLevel::EMERGENCY, $message, $context); + } + + public function alert($message, array $context = array()) + { + $this->log(LogLevel::ALERT, $message, $context); + } + + public function critical($message, array $context = array()) + { + $this->log(LogLevel::CRITICAL, $message, $context); + } + + public function error($message, array $context = array()) + { + $this->log(LogLevel::ERROR, $message, $context); + } + + public function warning($message, array $context = array()) + { + $this->log(LogLevel::WARNING, $message, $context); + } + + public function notice($message, array $context = array()) + { + $this->log(LogLevel::NOTICE, $message, $context); + } + + public function info($message, array $context = array()) + { + $this->log(LogLevel::INFO, $message, $context); + } + + public function debug($message, array $context = array()) + { + $this->log(LogLevel::DEBUG, $message, $context); + } + + public function log($level, $message, array $context = array()) + { + $this->output[] = sprintf("%s: %s %s", $level, $message, json_encode($context)); + } +} diff --git a/tests/Elasticsearch/Tests/ClientBuilder/DummyLogger.php b/tests/Elasticsearch/Tests/ClientBuilder/DummyLogger.php index 87f1e1d81..6887853c2 100644 --- a/tests/Elasticsearch/Tests/ClientBuilder/DummyLogger.php +++ b/tests/Elasticsearch/Tests/ClientBuilder/DummyLogger.php @@ -3,55 +3,7 @@ namespace Elasticsearch\Tests\ClientBuilder; -use Psr\Log\LoggerInterface; -use Psr\Log\LogLevel; - -class DummyLogger implements LoggerInterface +class DummyLogger { - public $output = []; - - public function emergency($message, array $context = array()) - { - $this->log(LogLevel::EMERGENCY, $message, $context); - } - - public function alert($message, array $context = array()) - { - $this->log(LogLevel::ALERT, $message, $context); - } - - public function critical($message, array $context = array()) - { - $this->log(LogLevel::CRITICAL, $message, $context); - } - - public function error($message, array $context = array()) - { - $this->log(LogLevel::ERROR, $message, $context); - } - - public function warning($message, array $context = array()) - { - $this->log(LogLevel::WARNING, $message, $context); - } - - public function notice($message, array $context = array()) - { - $this->log(LogLevel::NOTICE, $message, $context); - } - - public function info($message, array $context = array()) - { - $this->log(LogLevel::INFO, $message, $context); - } - - public function debug($message, array $context = array()) - { - $this->log(LogLevel::DEBUG, $message, $context); - } - - public function log($level, $message, array $context = array()) - { - $this->output[] = sprintf("%s: %s %s", $level, $message, json_encode($context)); - } + } diff --git a/tests/Elasticsearch/Tests/ClientIntegrationTests.php b/tests/Elasticsearch/Tests/ClientIntegrationTests.php index 299ae7de6..cf36725fe 100644 --- a/tests/Elasticsearch/Tests/ClientIntegrationTests.php +++ b/tests/Elasticsearch/Tests/ClientIntegrationTests.php @@ -6,7 +6,7 @@ use Elasticsearch\ClientBuilder; use Elasticsearch\Common\Exceptions\Missing404Exception; -use Elasticsearch\Tests\ClientBuilder\DummyLogger; +use Elasticsearch\Tests\ClientBuilder\ArrayLogger; use Psr\Log\LogLevel; /** @@ -26,40 +26,38 @@ public function setUp() if (empty(getenv('ES_TEST_HOST'))) { $this->markTestSkipped('I cannot execute integration test without ES_TEST_HOST env'); } + $this->logger = new ArrayLogger(); } public function testLogRequestSuccessHasInfoNotEmpty() { - $logger = new DummyLogger(); $client = ClientBuilder::create() ->setHosts([getenv('ES_TEST_HOST')]) - ->setLogger($logger) + ->setLogger($this->logger) ->build(); $result = $client->info(); - $this->assertNotEmpty($this->getLevelOutput(LogLevel::INFO, $logger->output)); + $this->assertNotEmpty($this->getLevelOutput(LogLevel::INFO, $this->logger->output)); } public function testLogRequestSuccessHasPortInInfo() { - $logger = new DummyLogger(); $client = ClientBuilder::create() ->setHosts([getenv('ES_TEST_HOST')]) - ->setLogger($logger) + ->setLogger($this->logger) ->build(); $result = $client->info(); - $this->assertContains('"port"', $this->getLevelOutput(LogLevel::INFO, $logger->output)); + $this->assertContains('"port"', $this->getLevelOutput(LogLevel::INFO, $this->logger->output)); } public function testLogRequestFailHasWarning() { - $logger = new DummyLogger(); $client = ClientBuilder::create() ->setHosts([getenv('ES_TEST_HOST')]) - ->setLogger($logger) + ->setLogger($this->logger) ->build(); try { @@ -68,7 +66,7 @@ public function testLogRequestFailHasWarning() 'id' => 'bar' ]); } catch (Missing404Exception $e) { - $this->assertNotEmpty($this->getLevelOutput(LogLevel::WARNING, $logger->output)); + $this->assertNotEmpty($this->getLevelOutput(LogLevel::WARNING, $this->logger->output)); } }