From: Tim Starling <tstarling@wikimedia.org>
Date: Fri, 28 Apr 2017 01:32:44 +0000 (+1000)
Subject: Improve HTTP logging
X-Git-Tag: 1.31.0-rc.0~3225^2
X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=cde44feba3c6fea041a179e357cea958d6399447

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
---

diff --git a/includes/config/EtcdConfig.php b/includes/config/EtcdConfig.php
index 880cf9ff8c..ae3df4996f 100644
--- a/includes/config/EtcdConfig.php
+++ b/includes/config/EtcdConfig.php
@@ -95,12 +95,14 @@ class EtcdConfig implements Config, LoggerAwareInterface {
 		$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 889cb60316..4f21ce2775 100644
--- a/includes/http/Http.php
+++ b/includes/http/Http.php
@@ -59,7 +59,8 @@ class Http {
 	 * @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 @@ class Http {
 			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 @@ class Http {
 
 		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 f0b44a5082..e9922b9edf 100644
--- a/includes/libs/MultiHttpClient.php
+++ b/includes/libs/MultiHttpClient.php
@@ -20,6 +20,10 @@
  * @file
  */
 
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
+
 /**
  * Class to handle concurrent HTTP requests
  *
@@ -42,7 +46,7 @@
  * @author Aaron Schulz
  * @since 1.23
  */
-class MultiHttpClient {
+class MultiHttpClient implements LoggerAwareInterface {
 	/** @var resource */
 	protected $multiHandle = null; // curl_multi handle
 	/** @var string|null SSL certificates path  */
@@ -59,6 +63,8 @@ class MultiHttpClient {
 	protected $proxy;
 	/** @var string */
 	protected $userAgent = 'wikimedia/multi-http-client v1.0';
+	/** @var LoggerInterface */
+	protected $logger;
 
 	/**
 	 * @param array $options
@@ -78,13 +84,17 @@ class MultiHttpClient {
 			}
 		}
 		static $opts = [
-			'connTimeout', 'reqTimeout', 'usePipelining', 'maxConnsPerHost', 'proxy', 'userAgent'
+			'connTimeout', 'reqTimeout', 'usePipelining', 'maxConnsPerHost',
+			'proxy', 'userAgent', 'logger'
 		];
 		foreach ( $opts as $key ) {
 			if ( isset( $options[$key] ) ) {
 				$this->$key = $options[$key];
 			}
 		}
+		if ( $this->logger === null ) {
+			$this->logger = new NullLogger;
+		}
 	}
 
 	/**
@@ -162,6 +172,7 @@ class MultiHttpClient {
 			} elseif ( !isset( $req['url'] ) ) {
 				throw new Exception( "Request has no 'url' field set." );
 			}
+			$this->logger->debug( "{$req['method']}: {$req['url']}" );
 			$req['query'] = isset( $req['query'] ) ? $req['query'] : [];
 			$headers = []; // normalized headers
 			if ( isset( $req['headers'] ) ) {
@@ -235,6 +246,8 @@ class MultiHttpClient {
 					if ( function_exists( 'curl_strerror' ) ) {
 						$req['response']['error'] .= " " . curl_strerror( $errno );
 					}
+					$this->logger->warning( "Error fetching URL \"{$req['url']}\": " .
+						$req['response']['error'] );
 				}
 			} else {
 				$req['response']['error'] = "(curl error: no status set)";
@@ -420,6 +433,15 @@ class MultiHttpClient {
 		return $this->multiHandle;
 	}
 
+	/**
+	 * Register a logger
+	 *
+	 * @param LoggerInterface
+	 */
+	public function setLogger( LoggerInterface $logger ) {
+		$this->logger = $logger;
+	}
+
 	function __destruct() {
 		if ( $this->multiHandle ) {
 			curl_multi_close( $this->multiHandle );