[MediaWiki-commits] [Gerrit] Allow disabling cirrus request logging from query string - change (mediawiki...CirrusSearch)

2015-07-30 Thread jenkins-bot (Code Review)
jenkins-bot has submitted this change and it was merged.

Change subject: Allow disabling cirrus request logging from query string
..


Allow disabling cirrus request logging from query string

We want to be able to try throwing various combinations of search parameters at
the production elasticsearch instances for indexes that are too big to test
elsewhere (such as enwiki).

This allows a web request to recuse itself from the CirrusSearchRequests log if
it knows the secret key. The key will need to be set from the cluster's private
data repository.

Change-Id: Iac79ad90e3ae32aa10a8eba5df63e5906ea41aa3
---
M CirrusSearch.php
M includes/ElasticsearchIntermediary.php
M includes/Hooks.php
M maintenance/runSearch.php
4 files changed, 48 insertions(+), 15 deletions(-)

Approvals:
  MaxSem: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/CirrusSearch.php b/CirrusSearch.php
index e8eea80..6abe6ae 100644
--- a/CirrusSearch.php
+++ b/CirrusSearch.php
@@ -663,6 +663,18 @@
  */
 $wgCirrusSearchEnableSearchLogging = false;
 
+/**
+ * Whether elasticsearch queries should be logged on the server side.
+ */
+$wgCirrusSearchLogElasticRequests = true;
+
+/**
+ * When truthy and this value is passed as the cirrusLogElasticRequests query
+ * variable $wgCirrusSearchLogElasticRequests will be set to false for that
+ * request.
+ */
+$wgCirrusSearchLogElasticRequestsSecret = false;
+
 // The maximum number of incategory:a|b|c items to OR together.
 $wgCirrusSearchMaxIncategoryOptions = 100;
 
diff --git a/includes/ElasticsearchIntermediary.php 
b/includes/ElasticsearchIntermediary.php
index dce6040..515bc5e 100644
--- a/includes/ElasticsearchIntermediary.php
+++ b/includes/ElasticsearchIntermediary.php
@@ -166,19 +166,32 @@
 * @return int number of milliseconds it took to complete the request
 */
private function finishRequest() {
+   global $wgCirrusSearchLogElasticRequests;
+
if ( !$this-requestStart ) {
wfLogWarning( 'finishRequest called without staring a 
request' );
return;
}
-   // No need to check description because it must be set by 
$this-start.
-
-   // Build the log message
$endTime = microtime( true );
$took = round( ( $endTime - $this-requestStart ) * 1000 );
+   if ( $wgCirrusSearchLogElasticRequests ) {
+   $logMessage = $this-buildLogMessage( 
$this-requestStart, $endTime, $took );
+   LoggerFactory::getInstance( 'CirrusSearchRequests' 
)-debug( $logMessage );
+   if ( $this-slowMillis  $took = $this-slowMillis ) {
+   $logMessage .= $this-user ? ' for ' . 
$this-user-getName() : '';
+   LoggerFactory::getInstance( 
'CirrusSearchSlowRequests' )-info( $logMessage );
+   }
+   }
+   $this-requestStart = null;
+   return $took;
+   }
+
+   private function buildLogMessage( $startTime, $endTime, $took ) {
\RequestContext::getMain()-getStats()-timing( 
'CirrusSearch.requestTime', $took );
+   // No need to check description because it must be set by 
$this-start.
$logMessage = $this-description;
 
-   $this-searchMetrics['wgCirrusStartTime'] = $this-requestStart;
+   $this-searchMetrics['wgCirrusStartTime'] = $startTime;
$this-searchMetrics['wgCirrusEndTime'] = $endTime;
 
$client = Connection::getClient();
@@ -213,7 +226,6 @@
$resultData[ 'suggest' ][ 'suggest' ][ 
0 ][ 'options' ][ 0 ][ 'text' ] . '\'';
}
}
-   $request = $client-getLastRequest();
 
if ( php_sapi_name() === 'cli' ) {
$source = 'cli';
@@ -224,14 +236,7 @@
}
$logMessage .= . Requested via $source by executor  . 
self::getExecutionId();
 
-   // Now log and clear our state.
-   LoggerFactory::getInstance( 'CirrusSearchRequests' )-debug( 
$logMessage );
-   if ( $this-slowMillis  $took = $this-slowMillis ) {
-   $logMessage .= $this-user ? ' for ' . 
$this-user-getName() : '';
-   LoggerFactory::getInstance( 'CirrusSearchSlowRequests' 
)-info( $logMessage );
-   }
-   $this-requestStart = null;
-   return $took;
+   return $logMessage;
}
 
