diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php index beafaad4054..6110ec64e0b 100644 --- a/includes/MediaWikiServices.php +++ b/includes/MediaWikiServices.php @@ -214,7 +214,6 @@ use Wikimedia\RequestTimeout\CriticalSectionProvider; use Wikimedia\Services\NoSuchServiceException; use Wikimedia\Services\SalvageableService; use Wikimedia\Services\ServiceContainer; -use Wikimedia\Stats\StatsCache; use Wikimedia\Stats\StatsFactory; use Wikimedia\UUID\GlobalIdGenerator; use Wikimedia\WRStats\WRStatsFactory; @@ -1968,14 +1967,6 @@ class MediaWikiServices extends ServiceContainer { return $this->getService( 'SpecialPageFactory' ); } - /** - * @since 1.41 - * @return StatsCache - */ - public function getStatsCache(): StatsCache { - return $this->getService( 'StatsCache' ); - } - /** * @since 1.27 * @return IBufferingStatsdDataFactory diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 059ee02e397..7bcf48e19a4 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -2055,10 +2055,6 @@ return [ ); }, - 'StatsCache' => static function ( MediaWikiServices $services ): StatsCache { - return new StatsCache(); - }, - 'StatsdDataFactory' => static function ( MediaWikiServices $services ): IBufferingStatsdDataFactory { return new BufferingStatsdDataFactory( rtrim( $services->getMainConfig()->get( MainConfigNames::StatsdMetricPrefix ), '.' ) @@ -2070,14 +2066,14 @@ return [ $format = \Wikimedia\Stats\OutputFormats::getFormatFromString( $config->get( MainConfigNames::StatsFormat ) ?? 'null' ); - $cache = $services->getService( 'StatsCache' ); + $cache = new StatsCache; $emitter = \Wikimedia\Stats\OutputFormats::getNewEmitter( $config->get( MainConfigNames::StatsPrefix ) ?? 'MediaWiki', $cache, \Wikimedia\Stats\OutputFormats::getNewFormatter( $format ), $config->get( MainConfigNames::StatsTarget ) ); - $factory = new StatsFactory( 'core', $cache, $emitter, LoggerFactory::getInstance( 'Stats' ) ); + $factory = new StatsFactory( $cache, $emitter, LoggerFactory::getInstance( 'Stats' ) ); return $factory->withStatsdDataFactory( $services->getStatsdDataFactory() ); }, diff --git a/includes/libs/Stats/Formatters/DogStatsdFormatter.php b/includes/libs/Stats/Formatters/DogStatsdFormatter.php index 57d03e612e1..6db5400cb13 100644 --- a/includes/libs/Stats/Formatters/DogStatsdFormatter.php +++ b/includes/libs/Stats/Formatters/DogStatsdFormatter.php @@ -33,10 +33,15 @@ class DogStatsdFormatter implements FormatterInterface { /** @inheritDoc */ public function getFormattedSamples( string $prefix, MetricInterface $metric ): array { $output = []; - foreach ( $metric->getSamples() as $sample ) { + // append component to prefix if set + if ( $metric->getComponent() !== '' ) { + $prefix .= ".{$metric->getComponent()}"; + } + + foreach ( $metric->getSamples() as $sample ) { // dot-separate prefix, component, and name `prefix.component.name` - $stat = implode( '.', [ $prefix, $metric->getComponent(), $metric->getName() ] ); + $stat = implode( '.', [ $prefix, $metric->getName() ] ); // merge value with separator `:42` $value = ':' . $sample->getValue(); diff --git a/includes/libs/Stats/Formatters/StatsdFormatter.php b/includes/libs/Stats/Formatters/StatsdFormatter.php index ba842477455..b586bd7a456 100644 --- a/includes/libs/Stats/Formatters/StatsdFormatter.php +++ b/includes/libs/Stats/Formatters/StatsdFormatter.php @@ -33,12 +33,15 @@ class StatsdFormatter implements FormatterInterface { /** @inheritDoc */ public function getFormattedSamples( string $prefix, MetricInterface $metric ): array { $output = []; - foreach ( $metric->getSamples() as $sample ) { + // append component to prefix if set + if ( $metric->getComponent() !== '' ) { + $prefix .= ".{$metric->getComponent()}"; + } + + foreach ( $metric->getSamples() as $sample ) { // dot-separate prefix, component, name, and label values `prefix.component.name.value1.value2` - $stat = implode( '.', - array_merge( [ $prefix, $metric->getComponent(), $metric->getName() ], $sample->getLabelValues() ) - ); + $stat = implode( '.', array_merge( [ $prefix, $metric->getName() ], $sample->getLabelValues() ) ); // merge value with separator `:42` $value = ':' . $sample->getValue(); diff --git a/includes/libs/Stats/Metrics/CounterMetric.php b/includes/libs/Stats/Metrics/CounterMetric.php index 25c7eade0fc..11ce3271da3 100644 --- a/includes/libs/Stats/Metrics/CounterMetric.php +++ b/includes/libs/Stats/Metrics/CounterMetric.php @@ -110,7 +110,7 @@ class CounterMetric implements MetricInterface { } /** @inheritDoc */ - public function withSampleRate( float $sampleRate ) { + public function setSampleRate( float $sampleRate ) { try { $this->baseMetric->setSampleRate( $sampleRate ); } catch ( IllegalOperationException | InvalidArgumentException $ex ) { @@ -127,7 +127,7 @@ class CounterMetric implements MetricInterface { } /** @inheritDoc */ - public function withLabel( string $key, string $value ) { + public function setLabel( string $key, string $value ) { try { $this->baseMetric->addLabel( $key, $value ); } catch ( IllegalOperationException | InvalidArgumentException $ex ) { diff --git a/includes/libs/Stats/Metrics/GaugeMetric.php b/includes/libs/Stats/Metrics/GaugeMetric.php index 3135ac1f0bd..adae653c889 100644 --- a/includes/libs/Stats/Metrics/GaugeMetric.php +++ b/includes/libs/Stats/Metrics/GaugeMetric.php @@ -100,7 +100,7 @@ class GaugeMetric implements MetricInterface { } /** @inheritDoc */ - public function withSampleRate( float $sampleRate ) { + public function setSampleRate( float $sampleRate ) { try { $this->baseMetric->setSampleRate( $sampleRate ); } catch ( IllegalOperationException | InvalidArgumentException $ex ) { @@ -117,7 +117,7 @@ class GaugeMetric implements MetricInterface { } /** @inheritDoc */ - public function withLabel( string $key, string $value ) { + public function setLabel( string $key, string $value ) { try { $this->baseMetric->addLabel( $key, $value ); } catch ( IllegalOperationException | InvalidArgumentException $ex ) { diff --git a/includes/libs/Stats/Metrics/MetricInterface.php b/includes/libs/Stats/Metrics/MetricInterface.php index c66a138e1ab..01a772998fd 100644 --- a/includes/libs/Stats/Metrics/MetricInterface.php +++ b/includes/libs/Stats/Metrics/MetricInterface.php @@ -60,7 +60,7 @@ interface MetricInterface { * @param float $sampleRate * @return CounterMetric|GaugeMetric|TimingMetric|NullMetric */ - public function withSampleRate( float $sampleRate ); + public function setSampleRate( float $sampleRate ); /** * Returns the list of defined label keys. @@ -76,7 +76,7 @@ interface MetricInterface { * @param string $value * @return CounterMetric|GaugeMetric|TimingMetric|NullMetric */ - public function withLabel( string $key, string $value ); + public function setLabel( string $key, string $value ); /** * Copies metric operation to StatsD at provided namespace. diff --git a/includes/libs/Stats/Metrics/TimingMetric.php b/includes/libs/Stats/Metrics/TimingMetric.php index b0a47975ce2..5f0f235577f 100644 --- a/includes/libs/Stats/Metrics/TimingMetric.php +++ b/includes/libs/Stats/Metrics/TimingMetric.php @@ -127,7 +127,7 @@ class TimingMetric implements MetricInterface { } /** @inheritDoc */ - public function withSampleRate( float $sampleRate ) { + public function setSampleRate( float $sampleRate ) { try { $this->baseMetric->setSampleRate( $sampleRate ); } catch ( IllegalOperationException | InvalidArgumentException $ex ) { @@ -144,7 +144,7 @@ class TimingMetric implements MetricInterface { } /** @inheritDoc */ - public function withLabel( string $key, string $value ) { + public function setLabel( string $key, string $value ) { try { $this->baseMetric->addLabel( $key, $value ); } catch ( IllegalOperationException | InvalidArgumentException $ex ) { diff --git a/includes/libs/Stats/StatsCache.php b/includes/libs/Stats/StatsCache.php index a5d3b69d7b5..598bd0e9994 100644 --- a/includes/libs/Stats/StatsCache.php +++ b/includes/libs/Stats/StatsCache.php @@ -29,7 +29,7 @@ use Wikimedia\Stats\Metrics\NullMetric; /** * Singleton cache for Metric instances. * - * For reuse and collision avoidance. Serves as the data source for Metric Renderers. + * For reuse and collision avoidance. Serves as the data source for Metric Emitters. * * @author Cole White * @since 1.41 @@ -93,13 +93,17 @@ class StatsCache { * Get the metric formatted name. * * Takes the provided name and constructs a more specific name by combining - * the service, component, and name options. + * component and name. * * @param string $component * @param string $name * @return string */ private static function cacheKey( string $component, string $name ): string { - return implode( self::DELIMITER, [ $component, $name ] ); + // mitigate collision of empty-component metric with a component metric + if ( $component !== '' ) { + return implode( self::DELIMITER, [ $component, $name ] ); + } + return $name; } } diff --git a/includes/libs/Stats/StatsFactory.php b/includes/libs/Stats/StatsFactory.php index ad10c6afe89..59d382106b2 100644 --- a/includes/libs/Stats/StatsFactory.php +++ b/includes/libs/Stats/StatsFactory.php @@ -70,33 +70,31 @@ class StatsFactory { /** * StatsFactory builds, configures, and caches Metrics. * - * @param string $component * @param StatsCache $cache * @param EmitterInterface $emitter * @param LoggerInterface $logger + * @param string $component */ public function __construct( - string $component, StatsCache $cache, EmitterInterface $emitter, - LoggerInterface $logger + LoggerInterface $logger, + string $component = '' ) { $this->component = StatsUtils::normalizeString( $component ); $this->cache = $cache; $this->emitter = $emitter; $this->logger = $logger; - $this->validateInstanceConfig(); } /** - * Throw exception on invalid instance configuration. + * Returns a new StatsFactory instance prefixed by component. * - * @return void + * @param string $component + * @return StatsFactory */ - private function validateInstanceConfig(): void { - if ( $this->component == '' ) { - throw new InvalidArgumentException( 'Stats: component cannot be empty.' ); - } + public function withComponent( string $component ): StatsFactory { + return new StatsFactory( $this->cache, $this->emitter, $this->logger, $component ); } /** @@ -106,7 +104,10 @@ class StatsFactory { * @param string $value * @return $this */ - public function withStaticLabel( string $key, string $value ): StatsFactory { + public function addStaticLabel( string $key, string $value ): StatsFactory { + if ( !$this->component ) { + throw new IllegalOperationException( 'Stats: cannot set static labels with undefined component.' ); + } if ( count( $this->cache->getAllMetrics() ) > 0 ) { throw new IllegalOperationException( 'Stats: cannot set static labels when metrics are in the cache.' ); } diff --git a/tests/phpunit/unit/includes/libs/Stats/MetricTest.php b/tests/phpunit/unit/includes/libs/Stats/MetricTest.php index dc725328c1c..5bd5feb15cf 100644 --- a/tests/phpunit/unit/includes/libs/Stats/MetricTest.php +++ b/tests/phpunit/unit/includes/libs/Stats/MetricTest.php @@ -17,7 +17,7 @@ use Wikimedia\TestingAccessWrapper; * @covers \Wikimedia\Stats\Metrics\CounterMetric * @covers \Wikimedia\Stats\Metrics\GaugeMetric * @covers \Wikimedia\Stats\Metrics\TimingMetric - * @covers \Wikimedia\Stats\MetricUtils + * @covers \Wikimedia\Stats\StatsUtils */ class MetricTest extends TestCase { @@ -28,31 +28,43 @@ class MetricTest extends TestCase { public const TESTS = [ 'basic' => [ 'config' => [ - 'name' => 'test.unit' + 'name' => 'test.unit', + 'component' => 'testComponent', ], 'value' => 2, - 'labels' => [] + 'labels' => [], ], 'invalidLabel' => [ 'config' => [ - 'name' => 'test.unit' + 'name' => 'test.unit', + 'component' => 'testComponent', ], 'value' => 2, - 'labels' => [ ': x' => 'labelOne' ] + 'labels' => [ ': x' => 'labelOne' ], ], 'oneLabel' => [ 'config' => [ - 'name' => 'test.unit' + 'name' => 'test.unit', + 'component' => 'testComponent', ], 'value' => 2, - 'labels' => [ 'x' => 'labelOne' ] + 'labels' => [ 'x' => 'labelOne' ], ], 'multiLabel' => [ 'config' => [ - 'name' => 'test.unit' + 'name' => 'test.unit', + 'component' => 'testComponent', ], 'value' => 2, - 'labels' => [ 'x' => 'labelOne', 'y' => 'labelTwo' ] + 'labels' => [ 'x' => 'labelOne', 'y' => 'labelTwo' ], + ], + 'noComponent' => [ + 'config' => [ + 'name' => 'test.unit', + 'component' => null, + ], + 'value' => 2, + 'labels' => [], ] ]; @@ -61,30 +73,36 @@ class MetricTest extends TestCase { 'statsd.counter.invalidLabel' => [ 'mediawiki.testComponent.test_unit.test_wiki.labelOne:2|c' ], 'statsd.counter.oneLabel' => [ 'mediawiki.testComponent.test_unit.test_wiki.labelOne:2|c' ], 'statsd.counter.multiLabel' => [ 'mediawiki.testComponent.test_unit.test_wiki.labelOne.labelTwo:2|c' ], + 'statsd.counter.noComponent' => [ 'mediawiki.test_unit:2|c' ], 'statsd.gauge.basic' => [ 'mediawiki.testComponent.test_unit.test_wiki:2|g' ], 'statsd.gauge.invalidLabel' => [ 'mediawiki.testComponent.test_unit.test_wiki.labelOne:2|g' ], 'statsd.gauge.oneLabel' => [ 'mediawiki.testComponent.test_unit.test_wiki.labelOne:2|g' ], 'statsd.gauge.multiLabel' => [ 'mediawiki.testComponent.test_unit.test_wiki.labelOne.labelTwo:2|g' ], + 'statsd.gauge.noComponent' => [ 'mediawiki.test_unit:2|g' ], 'statsd.timing.basic' => [ 'mediawiki.testComponent.test_unit.test_wiki:2|ms' ], 'statsd.timing.invalidLabel' => [ 'mediawiki.testComponent.test_unit.test_wiki.labelOne:2|ms' ], 'statsd.timing.oneLabel' => [ 'mediawiki.testComponent.test_unit.test_wiki.labelOne:2|ms' ], 'statsd.timing.multiLabel' => [ 'mediawiki.testComponent.test_unit.test_wiki.labelOne.labelTwo:2|ms' ], + 'statsd.timing.noComponent' => [ 'mediawiki.test_unit:2|ms' ], 'dogstatsd.counter.basic' => [ 'mediawiki.testComponent.test_unit:2|c|#wiki:test_wiki' ], 'dogstatsd.counter.invalidLabel' => [ 'mediawiki.testComponent.test_unit:2|c|#wiki:test_wiki,x:labelOne' ], 'dogstatsd.counter.oneLabel' => [ 'mediawiki.testComponent.test_unit:2|c|#wiki:test_wiki,x:labelOne' ], 'dogstatsd.counter.multiLabel' => [ 'mediawiki.testComponent.test_unit:2|c|#wiki:test_wiki,x:labelOne,y:labelTwo' ], + 'dogstatsd.counter.noComponent' => [ 'mediawiki.test_unit:2|c' ], 'dogstatsd.gauge.basic' => [ 'mediawiki.testComponent.test_unit:2|g|#wiki:test_wiki' ], 'dogstatsd.gauge.invalidLabel' => [ 'mediawiki.testComponent.test_unit:2|g|#wiki:test_wiki,x:labelOne' ], 'dogstatsd.gauge.oneLabel' => [ 'mediawiki.testComponent.test_unit:2|g|#wiki:test_wiki,x:labelOne' ], 'dogstatsd.gauge.multiLabel' => [ 'mediawiki.testComponent.test_unit:2|g|#wiki:test_wiki,x:labelOne,y:labelTwo' ], + 'dogstatsd.gauge.noComponent' => [ 'mediawiki.test_unit:2|g' ], 'dogstatsd.timing.basic' => [ 'mediawiki.testComponent.test_unit:2|ms|#wiki:test_wiki' ], 'dogstatsd.timing.invalidLabel' => [ 'mediawiki.testComponent.test_unit:2|ms|#wiki:test_wiki,x:labelOne' ], 'dogstatsd.timing.oneLabel' => [ 'mediawiki.testComponent.test_unit:2|ms|#wiki:test_wiki,x:labelOne' ], 'dogstatsd.timing.multiLabel' => [ 'mediawiki.testComponent.test_unit:2|ms|#wiki:test_wiki,x:labelOne,y:labelTwo' ], + 'dogstatsd.timing.noComponent' => [ 'mediawiki.test_unit:2|ms' ], ]; private $cache; @@ -96,27 +114,30 @@ class MetricTest extends TestCase { $this->cache->clear(); $formatter = OutputFormats::getNewFormatter( OutputFormats::getFormatFromString( $format ) ); $emitter = OutputFormats::getNewEmitter( 'mediawiki', $this->cache, $formatter ); - $statsFactory = new StatsFactory( 'testComponent', $this->cache, $emitter, new NullLogger ); - $statsFactory->withStaticLabel( 'wiki', 'test_wiki' ); + $statsFactory = new StatsFactory( $this->cache, $emitter, new NullLogger ); + if ( $config['config']['component'] !== null ) { + $statsFactory = $statsFactory->withComponent( $config['config']['component'] ); + $statsFactory->addStaticLabel( 'wiki', 'test_wiki' ); + } switch ( $type ) { case 'counter': $metric = $statsFactory->getCounter( $config['config']['name'] ); foreach ( $config['labels'] as $key => $value ) { - $metric->withLabel( $key, $value ); + $metric->setLabel( $key, $value ); } $metric->incrementBy( $config['value'] ); break; case 'gauge': $metric = $statsFactory->getGauge( $config['config']['name'] ); foreach ( $config['labels'] as $key => $value ) { - $metric->withLabel( $key, $value ); + $metric->setLabel( $key, $value ); } $metric->set( $config['value'] ); break; case 'timing': $metric = $statsFactory->getTiming( $config['config']['name'] ); foreach ( $config['labels'] as $key => $value ) { - $metric->withLabel( $key, $value ); + $metric->setLabel( $key, $value ); } $metric->observe( $config['value'] ); break; @@ -147,12 +168,12 @@ class MetricTest extends TestCase { public function testSampledMetrics() { $rounds = 10; - $m = new StatsFactory( 'counter', new StatsCache, new NullEmitter, new NullLogger ); - $ten_percent_metrics = $m->getCounter( 'test.sampled.ten' )->withSampleRate( 0.1 ); - $m = new StatsFactory( 'counter', new StatsCache, new NullEmitter, new NullLogger ); - $all_metrics = $m->getCounter( 'test.sampled.hundred' )->withSampleRate( 1.0 ); - $m = new StatsFactory( 'counter', new StatsCache, new NullEmitter, new NullLogger ); - $zero_metrics = $m->getCounter( 'test.sampled.zero' )->withSampleRate( 0.0 ); + $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger ); + $ten_percent_metrics = $m->getCounter( 'test.sampled.ten' )->setSampleRate( 0.1 ); + $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger ); + $all_metrics = $m->getCounter( 'test.sampled.hundred' )->setSampleRate( 1.0 ); + $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger ); + $zero_metrics = $m->getCounter( 'test.sampled.zero' )->setSampleRate( 0.0 ); for ( $i = 0; $i < $rounds; $i++ ) { $ten_percent_metrics->increment(); $all_metrics->increment(); @@ -164,63 +185,63 @@ class MetricTest extends TestCase { } public function testTimerNotStarted() { - $m = new StatsFactory( 'test', new StatsCache, new NullEmitter, new NullLogger ); + $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger ); $this->expectWarning(); $this->expectWarningMessage( 'Stats: stop() called before start() for metric \'test\'' ); $m->getTiming( 'test' )->stop(); } public function testErrorOnChangingStaticLabelsWithMetricsInCache() { - $m = new StatsFactory( 'test', new StatsCache, new NullEmitter, new NullLogger ); + $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger ); $m->getCounter( 'testMetric' ); $this->expectException( IllegalOperationException::class ); - $m->withStaticLabel( 'a', '1' ); + $m->addStaticLabel( 'a', '1' ); } public function testLabelCollision() { - $m = new StatsFactory( 'test', new StatsCache, new NullEmitter, new NullLogger ); - $m->withStaticLabel( 'collide', 'test' ); - $metric = @$m->getCounter( 'testMetricCounter' )->withLabel( 'collide', 'value' ); + $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger, 'test' ); + $m->addStaticLabel( 'collide', 'test' ); + $metric = @$m->getCounter( 'testMetricCounter' )->setLabel( 'collide', 'value' ); $this->assertInstanceOf( NullMetric::class, $metric ); - $metric = @$m->getGauge( 'testMetricGauge' )->withLabel( 'collide', 'value' ); + $metric = @$m->getGauge( 'testMetricGauge' )->setLabel( 'collide', 'value' ); $this->assertInstanceOf( NullMetric::class, $metric ); - $metric = @$m->getTiming( 'testMetricTiming' )->withLabel( 'collide', 'value' ); + $metric = @$m->getTiming( 'testMetricTiming' )->setLabel( 'collide', 'value' ); $this->assertInstanceOf( NullMetric::class, $metric ); } public function testChangingLabelsToUsedMetric() { - $m = new StatsFactory( 'test', new StatsCache, new NullEmitter, new NullLogger ); - $m->getCounter( 'testMetricCounter' )->withLabel( 'labelOne', 'a' )->increment(); - $counter = @$m->getCounter( 'testMetricCounter' )->withLabel( 'labelTwo', 'b' ); + $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger ); + $m->getCounter( 'testMetricCounter' )->setLabel( 'labelOne', 'a' )->increment(); + $counter = @$m->getCounter( 'testMetricCounter' )->setLabel( 'labelTwo', 'b' ); $this->assertInstanceOf( NullMetric::class, $counter ); - $m->getGauge( 'testMetricGauge' )->withLabel( 'labelOne', 'a' )->set( 1 ); - $gauge = @$m->getGauge( 'testMetricGauge' )->withLabel( 'labelTwo', 'b' ); + $m->getGauge( 'testMetricGauge' )->setLabel( 'labelOne', 'a' )->set( 1 ); + $gauge = @$m->getGauge( 'testMetricGauge' )->setLabel( 'labelTwo', 'b' ); $this->assertInstanceOf( NullMetric::class, $gauge ); - $m->getTiming( 'testMetricTiming' )->withLabel( 'labelOne', 'a' )->observe( 1 ); - $timer = @$m->getTiming( 'testMetricTiming' )->withLabel( 'labelTwo', 'b' ); + $m->getTiming( 'testMetricTiming' )->setLabel( 'labelOne', 'a' )->observe( 1 ); + $timer = @$m->getTiming( 'testMetricTiming' )->setLabel( 'labelTwo', 'b' ); $this->assertInstanceOf( NullMetric::class, $timer ); } public function testSampleRateOOB() { - $m = new StatsFactory( 'test', new StatsCache, new NullEmitter, new NullLogger ); - $metric = @$m->getCounter( 'testMetricCounter' )->withSampleRate( 1.1 ); + $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger ); + $metric = @$m->getCounter( 'testMetricCounter' )->setSampleRate( 1.1 ); $this->assertInstanceOf( NullMetric::class, $metric ); - $metric = @$m->getGauge( 'testMetricGauge' )->withSampleRate( -1 ); + $metric = @$m->getGauge( 'testMetricGauge' )->setSampleRate( -1 ); $this->assertInstanceOf( NullMetric::class, $metric ); - $metric = @$m->getTiming( 'testMetricTimer' )->withSampleRate( 20 ); + $metric = @$m->getTiming( 'testMetricTimer' )->setSampleRate( 20 ); $this->assertInstanceOf( NullMetric::class, $metric ); } public function testSampleRateDisallowed() { - $m = new StatsFactory( 'test', new StatsCache, new NullEmitter, new NullLogger ); + $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger ); $m->getCounter( 'testMetric' )->increment(); - $metric = @$m->getCounter( 'testMetric' )->withSampleRate( 0.5 ); + $metric = @$m->getCounter( 'testMetric' )->setSampleRate( 0.5 ); $this->assertInstanceOf( NullMetric::class, $metric ); $m->getGauge( 'testMetricGauge' )->set( 1 ); - $metric = @$m->getGauge( 'testMetricGauge' )->withSampleRate( 0.5 ); + $metric = @$m->getGauge( 'testMetricGauge' )->setSampleRate( 0.5 ); $this->assertInstanceOf( NullMetric::class, $metric ); $m->getTiming( 'testMetricTiming' )->observe( 1 ); - $metric = @$m->getTiming( 'testMetricTiming' )->withSampleRate( 0.5 ); + $metric = @$m->getTiming( 'testMetricTiming' )->setSampleRate( 0.5 ); $this->assertInstanceOf( NullMetric::class, $metric ); } } diff --git a/tests/phpunit/unit/includes/libs/Stats/StatsEmitterTest.php b/tests/phpunit/unit/includes/libs/Stats/StatsEmitterTest.php index 25f15f6371c..96064a3fadd 100644 --- a/tests/phpunit/unit/includes/libs/Stats/StatsEmitterTest.php +++ b/tests/phpunit/unit/includes/libs/Stats/StatsEmitterTest.php @@ -11,9 +11,9 @@ use Wikimedia\Stats\StatsCache; use Wikimedia\Stats\StatsFactory; /** - * @covers \Wikimedia\Stats\MetricsUDPEmitter - * @covers \Wikimedia\Stats\MetricsCache - * @covers \Wikimedia\Stats\Formatters\DogStatsD + * @covers \Wikimedia\Stats\Emitters\UDPEmitter + * @covers \Wikimedia\Stats\StatsCache + * @covers \Wikimedia\Stats\Formatters\DogStatsdFormatter * @covers \Wikimedia\Stats\OutputFormats * @covers \Wikimedia\Stats\MetricsRenderer */ @@ -43,7 +43,7 @@ class StatsEmitterTest extends TestCase { $emitter = $emitter->withTransport( $transport ); // initialize metrics factory - $m = new StatsFactory( 'test', $cache, $emitter, new NullLogger ); + $m = new StatsFactory( $cache, $emitter, new NullLogger, 'test' ); // inject statsd factory $m->withStatsdDataFactory( $statsd ); diff --git a/tests/phpunit/unit/includes/libs/Stats/StatsFactoryTest.php b/tests/phpunit/unit/includes/libs/Stats/StatsFactoryTest.php index ae6f30b2d18..ce2f72f713b 100644 --- a/tests/phpunit/unit/includes/libs/Stats/StatsFactoryTest.php +++ b/tests/phpunit/unit/includes/libs/Stats/StatsFactoryTest.php @@ -6,6 +6,7 @@ use InvalidArgumentException; use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; use Wikimedia\Stats\Emitters\NullEmitter; +use Wikimedia\Stats\Exceptions\IllegalOperationException; use Wikimedia\Stats\Exceptions\UnsupportedFormatException; use Wikimedia\Stats\Metrics\CounterMetric; use Wikimedia\Stats\Metrics\GaugeMetric; @@ -23,17 +24,17 @@ use Wikimedia\Stats\StatsUtils; class StatsFactoryTest extends TestCase { public function testGetCounter() { - $m = new StatsFactory( 'testExtension', new StatsCache, new NullEmitter, new NullLogger ); + $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger ); $this->assertInstanceOf( CounterMetric::class, $m->getCounter( 'test' ) ); } public function testGetGauge() { - $m = new StatsFactory( 'testExtension', new StatsCache, new NullEmitter, new NullLogger ); + $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger ); $this->assertInstanceOf( GaugeMetric::class, $m->getGauge( 'test' ) ); } public function testGetTiming() { - $m = new StatsFactory( 'testExtension', new StatsCache, new NullEmitter, new NullLogger ); + $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger ); $this->assertInstanceOf( TimingMetric::class, $m->getTiming( 'test' ) ); } @@ -48,14 +49,15 @@ class StatsFactoryTest extends TestCase { } public function testUnsetNameConfig() { - $m = new StatsFactory( 'test', new StatsCache, new NullEmitter, new NullLogger ); + $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger ); $this->expectException( InvalidArgumentException::class ); $m->getCounter( '' ); } - public function testUnsetComponentConfig() { - $this->expectException( InvalidArgumentException::class ); - new StatsFactory( '', new StatsCache, new NullEmitter, new NullLogger ); + public function testDisallowSettingStaticLabelsOnUndefinedComponent() { + $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger ); + $this->expectException( IllegalOperationException::class ); + $m->addStaticLabel( 'a', 'a' ); } public function testNormalizeString() { @@ -73,16 +75,16 @@ class StatsFactoryTest extends TestCase { } public function testGetNullMetricWithLabelMismatch() { - $m = new StatsFactory( 'test', new StatsCache, new NullEmitter, new NullLogger ); + $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger ); // initialize a counter and add a sample - $m->getCounter( 'test_metric' )->withLabel( 'a', 'a' )->increment(); + $m->getCounter( 'test_metric' )->setLabel( 'a', 'a' )->increment(); // get the same counter and attempt to add a different label key - $metric = @$m->getCounter( 'test_metric' )->withLabel( 'b', 'b' ); + $metric = @$m->getCounter( 'test_metric' )->setLabel( 'b', 'b' ); $this->assertInstanceOf( NullMetric::class, $metric ); } public function testGetNullMetricOnNameCollision() { - $m = new StatsFactory( 'testComponent', new StatsCache, new NullEmitter, new NullLogger ); + $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger ); // define metric as counter 'test' $m->getCounter( 'test' ); // redefine metric as timing 'test'