From 82b19bd96a1735e4e31facff2c6f5ba9492dbe60 Mon Sep 17 00:00:00 2001 From: addshore Date: Sat, 18 Aug 2018 11:22:38 +0100 Subject: [PATCH] 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 --- autoload.php | 1 + includes/MediaWikiServices.php | 3 +- includes/ServiceWiring.php | 8 +- .../stats/PrefixingStatsdDataFactoryProxy.php | 96 +++++++++++++++++++ .../PrefixingStatsdDataFactoryProxyTest.php | 58 +++++++++++ 5 files changed, 162 insertions(+), 4 deletions(-) create mode 100644 includes/libs/stats/PrefixingStatsdDataFactoryProxy.php create mode 100644 tests/phpunit/includes/libs/stats/PrefixingStatsdDataFactoryProxyTest.php diff --git a/autoload.php b/autoload.php index 3b4f0253a9..a372dd19f0 100644 --- 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 f9d9aab32e..4a6046d715 100644 --- 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 286dde197d..1a19465ed5 100644 --- 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 index 0000000000..136a614c3c --- /dev/null +++ b/includes/libs/stats/PrefixingStatsdDataFactoryProxy.php @@ -0,0 +1,96 @@ +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 index 0000000000..b55d8697db --- /dev/null +++ b/tests/phpunit/includes/libs/stats/PrefixingStatsdDataFactoryProxyTest.php @@ -0,0 +1,58 @@ +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 ); + } + +} -- 2.20.1