Revert "REST: collect metrics on endpoint access"
This reverts commit d32c260ed0.
Reason for revert: Timo has reservations, I'll submit an updated version later.
Change-Id: I71d4d61a879fda4dccfc105127446cfedde75a7b
This commit is contained in:
parent
d32c260ed0
commit
5c7cca8776
5 changed files with 5 additions and 60 deletions
|
|
@ -63,8 +63,6 @@ class EntryPoint {
|
|||
$authority
|
||||
);
|
||||
|
||||
$metrics = $services->getStatsdDataFactory();
|
||||
|
||||
return ( new Router(
|
||||
self::getRouteFiles( $conf ),
|
||||
ExtensionRegistry::getInstance()->getAttribute( 'RestRoutes' ),
|
||||
|
|
@ -78,9 +76,7 @@ class EntryPoint {
|
|||
new MWErrorReporter(),
|
||||
$services->getHookContainer(),
|
||||
$context->getRequest()->getSession()
|
||||
) )
|
||||
->setCors( $cors )
|
||||
->setMetrics( $metrics );
|
||||
) )->setCors( $cors );
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -87,15 +87,6 @@ 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.
|
||||
|
|
|
|||
|
|
@ -4,7 +4,6 @@ namespace MediaWiki\Rest;
|
|||
|
||||
use AppendIterator;
|
||||
use BagOStuff;
|
||||
use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface;
|
||||
use MediaWiki\Config\ServiceOptions;
|
||||
use MediaWiki\HookContainer\HookContainer;
|
||||
use MediaWiki\MainConfigNames;
|
||||
|
|
@ -14,7 +13,6 @@ 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;
|
||||
|
|
@ -82,9 +80,6 @@ class Router {
|
|||
/** @var Session */
|
||||
private $session;
|
||||
|
||||
/** @var StatsdDataFactoryInterface */
|
||||
private $metrics;
|
||||
|
||||
/**
|
||||
* @internal
|
||||
* @var array
|
||||
|
|
@ -140,8 +135,6 @@ class Router {
|
|||
$this->errorReporter = $errorReporter;
|
||||
$this->hookContainer = $hookContainer;
|
||||
$this->session = $session;
|
||||
|
||||
$this->metrics = new NullStatsdDataFactory();
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -409,25 +402,14 @@ 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.
|
||||
$metricsKey = 'REST/endpoint/' . strtr( $handler->getPath(), [
|
||||
'{' => '_',
|
||||
'}' => '_',
|
||||
] );
|
||||
|
||||
$this->metrics->increment( "$metricsKey.req.$requestMethod" );
|
||||
|
||||
try {
|
||||
$response = $this->executeHandler( $handler );
|
||||
return $this->executeHandler( $handler );
|
||||
} catch ( HttpException $e ) {
|
||||
$response = $this->responseFactory->createFromException( $e );
|
||||
return $this->responseFactory->createFromException( $e );
|
||||
} catch ( Throwable $e ) {
|
||||
$this->errorReporter->reportError( $e, $handler, $request );
|
||||
$response = $this->responseFactory->createFromException( $e );
|
||||
return $this->responseFactory->createFromException( $e );
|
||||
}
|
||||
|
||||
$this->metrics->increment( "$metricsKey.response.status" . $response->getStatusCode() );
|
||||
return $response;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -525,14 +507,4 @@ class Router {
|
|||
return $this;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param StatsdDataFactoryInterface $metrics
|
||||
* @return self
|
||||
*/
|
||||
public function setMetrics( StatsdDataFactoryInterface $metrics ): self {
|
||||
$this->metrics = $metrics;
|
||||
|
||||
return $this;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -47,19 +47,12 @@ class RestStructureTest extends MediaWikiIntegrationTestCase {
|
|||
|
||||
$services = $this->createNoOpMock(
|
||||
MediaWikiServices::class,
|
||||
[
|
||||
'getMainConfig',
|
||||
'getHookContainer',
|
||||
'getObjectFactory',
|
||||
'getLocalServerObjectCache',
|
||||
'getStatsdDataFactory',
|
||||
]
|
||||
[ 'getMainConfig', 'getHookContainer', 'getObjectFactory', 'getLocalServerObjectCache' ]
|
||||
);
|
||||
$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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -113,13 +113,6 @@ 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() );
|
||||
|
|
|
|||
Loading…
Reference in a new issue