Create and use PrefixingStatsdDataFactoryProxy in PerDbNameStatsdDataFactory

MediaWiki::emitBufferedStatsdData() is never called for the PerDbName factory,
so stats were being dropped. Instead of having two factories, turn the
PerDbName one into a proxy/wrapper around the main factory that just adds a
prefix in front of all of the keys.

Bug: T202144
Change-Id: I31e376446abb58e41353b4ca3814120d2e914104

diff --git a/autoload.php b/autoload.php
index 3b4f025..a372dd1 100644 (file)
--- a/autoload.php
+++ b/autoload.php
@@ -1132,6 +1132,7 @@ $wgAutoloadLocalClasses = [
        'PreferencesFormLegacy' => __DIR__ . '/includes/specials/forms/PreferencesFormLegacy.php',
        'PreferencesFormOOUI' => __DIR__ . '/includes/specials/forms/PreferencesFormOOUI.php',
        'PrefixSearch' => __DIR__ . '/includes/PrefixSearch.php',
+       'PrefixingStatsdDataFactoryProxy' => __DIR__ . '/includes/libs/stats/PrefixingStatsdDataFactoryProxy.php',
        'PreprocessDump' => __DIR__ . '/maintenance/preprocessDump.php',
        'Preprocessor' => __DIR__ . '/includes/parser/Preprocessor.php',
        'Preprocessor_DOM' => __DIR__ . '/includes/parser/Preprocessor_DOM.php',

diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php
index f9d9aab..4a6046d 100644 (file)
--- a/includes/MediaWikiServices.php
+++ b/includes/MediaWikiServices.php
@@ -12,6 +12,7 @@ use GenderCache;
 use GlobalVarConfig;
 use Hooks;
 use IBufferingStatsdDataFactory;
+use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface;
 use MediaWiki\Http\HttpRequestFactory;
 use MediaWiki\Preferences\PreferencesFactory;
 use MediaWiki\Shell\CommandFactory;
@@ -702,7 +703,7 @@ class MediaWikiServices extends ServiceContainer {
 
        /**
         * @since 1.32
-        * @return IBufferingStatsdDataFactory
+        * @return StatsdDataFactoryInterface
         */
        public function getPerDbNameStatsdDataFactory() {
                return $this->getService( 'PerDbNameStatsdDataFactory' );

diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index 286dde1..1a19465 100644 (file)
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -37,6 +37,7 @@
  *      MediaWiki code base.
  */
 
+use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface;
 use MediaWiki\Auth\AuthManager;
 use MediaWiki\Config\ConfigRepository;
 use MediaWiki\Interwiki\ClassicInterwikiLookup;
@@ -400,11 +401,12 @@ return [
        },
 
        'PerDbNameStatsdDataFactory' =>
-       function ( MediaWikiServices $services ) : IBufferingStatsdDataFactory {
+       function ( MediaWikiServices $services ) : StatsdDataFactoryInterface {
                $config = $services->getMainConfig();
                $wiki = $config->get( 'DBname' );
-               return new BufferingStatsdDataFactory(
-                       rtrim( $services->getMainConfig()->get( 'StatsdMetricPrefix' ), '.' ) . '.' . $wiki
+               return new PrefixingStatsdDataFactoryProxy(
+                       $services->getStatsdDataFactory(),
+                       $wiki
                );
        },
 

diff --git a/includes/libs/stats/PrefixingStatsdDataFactoryProxy.php b/includes/libs/stats/PrefixingStatsdDataFactoryProxy.php
new file mode 100644 (file)
index 0000000..136a614
--- /dev/null
+++ b/includes/libs/stats/PrefixingStatsdDataFactoryProxy.php
@@ -0,0 +1,96 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface;
+use Liuggio\StatsdClient\Entity\StatsdDataInterface;
+
+/**
+ * Proxy to prefix metric keys sent to a StatsdDataFactoryInterface
+ *
+ * @since 1.32
+ */
+class PrefixingStatsdDataFactoryProxy implements StatsdDataFactoryInterface {
+
+       /**
+        * @var string
+        */
+       private $prefix;
+
+       /**
+        * @var StatsdDataFactoryInterface
+        */
+       private $factory;
+
+       /**
+        * @param StatsdDataFactoryInterface $factory
+        * @param string $prefix
+        */
+       public function __construct(
+               StatsdDataFactoryInterface $factory,
+               $prefix
+       ) {
+               $this->factory = $factory;
+               $this->prefix = rtrim( $prefix, '.' );
+       }
+
+       /**
+        * @param string $key
+        * @return string
+        */
+       private function addPrefixToKey( $key ) {
+               return $this->prefix . '.' . $key;
+       }
+
+       public function timing( $key, $time ) {
+               return $this->factory->timing( $this->addPrefixToKey( $key ), $time );
+       }
+
+       public function gauge( $key, $value ) {
+               return $this->factory->gauge( $this->addPrefixToKey( $key ), $value );
+       }
+
+       public function set( $key, $value ) {
+               return $this->factory->set( $this->addPrefixToKey( $key ), $value );
+       }
+
+       public function increment( $key ) {
+               return $this->factory->increment( $this->addPrefixToKey( $key ) );
+       }
+
+       public function decrement( $key ) {
+               return $this->factory->decrement( $this->addPrefixToKey( $key ) );
+       }
+
+       public function updateCount( $key, $delta ) {
+               return $this->factory->updateCount( $this->addPrefixToKey( $key ), $delta );
+       }
+
+       public function produceStatsdData(
+               $key,
+               $value = 1,
+               $metric = StatsdDataInterface::STATSD_METRIC_COUNT
+       ) {
+               return $this->factory->produceStatsdData(
+                       $this->addPrefixToKey( $key ),
+                       $value,
+                       $metric
+               );
+       }
+}

diff --git a/tests/phpunit/includes/libs/stats/PrefixingStatsdDataFactoryProxyTest.php b/tests/phpunit/includes/libs/stats/PrefixingStatsdDataFactoryProxyTest.php
new file mode 100644 (file)
index 0000000..b55d869
--- /dev/null
+++ b/tests/phpunit/includes/libs/stats/PrefixingStatsdDataFactoryProxyTest.php
@@ -0,0 +1,58 @@
+<?php
+
+use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface;
+
+/**
+ * @covers PrefixingStatsdDataFactoryProxy
+ */
+class PrefixingStatsdDataFactoryProxyTest extends PHPUnit\Framework\TestCase {
+
+       use PHPUnit4And6Compat;
+
+       public function provideMethodNames() {
+               return [
+                       [ 'timing' ],
+                       [ 'gauge' ],
+                       [ 'set' ],
+                       [ 'increment' ],
+                       [ 'decrement' ],
+                       [ 'updateCount' ],
+                       [ 'produceStatsdData' ],
+               ];
+       }
+
+       /**
+        * @dataProvider provideMethodNames
+        */
+       public function testPrefixingAndPassthrough( $method ) {
+               /** @var StatsdDataFactoryInterface|PHPUnit_Framework_MockObject_MockObject $innerFactory */
+               $innerFactory = $this->getMock(
+                       \Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface::class
+               );
+               $innerFactory->expects( $this->once() )
+                       ->method( $method )
+                       ->with( 'testprefix.' . 'metricname' );
+
+               $proxy = new PrefixingStatsdDataFactoryProxy( $innerFactory, 'testprefix' );
+               // 1,2,3,4 simply makes sure we provide enough parameters, without caring what they are
+               $proxy->$method( 'metricname', 1, 2, 3, 4 );
+       }
+
+       /**
+        * @dataProvider provideMethodNames
+        */
+       public function testPrefixIsTrimmed( $method ) {
+               /** @var StatsdDataFactoryInterface|PHPUnit_Framework_MockObject_MockObject $innerFactory */
+               $innerFactory = $this->getMock(
+                       \Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface::class
+               );
+               $innerFactory->expects( $this->once() )
+                       ->method( $method )
+                       ->with( 'testprefix.' . 'metricname' );
+
+               $proxy = new PrefixingStatsdDataFactoryProxy( $innerFactory, 'testprefix...' );
+               // 1,2,3,4 simply makes sure we provide enough parameters, without caring what they are
+               $proxy->$method( 'metricname', 1, 2, 3, 4 );
+       }
+
+}