refactor: move to supported node-prometheus-gc-stats (#1210)

* refactor: move to supported node-prometheus-gc-stats * Create great-bugs-rush.md * test: to not assert default metrics
tdeekens · Apr 12, 2024 · 7668337 · 7668337
1 parent 972fe5a
commit 7668337
Show file tree

Hide file tree

Showing 9 changed files with 45 additions and 115 deletions.
diff --git a/.changeset/great-bugs-rush.md b/.changeset/great-bugs-rush.md
@@ -0,0 +1,14 @@
+---
+"@promster/metrics": major
+"@promster/types": patch
+---
+
+Replace `@sematext/gc-stats` with `prometheus-gc-stats`.
+
+The latter is better supported and doesn't require any userland install. The module however does not allow to full configuration of metric names. Hence the metric names have changed:
+
+We now expose:
+
+1. nodejs_gc_runs_total: Counts the number of time GC is invoked
+2. nodejs_gc_pause_seconds_total: Time spent in GC in seconds
+3. nodejs_gc_reclaimed_bytes_total: The number of bytes GC has freed
diff --git a/packages/metrics/package.json b/packages/metrics/package.json
@@ -38,9 +38,9 @@
   "dependencies": {
     "@promster/types": "^13.0.0",
     "lodash.memoize": "4.1.2",
+    "prometheus-gc-stats": "1.1.0",
     "lodash.once": "4.1.1",
     "merge-options": "3.0.4",
-    "optional": "0.1.4",
     "tslib": "2.6.2",
     "url-value-parser": "2.2.0"
   },
@@ -50,9 +50,6 @@
     "@types/node": "20.11.30",
     "@types/lodash.once": "4.1.9"
   },
-  "optionalDependencies": {
-    "@sematext/gc-stats": "1.5.9"
-  },
   "peerDependencies": {
     "prom-client": "13.x.x || 14.x || 15.x"
   }

diff --git a/packages/metrics/src/client/client.ts b/packages/metrics/src/client/client.ts
@@ -1,4 +1,5 @@
 import once from 'lodash.once';
+import type { PrometheusContentType } from 'prom-client';
 import * as Prometheus from 'prom-client';
 import { skipMetricsInEnvironment } from '../environment';
 
@@ -9,8 +10,9 @@ const defaultRegister = Prometheus.register;
 
 // eslint-disable-next-line @typescript-eslint/consistent-type-definitions
 interface TClientOptions
