From f2e9d5108dfc5411d2557fe6988abf8dea7a87e2 Mon Sep 17 00:00:00 2001
From: Daniel Kinzler <dkinzler@wikimedia.org>
Date: Tue, 13 Dec 2022 21:00:59 +0000
Subject: [PATCH] REST: collect metrics on endpoint access

This is a modified version of Ie282bc5b5f5df0bbd6a40c8362ba73fcbbf36c2e
which was reverted in 5c7cca87763b5f40cf9a3788aca957082c3bd4e7.

Bug: T321969
Change-Id: I566d54a473aa51c4cdaada21a49d63c0624aab93
---
 includes/Rest/EntryPoint.php                  |  6 ++-
 includes/Rest/Handler.php                     |  9 ++++
 includes/Rest/Router.php                      | 44 +++++++++++++++++--
 tests/phpunit/structure/RestStructureTest.php |  9 +++-
 .../includes/Rest/Handler/HandlerTest.php     |  7 +++
 5 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/includes/Rest/EntryPoint.php b/includes/Rest/EntryPoint.php
index 1beb79f22c3..dfc288b882a 100644
--- a/includes/Rest/EntryPoint.php
+++ b/includes/Rest/EntryPoint.php
@@ -63,6 +63,8 @@ class EntryPoint {
 			$authority
 		);
 
+		$stats = $services->getStatsdDataFactory();
+
 		return ( new Router(
 			self::getRouteFiles( $conf ),
 			ExtensionRegistry::getInstance()->getAttribute( 'RestRoutes' ),
@@ -76,7 +78,9 @@ class EntryPoint {
 			new MWErrorReporter(),
 			$services->getHookContainer(),
 			$context->getRequest()->getSession()
-		) )->setCors( $cors );
+		) )
+			->setCors( $cors )
+			->setStats( $stats );
 	}
 
 	/**
diff --git a/includes/Rest/Handler.php b/includes/Rest/Handler.php
index 74de58aed52..b6507842dca 100644
--- a/includes/Rest/Handler.php
+++ b/includes/Rest/Handler.php
@@ -87,6 +87,15 @@ abstract class Handler {
 		$this->postInitSetup();
 	}
 
+	/**
+	 * Returns the path this handler is bound to, including path variables.
+	 *
+	 * @return string
+	 */
+	public function getPath(): string {
+		return $this->getConfig()['path'];
+	}
+
 	/**
 	 * Get the Router. The return type declaration causes it to raise
 	 * a fatal error if init() has not yet been called.
diff --git a/includes/Rest/Router.php b/includes/Rest/Router.php
index b4c9b1d65e4..c304840b123 100644
--- a/includes/Rest/Router.php
+++ b/includes/Rest/Router.php
@@ -4,6 +4,7 @@ namespace MediaWiki\Rest;
 
 use AppendIterator;
 use BagOStuff;
+use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface;
 use MediaWiki\Config\ServiceOptions;
 use MediaWiki\HookContainer\HookContainer;
 use MediaWiki\MainConfigNames;
@@ -13,6 +14,7 @@ use MediaWiki\Rest\PathTemplateMatcher\PathMatcher;
 use MediaWiki\Rest\Reporter\ErrorReporter;
 use MediaWiki\Rest\Validator\Validator;
 use MediaWiki\Session\Session;
+use NullStatsdDataFactory;
 use Throwable;
 use Wikimedia\Message\MessageValue;
 use Wikimedia\ObjectFactory\ObjectFactory;
@@ -80,6 +82,9 @@ class Router {
 	/** @var Session */
 	private $session;
 
