Tim Starling has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/350787 )

Change subject: Improve HTTP logging
......................................................................

Improve HTTP logging

* Log HTTP debug lines to the "http" channel instead of wfDebug()
* Add the ability to do debug logging to MultiHttpClient
* Add a static method Http::createMultiClient() which provides a
  MultiHttpClient configured similarly to the way individual requests
  are configured, respecting the wiki's $wgHTTPTimeout and writing debug
  logs.
* In EtcdConfig, pass the logger instance through to the MultiHttpClient
  backend.

Change-Id: Ic5bdcb0cae95d7b3715ab5261758be082751c3ff
---
M includes/config/EtcdConfig.php
M includes/http/Http.php
M includes/libs/MultiHttpClient.php
3 files changed, 60 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/87/350787/1

diff --git a/includes/config/EtcdConfig.php b/includes/config/EtcdConfig.php
index d3fbd65..1a488bd 100644
--- a/includes/config/EtcdConfig.php
+++ b/includes/config/EtcdConfig.php
@@ -23,6 +23,7 @@
 
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerInterface;
+use MediaWiki\Logger\LoggerFactory;
 use Wikimedia\WaitConditionLoop;
 
 /**
@@ -98,12 +99,14 @@
                $this->logger = new Psr\Log\NullLogger();
                $this->http = new MultiHttpClient( [
                        'connTimeout' => $this->timeout,
-                       'reqTimeout' => $this->timeout
+                       'reqTimeout' => $this->timeout,
+                       'logger' => $this->logger
                ] );
        }
 
        public function setLogger( LoggerInterface $logger ) {
                $this->logger = $logger;
+               $this->http->setLogger( $logger );
        }
 
        public function has( $name ) {
diff --git a/includes/http/Http.php b/includes/http/Http.php
index 889cb60..4f21ce2 100644
--- a/includes/http/Http.php
+++ b/includes/http/Http.php
@@ -59,7 +59,8 @@
         * @return string|bool (bool)false on failure or a string on success
         */
        public static function request( $method, $url, $options = [], $caller = 
__METHOD__ ) {
-               wfDebug( "HTTP: $method: $url\n" );
+               $logger = LoggerFactory::getInstance( 'http' );
+               $logger->debug( "$method: $url" );
 
                $options['method'] = strtoupper( $method );
 
@@ -77,7 +78,6 @@
                        return $req->getContent();
                } else {
                        $errors = $status->getErrorsByType( 'error' );
-                       $logger = LoggerFactory::getInstance( 'http' );
                        $logger->warning( Status::wrap( $status )->getWikiText( 
false, false, 'en' ),
                                [ 'error' => $errors, 'caller' => $caller, 
'content' => $req->getContent() ] );
                        return false;
@@ -164,4 +164,20 @@
 
                return "";
        }
+
+       /**
+        * Get a configured MultiHttpClient
+        * @param array $options
+        */
+       public static function createMultiClient( $options = [] ) {
+               global $wgHTTPConnectTimeout, $wgHTTPTimeout, $wgHTTPProxy;
+
+               return new MultiHttpClient( $options + [
+                       'connTimeout' => $wgHTTPConnectTimeout,
+                       'reqTimeout' => $wgHTTPTimeout,
+                       'userAgent' => self::userAgent(),
+                       'proxy' => $wgHTTPProxy,
+                       'logger' => LoggerFactory::getInstance( 'http' )
+               ] );
+       }
 }
diff --git a/includes/libs/MultiHttpClient.php 
b/includes/libs/MultiHttpClient.php
index f0b44a5..b077059 100644
--- a/includes/libs/MultiHttpClient.php
+++ b/includes/libs/MultiHttpClient.php
@@ -59,6 +59,8 @@
        protected $proxy;
        /** @var string */
        protected $userAgent = 'wikimedia/multi-http-client v1.0';
+       /** @var Psr\Log\LoggerInterface */
+       protected $logger;
 
        /**
         * @param array $options
@@ -78,7 +80,8 @@
                        }
                }
                static $opts = [
-                       'connTimeout', 'reqTimeout', 'usePipelining', 
'maxConnsPerHost', 'proxy', 'userAgent'
+                       'connTimeout', 'reqTimeout', 'usePipelining', 
'maxConnsPerHost',
+                       'proxy', 'userAgent', 'logger'
                ];
                foreach ( $opts as $key ) {
                        if ( isset( $options[$key] ) ) {
@@ -162,6 +165,7 @@
                        } elseif ( !isset( $req['url'] ) ) {
                                throw new Exception( "Request has no 'url' 
field set." );
                        }
+                       $this->logDebug( "{$req['method']}: {$req['url']}" );
                        $req['query'] = isset( $req['query'] ) ? $req['query'] 
: [];
                        $headers = []; // normalized headers
                        if ( isset( $req['headers'] ) ) {
@@ -235,6 +239,8 @@
                                        if ( function_exists( 'curl_strerror' ) 
) {
                                                $req['response']['error'] .= " 
" . curl_strerror( $errno );
                                        }
+                                       $this->logWarning( "Error fetching URL 
\"{$req['url']}\": " .
+                                               $req['response']['error'] );
                                }
                        } else {
                                $req['response']['error'] = "(curl error: no 
status set)";
@@ -420,6 +426,37 @@
                return $this->multiHandle;
        }
 
+       /**
+        * Log a message with debug priority
+        *
+        * @param string $message
+        */
+       protected function logDebug( $message ) {
+               if ( $this->logger ) {
+                       $this->logger->debug( $message );
+               }
+       }
+
+       /**
+        * Log a message with warning priority
+        *
+        * @param string $message
+        */
+       protected function logWarning( $message ) {
+               if ( $this->logger ) {
+                       $this->logger->warning( $message );
+               }
+       }
+
+       /**
+        * Register a logger
+        *
+        * @param Psr\Log\LoggerInterface
+        */
+       public function setLogger( Psr\Log\LoggerInterface $logger ) {
+               $this->logger = $logger;
+       }
+
        function __destruct() {
                if ( $this->multiHandle ) {
                        curl_multi_close( $this->multiHandle );

-- 
To view, visit https://gerrit.wikimedia.org/r/350787
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic5bdcb0cae95d7b3715ab5261758be082751c3ff
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Tim Starling <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to