-  extends Prometheus.DefaultMetricsCollectorConfiguration {
+  extends Prometheus.DefaultMetricsCollectorConfiguration<PrometheusContentType> {
   detectKubernetes?: boolean;
+  prefix?: string;
 }
 
 const configure = once((options: TClientOptions) => {

diff --git a/packages/metrics/src/create-gc-metrics/create-gc-metrics.spec.js b/packages/metrics/src/create-gc-metrics/create-gc-metrics.spec.js
@@ -20,16 +20,4 @@ describe('createGcMetrics', () => {
   it('should have `up` metric', () => {
     expect(metrics).toHaveProperty('up');
   });
-
-  it('should have `countOfGcs` metric', () => {
-    expect(metrics).toHaveProperty('countOfGcs');
-  });
-
-  it('should have `durationOfGc` metric', () => {
-    expect(metrics).toHaveProperty('durationOfGc');
-  });
-
-  it('should have `reclaimedInGc` metric', () => {
-    expect(metrics).toHaveProperty('reclaimedInGc');
-  });
 });
diff --git a/packages/metrics/src/create-gc-metrics/create-gc-metrics.ts b/packages/metrics/src/create-gc-metrics/create-gc-metrics.ts
@@ -6,7 +6,6 @@ import {
 import merge from 'merge-options';
 import { configure, Prometheus } from '../client';
 
-const defaultLabels = ['gc_type'];
 const asArray = (maybeArray: Readonly<string[] | string>) =>
   Array.isArray(maybeArray) ? maybeArray : [maybeArray];
 
@@ -16,9 +15,6 @@ const defaultOptions = {
   metricPrefix: '',
   metricNames: {
     up: ['nodejs_up'],
-    countOfGcs: ['nodejs_gc_runs_total'],
-    durationOfGc: ['nodejs_gc_pause_seconds_total'],
-    reclaimedInGc: ['nodejs_gc_reclaimed_bytes_total'],
   },
 };
 
@@ -30,30 +26,6 @@ const getMetrics = (options: TDefaultedPromsterOptions) => ({
         help: '1 = nodejs server is up, 0 = nodejs server is not up',
       })
   ),
-  countOfGcs: asArray(options.metricNames.countOfGcs).map(
-    (nameOfCountOfGcsMetric: string) =>
-      new Prometheus.Counter({
-        name: `${options.metricPrefix}${nameOfCountOfGcsMetric}`,
-        help: 'Count of total garbage collections.',
-        labelNames: defaultLabels,
-      })
-  ),
-  durationOfGc: asArray(options.metricNames.durationOfGc).map(
-    (nameOfDurationOfGcMetric: string) =>
-      new Prometheus.Counter({
-        name: `${options.metricPrefix}${nameOfDurationOfGcMetric}`,
-        help: 'Time spent in GC Pause in seconds.',
-        labelNames: defaultLabels,
-      })
-  ),
-  reclaimedInGc: asArray(options.metricNames.reclaimedInGc).map(
-    (nameOfReclaimedInGcMetric: string) =>
-      new Prometheus.Counter({
-        name: `${options.metricPrefix}${nameOfReclaimedInGcMetric}`,
-        help: 'Total number of bytes reclaimed by GC.',
-        labelNames: defaultLabels,
-      })
-  ),
 });
 
 const createGcMetrics = (options: TDefaultedPromsterOptions): TGcMetrics => {

diff --git a/packages/metrics/src/create-gc-observer/create-gc-observer.ts b/packages/metrics/src/create-gc-observer/create-gc-observer.ts
@@ -1,65 +1,24 @@
 import {
   type TOptionalPromsterOptions,
   type TGcMetrics,
-  type TValueOf,
 } from '@promster/types';
 
 import once from 'lodash.once';
 // @ts-expect-error
-import requireOptional from 'optional';
-
-const gc = requireOptional('@sematext/gc-stats');
-
-const gcTypes = {
-  0: 'unknown',
-  1: 'scavenge',
-  2: 'mark_sweep_compact',
-  3: 'scavenge_and_mark_sweep_compact',
-  4: 'incremental_marking',
-  8: 'weak_phantom',
-  15: 'all',
-} as const;
-
-type TGcTypes = TValueOf<typeof gcTypes>;
-type TStats = {
-  gctype: TGcTypes;
-  pause: number;
-  diff: {
-    usedHeapSize: number;
-  };
-};
+import gcStats from 'prometheus-gc-stats';
+import { defaultRegister } from '../client/client';
 
 const defaultOptions = {
   disableGcMetrics: false,
 };
 
 const createGcObserver = once(
   (metrics: TGcMetrics, options: TOptionalPromsterOptions) => () => {
-    if (typeof gc !== 'function') {
-      return;
-    }
-
-    if (options.disableGcMetrics) return;
-
-    gc().on('stats', (stats: TStats) => {
-      // @ts-expect-error
-      const gcType: TGcTypes = gcTypes[stats.gctype];
-
-      metrics.countOfGcs.forEach((countOfGcMetricType) => {
-        countOfGcMetricType.labels(gcType).inc();
-      });
-      metrics.durationOfGc.forEach((durationOfGcMetricType) => {
-        durationOfGcMetricType.labels(gcType).inc(stats.pause / 1e9);
-      });
-
-      if (stats.diff.usedHeapSize < 0) {
-        metrics.reclaimedInGc.forEach((reclaimedInGcMetricType) => {
-          reclaimedInGcMetricType
-            .labels(gcType)
-            .inc(stats.diff.usedHeapSize * -1);
-        });
-      }
+    const startGcStats = gcStats(defaultRegister, {
+      prefix: options.metricPrefix,
     });
+
+    startGcStats();
   }
 );
 

diff --git a/packages/server/src/server/server.spec.js b/packages/server/src/server/server.spec.js
@@ -65,6 +65,9 @@ it('should expose garbage collection metrics', async () => {
 
   expect(parsedMetrics).toEqual(
     expect.arrayContaining([
+      expect.objectContaining({
+        name: 'nodejs_version_info',
+      }),
       expect.objectContaining({
         name: 'process_cpu_user_seconds_total',
       }),
@@ -83,21 +86,12 @@ it('should expose garbage collection metrics', async () => {
       expect.objectContaining({
         name: 'nodejs_eventloop_lag_seconds',
       }),
-      expect.objectContaining({
-        name: 'nodejs_gc_runs_total',
-      }),
-      expect.objectContaining({
-        name: 'nodejs_gc_duration_seconds',
-      }),
       expect.objectContaining({
         name: 'nodejs_eventloop_lag_max_seconds',
       }),
       expect.objectContaining({
         name: 'nodejs_eventloop_lag_p50_seconds',
       }),
-      expect.objectContaining({
-        name: 'nodejs_version_info',
-      }),
     ])
   );
 });
diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts
@@ -37,9 +37,6 @@ export type THttpMetrics = {
 };
 export type TGcMetrics = {
   up: Gauge[];
-  countOfGcs: Counter[];
-  durationOfGc: Counter[];
-  reclaimedInGc: Counter[];
 };
 export type TGraphQlMetrics = {
   graphQlParseDuration?: Histogram[];

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml