jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/368163 )
Change subject: Move constraint check tracking to LoggingHelper
......................................................................
Move constraint check tracking to LoggingHelper
LoggingHelper gets a statsd data factory inserted and is now responsible
for statsd tracking as well as structured logging. The tracking of
constraint check duration is moved from the DelegatingConstraintChecker
to the LoggingHelper.
Since LoggingHelper already has the short class name of the constraint
checker, that is also included in the tracked metric, which lets us show
just that short name in Grafana (instead of the constraint type item ID,
which is less useful.)
Bug: T171285
Change-Id: I2be54579238e7b92b0a8a384b0214c8142c029f1
---
M includes/ConstraintCheck/DelegatingConstraintChecker.php
M includes/ConstraintCheck/Helper/LoggingHelper.php
M includes/ConstraintReportFactory.php
M tests/phpunit/Api/CheckConstraintsTest.php
M tests/phpunit/Helper/LoggingHelperTest.php
5 files changed, 46 insertions(+), 13 deletions(-)
Approvals:
Jonas Kress (WMDE): Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/ConstraintCheck/DelegatingConstraintChecker.php
b/includes/ConstraintCheck/DelegatingConstraintChecker.php
index 4706aa5..3cb708a 100644
--- a/includes/ConstraintCheck/DelegatingConstraintChecker.php
+++ b/includes/ConstraintCheck/DelegatingConstraintChecker.php
@@ -4,7 +4,6 @@
use InvalidArgumentException;
use LogicException;
-use MediaWiki\MediaWikiServices;
use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\DataModel\Services\Lookup\EntityLookup;
@@ -343,7 +342,6 @@
private function getCheckResultFor( Statement $statement, Constraint
$constraint, EntityDocument $entity ) {
if ( array_key_exists( $constraint->getConstraintTypeItemId(),
$this->checkerMap ) ) {
$checker =
$this->checkerMap[$constraint->getConstraintTypeItemId()];
- $statsd =
MediaWikiServices::getInstance()->getStatsdDataFactory();
$startTime = microtime( true );
try {
@@ -368,10 +366,6 @@
);
}
$endTime = microtime( true );
- $statsd->timing(
- 'wikibase.quality.constraints.check.timing.' .
$constraint->getConstraintTypeItemId(),
- ( $endTime - $startTime ) * 1000
- );
try {
$constraintStatus =
$this->constraintParameterParser
diff --git a/includes/ConstraintCheck/Helper/LoggingHelper.php
b/includes/ConstraintCheck/Helper/LoggingHelper.php
index 01e1552..cd48ce0 100644
--- a/includes/ConstraintCheck/Helper/LoggingHelper.php
+++ b/includes/ConstraintCheck/Helper/LoggingHelper.php
@@ -3,6 +3,7 @@
namespace WikibaseQuality\ConstraintReport\ConstraintCheck\Helper;
use Config;
+use IBufferingStatsdDataFactory;
use Psr\Log\LoggerInterface;
use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Statement\Statement;
@@ -10,13 +11,18 @@
use WikibaseQuality\ConstraintReport\ConstraintCheck\Result\CheckResult;
/**
- * Helper class for logging messages if necessary.
+ * Helper class for tracking and logging messages.
*
* @package WikibaseQuality\ConstraintReport\ConstraintCheck\Helper
* @author Lucas Werkmeister
* @license GNU GPL v2+
*/
class LoggingHelper {
+
+ /**
+ * @var IBufferingStatsdDataFactory
+ */
+ private $dataFactory;
/**
* @type LoggerInterface
@@ -29,13 +35,16 @@
private $constraintCheckDurationLimits;
/**
+ * @param IBufferingStatsdDataFactory $dataFactory,
* @param LoggerInterface $logger
* @param Config $config
*/
public function __construct(
+ IBufferingStatsdDataFactory $dataFactory,
LoggerInterface $logger,
Config $config
) {
+ $this->dataFactory = $dataFactory;
$this->logger = $logger;
$this->constraintCheckDurationLimits = [
'info' => $config->get(
'WBQualityConstraintsCheckDurationInfoSeconds' ),
@@ -44,7 +53,9 @@
}
/**
- * Log a constraint check if it took longer than a certain time.
+ * Log a constraint check.
+ * The constraint check is tracked on the statsd data factory,
+ * and also logged with the logger interface if it took longer than a
certain time.
* Multiple limits corresponding to different log levels can be
specified in the configuration;
* checks that exceed a higher limit are logged at a more severe level.
*
@@ -65,6 +76,16 @@
$durationSeconds,
$method
) {
+ $constraintCheckerClassShortName = substr( strrchr(
$constraintCheckerClass, '\\' ), 1 );
+ $constraintTypeItemId = $constraint->getConstraintTypeItemId();
+
+ $this->dataFactory->timing(
+ 'wikibase.quality.constraints.check.timing.' .
+ $constraintTypeItemId . '-' .
+ $constraintCheckerClassShortName,
+ $durationSeconds * 1000
+ );
+
// find the longest limit (and associated log level) that the
duration exceeds
foreach ( $this->constraintCheckDurationLimits as $level =>
$limit ) {
if (
@@ -82,8 +103,6 @@
return;
}
- $constraintCheckerClassShortName = substr( strrchr(
$constraintCheckerClass, '\\' ), 1 );
-
$this->logger->log(
$logLevel,
'Constraint check with
{constraintCheckerClassShortName} ' .
@@ -96,7 +115,7 @@
'limitSeconds' => $limitSeconds,
'constraintId' =>
$constraint->getConstraintId(),
'constraintPropertyId' =>
$constraint->getPropertyId()->getSerialization(),
- 'constraintTypeItemId' =>
$constraint->getConstraintTypeItemId(),
+ 'constraintTypeItemId' => $constraintTypeItemId,
'constraintParameters' =>
$constraint->getConstraintParameters(),
'constraintCheckerClass' =>
$constraintCheckerClass,
'constraintCheckerClassShortName' =>
$constraintCheckerClassShortName,
diff --git a/includes/ConstraintReportFactory.php
b/includes/ConstraintReportFactory.php
index 43d0139..87ff66c 100644
--- a/includes/ConstraintReportFactory.php
+++ b/includes/ConstraintReportFactory.php
@@ -187,6 +187,7 @@
$this->constraintParameterParser,
$this->statementGuidParser,
new LoggingHelper(
+
MediaWikiServices::getInstance()->getStatsdDataFactory(),
LoggerFactory::getInstance(
'WikibaseQualityConstraints' ),
$this->config
)
diff --git a/tests/phpunit/Api/CheckConstraintsTest.php
b/tests/phpunit/Api/CheckConstraintsTest.php
index 49a0ae9..be77be6 100644
--- a/tests/phpunit/Api/CheckConstraintsTest.php
+++ b/tests/phpunit/Api/CheckConstraintsTest.php
@@ -6,6 +6,7 @@
use DataValues\UnknownValue;
use HashConfig;
use MediaWiki\Logger\LoggerFactory;
+use MediaWiki\MediaWikiServices;
use RequestContext;
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
@@ -122,6 +123,7 @@
$constraintParameterParser,
$repo->getStatementGuidParser(),
new LoggingHelper(
+
MediaWikiServices::getInstance()->getStatsdDataFactory(),
LoggerFactory::getInstance(
'WikibaseQualityConstraints' ),
$config
)
diff --git a/tests/phpunit/Helper/LoggingHelperTest.php
b/tests/phpunit/Helper/LoggingHelperTest.php
index 8cb2e0e..c0d7b77 100644
--- a/tests/phpunit/Helper/LoggingHelperTest.php
+++ b/tests/phpunit/Helper/LoggingHelperTest.php
@@ -3,6 +3,7 @@
namespace WikibaseQuality\ConstraintReport\Test\ConstraintChecker;
use HashConfig;
+use IBufferingStatsdDataFactory;
use Psr\Log\LoggerInterface;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\Repo\Tests\NewItem;
@@ -37,6 +38,14 @@
'test message'
);
+ $dataFactory = $this->getMock(
IBufferingStatsdDataFactory::class );
+ $dataFactory->expects( $this->once() )
+ ->method( 'timing' )
+ ->with(
+ $this->identicalTo(
'wikibase.quality.constraints.check.timing.Q100-TestChecker' ),
+ $this->identicalTo( $durationSeconds * 1000 )
+ );
+
$logger = $this->getMock( LoggerInterface::class );
$logger->expects( $expectedLevel !== null ? $this->once() :
$this->never() )
->method( 'log' )
@@ -68,7 +77,7 @@
)
);
- $loggingHelper = new LoggingHelper( $logger, new HashConfig( [
+ $loggingHelper = new LoggingHelper( $dataFactory, $logger, new
HashConfig( [
'WBQualityConstraintsCheckDurationInfoSeconds' => 1.0,
'WBQualityConstraintsCheckDurationWarningSeconds' =>
10.0,
] ) );
@@ -102,10 +111,18 @@
'test message'
);
+ $dataFactory = $this->getMock(
IBufferingStatsdDataFactory::class );
+ $dataFactory->expects( $this->once() )
+ ->method( 'timing' )
+ ->with(
+ $this->identicalTo(
'wikibase.quality.constraints.check.timing.Q100-TestChecker' ),
+ $this->identicalTo( 5000.0 )
+ );
+
$logger = $this->getMock( LoggerInterface::class );
$logger->expects( $this->never() )->method( 'log' );
- $loggingHelper = new LoggingHelper( $logger, new HashConfig( [
+ $loggingHelper = new LoggingHelper( $dataFactory, $logger, new
HashConfig( [
'WBQualityConstraintsCheckDurationInfoSeconds' => null,
'WBQualityConstraintsCheckDurationWarningSeconds' =>
null,
] ) );
--
To view, visit https://gerrit.wikimedia.org/r/368163
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I2be54579238e7b92b0a8a384b0214c8142c029f1
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/WikibaseQualityConstraints
Gerrit-Branch: master
Gerrit-Owner: Lucas Werkmeister (WMDE) <[email protected]>
Gerrit-Reviewer: Jonas Kress (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
