Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Append missing _total suffix when registering Counter metrics. #177

Merged
merged 5 commits into from
Sep 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public String getPrometheusOpenMetrics()
}

@GET
@Produces({MediaType.APPLICATION_JSON, "*/*;q=0"})
@Produces(MediaType.APPLICATION_JSON)
public String getJsonString()
{
return metricsContainer.getJsonString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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.
*/
Expand All @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down