+	/** @var StatsdDataFactoryInterface */
+	private $stats;
+
 	/**
 	 * @internal
 	 * @var array
@@ -135,6 +140,8 @@ class Router {
 		$this->errorReporter = $errorReporter;
 		$this->hookContainer = $hookContainer;
 		$this->session = $session;
+
+		$this->stats = new NullStatsdDataFactory();
 	}
 
 	/**
@@ -402,14 +409,34 @@ class Router {
 		$request->setPathParams( array_map( 'rawurldecode', $match['params'] ) );
 		$handler = $this->createHandler( $request, $match['userData'] );
 
+		// Replace any characters that may have a special meaning in the metrics DB.
+		$pathForMetrics = $handler->getPath();
+		$pathForMetrics = strtr( $pathForMetrics, '{}:', '-' );
+		$pathForMetrics = strtr( $pathForMetrics, '/.', '_' );
+
+		$statTime = microtime( true );
+
 		try {
-			return $this->executeHandler( $handler );
+			$response = $this->executeHandler( $handler );
 		} catch ( HttpException $e ) {
-			return $this->responseFactory->createFromException( $e );
+			$response = $this->responseFactory->createFromException( $e );
 		} catch ( Throwable $e ) {
 			$this->errorReporter->reportError( $e, $handler, $request );
-			return $this->responseFactory->createFromException( $e );
+			$response = $this->responseFactory->createFromException( $e );
 		}
+
+		// gather metrics
+		if ( $response->getStatusCode() >= 400 ) {
+			// count how often we return which error code
+			$statusCode = $response->getStatusCode();
+			$this->stats->increment( "rest_api_errors.$pathForMetrics.$requestMethod.$statusCode" );
+		} else {
+			// measure how long it takes to generate a response
+			$microtime = ( microtime( true ) - $statTime ) * 1000;
+			$this->stats->timing( "rest_api_latency.$pathForMetrics.$requestMethod", $microtime );
+		}
+
+		return $response;
 	}
 
 	/**
@@ -507,4 +534,15 @@ class Router {
 		return $this;
 	}
 
+	/**
+	 * @param StatsdDataFactoryInterface $stats
+	 *
+	 * @return self
+	 */
+	public function setStats( StatsdDataFactoryInterface $stats ): self {
+		$this->stats = $stats;
+
+		return $this;
+	}
+
 }
diff --git a/tests/phpunit/structure/RestStructureTest.php b/tests/phpunit/structure/RestStructureTest.php
index 9d1c754e548..e17fcfb3315 100644
--- a/tests/phpunit/structure/RestStructureTest.php
+++ b/tests/phpunit/structure/RestStructureTest.php
@@ -47,12 +47,19 @@ class RestStructureTest extends MediaWikiIntegrationTestCase {
 
 		$services = $this->createNoOpMock(
 			MediaWikiServices::class,
-			[ 'getMainConfig', 'getHookContainer', 'getObjectFactory', 'getLocalServerObjectCache' ]
+			[
+				'getMainConfig',
+				'getHookContainer',
+				'getObjectFactory',
+				'getLocalServerObjectCache',
+				'getStatsdDataFactory',
+			]
 		);
 		$services->method( 'getMainConfig' )->willReturn( $config );
 		$services->method( 'getHookContainer' )->willReturn( $hookContainer );
 		$services->method( 'getObjectFactory' )->willReturn( $objectFactory );
 		$services->method( 'getLocalServerObjectCache' )->willReturn( new EmptyBagOStuff() );
+		$services->method( 'getStatsdDataFactory' )->willReturn( new NullStatsdDataFactory() );
 
 		return $services;
 	}
diff --git a/tests/phpunit/unit/includes/Rest/Handler/HandlerTest.php b/tests/phpunit/unit/includes/Rest/Handler/HandlerTest.php
index e1f8d724bff..6b570bb3c36 100644
--- a/tests/phpunit/unit/includes/Rest/Handler/HandlerTest.php
+++ b/tests/phpunit/unit/includes/Rest/Handler/HandlerTest.php
@@ -113,6 +113,13 @@ class HandlerTest extends \MediaWikiUnitTestCase {
 		$this->assertStringEndsWith( $expected, $url );
 	}
 
+	public function testGetPath() {
+		$handler = $this->newHandler();
+		$request = new RequestData();
+		$this->initHandler( $handler, $request, [ 'path' => 'just/some/path' ] );
+		$this->assertSame( 'just/some/path', $handler->getPath() );
+	}
+
 	public function testGetResponseFactory() {
 		$handler = $this->newHandler();
 		$this->initHandler( $handler, new RequestData() );