Stats: simplify metrics configuration, enforce builder pattern

BREAKING INTERFACE CHANGE

 * getMetric() (and all subtypes) now only require a $name.
 * Labels are added dynamically and order enforced by BaseMetric labelKeys.
 * Metric component is now defined in the service instance.
 * Remove InvalidLabelsException.
 * Make addLabelKey() private.
 * Removed unused functions.
 * NullMetric to return $this - support builder pattern.
 * Update tests.

Bug: T240685
Change-Id: Id8cb62ec74907cb54dc39b6a228a230eed0c957d

7 files changed, 27 insertions, 69 deletions

diff --git a/includes/libs/Stats/Metrics/BaseMetric.php b/includes/libs/Stats/Metrics/BaseMetric.php
index 04a34d7f2f91..3e60eb6db6ea 100644
--- a/includes/libs/Stats/Metrics/BaseMetric.php
+++ b/includes/libs/Stats/Metrics/BaseMetric.php
@@ -41,7 +41,7 @@ use Wikimedia\Stats\StatsUtils;
 class BaseMetric implements BaseMetricInterface {
 
 	/** @var float */
-	private $sampleRate = StatsUtils::DEFAULT_SAMPLE_RATE;
+	private float $sampleRate = StatsUtils::DEFAULT_SAMPLE_RATE;
 
 	/** @var string */
 	private string $name;
@@ -106,8 +106,13 @@ class BaseMetric implements BaseMetricInterface {
 		$this->workingLabels[$key] = StatsUtils::normalizeString( $value );
 	}
 
-	/** @inheritDoc */
-	public function addLabelKey( string $key ): void {
+	/**
+	 * Registers a label key
+	 *
+	 * @param string $key
+	 * @return void
+	 */
+	private function addLabelKey( string $key ): void {
 		if ( in_array( $key, $this->labelKeys, true ) ) {
 			return;  // key already exists
 		}
diff --git a/includes/libs/Stats/Metrics/BaseMetricInterface.php b/includes/libs/Stats/Metrics/BaseMetricInterface.php
index 2e16cae0c2b4..7deb18e4c0c9 100644
--- a/includes/libs/Stats/Metrics/BaseMetricInterface.php
+++ b/includes/libs/Stats/Metrics/BaseMetricInterface.php
@@ -85,14 +85,6 @@ interface BaseMetricInterface {
 	public function addLabel( string $key, string $value ): void;
 
 	/**
-	 * Registers a label key
-	 *
-	 * @param string $key
-	 * @return void
-	 */
-	public function addLabelKey( string $key ): void;
-
-	/**
 	 * Returns array of label keys.
 	 *
 	 * @return string[]
diff --git a/includes/libs/Stats/Metrics/CounterMetric.php b/includes/libs/Stats/Metrics/CounterMetric.php
index 0e020d841f10..2ba4d8aee248 100644
--- a/includes/libs/Stats/Metrics/CounterMetric.php
+++ b/includes/libs/Stats/Metrics/CounterMetric.php
@@ -25,7 +25,6 @@ use InvalidArgumentException;
 use Psr\Log\LoggerInterface;
 use Wikimedia\Stats\Exceptions\IllegalOperationException;
 use Wikimedia\Stats\Sample;
-use Wikimedia\Stats\StatsUtils;
 
 /**
  * Counter Metric Implementation
@@ -61,11 +60,10 @@ class CounterMetric implements MetricInterface {
 	/**
 	 * Increments metric by one.
 	 *
-	 * @param string[] $labels
 	 * @return void
 	 */
-	public function increment( array $labels = [] ): void {
-		$this->incrementBy( 1, $labels );
+	public function increment(): void {
+		$this->incrementBy( 1 );
 	}
 
 	/**
@@ -76,10 +74,6 @@ class CounterMetric implements MetricInterface {
 	 * @return void
 	 */
 	public function incrementBy( int $value, array $labels = [] ): void {
-		StatsUtils::validateLabels( $this->baseMetric->getLabelKeys(), $labels );
-		foreach ( $this->baseMetric->getLabelKeys() as $i => $labelKey ) {
-			$this->baseMetric->addLabel( $labelKey, $labels[$i] );
-		}
 		$this->baseMetric->addSample( new Sample( $this->baseMetric->getLabelValues(), $value ) );
 	}
 
@@ -113,7 +107,8 @@ class CounterMetric implements MetricInterface {
 		try {
 			$this->baseMetric->setSampleRate( $sampleRate );
 		} catch ( IllegalOperationException | InvalidArgumentException $ex ) {
-			$this->logger->error( $ex->getMessage() );
+			// Log the condition and give the caller something that will absorb calls.
+			trigger_error( $ex->getMessage(), E_USER_WARNING );
 			return new NullMetric;
 		}
 		return $this;
@@ -125,18 +120,12 @@ class CounterMetric implements MetricInterface {
 	}
 
 	/** @inheritDoc */
-	public function withLabelKey( string $key ): CounterMetric {
-		$this->baseMetric->addLabelKey( $key );
-		return $this;
-	}
-
-	/** @inheritDoc */
 	public function withLabel( string $key, string $value ) {
 		try {
 			$this->baseMetric->addLabel( $key, $value );
-			$this->baseMetric->clearLabels(); // Support legacy behavior for now
 		} catch ( IllegalOperationException | InvalidArgumentException $ex ) {
-			$this->logger->error( $ex->getMessage() );
+			// Log the condition and give the caller something that will absorb calls.
+			trigger_error( $ex->getMessage(), E_USER_WARNING );
 			return new NullMetric;
 		}
 		return $this;
diff --git a/includes/libs/Stats/Metrics/GaugeMetric.php b/includes/libs/Stats/Metrics/GaugeMetric.php
index 50f3acb5a336..be2092ef32e9 100644
--- a/includes/libs/Stats/Metrics/GaugeMetric.php
+++ b/includes/libs/Stats/Metrics/GaugeMetric.php
@@ -61,14 +61,9 @@ class GaugeMetric implements MetricInterface {
 	 * Sets metric to value.
 	 *
 	 * @param float $value
-	 * @param string[] $labels
 	 * @return void
 	 */
-	public function set( float $value, array $labels = [] ): void {
-		$this->baseMetric->clearLabels();
-		foreach ( $this->baseMetric->getLabelKeys() as $i => $labelKey ) {
-			$this->baseMetric->addLabel( $labelKey, $labels[$i] );
-		}
+	public function set( float $value ): void {
 		$this->baseMetric->addSample( new Sample( $this->baseMetric->getLabelValues(), $value ) );
 	}
 
@@ -102,7 +97,8 @@ class GaugeMetric implements MetricInterface {
 		try {
 			$this->baseMetric->setSampleRate( $sampleRate );
 		} catch ( IllegalOperationException | InvalidArgumentException $ex ) {
-			$this->logger->error( $ex->getMessage() );
+			// Log the condition and give the caller something that will absorb calls.
+			trigger_error( $ex->getMessage(), E_USER_WARNING );
 			return new NullMetric;
 		}
 		return $this;
@@ -114,18 +110,12 @@ class GaugeMetric implements MetricInterface {
 	}
 
 	/** @inheritDoc */
-	public function withLabelKey( string $key ): GaugeMetric {
-		$this->baseMetric->addLabelKey( $key );
-		return $this;
-	}
-
-	/** @inheritDoc */
 	public function withLabel( string $key, string $value ) {
 		try {
 			$this->baseMetric->addLabel( $key, $value );
-			$this->baseMetric->clearLabels(); // Support legacy behavior for now
 		} catch ( IllegalOperationException | InvalidArgumentException $ex ) {
-			$this->logger->error( $ex->getMessage() );
+			// Log the condition and give the caller something that will absorb calls.
+			trigger_error( $ex->getMessage(), E_USER_WARNING );
 			return new NullMetric;
 		}
 		return $this;
diff --git a/includes/libs/Stats/Metrics/MetricInterface.php b/includes/libs/Stats/Metrics/MetricInterface.php
index 8881a805d41d..f8f9a9b18a3c 100644
--- a/includes/libs/Stats/Metrics/MetricInterface.php
+++ b/includes/libs/Stats/Metrics/MetricInterface.php
@@ -79,14 +79,6 @@ interface MetricInterface {
 	public function withLabel( string $key, string $value );
 
 	/**
-	 * Adds a label key.
-	 *
-	 * @param string $key
-	 * @return CounterMetric|GaugeMetric|TimingMetric|NullMetric
-	 */
-	public function withLabelKey( string $key );
-
-	/**
 	 * Returns metric with cleared labels.
 	 *
 	 * @return CounterMetric|GaugeMetric|TimingMetric|NullMetric
diff --git a/includes/libs/Stats/Metrics/NullMetric.php b/includes/libs/Stats/Metrics/NullMetric.php
index 5f7951551eff..6060f929e682 100644
--- a/includes/libs/Stats/Metrics/NullMetric.php
+++ b/includes/libs/Stats/Metrics/NullMetric.php
@@ -39,10 +39,10 @@ class NullMetric {
 	 *
 	 * @param $method_name string
 	 * @param $args array
-	 * @return null
+	 * @return NullMetric
 	 */
-	public function __call( string $method_name, array $args ) {
-		return null;
+	public function __call( string $method_name, array $args ): NullMetric {
+		return $this;
 	}
 
 }
diff --git a/includes/libs/Stats/Metrics/TimingMetric.php b/includes/libs/Stats/Metrics/TimingMetric.php
index 648b880a916c..1c9a38697448 100644
--- a/includes/libs/Stats/Metrics/TimingMetric.php
+++ b/includes/libs/Stats/Metrics/TimingMetric.php
@@ -62,14 +62,9 @@ class TimingMetric implements MetricInterface {
 	 * Records a previously calculated observation.
 	 *
 	 * @param float $value
-	 * @param string[] $labels
 	 * @return void
 	 */
-	public function observe( float $value, array $labels = [] ): void {
-		$this->baseMetric->clearLabels();
-		foreach ( $this->baseMetric->getLabelKeys() as $i => $labelKey ) {
-			$this->baseMetric->addLabel( $labelKey, $labels[$i] );
-		}
+	public function observe( float $value ): void {
 		$this->baseMetric->addSample( new Sample( $this->baseMetric->getLabelValues(), $value ) );
 	}
 
@@ -103,7 +98,8 @@ class TimingMetric implements MetricInterface {
 		try {
 			$this->baseMetric->setSampleRate( $sampleRate );
 		} catch ( IllegalOperationException | InvalidArgumentException $ex ) {
-			$this->logger->error( $ex->getMessage() );
+			// Log the condition and give the caller something that will absorb calls.
+			trigger_error( $ex->getMessage(), E_USER_WARNING );
 			return new NullMetric;
 		}
 		return $this;
@@ -115,18 +111,12 @@ class TimingMetric implements MetricInterface {
 	}
 
 	/** @inheritDoc */
-	public function withLabelKey( string $key ): TimingMetric {
-		$this->baseMetric->addLabelKey( $key );
-		return $this;
-	}
-
-	/** @inheritDoc */
 	public function withLabel( string $key, string $value ) {
 		try {
 			$this->baseMetric->addLabel( $key, $value );
-			$this->baseMetric->clearLabels(); // Support legacy behavior for now
 		} catch ( IllegalOperationException | InvalidArgumentException $ex ) {
-			$this->logger->error( $ex->getMessage() );
+			// Log the condition and give the caller something that will absorb calls.
+			trigger_error( $ex->getMessage(), E_USER_WARNING );
 			return new NullMetric;
 		}
 		return $this;