fix: allow multiple consumers of metrics (#6)

To support using labels as disambiguators across multiple reporters of the same metric, cache metrics globally then return the pre-exsting metric when it's registered on subsequent occasions. Co-authored-by: Marin Petrunic <[email protected]>
libp2p · Nov 21, 2022 · 92bde9b · 92bde9b
1 parent 123fb81
commit 92bde9b
Show file tree

Hide file tree

Showing 10 changed files with 221 additions and 32 deletions.
diff --git a/src/counter-group.ts b/src/counter-group.ts
@@ -1,28 +1,33 @@
 import type { CounterGroup, CalculateMetric } from '@libp2p/interface-metrics'
 import { Counter as PromCounter, CollectFunction } from 'prom-client'
+import { normaliseString, CalculatedMetric } from './utils.js'
 import type { PrometheusCalculatedMetricOptions } from './index.js'
-import { normaliseString } from './utils.js'
 
-export class PrometheusCounterGroup implements CounterGroup {
+export class PrometheusCounterGroup implements CounterGroup, CalculatedMetric<Record<string, number>> {
   private readonly counter: PromCounter
   private readonly label: string
+  private readonly calculators: Array<CalculateMetric<Record<string, number>>>
 
   constructor (name: string, opts: PrometheusCalculatedMetricOptions<Record<string, number>>) {
     name = normaliseString(name)
     const help = normaliseString(opts.help ?? name)
     const label = this.label = normaliseString(opts.label ?? name)
     let collect: CollectFunction<PromCounter<any>> | undefined
+    this.calculators = []
 
     // calculated metric
     if (opts?.calculate != null) {
-      const calculate: CalculateMetric<Record<string, number>> = opts.calculate
+      this.calculators.push(opts.calculate)
+      const self = this
 
       collect = async function () {
-        const values = await calculate()
+        await Promise.all(self.calculators.map(async calculate => {
+          const values = await calculate()
 
-        Object.entries(values).forEach(([key, value]) => {
-          this.inc({ [label]: key }, value)
-        })
+          Object.entries(values).forEach(([key, value]) => {
+            this.inc({ [label]: key }, value)
+          })
+        }))
       }
     }
 
@@ -35,6 +40,10 @@ export class PrometheusCounterGroup implements CounterGroup {
     })
   }
 
+  addCalculator (calculator: CalculateMetric<Record<string, number>>) {
+    this.calculators.push(calculator)
+  }
+
   increment (values: Record<string, number | unknown>): void {
     Object.entries(values).forEach(([key, value]) => {
       const inc = typeof value === 'number' ? value : 1

diff --git a/src/counter.ts b/src/counter.ts
@@ -1,25 +1,29 @@
-import type { Counter } from '@libp2p/interface-metrics'
+import type { CalculateMetric, Counter } from '@libp2p/interface-metrics'
 import { CollectFunction, Counter as PromCounter } from 'prom-client'
 import type { PrometheusCalculatedMetricOptions } from './index.js'
-import { normaliseString } from './utils.js'
+import { normaliseString, CalculatedMetric } from './utils.js'
 
-export class PrometheusCounter implements Counter {
+export class PrometheusCounter implements Counter, CalculatedMetric {
   private readonly counter: PromCounter
+  private readonly calculators: CalculateMetric[]
 
   constructor (name: string, opts: PrometheusCalculatedMetricOptions) {
     name = normaliseString(name)
     const help = normaliseString(opts.help ?? name)
     const labels = opts.label != null ? [normaliseString(opts.label)] : []
     let collect: CollectFunction<PromCounter<any>> | undefined
+    this.calculators = []
 
     // calculated metric
     if (opts?.calculate != null) {
-      const calculate = opts.calculate
+      this.calculators.push(opts.calculate)
+      const self = this
 
       collect = async function () {
-        const value = await calculate()
+        const values = await Promise.all(self.calculators.map(async calculate => await calculate()))
+        const sum = values.reduce((acc, curr) => acc + curr, 0)
 
-        this.inc(value)
+        this.inc(sum)
       }
     }
 
@@ -32,6 +36,10 @@ export class PrometheusCounter implements Counter {
     })
   }
 
+  addCalculator (calculator: CalculateMetric) {
+    this.calculators.push(calculator)
+  }
+
   increment (value: number = 1): void {
     this.counter.inc(value)
   }

diff --git a/src/index.ts b/src/index.ts
@@ -11,6 +11,9 @@ import { logger } from '@libp2p/logger'
 
 const log = logger('libp2p:prometheus-metrics')
 
+// prom-client metrics are global
+const metrics = new Map<string, any>()
+
 export interface PrometheusMetricsInit {
   /**
    * Use a custom registry to register metrics.
@@ -50,6 +53,7 @@ class PrometheusMetrics implements Metrics {
 
     if (init?.preserveExistingMetrics !== true) {
       log('Clearing existing metrics')
+      metrics.clear()
       ;(this.registry ?? register).clear()
     }
 
@@ -140,8 +144,22 @@ class PrometheusMetrics implements Metrics {
       throw new Error('Metric name is required')
     }
 
+    let metric = metrics.get(name)
+
+    if (metrics.has(name)) {
+      log('Reuse existing metric', name)
+
+      if (opts.calculate != null) {
+        metric.addCalculator(opts.calculate)
+      }
+
+      return metrics.get(name)
+    }
+
     log('Register metric', name)
-    const metric = new PrometheusMetric(name, { registry: this.registry, ...opts })
+    metric = new PrometheusMetric(name, { registry: this.registry, ...opts })
+
+    metrics.set(name, metric)
 
     if (opts.calculate == null) {
       return metric
@@ -152,14 +170,28 @@ class PrometheusMetrics implements Metrics {
   registerMetricGroup (name: string, opts?: MetricOptions): MetricGroup
   registerMetricGroup (name: string, opts: any = {}): any {
     if (name == null ?? name.trim() === '') {
-      throw new Error('Metric name is required')
+      throw new Error('Metric group name is required')
+    }
+
+    let metricGroup = metrics.get(name)
+
+    if (metricGroup != null) {
+      log('Reuse existing metric group', name)
+
+      if (opts.calculate != null) {
+        metricGroup.addCalculator(opts.calculate)
+      }
+
+      return metricGroup
     }
 
     log('Register metric group', name)
-    const group = new PrometheusMetricGroup(name, { registry: this.registry, ...opts })
+    metricGroup = new PrometheusMetricGroup(name, { registry: this.registry, ...opts })
+
+    metrics.set(name, metricGroup)
 
     if (opts.calculate == null) {
-      return group
+      return metricGroup
     }
   }
 
@@ -170,8 +202,22 @@ class PrometheusMetrics implements Metrics {
       throw new Error('Counter name is required')
     }
 
+    let counter = metrics.get(name)
+
+    if (counter != null) {
+      log('Reuse existing counter', name)
+
+      if (opts.calculate != null) {
+        counter.addCalculator(opts.calculate)
+      }
+
+      return metrics.get(name)
+    }
+
     log('Register counter', name)
-    const counter = new PrometheusCounter(name, { registry: this.registry, ...opts })
+    counter = new PrometheusCounter(name, { registry: this.registry, ...opts })
+
+    metrics.set(name, counter)
 
     if (opts.calculate == null) {
       return counter
@@ -182,14 +228,28 @@ class PrometheusMetrics implements Metrics {
   registerCounterGroup (name: string, opts?: MetricOptions): CounterGroup
   registerCounterGroup (name: string, opts: any = {}): any {
     if (name == null ?? name.trim() === '') {
-      throw new Error('Metric name is required')
+      throw new Error('Counter group name is required')
+    }
+
+    let counterGroup = metrics.get(name)
+
+    if (counterGroup != null) {
+      log('Reuse existing counter group', name)
+
+      if (opts.calculate != null) {
+        counterGroup.addCalculator(opts.calculate)
+      }
+
+      return counterGroup
     }
 
     log('Register counter group', name)
-    const group = new PrometheusCounterGroup(name, { registry: this.registry, ...opts })
+    counterGroup = new PrometheusCounterGroup(name, { registry: this.registry, ...opts })
+
+    metrics.set(name, counterGroup)
 
     if (opts.calculate == null) {
-      return group
+      return counterGroup
     }
   }
 }

diff --git a/src/metric-group.ts b/src/metric-group.ts
@@ -1,28 +1,33 @@
 import type { CalculateMetric, MetricGroup, StopTimer } from '@libp2p/interface-metrics'
 import { CollectFunction, Gauge } from 'prom-client'
 import type { PrometheusCalculatedMetricOptions } from './index.js'
-import { normaliseString } from './utils.js'
+import { normaliseString, CalculatedMetric } from './utils.js'
 
-export class PrometheusMetricGroup implements MetricGroup {
+export class PrometheusMetricGroup implements MetricGroup, CalculatedMetric<Record<string, number>> {
   private readonly gauge: Gauge
   private readonly label: string
+  private readonly calculators: Array<CalculateMetric<Record<string, number>>>
 
   constructor (name: string, opts: PrometheusCalculatedMetricOptions<Record<string, number>>) {
     name = normaliseString(name)
     const help = normaliseString(opts.help ?? name)
     const label = this.label = normaliseString(opts.label ?? name)
     let collect: CollectFunction<Gauge<any>> | undefined
+    this.calculators = []
 
     // calculated metric
     if (opts?.calculate != null) {
-      const calculate: CalculateMetric<Record<string, number>> = opts.calculate
+      this.calculators.push(opts.calculate)
+      const self = this
 
       collect = async function () {
-        const values = await calculate()
+        await Promise.all(self.calculators.map(async calculate => {
+          const values = await calculate()
 
-        Object.entries(values).forEach(([key, value]) => {
-          this.set({ [label]: key }, value)
-        })
+          Object.entries(values).forEach(([key, value]) => {
+            this.set({ [label]: key }, value)
+          })
+        }))
       }
     }
 
@@ -35,6 +40,10 @@ export class PrometheusMetricGroup implements MetricGroup {
     })
   }
 
+  addCalculator (calculator: CalculateMetric<Record<string, number>>) {
+    this.calculators.push(calculator)
+  }
+
   update (values: Record<string, number>): void {
     Object.entries(values).forEach(([key, value]) => {
       this.gauge.set({ [this.label]: key }, value)

diff --git a/src/metric.ts b/src/metric.ts
@@ -1,25 +1,29 @@
-import type { Metric, StopTimer } from '@libp2p/interface-metrics'
+import type { Metric, StopTimer, CalculateMetric } from '@libp2p/interface-metrics'
 import { CollectFunction, Gauge } from 'prom-client'
 import type { PrometheusCalculatedMetricOptions } from './index.js'
 import { normaliseString } from './utils.js'
 
 export class PrometheusMetric implements Metric {
   private readonly gauge: Gauge
+  private readonly calculators: CalculateMetric[]
 
   constructor (name: string, opts: PrometheusCalculatedMetricOptions) {
     name = normaliseString(name)
     const help = normaliseString(opts.help ?? name)
     const labels = opts.label != null ? [normaliseString(opts.label)] : []
     let collect: CollectFunction<Gauge<any>> | undefined
+    this.calculators = []
 
     // calculated metric
     if (opts?.calculate != null) {
-      const calculate = opts.calculate
+      this.calculators.push(opts.calculate)
+      const self = this
 
       collect = async function () {
-        const value = await calculate()
+        const values = await Promise.all(self.calculators.map(async calculate => await calculate()))
+        const sum = values.reduce((acc, curr) => acc + curr, 0)
 
-        this.set(value)
+        this.set(sum)
       }
     }
 
@@ -32,6 +36,10 @@ export class PrometheusMetric implements Metric {
     })
   }
 
+  addCalculator (calculator: CalculateMetric) {
+    this.calculators.push(calculator)
+  }
+
   update (value: number): void {
     this.gauge.set(value)
   }

diff --git a/src/utils.ts b/src/utils.ts
@@ -1,3 +1,8 @@
+import type { CalculateMetric } from '@libp2p/interface-metrics'
+
+export interface CalculatedMetric <T = number> {
+  addCalculator: (calculator: CalculateMetric<T>) => void
+}
 
 export const ONE_SECOND = 1000
 export const ONE_MINUTE = 60 * ONE_SECOND

diff --git a/test/counter-groups.spec.ts b/test/counter-groups.spec.ts
@@ -90,4 +90,31 @@ describe('counter groups', () => {
 
     await expect(client.register.metrics()).to.eventually.not.include(metricKey, 'still included metric key')
   })
+
+  it('should allow use of the same counter group from multiple reporters', async () => {
+    const metricName = randomMetricName()
+    const metricKey1 = randomMetricName('key_')
+    const metricKey2 = randomMetricName('key_')
+    const metricLabel = randomMetricName('label_')
+    const metricValue1 = 5
+    const metricValue2 = 7
+    const metrics = prometheusMetrics()()
+    const metric1 = metrics.registerCounterGroup(metricName, {
+      label: metricLabel
+    })
+    metric1.increment({
+      [metricKey1]: metricValue1
+    })
+    const metric2 = metrics.registerCounterGroup(metricName, {
+      label: metricLabel
+    })
+    metric2.increment({
+      [metricKey2]: metricValue2
+    })
+
+    const reportedMetrics = await client.register.metrics()
+
+    expect(reportedMetrics).to.include(`${metricName}{${metricLabel}="${metricKey1}"} ${metricValue1}`, 'did not include updated metric')
+    expect(reportedMetrics).to.include(`${metricName}{${metricLabel}="${metricKey2}"} ${metricValue2}`, 'did not include updated metric')
+  })
 })
diff --git a/test/counters.spec.ts b/test/counters.spec.ts
@@ -64,4 +64,22 @@ describe('counters', () => {
 
     await expect(client.register.metrics()).to.eventually.include(`${metricName} 0`, 'did not include updated metric')
   })
+
+  it('should allow use of the same counter from multiple reporters', async () => {
+    const metricName = randomMetricName()
+    const metricLabel = randomMetricName('label_')
+    const metricValue1 = 5
+    const metricValue2 = 7
+    const metrics = prometheusMetrics()()
+    const metric1 = metrics.registerCounter(metricName, {
+      label: metricLabel
+    })
+    metric1.increment(metricValue1)
+    const metric2 = metrics.registerCounter(metricName, {
+      label: metricLabel
+    })
+    metric2.increment(metricValue2)
+
+    await expect(client.register.metrics()).to.eventually.include(`${metricName} ${metricValue1 + metricValue2}`)
+  })
 })