Skip to content

Commit 3d041a4

Browse files
merkruflin
merk
authored and
ruflin
committed
Simplify Client logger (#1069)
* Improve Client Logging * Update Client to mitigate _log being overwritten
1 parent 832d570 commit 3d041a4

File tree

4 files changed

+119
-51
lines changed

4 files changed

+119
-51
lines changed

‎CHANGELOG.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@ All notable changes to this project will be documented in this file based on the
1313
### Improvements
1414
- `Elastica\Type->deleteByQuery($query, $options)` $query param can be a query `array` again https://github.com/ruflin/Elastica/issues/1072
1515
- `Elastica\Client->connect()` allows to establish a connection to ES server when the config was set using method `Elastica\Client->setConfigValue()` https://github.com/ruflin/Elastica/issues/1076
16+
- Elastica\Client constructor now accepts a LoggerInterface and will log both successful and failed requests. #1069
1617

1718
### Deprecated
18-
19+
- Configuring the logger in \Elastica\Client $config constructor is deprecated and will be removed. Use the $logger argument instead.
1920

2021
## [3.1.1](https://github.com/ruflin/Elastica/compare/3.1.0...3.1.1)
2122

‎lib/Elastica/Client.php

+63-46
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55
use Elastica\Bulk\Action;
66
use Elastica\Exception\ConnectionException;
77
use Elastica\Exception\InvalidException;
8-
use Elastica\Exception\RuntimeException;
98
use Elastica\Script\AbstractScript;
109
use Psr\Log\LoggerInterface;
10+
use Psr\Log\NullLogger;
1111

1212
/**
1313
* Client to connect the the elasticsearch server.
@@ -46,7 +46,7 @@ class Client
4646
/**
4747
* @var callback
4848
*/
49-
protected $_callback = null;
49+
protected $_callback;
5050

5151
/**
5252
* @var \Elastica\Request
@@ -61,22 +61,30 @@ class Client
6161
/**
6262
* @var LoggerInterface
6363
*/
64-
protected $_logger = null;
64+
protected $_logger;
65+
6566
/**
6667
* @var Connection\ConnectionPool
6768
*/
68-
protected $_connectionPool = null;
69+
protected $_connectionPool;
6970

7071
/**
7172
* Creates a new Elastica client.
7273
*
73-
* @param array $config OPTIONAL Additional config options
74+
* @param array $config OPTIONAL Additional config options
7475
* @param callback $callback OPTIONAL Callback function which can be used to be notified about errors (for example connection down)
76+
* @param LoggerInterface $logger
7577
*/
76-
public function __construct(array $config = array(), $callback = null)
78+
public function __construct(array $config = array(), $callback = null, LoggerInterface $logger = null)
7779
{
78-
$this->setConfig($config);
7980
$this->_callback = $callback;
81+
82+
if (!$logger && isset($config['log']) && $config['log']) {
83+
$logger = new Log($config['log']);
84+
}
85+
$this->_logger = $logger ?: new NullLogger();
86+
87+
$this->setConfig($config);
8088
$this->_initConnections();
8189
}
8290

@@ -622,32 +630,66 @@ public function bulk(array $params)
622630
*
623631
* @throws Exception\ConnectionException|\Exception
624632
*
625-
* @return \Elastica\Response Response object
633+
* @return Response Response object
626634
*/
627635
public function request($path, $method = Request::GET, $data = array(), array $query = array())
628636
{
629637
$connection = $this->getConnection();
630-
try {
631-
$request = new Request($path, $method, $data, $query, $connection);
632-
633-
$this->_log($request);
634-
635-
$response = $request->send();
636-
637-
$this->_lastRequest = $request;
638-
$this->_lastResponse = $response;
638+
$request = $this->_lastRequest = new Request($path, $method, $data, $query, $connection);
639+
$this->_lastResponse = null;
639640

640-
return $response;
641+
try {
642+
$response = $this->_lastResponse = $request->send();
641643
} catch (ConnectionException $e) {
642644
$this->_connectionPool->onFail($connection, $e, $this);
643645

646+
$this->_log($e);
647+
644648
// In case there is no valid connection left, throw exception which caused the disabling of the connection.
645649
if (!$this->hasConnection()) {
646650
throw $e;
647651
}
648652

649653
return $this->request($path, $method, $data, $query);
650654
}
655+
656+
$this->_log($request, $response);
657+
658+
return $response;
659+
}
660+
661+
/**
662+
* logging.
663+
*
664+
* @deprecated Overwriting Client->_log is deprecated. Handle logging functionality by using a custom LoggerInterface.
665+
*
666+
* @param mixed $context
667+
*/
668+
protected function _log($context)
669+
{
670+
if ($context instanceof ConnectionException) {
671+
$this->_logger->error('Elastica Request Failure', [
672+
'exception' => $context,
673+
'request' => $context->getRequest()->toArray(),
674+
'retry' => $this->hasConnection()
675+
]);
676+
677+
return;
678+
}
679+
680+
if ($context instanceof Request) {
681+
$this->_logger->debug('Elastica Request', [
682+
'request' => $context->toArray(),
683+
'response' => $this->_lastResponse ? $this->_lastResponse->getData() : null,
684+
'responseStatus' => $this->_lastResponse ? $this->_lastResponse->getStatus() : null
685+
]);
686+
687+
return;
688+
}
689+
690+
$this->_logger->debug('Elastica Request', [
691+
'message' => $context
692+
]);
651693
}
652694

653695
/**
@@ -677,48 +719,23 @@ public function refreshAll()
677719
}
678720

679721
/**
680-
* logging.
681-
*
682-
* @param string|\Elastica\Request $context
683-
*
684-
* @throws Exception\RuntimeException
685-
*/
686-
protected function _log($context)
687-
{
688-
$log = $this->getConfig('log');
689-
if ($log && !class_exists('Psr\Log\AbstractLogger')) {
690-
throw new RuntimeException('Class Psr\Log\AbstractLogger not found');
691-
} elseif (!$this->_logger && $log) {
692-
$this->setLogger(new Log($this->getConfig('log')));
693-
}
694-
if ($this->_logger) {
695-
if ($context instanceof Request) {
696-
$data = $context->toArray();
697-
} else {
698-
$data = array('message' => $context);
699-
}
700-
$this->_logger->debug('logging Request', $data);
701-
}
702-
}
703-
704-
/**
705-
* @return \Elastica\Request
722+
* @return Request
706723
*/
707724
public function getLastRequest()
708725
{
709726
return $this->_lastRequest;
710727
}
711728

712729
/**
713-
* @return \Elastica\Response
730+
* @return Response
714731
*/
715732
public function getLastResponse()
716733
{
717734
return $this->_lastResponse;
718735
}
719736

720737
/**
721-
* set Logger.
738+
* Replace the existing logger.
722739
*
723740
* @param LoggerInterface $logger
724741
*

‎test/lib/Elastica/Test/Base.php

+5-3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Elastica\Client;
66
use Elastica\Connection;
77
use Elastica\Index;
8+
use Psr\Log\LoggerInterface;
89

910
class Base extends \PHPUnit_Framework_TestCase
1011
{
@@ -54,12 +55,13 @@ protected function finishCollectErrors()
5455
}
5556

5657
/**
57-
* @param array $params Additional configuration params. Host and Port are already set
58+
* @param array $params Additional configuration params. Host and Port are already set
5859
* @param callback $callback
60+
* @param LoggerInterface $logger
5961
*
6062
* @return Client
6163
*/
62-
protected function _getClient(array $params = array(), $callback = null)
64+
protected function _getClient(array $params = array(), $callback = null, LoggerInterface $logger = null)
6365
{
6466
$config = array(
6567
'host' => $this->_getHost(),
@@ -68,7 +70,7 @@ protected function _getClient(array $params = array(), $callback = null)
6870

6971
$config = array_merge($config, $params);
7072

71-
return new Client($config, $callback);
73+
return new Client($config, $callback, $logger);
7274
}
7375

7476
/**

‎test/lib/Elastica/Test/ClientTest.php

+49-1
Original file line numberDiff line numberDiff line change
@@ -1180,7 +1180,7 @@ public function testClientConnectWithConfigSetByMethod()
11801180
{
11811181
$client = new Client();
11821182
$client->setConfigValue('host', $this->_getHost());
1183-
$client->setConfigValue('port',$this->_getPort());
1183+
$client->setConfigValue('port', $this->_getPort());
11841184

11851185
$client->connect();
11861186
$this->assertTrue($client->hasConnection());
@@ -1190,4 +1190,52 @@ public function testClientConnectWithConfigSetByMethod()
11901190
$this->assertEquals($this->_getHost(), $connection->getHost());
11911191
$this->assertEquals($this->_getPort(), $connection->getPort());
11921192
}
1193+
1194+
/**
1195+
* @group functional
1196+
*/
1197+
public function testLogger()
1198+
{
1199+
$logger = $this->getMock('Psr\\Log\\LoggerInterface');
1200+
$client = $this->_getClient([], null, $logger);
1201+
1202+
$logger->expects($this->once())
1203+
->method('debug')
1204+
->with(
1205+
'Elastica Request',
1206+
$this->logicalAnd(
1207+
$this->arrayHasKey('request'),
1208+
$this->arrayHasKey('response'),
1209+
$this->arrayHasKey('responseStatus')
1210+
)
1211+
);
1212+
1213+
$client->request('_stats', Request::GET);
1214+
}
1215+
1216+
/**
1217+
* @expectedException \Elastica\Exception\Connection\HttpException
1218+
* @group functional
1219+
*/
1220+
public function testLoggerOnFailure()
1221+
{
1222+
$logger = $this->getMock('Psr\\Log\\LoggerInterface');
1223+
$client = $this->_getClient(array('connections' => array(
1224+
array('host' => $this->_getHost(), 'port' => 9201),
1225+
)), null, $logger);
1226+
1227+
$logger->expects($this->once())
1228+
->method('error')
1229+
->with(
1230+
'Elastica Request Failure',
1231+
$this->logicalAnd(
1232+
$this->arrayHasKey('exception'),
1233+
$this->arrayHasKey('request'),
1234+
$this->arrayHasKey('retry'),
1235+
$this->logicalNot($this->arrayHasKey('response'))
1236+
)
1237+
);
1238+
1239+
$client->request('_stats', Request::GET);
1240+
}
11931241
}

0 commit comments

Comments
 (0)