private function extractMessageAndStatus( $exception ) {
diff --git a/includes/Hooks.php b/includes/Hooks.php
index c7bf082..4cf1efd 100644
--- a/includes/Hooks.php
+++ b/includes/Hooks.php
@@ -82,7 +82,9 @@
$wgCirrusSearchBoostLinks,
 

[MediaWiki-commits] [Gerrit] Allow disabling cirrus request logging from query string - change (mediawiki...CirrusSearch)

2015-07-29 Thread EBernhardson (Code Review)
EBernhardson has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/227870

Change subject: Allow disabling cirrus request logging from query string
..

Allow disabling cirrus request logging from query string

We want to be able to try throwing various combinations of search parameters at
the production elasticsearch instances for indexes that are too big to test
elsewhere (such as enwiki).

This allows a web request to recuse itself from the CirrusSearchRequests log if
it knows the secret key. The key will need to be set from the cluster's private
data repository.

Change-Id: Iac79ad90e3ae32aa10a8eba5df63e5906ea41aa3
---
M CirrusSearch.php
M includes/ElasticsearchIntermediary.php
M includes/Hooks.php
3 files changed, 43 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CirrusSearch 
refs/changes/70/227870/1

diff --git a/CirrusSearch.php b/CirrusSearch.php
index bd69af8..03b1dc2 100644
--- a/CirrusSearch.php
+++ b/CirrusSearch.php
@@ -611,6 +611,18 @@
  */
 $wgCirrusSearchEnableSearchLogging = false;
 
+/**
+ * Whether elasticsearch queries should be logged on the server side.
+ */
+$wgCirrusSearchLogElasticRequests = true;
+
+/**
+ * When truthy and this value is passed as the cirrusLogElasticRequests query
+ * variable $wgCirrusSearchLogElasticRequests will be set to false for that
+ * request.
+ */
+$wgCirrusSearchLogElasticRequestsSecret = false;
+
 // The maximum number of incategory:a|b|c items to OR together.
 $wgCirrusSearchMaxIncategoryOptions = 100;
 
diff --git a/includes/ElasticsearchIntermediary.php 
b/includes/ElasticsearchIntermediary.php
index dce6040..ec4ff09 100644
--- a/includes/ElasticsearchIntermediary.php
+++ b/includes/ElasticsearchIntermediary.php
@@ -166,19 +166,32 @@
 * @return int number of milliseconds it took to complete the request
 */
private function finishRequest() {
+   global $wgCirrusSearchLogRequests;
+
if ( !$this-requestStart ) {
wfLogWarning( 'finishRequest called without staring a 
request' );
return;
}
-   // No need to check description because it must be set by 
$this-start.
-
-   // Build the log message
$endTime = microtime( true );
$took = round( ( $endTime - $this-requestStart ) * 1000 );
+   if ( $wgCirrusSearchLogRequests ) {
+   $logMessage = $this-buildLogMessage( 
$this-requestStart, $endTime, $took );
+   LoggerFactory::getInstance( 'CirrusSearchRequests' 
)-debug( $logMessage );
+   if ( $this-slowMillis  $took = $this-slowMillis ) {
+   $logMessage .= $this-user ? ' for ' . 
$this-user-getName() : '';
+   LoggerFactory::getInstance( 
'CirrusSearchSlowRequests' )-info( $logMessage );
+   }
+   }
+   $this-requestStart = null;
+   return $took;
+   }
+
+   private function buildLogMessage( $startTime, $endTime, $took ) {
\RequestContext::getMain()-getStats()-timing( 
'CirrusSearch.requestTime', $took );
+   // No need to check description because it must be set by 
$this-start.
$logMessage = $this-description;
 
-   $this-searchMetrics['wgCirrusStartTime'] = $this-requestStart;
+   $this-searchMetrics['wgCirrusStartTime'] = $startTime;
$this-searchMetrics['wgCirrusEndTime'] = $endTime;
 
$client = Connection::getClient();
@@ -213,7 +226,6 @@
$resultData[ 'suggest' ][ 'suggest' ][ 
0 ][ 'options' ][ 0 ][ 'text' ] . '\'';
}
}
-   $request = $client-getLastRequest();
 
if ( php_sapi_name() === 'cli' ) {
$source = 'cli';
@@ -224,14 +236,7 @@
}
$logMessage .= . Requested via $source by executor  . 
self::getExecutionId();
 
-   // Now log and clear our state.
-   LoggerFactory::getInstance( 'CirrusSearchRequests' )-debug( 
$logMessage );
-   if ( $this-slowMillis  $took = $this-slowMillis ) {
-   $logMessage .= $this-user ? ' for ' . 
$this-user-getName() : '';
-   LoggerFactory::getInstance( 'CirrusSearchSlowRequests' 
)-info( $logMessage );
-   }
-   $this-requestStart = null;
-   return $took;
+   return $logMessage;
}
 
private function extractMessageAndStatus( $exception ) {
diff --git a/includes/Hooks.php b/includes/Hooks.php
index c425c56..2fc51e7 100644
--- a/includes/Hooks.php
+++ b/includes/Hooks.php
@@ -82,7 +82,9 @@