stats: allow passing a single/multiple namespaces to copyToStatsdAt()

There are a few places in MediaWiki code where we copy the same
measurement to multiple statsd namespaces.  This patch enables StatsLib
to take an array of namespaces and pass the single measurement call to
each configured statsd namespace.

Refactors handling of $statsdNamespaces to BaseMetric to consolidate
validation logic.

Adds tests.

Bug: T355361
Change-Id: Ib407d0d33dbcb5ae9b77518e01ee0917217685b4

includes/libs/Stats/Metrics/BaseMetric.php

@@ -22,6 +22,7 @@
 namespace Wikimedia\Stats\Metrics;
 
 use IBufferingStatsdDataFactory;
+use InvalidArgumentException;
 use Wikimedia\Stats\Exceptions\IllegalOperationException;
 use Wikimedia\Stats\Sample;
 use Wikimedia\Stats\StatsUtils;
@@ -65,6 +66,9 @@ class BaseMetric implements BaseMetricInterface {
  /** @var IBufferingStatsdDataFactory|null */
  private ?IBufferingStatsdDataFactory $statsdDataFactory = null;
 
+ /** @var string[] */
+ private array $statsdNamespaces = [];
+
  /** @inheritDoc */
  public function __construct( string $component, string $name ) {
  $this->component = $component;
@@ -133,6 +137,29 @@ public function withStatsdDataFactory( $statsdDataFactory ): BaseMetric {
  return $this;
  }
 
+ /** @inheritDoc */
+ public function setStatsdNamespaces( $statsdNamespaces ): void {
+ if ( $this->statsdDataFactory === null ) {
+ return;
+ }
+ $statsdNamespaces = is_array( $statsdNamespaces ) ? $statsdNamespaces : [ $statsdNamespaces ];
+
+ foreach ( $statsdNamespaces as $namespace ) {
+ if ( $namespace === '' ) {
+ throw new InvalidArgumentException( "Stats: StatsD namespace cannot be empty." );
+ }
+ if ( !is_string( $namespace ) ) {
+ throw new InvalidArgumentException( "Stats: StatsD namespace must be a string." );
+ }
+ }
+ $this->statsdNamespaces = $statsdNamespaces;
+ }
+
+ /** @inheritDoc */
+ public function getStatsdNamespaces(): array {
+ return $this->statsdNamespaces;
+ }
+
  /**
  * Registers a label key
  *

includes/libs/Stats/Metrics/BaseMetricInterface.php

@@ -134,6 +134,21 @@ public function clearLabels(): void;
  */
  public function getStatsdDataFactory();
 
+ /**
+ * Validates and sets legacy StatsD namespaces.
+ *
+ * @param string|string[] $statsdNamespaces
+ * @return void
+ */
+ public function setStatsdNamespaces( $statsdNamespaces ): void;
+
+ /**
+ * Returns the configured legacy StatsD namespaces.
+ *
+ * @return string[]
+ */
+ public function getStatsdNamespaces(): array;
+
  /**
  * StatsD Data Factory instance to copy metrics to.
  *

includes/libs/Stats/Metrics/CounterMetric.php

@@ -45,9 +45,6 @@ class CounterMetric implements MetricInterface {
  */
  private const TYPE_INDICATOR = "c";
 
- /** @var string|null */
- private ?string $statsdNamespace = null;
-
  /** @var BaseMetricInterface */
  private BaseMetricInterface $baseMetric;
 
@@ -76,9 +73,8 @@ public function increment(): void {
  * @return void
  */
  public function incrementBy( float $value ): void {
- if ( $this->statsdNamespace !== null ) {
- $this->baseMetric->getStatsdDataFactory()->updateCount( $this->statsdNamespace, $value );
- $this->statsdNamespace = null;
+ foreach ( $this->baseMetric->getStatsdNamespaces() as $namespace ) {
+ $this->baseMetric->getStatsdDataFactory()->updateCount( $namespace, $value );
  }
 
  try {
@@ -149,9 +145,13 @@ public function setLabel( string $key, string $value ) {
  }
 
  /** @inheritDoc */
- public function copyToStatsdAt( string $statsdNamespace ): CounterMetric {
- if ( $this->baseMetric->getStatsdDataFactory() !== null ) {
- $this->statsdNamespace = $statsdNamespace;
+ public function copyToStatsdAt( $statsdNamespaces ) {
+ try {
+ $this->baseMetric->setStatsdNamespaces( $statsdNamespaces );
+ } catch ( InvalidArgumentException $ex ) {
+ // Log the condition and give the caller something that will absorb calls.
+ trigger_error( $ex->getMessage(), E_USER_WARNING );
+ return new NullMetric;
  }
  return $this;
  }

includes/libs/Stats/Metrics/GaugeMetric.php

@@ -45,9 +45,6 @@ class GaugeMetric implements MetricInterface {
  */
  private const TYPE_INDICATOR = "g";
 
- /** @var string|null */
- private ?string $statsdNamespace = null;
-
  /** @var BaseMetricInterface */
  private BaseMetricInterface $baseMetric;
 
@@ -67,9 +64,8 @@ public function __construct( $baseMetric, $logger ) {
  * @return void
  */
  public function set( float $value ): void {
- if ( $this->statsdNamespace !== null ) {
- $this->baseMetric->getStatsdDataFactory()->updateCount( $this->statsdNamespace, $value );
- $this->statsdNamespace = null;
+ foreach ( $this->baseMetric->getStatsdNamespaces() as $namespace ) {
+ $this->baseMetric->getStatsdDataFactory()->updateCount( $namespace, $value );
  }
 
  try {
@@ -140,9 +136,13 @@ public function setLabel( string $key, string $value ) {
  }
 
  /** @inheritDoc */
- public function copyToStatsdAt( string $statsdNamespace ) {
- if ( $this->baseMetric->getStatsdDataFactory() !== null ) {
- $this->statsdNamespace = $statsdNamespace;
+ public function copyToStatsdAt( $statsdNamespaces ) {
+ try {
+ $this->baseMetric->setStatsdNamespaces( $statsdNamespaces );
+ } catch ( InvalidArgumentException $ex ) {
+ // Log the condition and give the caller something that will absorb calls.
+ trigger_error( $ex->getMessage(), E_USER_WARNING );
+ return new NullMetric;
  }
  return $this;
  }

includes/libs/Stats/Metrics/MetricInterface.php

@@ -88,10 +88,12 @@ public function setLabel( string $key, string $value );
  /**
  * Copies metric operation to StatsD at provided namespace.
  *
- * @param string $statsdNamespace
+ * Takes a namespace or multiple namespaces.
+ *
+ * @param string|string[] $statsdNamespaces
  * @return CounterMetric|GaugeMetric|TimingMetric|NullMetric
  */
- public function copyToStatsdAt( string $statsdNamespace );
+ public function copyToStatsdAt( $statsdNamespaces );
 
  /**
  * Returns metric with cleared labels.

includes/libs/Stats/Metrics/TimingMetric.php

@@ -46,9 +46,6 @@ class TimingMetric implements MetricInterface {
  */
  private const TYPE_INDICATOR = "ms";
 
- /** @var string|null */
- private ?string $statsdNamespace = null;
-
  /** @var BaseMetricInterface */
  private BaseMetricInterface $baseMetric;
 
@@ -97,9 +94,8 @@ public function stop(): void {
  * @return void
  */
  public function observe( float $value ): void {
- if ( $this->statsdNamespace !== null ) {
- $this->baseMetric->getStatsdDataFactory()->timing( $this->statsdNamespace, $value );
- $this->statsdNamespace = null;
+ foreach ( $this->baseMetric->getStatsdNamespaces() as $namespace ) {
+ $this->baseMetric->getStatsdDataFactory()->timing( $namespace, $value );
  }
 
  try {
@@ -170,9 +166,13 @@ public function setLabel( string $key, string $value ) {
  }
 
  /** @inheritDoc */
- public function copyToStatsdAt( string $statsdNamespace ) {
- if ( $this->baseMetric->getStatsdDataFactory() !== null ) {
- $this->statsdNamespace = $statsdNamespace;
+ public function copyToStatsdAt( $statsdNamespaces ) {
+ try {
+ $this->baseMetric->setStatsdNamespaces( $statsdNamespaces );
+ } catch ( InvalidArgumentException $ex ) {
+ // Log the condition and give the caller something that will absorb calls.
+ trigger_error( $ex->getMessage(), E_USER_WARNING );
+ return new NullMetric;
  }
  return $this;
  }

tests/phpunit/unit/includes/libs/Stats/MetricTest.php

@@ -2,10 +2,13 @@
 
 namespace Wikimedia\Tests\Stats;
 
+use IBufferingStatsdDataFactory;
 use PHPUnit\Framework\TestCase;
 use Psr\Log\NullLogger;
 use Wikimedia\Stats\Emitters\NullEmitter;
 use Wikimedia\Stats\Exceptions\IllegalOperationException;
+use Wikimedia\Stats\Metrics\BaseMetric;
+use Wikimedia\Stats\Metrics\CounterMetric;
 use Wikimedia\Stats\Metrics\NullMetric;
 use Wikimedia\Stats\OutputFormats;
 use Wikimedia\Stats\StatsCache;
@@ -273,4 +276,43 @@ public function testSampleRateDisallowed() {
  $metric = @$m->getTiming( 'testMetricTiming' )->setSampleRate( 0.5 );
  $this->assertInstanceOf( NullMetric::class, $metric );
  }
+
+ public function testCopyToStatsdAtEmptyArrayResetsValue() {
+ $baseMetric = new BaseMetric( '', 'testMetric' );
+ $metric = new CounterMetric(
+ $baseMetric->withStatsdDataFactory( $this->createMock( IBufferingStatsdDataFactory::class ) ),
+ new NullLogger
+ );
+ $metric->copyToStatsdAt( 'test' );
+ $this->assertEquals( [ 'test' ], $baseMetric->getStatsdNamespaces() );
+ $metric->copyToStatsdAt( [] );
+ $this->assertEquals( [], $baseMetric->getStatsdNamespaces() );
+ }
+
+ public function testHandleInvalidStatsdNamespace() {
+ $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger );
+ $m = $m->withStatsdDataFactory( $this->createMock( IBufferingStatsdDataFactory::class ) );
+ $this->expectWarning();
+ $this->expectWarningMessage( 'Stats: StatsD namespace must be a string.' );
+ $metric = $m->getCounter( 'testMetricCounter' )->copyToStatsdAt( null );
+ $this->assertInstanceOf( NullMetric::class, $metric );
+ }
+
+ public function testHandleEmptyStatsdNamespace() {
+ $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger );
+ $m = $m->withStatsdDataFactory( $this->createMock( IBufferingStatsdDataFactory::class ) );
+ $this->expectWarning();
+ $this->expectWarningMessage( 'Stats: StatsD namespace cannot be empty.' );
+ $metric = $m->getCounter( 'testMetricCounter' )->copyToStatsdAt( '' );
+ $this->assertInstanceOf( NullMetric::class, $metric );
+ }
+
+ public function testHandleNonStringStatsdNamespaceInArray() {
+ $m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger );
+ $m = $m->withStatsdDataFactory( $this->createMock( IBufferingStatsdDataFactory::class ) );
+ $this->expectWarning();
+ $this->expectWarningMessage( 'Stats: StatsD namespace must be a string.' );
+ $metric = $m->getCounter( 'testMetricCounter' )->copyToStatsdAt( [ null ] );
+ $this->assertInstanceOf( NullMetric::class, $metric );
+ }
 }