WatchedItemStore: Switch to StatsFactory

Switch WatchedItemStore metrics to StatsFactory, preserving the existing StatsD metric names as compatibility mappings. Add assertions for the metrics in unit tests. Bug: T359256 Change-Id: I7cc84abbbbb005e598423099af7bad818c78123b
2024-05-03 17:37:04 +02:00 · 2024-05-03 17:37:04 +02:00 · ae5c0ffe80
commit ae5c0ffe80
parent 40b664ac38
3 changed files with 123 additions and 29 deletions
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@ -2461,9 +2461,9 @@ return [
 			$services->getReadOnlyMode(),
 			$services->getNamespaceInfo(),
 			$services->getRevisionLookup(),
-			$services->getLinkBatchFactory()
+			$services->getLinkBatchFactory(),
+			$services->getStatsFactory()
 		);
-		$store->setStatsdDataFactory( $services->getStatsdDataFactory() );

 		if ( $services->getMainConfig()->get( MainConfigNames::ReadOnlyWatchedItemStore ) ) {
 			$store = new NoWriteWatchedItemStore( $store );
--- a/includes/watcheditem/WatchedItemStore.php
+++ b/includes/watcheditem/WatchedItemStore.php
@ -1,6 +1,5 @@
 <?php

-use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface;
 use MediaWiki\Cache\LinkBatchFactory;
 use MediaWiki\Config\ServiceOptions;
 use MediaWiki\Deferred\DeferredUpdates;
@ -21,6 +20,7 @@ use Wikimedia\Rdbms\IResultWrapper;
 use Wikimedia\Rdbms\ReadOnlyMode;
 use Wikimedia\Rdbms\SelectQueryBuilder;
 use Wikimedia\ScopedCallback;
+use Wikimedia\Stats\StatsFactory;

 /**
 * Storage layer class for WatchedItems.
@ -30,7 +30,7 @@ use Wikimedia\ScopedCallback;
 * @author Addshore
 * @since 1.27
 */
-class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterface {
+class WatchedItemStore implements WatchedItemStoreInterface {

 	/**
 	 * @internal For use by ServiceWiring
@ -101,11 +101,6 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
 	 */
 	private $revisionLookup;

-	/**
-	 * @var StatsdDataFactoryInterface
-	 */
-	private $stats;
-
 	/**
 	 * @var bool Correlates to $wgWatchlistExpiry feature flag.
 	 */
@ -116,6 +111,9 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
 	 */
 	private $linkBatchFactory;

+	/** @var StatsFactory */
+	private $statsFactory;
+
 	/**
 	 * @var string|null Maximum configured relative expiry.
 	 */
@ -134,6 +132,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
 	 * @param NamespaceInfo $nsInfo
 	 * @param RevisionLookup $revisionLookup
 	 * @param LinkBatchFactory $linkBatchFactory
+	 * @param StatsFactory $statsFactory
 	 */
 	public function __construct(
 		ServiceOptions $options,
@ -144,7 +143,8 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
 		ReadOnlyMode $readOnlyMode,
 		NamespaceInfo $nsInfo,
 		RevisionLookup $revisionLookup,
-		LinkBatchFactory $linkBatchFactory
+		LinkBatchFactory $linkBatchFactory,
+		StatsFactory $statsFactory
 	) {
 		$options->assertRequiredOptions( self::CONSTRUCTOR_OPTIONS );
 		$this->updateRowsPerQuery = $options->get( MainConfigNames::UpdateRowsPerQuery );
@ -157,23 +157,16 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
 		$this->stash = $stash;
 		$this->cache = $cache;
 		$this->readOnlyMode = $readOnlyMode;
-		$this->stats = new NullStatsdDataFactory();
 		$this->deferredUpdatesAddCallableUpdateCallback =
 			[ DeferredUpdates::class, 'addCallableUpdate' ];
 		$this->nsInfo = $nsInfo;
 		$this->revisionLookup = $revisionLookup;
 		$this->linkBatchFactory = $linkBatchFactory;
+		$this->statsFactory = $statsFactory;

 		$this->latestUpdateCache = new HashBagOStuff( [ 'maxKeys' => 3 ] );
 	}

-	/**
-	 * @param StatsdDataFactoryInterface $stats
-	 */
-	public function setStatsdDataFactory( StatsdDataFactoryInterface $stats ) {
-		$this->stats = $stats;
-	}
-
 	/**
 	 * Overrides the DeferredUpdates::addCallableUpdate callback
 	 * This is intended for use while testing and will fail if MW_PHPUNIT_TEST is not defined.
@ -219,7 +212,9 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
 		$key = $this->getCacheKey( $user, $target );
 		$this->cache->set( $key, $item );
 		$this->cacheIndex[$target->getNamespace()][$target->getDBkey()][$user->getId()] = $key;
-		$this->stats->increment( 'WatchedItemStore.cache' );
+		$this->statsFactory->getCounter( 'WatchedItemStore_cache_total' )
+			->copyToStatsdAt( 'WatchedItemStore.cache' )
+			->increment();
 	}

 	/**
@ -229,19 +224,28 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
 	private function uncache( UserIdentity $user, $target ) {
 		$this->cache->delete( $this->getCacheKey( $user, $target ) );
 		unset( $this->cacheIndex[$target->getNamespace()][$target->getDBkey()][$user->getId()] );
-		$this->stats->increment( 'WatchedItemStore.uncache' );
+		$this->statsFactory->getCounter( 'WatchedItemStore_uncache_total' )
+			->copyToStatsdAt( 'WatchedItemStore.uncache' )
+			->increment();
 	}

 	/**
 	 * @param LinkTarget|PageIdentity $target
 	 */
 	private function uncacheLinkTarget( $target ) {
-		$this->stats->increment( 'WatchedItemStore.uncacheLinkTarget' );
+		$this->statsFactory->getCounter( 'WatchedItemStore_uncacheLinkTarget_total' )
+			->copyToStatsdAt( 'WatchedItemStore.uncacheLinkTarget' )
+			->increment();
 		if ( !isset( $this->cacheIndex[$target->getNamespace()][$target->getDBkey()] ) ) {
 			return;
 		}
+
+		$uncacheLinkTargetItemsTotal = $this->statsFactory
+			->getCounter( 'WatchedItemStore_uncacheLinkTarget_items_total' )
+			->copyToStatsdAt( 'WatchedItemStore.uncacheLinkTarget.items' );
+
 		foreach ( $this->cacheIndex[$target->getNamespace()][$target->getDBkey()] as $key ) {
-			$this->stats->increment( 'WatchedItemStore.uncacheLinkTarget.items' );
+			$uncacheLinkTargetItemsTotal->increment();
 			$this->cache->delete( $key );
 		}
 	}
@ -250,11 +254,16 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
 	 * @param UserIdentity $user
 	 */
 	private function uncacheUser( UserIdentity $user ) {
-		$this->stats->increment( 'WatchedItemStore.uncacheUser' );
+		$this->statsFactory->getCounter( 'WatchedItemStore_uncacheUser_total' )
+			->copyToStatsdAt( 'WatchedItemStore.uncacheUser' )
+			->increment();
+		$uncacheUserItemsTotal = $this->statsFactory->getCounter( 'WatchedItemStore_uncacheUser_items_total' )
+			->copyToStatsdAt( 'WatchedItemStore.uncacheUser.items' );
+
 		foreach ( $this->cacheIndex as $dbKeyArray ) {
 			foreach ( $dbKeyArray as $userArray ) {
 				if ( isset( $userArray[$user->getId()] ) ) {
-					$this->stats->increment( 'WatchedItemStore.uncacheUser.items' );
+					$uncacheUserItemsTotal->increment();
 					$this->cache->delete( $userArray[$user->getId()] );
 				}
 			}
@ -693,10 +702,16 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac

 		$cached = $this->getCached( $user, $target );
 		if ( $cached && !$cached->isExpired() ) {
-			$this->stats->increment( 'WatchedItemStore.getWatchedItem.cached' );
+			$this->statsFactory->getCounter( 'WatchedItemStore_getWatchedItem_accesses_total' )
+				->setLabel( 'status', 'hit' )
+				->copyToStatsdAt( 'WatchedItemStore.getWatchedItem.cached' )
+				->increment();
 			return $cached;
 		}
-		$this->stats->increment( 'WatchedItemStore.getWatchedItem.load' );
+		$this->statsFactory->getCounter( 'WatchedItemStore_getWatchedItem_accesses_total' )
+			->setLabel( 'status', 'miss' )
+			->copyToStatsdAt( 'WatchedItemStore.getWatchedItem.load' )
+			->increment();
 		return $this->loadWatchedItem( $user, $target );
 	}

--- a/tests/phpunit/unit/includes/watcheditem/WatchedItemStoreUnitTest.php
+++ b/tests/phpunit/unit/includes/watcheditem/WatchedItemStoreUnitTest.php
@ -25,19 +25,27 @@ use Wikimedia\Rdbms\LBFactory;
 use Wikimedia\Rdbms\ReplaceQueryBuilder;
 use Wikimedia\Rdbms\SelectQueryBuilder;
 use Wikimedia\Rdbms\UpdateQueryBuilder;
+use Wikimedia\Stats\StatsFactory;
 use Wikimedia\TestingAccessWrapper;

 /**
 * @author Addshore
 * @author DannyS712
- * @todo This test should become unittest again once LinksMigration is done (T300222)
 *
 * @covers \WatchedItemStore
 */
-class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
+class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase {
 	use DummyServicesTrait;
 	use MockTitleTrait;

+	/** @var StatsFactory */
+	private $statsFactory;
+
+	protected function setUp(): void {
+		parent::setUp();
+		$this->statsFactory = StatsFactory::newNull();
+	}
+
 	/**
 	 * @return MockObject&IDatabase
 	 */
@ -190,7 +198,8 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
 			$mocks['readOnlyMode'] ?? $this->getDummyReadOnlyMode( false ),
 			$nsInfo,
 			$mocks['revisionLookup'] ?? $this->getMockRevisionLookup(),
-			$this->getMockLinkBatchFactory( $db )
+			$this->getMockLinkBatchFactory( $db ),
+			$this->statsFactory
 		);
 	}

@ -1169,6 +1178,15 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
 			new UserIdentityValue( 1, 'MockUser' ),
 			$testPageFactory( 100, 0, 'Some_Page' )
 		);
+
+		$this->assertSame(
+			1,
+			$this->statsFactory->getCounter( 'WatchedItemStore_uncache_total' )->getSampleCount()
+		);
+		$this->assertSame(
+			0,
+			$this->statsFactory->getCounter( 'WatchedItemStore_cache_total' )->getSampleCount()
+		);
 	}

 	/**
@ -1189,6 +1207,14 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
 			new UserIdentityValue( 0, 'AnonUser' ),
 			$testPageFactory( 100, 0, 'Some_Page' )
 		);
+		$this->assertSame(
+			0,
+			$this->statsFactory->getCounter( 'WatchedItemStore_uncache_total' )->getSampleCount()
+		);
+		$this->assertSame(
+			0,
+			$this->statsFactory->getCounter( 'WatchedItemStore_cache_total' )->getSampleCount()
+		);
 	}

 	/**
@ -1255,6 +1281,10 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
 				[ $testPageFactory( 100, 0, 'Some_Page' ), $testPageFactory( 101, 1, 'Some_Page' ) ]
 			)
 		);
+		$this->assertSame(
+			2,
+			$this->statsFactory->getCounter( 'WatchedItemStore_uncache_total' )->getSampleCount()
+		);
 	}

 	/**
@ -1277,6 +1307,10 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
 				[ $testPageFactory( 100, 0, 'Other_Page' ) ]
 			)
 		);
+		$this->assertSame(
+			0,
+			$this->statsFactory->getCounter( 'WatchedItemStore_uncache_total' )->getSampleCount()
+		);
 	}

 	public function testAddWatchBatchReturnsTrue_whenGivenEmptyList() {
@ -1294,6 +1328,10 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
 		$this->assertTrue(
 			$store->addWatchBatchForUser( $user, [] )
 		);
+		$this->assertSame(
+			0,
+			$this->statsFactory->getCounter( 'WatchedItemStore_uncache_total' )->getSampleCount()
+		);
 	}

 	/**
@ -1346,6 +1384,11 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
 		$this->assertEquals( 'SomeDbKey', $watchedItem->getTarget()->getDBkey() );
 		$this->assertSame( '20300101000000', $watchedItem->getExpiry() );
 		$this->assertSame( 0, $watchedItem->getTarget()->getNamespace() );
+
+		$this->assertSame(
+			1,
+			$this->statsFactory->getCounter( 'WatchedItemStore_cache_total' )->getSampleCount()
+		);
 	}

 	/**
@ -1372,6 +1415,10 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
 				$testPageFactory( 100, 0, 'SomeDbKey' )
 			)
 		);
+		$this->assertSame(
+			0,
+			$this->statsFactory->getCounter( 'WatchedItemStore_cache_total' )->getSampleCount()
+		);
 	}

 	/**
@ -1394,6 +1441,10 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
 				$testPageFactory( 100, 0, 'SomeDbKey' )
 			)
 		);
+		$this->assertSame(
+			0,
+			$this->statsFactory->getCounter( 'WatchedItemStore_cache_total' )->getSampleCount()
+		);
 	}

 	/**
@ -1439,6 +1490,10 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
 				$testPageFactory( 100, 0, 'SomeDbKey' )
 			)
 		);
+		$this->assertSame(
+			1,
+			$this->statsFactory->getCounter( 'WatchedItemStore_uncache_total' )->getSampleCount()
+		);
 	}

 	/**
@ -1468,6 +1523,10 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
 				$testPageFactory( 100, 0, 'SomeDbKey' )
 			)
 		);
+		$this->assertSame(
+			1,
+			$this->statsFactory->getCounter( 'WatchedItemStore_uncache_total' )->getSampleCount()
+		);
 	}

 	/**
@ -1491,6 +1550,10 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
 				$testPageFactory( 100, 0, 'SomeDbKey' )
 			)
 		);
+		$this->assertSame(
+			0,
+			$this->statsFactory->getCounter( 'WatchedItemStore_uncache_total' )->getSampleCount()
+		);
 	}

 	/**
@ -1583,6 +1646,11 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
 				$linkTarget
 			)
 		);
+		$this->assertSame(
+			1,
+			$this->statsFactory->getCounter( 'WatchedItemStore_getWatchedItem_accesses_total' )
+				->getSampleCount()
+		);
 	}

 	/**
@ -1626,6 +1694,12 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
 				$testPageFactory( 100, 0, 'SomeDbKey' )
 			)
 		);
+
+		$this->assertSame(
+			1,
+			$this->statsFactory->getCounter( 'WatchedItemStore_getWatchedItem_accesses_total' )
+				->getSampleCount()
+		);
 	}

 	/**
@ -1649,6 +1723,11 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
 				$testPageFactory( 100, 0, 'SomeDbKey' )
 			)
 		);
+		$this->assertSame(
+			0,
+			$this->statsFactory->getCounter( 'WatchedItemStore_getWatchedItem_accesses_total' )
+				->getSampleCount()
+		);
 	}

 	public function testGetWatchedItemsForUser() {