Refactor Statsd classes to enable null collector to work.

The following changes are added:
- Created MediawikiStatsdDataFactory interface
- Added hasData() method to see if there are any data to send
- Added getData() method to fetch data
- Made service infrastructure use MediawikiStatsdDataFactory interface
- Made wfLogProfilingData() use MediawikiStatsdDataFactory interface
- Added capability to enable/disable buffering collector

Bug: T166354
Change-Id: I2874175647e987996a9a399829b3319674471aaa

diff --git a/autoload.php b/autoload.php
index fbdee83..018c4fc 100644 (file)
--- a/autoload.php
+++ b/autoload.php
@@ -960,6 +960,7 @@ $wgAutoloadLocalClasses = [
        'MediaWiki\\Widget\\TitleInputWidget' => __DIR__ . '/includes/widget/TitleInputWidget.php',
        'MediaWiki\\Widget\\UserInputWidget' => __DIR__ . '/includes/widget/UserInputWidget.php',
        'MediaWiki\\Widget\\UsersMultiselectWidget' => __DIR__ . '/includes/widget/UsersMultiselectWidget.php',
+       'MediawikiStatsdDataFactory' => __DIR__ . '/includes/libs/stats/MediawikiStatsdDataFactory.php',
        'MemCachedClientforWiki' => __DIR__ . '/includes/compat/MemcachedClientCompat.php',
        'MemcLockManager' => __DIR__ . '/includes/libs/lockmanager/MemcLockManager.php',
        'MemcachedBagOStuff' => __DIR__ . '/includes/libs/objectcache/MemcachedBagOStuff.php',

diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index 9c70639..8f2c24e 100644 (file)
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -1191,7 +1191,8 @@ function wfLogProfilingData() {
        $profiler->logData();
 
        $config = $context->getConfig();
-       if ( $config->get( 'StatsdServer' ) ) {
+       $stats = MediaWikiServices::getInstance()->getStatsdDataFactory();
+       if ( $config->get( 'StatsdServer' ) && $stats->hasData() ) {
                try {
                        $statsdServer = explode( ':', $config->get( 'StatsdServer' ) );
                        $statsdHost = $statsdServer[0];
@@ -1199,9 +1200,7 @@ function wfLogProfilingData() {
                        $statsdSender = new SocketSender( $statsdHost, $statsdPort );
                        $statsdClient = new SamplingStatsdClient( $statsdSender, true, false );
                        $statsdClient->setSamplingRates( $config->get( 'StatsdSamplingRates' ) );
-                       $statsdClient->send(
-                               MediaWikiServices::getInstance()->getStatsdDataFactory()->getBuffer()
-                       );
+                       $statsdClient->send( $stats->getData() );
                } catch ( Exception $ex ) {
                        MWExceptionHandler::logException( $ex );
                }

diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php
index 3bf6d78..b63c769 100644 (file)
--- a/includes/MediaWikiServices.php
+++ b/includes/MediaWikiServices.php
@@ -9,9 +9,9 @@ use EventRelayerGroup;
 use GenderCache;
 use GlobalVarConfig;
 use Hooks;
+use MediawikiStatsdDataFactory;
 use Wikimedia\Rdbms\LBFactory;
 use LinkCache;
-use Liuggio\StatsdClient\Factory\StatsdDataFactory;
 use Wikimedia\Rdbms\LoadBalancer;
 use MediaHandlerFactory;
 use MediaWiki\Linker\LinkRenderer;
@@ -446,7 +446,7 @@ class MediaWikiServices extends ServiceContainer {
 
        /**
         * @since 1.27
-        * @return StatsdDataFactory
+        * @return MediawikiStatsdDataFactory
         */
        public function getStatsdDataFactory() {
                return $this->getService( 'StatsdDataFactory' );

diff --git a/includes/context/ContextSource.php b/includes/context/ContextSource.php
index 135c9b2..2264670 100644 (file)
--- a/includes/context/ContextSource.php
+++ b/includes/context/ContextSource.php
@@ -170,7 +170,7 @@ abstract class ContextSource implements IContextSource {
         * @deprecated since 1.27 use a StatsdDataFactory from MediaWikiServices (preferably injected)
         *
         * @since 1.25
-        * @return StatsdDataFactory
+        * @return MediawikiStatsdDataFactory
         */
        public function getStats() {
                return MediaWikiServices::getInstance()->getStatsdDataFactory();

diff --git a/includes/context/DerivativeContext.php b/includes/context/DerivativeContext.php
index e77a058..2939510 100644 (file)
--- a/includes/context/DerivativeContext.php
+++ b/includes/context/DerivativeContext.php
@@ -109,7 +109,7 @@ class DerivativeContext extends ContextSource implements MutableContext {
         *
         * @deprecated since 1.27 use a StatsdDataFactory from MediaWikiServices (preferably injected)
         *
-        * @return StatsdDataFactory
+        * @return MediawikiStatsdDataFactory
         */
        public function getStats() {
                return MediaWikiServices::getInstance()->getStatsdDataFactory();

diff --git a/includes/context/IContextSource.php b/includes/context/IContextSource.php
index ccefc72..8e9fc6f 100644 (file)
--- a/includes/context/IContextSource.php
+++ b/includes/context/IContextSource.php
@@ -131,7 +131,7 @@ interface IContextSource {
         * @deprecated since 1.27 use a StatsdDataFactory from MediaWikiServices (preferably injected)
         *
         * @since 1.25
-        * @return StatsdDataFactory
+        * @return MediawikiStatsdDataFactory
         */
        public function getStats();
 

diff --git a/includes/context/RequestContext.php b/includes/context/RequestContext.php
index 3dfa456..0e1de50 100644 (file)
--- a/includes/context/RequestContext.php
+++ b/includes/context/RequestContext.php
@@ -138,7 +138,7 @@ class RequestContext implements IContextSource, MutableContext {
         *
         * @deprecated since 1.27 use a StatsdDataFactory from MediaWikiServices (preferably injected)
         *
-        * @return StatsdDataFactory
+        * @return MediawikiStatsdDataFactory
         */
        public function getStats() {
                return MediaWikiServices::getInstance()->getStatsdDataFactory();

diff --git a/includes/libs/stats/BufferingStatsdDataFactory.php b/includes/libs/stats/BufferingStatsdDataFactory.php
index 9c18b10..f687254 100644 (file)
--- a/includes/libs/stats/BufferingStatsdDataFactory.php
+++ b/includes/libs/stats/BufferingStatsdDataFactory.php
@@ -32,8 +32,17 @@ use Liuggio\StatsdClient\Factory\StatsdDataFactory;
  *
  * @since 1.25
  */
-class BufferingStatsdDataFactory extends StatsdDataFactory {
+class BufferingStatsdDataFactory extends StatsdDataFactory implements MediawikiStatsdDataFactory {
        protected $buffer = [];
+       /**
+        * Collection enabled?
+        * @var bool
+        */
+       protected $enabled = true;
+       /**
+        * @var string
+        */
+       private $prefix;
 
        public function __construct( $prefix ) {
                parent::__construct();
@@ -49,6 +58,7 @@ class BufferingStatsdDataFactory extends StatsdDataFactory {
         *
         * @param string $key
         * @since 1.26
+        * @return string
         */
        private static function normalizeMetricKey( $key ) {
                $key = preg_replace( '/[:.]+/', '.', $key );
@@ -61,6 +71,9 @@ class BufferingStatsdDataFactory extends StatsdDataFactory {
                $key, $value = 1, $metric = StatsdDataInterface::STATSD_METRIC_COUNT
        ) {
                $entity = $this->produceStatsdDataEntity();
+               if ( !$this->enabled ) {
+                       return $entity;
+               }
                if ( $key !== null ) {
                        $key = self::normalizeMetricKey( "{$this->prefix}.{$key}" );
                        $entity->setKey( $key );
@@ -79,9 +92,35 @@ class BufferingStatsdDataFactory extends StatsdDataFactory {
        }
 
        /**
+        * @deprecated Use getData()
         * @return StatsdData[]
         */
        public function getBuffer() {
                return $this->buffer;
        }
+
+       /**
+        * Check whether this data factory has any data.
+        * @return boolean
+        */
+       public function hasData() {
+               return !empty( $this->buffer );
+       }
+
+       /**
+        * Return data from the factory.
+        * @return StatsdData[]
+        */
+       public function getData() {
+               return $this->buffer;
+       }
+
+       /**
+        * Set collection enable status.
+        * @param bool $enabled Will collection be enabled?
+        * @return void
+        */
+       public function setEnabled( $enabled ) {
+               $this->enabled = $enabled;
+       }
 }

diff --git a/includes/libs/stats/MediawikiStatsdDataFactory.php b/includes/libs/stats/MediawikiStatsdDataFactory.php
new file mode 100644 (file)
index 0000000..d560f18
--- /dev/null
+++ b/includes/libs/stats/MediawikiStatsdDataFactory.php
@@ -0,0 +1,28 @@
+<?php
+use Liuggio\StatsdClient\Entity\StatsdData;
+use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface;
+
+/**
+ * Mediawiki adaptation of Statsd data factory.
+ */
+interface MediawikiStatsdDataFactory extends StatsdDataFactoryInterface {
+       /**
+        * Check whether this data factory has any data.
+        * @return boolean
+        */
+       public function hasData();
+
+       /**
+        * Return data from the factory.
+        * @return StatsdData[]
+        */
+       public function getData();
+
+       /**
+        * Set collection enable status.
+        * @param bool $enabled Will collection be enabled?
+        * @return void
+        */
+       public function setEnabled( $enabled );
+
+}

diff --git a/includes/libs/stats/NullStatsdDataFactory.php b/includes/libs/stats/NullStatsdDataFactory.php
index fee792f..4fa0248 100644 (file)
--- a/includes/libs/stats/NullStatsdDataFactory.php
+++ b/includes/libs/stats/NullStatsdDataFactory.php
@@ -8,7 +8,7 @@ use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface;
  * @author Addshore
  * @since 1.27
  */
-class NullStatsdDataFactory implements StatsdDataFactoryInterface {
+class NullStatsdDataFactory implements MediawikiStatsdDataFactory {
 
        /**
         * This function creates a 'timing' StatsdData.
@@ -108,4 +108,28 @@ class NullStatsdDataFactory implements StatsdDataFactoryInterface {
                return $data;
        }
 
+       /**
+        * Check whether this data factory has any data.
+        * @return boolean
+        */
+       public function hasData() {
+               return false;
+       }
+
+       /**
+        * Return data from the factory.
+        * @return StatsdData[]
+        */
+       public function getData() {
+               return [];
+       }
+
+       /**
+        * Set collection enable status.
+        * @param bool $enabled Will collection be enabled?
+        * @return void
+        */
+       public function setEnabled( $enabled ) {
+               // Nothing to do, null factory is always disabled.
+       }
 }

diff --git a/tests/phpunit/includes/MediaWikiServicesTest.php b/tests/phpunit/includes/MediaWikiServicesTest.php
index a72662f..037faa6 100644 (file)
--- a/tests/phpunit/includes/MediaWikiServicesTest.php
+++ b/tests/phpunit/includes/MediaWikiServicesTest.php
@@ -303,7 +303,7 @@ class MediaWikiServicesTest extends MediaWikiTestCase {
                        'MainConfig' => [ 'MainConfig', Config::class ],
                        'SiteStore' => [ 'SiteStore', SiteStore::class ],
                        'SiteLookup' => [ 'SiteLookup', SiteLookup::class ],
-                       'StatsdDataFactory' => [ 'StatsdDataFactory', StatsdDataFactory::class ],
+                       'StatsdDataFactory' => [ 'StatsdDataFactory', MediawikiStatsdDataFactory::class ],
                        'InterwikiLookup' => [ 'InterwikiLookup', InterwikiLookup::class ],
                        'EventRelayerGroup' => [ 'EventRelayerGroup', EventRelayerGroup::class ],
                        'SearchEngineFactory' => [ 'SearchEngineFactory', SearchEngineFactory::class ],