Append missing _total suffix when registering Counter metrics. (#177)

* Append missing _total suffix when registering Counter metrics.

This ensures consistent metric names across the Prometheus registry,
the MetricsContainer and across the JSON and Prometheus exposition
formats available at /metrics. Also adds basic logging and fixes docs.

See: jitsi/jitsi-videobridge#1942

* fix: MediaType annotation for application/json.

Fixes JSON output showing up in text/html when querying via browser.

* log: Change log level to DEBUG.

* test: Add tests to cover counter "_total" suffix.

* Add check for base metric name when registering a counter.

jicoco-metrics/src/main/java/org/jitsi/rest/prometheus/Prometheus.java

@@ -58,7 +58,7 @@ public String getPrometheusOpenMetrics()
     }
 
     @GET
-    @Produces({MediaType.APPLICATION_JSON, "*/*;q=0"})
+    @Produces(MediaType.APPLICATION_JSON)
     public String getJsonString()
     {
         return metricsContainer.getJsonString();

jicoco-metrics/src/main/kotlin/org/jitsi/metrics/MetricsContainer.kt

@@ -17,6 +17,7 @@ package org.jitsi.metrics
 
 import io.prometheus.client.CollectorRegistry
 import io.prometheus.client.exporter.common.TextFormat
+import org.jitsi.utils.logging2.createLogger
 import org.json.simple.JSONObject
 import java.io.IOException
 import java.io.StringWriter
@@ -28,6 +29,7 @@ open class MetricsContainer @JvmOverloads constructor(
     /** the registry used to register metrics */
     val registry: CollectorRegistry = CollectorRegistry.defaultRegistry
 ) {
+    private val logger = createLogger()
 
     /**
      * Namespace prefix added to all metrics.
@@ -98,6 +100,7 @@ open class MetricsContainer @JvmOverloads constructor(
 
     /**
      * Creates and registers a [CounterMetric] with the given [name], [help] string and optional [initialValue].
+     * If omitted, a "_total" suffix is added to the metric name to ensure consistent (Counter) metric naming.
      *
      * Throws an exception if a metric with the same name but a different type exists.
      */
@@ -110,17 +113,20 @@ open class MetricsContainer @JvmOverloads constructor(
         /** the optional initial value of the metric */
         initialValue: Long = 0
     ): CounterMetric {
-        if (metrics.containsKey(name)) {
+        val newName = if (name.endsWith("_total")) name else "${name}_total".also {
+            logger.debug("Counter '$name' was renamed to '$it' to ensure consistent metric naming.")
+        }
+        if (metrics.containsKey(newName) or metrics.containsKey(name)) {
             if (checkForNameConflicts) {
-                throw RuntimeException("Could not register metric '$name'. A metric with that name already exists.")
+                throw RuntimeException("Could not register metric '$newName'. A metric with that name already exists.")
             }
-            return metrics[name] as CounterMetric
+            return metrics[newName] as CounterMetric
         }
-        return CounterMetric(name, help, namespace, initialValue).apply { metrics[name] = register(registry) }
+        return CounterMetric(newName, help, namespace, initialValue).apply { metrics[newName] = register(registry) }
     }
 
     /**
-     * Creates and registers a [CounterMetric] with the given [name], [help] string and optional [initialValue].
+     * Creates and registers a [LongGaugeMetric] with the given [name], [help] string and optional [initialValue].
      *
      * Throws an exception if a metric with the same name but a different type exists.
      */

jicoco-metrics/src/test/kotlin/org/jitsi/metrics/MetricsContainerTest.kt

@@ -38,13 +38,20 @@ class MetricsContainerTest : ShouldSpec() {
                 context("while checking for name conflicts") {
                     should("throw a RuntimeException") {
                         shouldThrow<RuntimeException> { mc.registerBooleanMetric("boolean", "A boolean metric") }
+                        // "counter" is renamed to "counter_total" so both should throw an exception
+                        shouldThrow<RuntimeException> { mc.registerCounter("counter", "A counter metric") }
+                        shouldThrow<RuntimeException> { mc.registerCounter("counter_total", "A counter metric") }
+                        // we test this because the Prometheus JVM library stores Counters without the "_total" suffix
+                        shouldThrow<RuntimeException> { mc.registerCounter("boolean_total", "A counter metric") }
                     }
                 }
                 context("without checking for name conflicts") {
                     mc.checkForNameConflicts = false
                     should("return an existing metric") {
                         booleanMetric shouldBe mc.registerBooleanMetric("boolean", "A boolean metric")
+                        // "counter" is renamed to "counter_total" so both should return the same metric
                         counter shouldBe mc.registerCounter("counter", "A counter metric")
+                        counter shouldBe mc.registerCounter("counter_total", "A counter metric")
                         info shouldBe mc.registerInfo("info", "An info metric", "value")
                         longGauge shouldBe mc.registerLongGauge("gauge", "A gauge metric")
                     }