fix: metrics should persist beyond node restarts (#3159)

Partial revert of #3154 - metrics should persist when a node is
stopped and then started again and re-registering the same metric
should be a no-op.

Author: achingbrain

packages/metrics-opentelemetry/src/index.ts

@@ -118,7 +118,7 @@ class OpenTelemetryMetrics implements Metrics {
         }
 
         // reset counts for next time
-        this.transferStats = new Map()
+        this.transferStats.clear()
 
         return output
       }
@@ -139,8 +139,6 @@ class OpenTelemetryMetrics implements Metrics {
 
   stop (): void {
     this.transferStats.clear()
-    this.metrics.clear()
-    this.observables.clear()
   }
 
   /**
@@ -195,7 +193,13 @@ class OpenTelemetryMetrics implements Metrics {
     }
 
     if (isCalculatedMetricOptions<CalculatedMetricOptions>(opts)) {
-      const gauge = this.observables.get(name) ?? this.meter.createObservableGauge(name, {
+      let gauge = this.observables.get(name)
+
+      if (gauge != null) {
+        return
+      }
+
+      gauge = this.meter.createObservableGauge(name, {
         description: opts?.help ?? name
       })
 
@@ -232,7 +236,13 @@ class OpenTelemetryMetrics implements Metrics {
     const label = opts?.label ?? name
 
     if (isCalculatedMetricOptions<CalculatedMetricOptions<Record<string, number>>>(opts)) {
-      const gauge = this.observables.get(name) ?? this.meter.createObservableGauge(name, {
+      let gauge = this.observables.get(name)
+
+      if (gauge != null) {
+        return
+      }
+
+      gauge = this.meter.createObservableGauge(name, {
         description: opts?.help ?? name
       })
 
@@ -273,7 +283,13 @@ class OpenTelemetryMetrics implements Metrics {
     }
 
     if (isCalculatedMetricOptions<CalculatedMetricOptions>(opts)) {
-      const counter = this.observables.get(name) ?? this.meter.createObservableCounter(name, {
+      let counter = this.observables.get(name)
+
+      if (counter != null) {
+        return
+      }
+
+      counter = this.meter.createObservableCounter(name, {
         description: opts?.help ?? name
       })
 
@@ -310,7 +326,13 @@ class OpenTelemetryMetrics implements Metrics {
     const label = opts?.label ?? name
 
     if (isCalculatedMetricOptions<CalculatedMetricOptions<Record<string, number>>>(opts)) {
-      const counter = this.observables.get(name) ?? this.meter.createObservableCounter(name, {
+      let counter = this.observables.get(name)
+
+      if (counter != null) {
+        return
+      }
+
+      counter = this.meter.createObservableCounter(name, {
         description: opts?.help ?? name
       })
 

packages/metrics-opentelemetry/test/index.spec.ts

@@ -27,7 +27,7 @@ describe('opentelemetry-metrics', () => {
     expect(wrapped).to.not.equal(target.wrapped)
   })
 
-  it('should not retain metrics after stop', async () => {
+  it('should retain metrics after stop', async () => {
     const metrics = openTelemetryMetrics()({
       nodeInfo: {
         name: 'test',
@@ -50,6 +50,6 @@ describe('opentelemetry-metrics', () => {
 
     const m3 = metrics.registerCounterGroup('test_metric')
 
-    expect(m3).to.not.equal(m1, 're-used metric')
+    expect(m3).to.equal(m1, 'did not re-use metric')
   })
 })

packages/metrics-prometheus/src/counter-group.ts

@@ -1,35 +1,27 @@
 import { Counter as PromCounter } from 'prom-client'
 import { normalizeString } from './utils.js'
 import type { PrometheusCalculatedMetricOptions } from './index.js'
-import type { CalculatedMetric } from './utils.js'
-import type { CounterGroup, CalculateMetric } from '@libp2p/interface'
+import type { CounterGroup } from '@libp2p/interface'
 import type { CollectFunction } from 'prom-client'
 
-export class PrometheusCounterGroup implements CounterGroup, CalculatedMetric<Record<string, number>> {
+export class PrometheusCounterGroup implements CounterGroup {
   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 = normalizeString(name)
     const help = normalizeString(opts.help ?? name)
     const label = this.label = normalizeString(opts.label ?? name)
     let collect: CollectFunction<PromCounter<any>> | undefined
-    this.calculators = []
 
     // calculated metric
     if (opts?.calculate != null) {
-      this.calculators.push(opts.calculate)
-      const self = this
-
       collect = async function () {
-        await Promise.all(self.calculators.map(async calculate => {
-          const values = await calculate()
+        const values = await opts.calculate()
 
-          Object.entries(values).forEach(([key, value]) => {
-            this.inc({ [label]: key }, value)
-          })
-        }))
+        Object.entries(values).forEach(([key, value]) => {
+          this.inc({ [label]: key }, value)
+        })
       }
     }
 
@@ -42,10 +34,6 @@ export class PrometheusCounterGroup implements CounterGroup, CalculatedMetric<Re
     })
   }
 
-  addCalculator (calculator: CalculateMetric<Record<string, number>>): void {
-    this.calculators.push(calculator)
-  }
-
   increment (values: Record<string, number | unknown>): void {
     Object.entries(values).forEach(([key, value]) => {
       const inc = typeof value === 'number' ? value : 1

packages/metrics-prometheus/src/counter.ts

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

packages/metrics-prometheus/src/histogram-group.ts

@@ -1,35 +1,27 @@
 import { Histogram as PromHistogram } from 'prom-client'
 import { normalizeString } from './utils.js'
 import type { PrometheusCalculatedHistogramOptions } from './index.js'
-import type { CalculatedMetric } from './utils.js'
-import type { CalculateMetric, HistogramGroup, StopTimer } from '@libp2p/interface'
+import type { HistogramGroup, StopTimer } from '@libp2p/interface'
 import type { CollectFunction } from 'prom-client'
 
-export class PrometheusHistogramGroup implements HistogramGroup, CalculatedMetric<Record<string, number>> {
+export class PrometheusHistogramGroup implements HistogramGroup {
   private readonly histogram: PromHistogram
   private readonly label: string
-  private readonly calculators: Array<CalculateMetric<Record<string, number>>>
 
   constructor (name: string, opts: PrometheusCalculatedHistogramOptions<Record<string, number>>) {
     name = normalizeString(name)
     const help = normalizeString(opts.help ?? name)
     const label = this.label = normalizeString(opts.label ?? name)
     let collect: CollectFunction<PromHistogram<any>> | undefined
-    this.calculators = []
 
     // calculated metric
     if (opts?.calculate != null) {
-      this.calculators.push(opts.calculate)
-      const self = this
-
       collect = async function () {
-        await Promise.all(self.calculators.map(async calculate => {
-          const values = await calculate()
+        const values = await opts.calculate()
 
-          Object.entries(values).forEach(([key, value]) => {
-            this.observe({ [label]: key }, value)
-          })
-        }))
+        Object.entries(values).forEach(([key, value]) => {
+          this.observe({ [label]: key }, value)
+        })
       }
     }
 
@@ -43,10 +35,6 @@ export class PrometheusHistogramGroup implements HistogramGroup, CalculatedMetri
     })
   }
 
-  addCalculator (calculator: CalculateMetric<Record<string, number>>): void {
-    this.calculators.push(calculator)
-  }
-
   observe (values: Record<string, number>): void {
     Object.entries(values).forEach(([key, value]) => {
       this.histogram.observe({ [this.label]: key }, value)

packages/metrics-prometheus/src/histogram.ts

@@ -1,30 +1,22 @@
 import { Histogram as PromHistogram } from 'prom-client'
 import { normalizeString } from './utils.js'
 import type { PrometheusCalculatedHistogramOptions } from './index.js'
-import type { StopTimer, CalculateMetric, Histogram } from '@libp2p/interface'
+import type { StopTimer, Histogram } from '@libp2p/interface'
 import type { CollectFunction } from 'prom-client'
 
 export class PrometheusHistogram implements Histogram {
   private readonly histogram: PromHistogram
-  private readonly calculators: CalculateMetric[]
 
   constructor (name: string, opts: PrometheusCalculatedHistogramOptions) {
     name = normalizeString(name)
     const help = normalizeString(opts.help ?? name)
     const labels = opts.label != null ? [normalizeString(opts.label)] : []
     let collect: CollectFunction<PromHistogram<any>> | undefined
-    this.calculators = []
 
     // calculated metric
     if (opts?.calculate != null) {
-      this.calculators.push(opts.calculate)
-      const self = this
-
       collect = async function () {
-        const values = await Promise.all(self.calculators.map(async calculate => calculate()))
-        for (const value of values) {
-          this.observe(value)
-        }
+        this.observe(await opts.calculate())
       }
     }
 
@@ -38,10 +30,6 @@ export class PrometheusHistogram implements Histogram {
     })
   }
 
-  addCalculator (calculator: CalculateMetric): void {
-    this.calculators.push(calculator)
-  }
-
   observe (value: number): void {
     this.histogram.observe(value)
   }