chore: types in metrics (#1492)

## Description As we increase our TypeScript standards, we want to enforce return types. This PR adds return types to metrics and metrics test ## Related Issue Fixes #1453  Relates to # ## Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [x] Other (security config, docs update, etc) ## Checklist before merging - [x] Unit, [Journey](https://github.com/defenseunicorns/pepr/tree/main/journey), [E2E Tests](https://github.com/defenseunicorns/pepr-excellent-examples), [docs](https://github.com/defenseunicorns/pepr/tree/main/docs), [adr](https://github.com/defenseunicorns/pepr/tree/main/adr) added or updated as needed - [x] [Contributor Guide Steps](https://docs.pepr.dev/main/contribute/#submitting-a-pull-request) followed Signed-off-by: Case Wylie <[email protected]>
defenseunicorns · Dec 3, 2024 · 4d367cc · 4d367cc
1 parent 4e79c36
commit 4d367cc
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 40 deletions.
diff --git a/src/lib/metrics.test.ts b/src/lib/metrics.test.ts
@@ -7,50 +7,50 @@ import { performance } from "perf_hooks";
 import { MetricsCollector } from "./metrics";
 
 test("constructor initializes counters correctly", () => {
-  const collector = new MetricsCollector("testPrefix");
+  const collector: MetricsCollector = new MetricsCollector("testPrefix");
 
   expect(collector).toBeTruthy();
 });
 
 test("error method increments error counter", async () => {
-  const collector = new MetricsCollector("testPrefix");
+  const collector: MetricsCollector = new MetricsCollector("testPrefix");
 
   collector.error();
 
-  const metrics = await collector.getMetrics();
+  const metrics: string = await collector.getMetrics();
   expect(metrics).toMatch(/testPrefix_errors 1/);
 });
 
 test("alert method increments alerts counter", async () => {
-  const collector = new MetricsCollector("testPrefix");
+  const collector: MetricsCollector = new MetricsCollector("testPrefix");
 
   collector.alert();
 
-  const metrics = await collector.getMetrics();
+  const metrics: string = await collector.getMetrics();
   expect(metrics).toMatch(/testPrefix_alerts 1/);
 });
 
 test("observeStart returns current timestamp", () => {
-  const timeBefore = performance.now();
-  const startTime = MetricsCollector.observeStart();
-  const timeAfter = performance.now();
+  const timeBefore: number = performance.now();
+  const startTime: number = MetricsCollector.observeStart();
+  const timeAfter: number = performance.now();
 
   expect(timeBefore <= startTime).toBe(true);
   expect(timeAfter >= startTime).toBe(true);
 });
 
 test("observeEnd updates summary", async () => {
-  const collector = new MetricsCollector("testPrefix");
+  const collector: MetricsCollector = new MetricsCollector("testPrefix");
 
-  const startTime = MetricsCollector.observeStart();
+  const startTime: number = MetricsCollector.observeStart();
   await new Promise(resolve => setTimeout(resolve, 100)); // Delay to simulate operation
   collector.observeEnd(startTime);
 
   await new Promise(resolve => setTimeout(resolve, 100)); // Delay to simulate operation
   collector.observeEnd(startTime, "validate");
   collector.observeEnd(startTime, "validate");
 
-  const metrics = await collector.getMetrics();
+  const metrics: string = await collector.getMetrics();
   expect(metrics).toMatch(/testPrefix_mutate_count 1/);
   expect(metrics).toMatch(/testPrefix_mutate_sum \d+\.\d+/);
 
@@ -59,19 +59,19 @@ test("observeEnd updates summary", async () => {
 });
 
 test("coverage tests, with duplicate counters, default prefix (pepr) and still works properly", async () => {
-  const collector = new MetricsCollector();
+  const collector: MetricsCollector = new MetricsCollector();
   collector.addCounter("testCounter", "testHelp");
   // second one should log, but still work fine TODO: validate log
   collector.addCounter("testCounter", "testHelp");
-  let metrics = await collector.getMetrics();
+  let metrics: string = await collector.getMetrics();
   expect(metrics).toMatch(/pepr_testCounter 0/);
   collector.incCounter("testCounter");
   metrics = await collector.getMetrics();
   expect(metrics).toMatch(/pepr_testCounter 1/);
   collector.addSummary("testSummary", "testHelp");
   // second one should log, but still work fine TODO: validate log
   collector.addSummary("testSummary", "testHelp");
-  const startTime = MetricsCollector.observeStart();
+  const startTime: number = MetricsCollector.observeStart();
 
   await new Promise(resolve => setTimeout(resolve, 100)); // Delay to simulate operation
   collector.observeEnd(startTime, "testSummary");
@@ -82,49 +82,49 @@ test("coverage tests, with duplicate counters, default prefix (pepr) and still w
 });
 
 test("incCacheMiss increments cache miss gauge", async () => {
-  const collector = new MetricsCollector("testPrefix");
+  const collector: MetricsCollector = new MetricsCollector("testPrefix");
 
   collector.incCacheMiss("window1");
 
-  const metrics = await collector.getMetrics();
+  const metrics: string = await collector.getMetrics();
   expect(metrics).toMatch(/testPrefix_cache_miss{window="window1"} 1/);
 });
 
 test("incRetryCount increments retry count gauge", async () => {
-  const collector = new MetricsCollector("testPrefix");
+  const collector: MetricsCollector = new MetricsCollector("testPrefix");
 
   collector.incRetryCount("1");
 
-  const metrics = await collector.getMetrics();
+  const metrics: string = await collector.getMetrics();
   expect(metrics).toMatch(/testPrefix_resync_failure_count{count="1"} 1/);
 });
 
 test("initCacheMissWindow initializes cache miss gauge to zero", async () => {
-  const collector = new MetricsCollector("testPrefix");
+  const collector: MetricsCollector = new MetricsCollector("testPrefix");
 
   collector.initCacheMissWindow("window1");
 
-  const metrics = await collector.getMetrics();
+  const metrics: string = await collector.getMetrics();
   expect(metrics).toMatch(/testPrefix_cache_miss{window="window1"} 0/);
 });
 
 test("should initialize cache miss window and maintain size limit", async () => {
   process.env.PEPR_MAX_CACHE_MISS_WINDOWS = "3";
-  const collector = new MetricsCollector("pepr");
+  const collector: MetricsCollector = new MetricsCollector("pepr");
   collector.initCacheMissWindow("window1");
   collector.initCacheMissWindow("window2");
   collector.initCacheMissWindow("window3");
   collector.initCacheMissWindow("window4");
 
-  const metrics = await collector.getMetrics();
+  const metrics: string = await collector.getMetrics();
   expect(metrics).not.toContain("window1");
   expect(metrics).toContain("window4");
 
   collector.initCacheMissWindow("window5");
   collector.initCacheMissWindow("window6");
   collector.initCacheMissWindow("window7");
 
-  const updatedMetrics = await collector.getMetrics();
+  const updatedMetrics: string = await collector.getMetrics();
   expect(updatedMetrics).not.toContain("window4");
   expect(updatedMetrics).toContain("window5");
   expect(updatedMetrics).toContain("window6");

diff --git a/src/lib/metrics.ts b/src/lib/metrics.ts
@@ -9,6 +9,7 @@ import Log from "./logger";
 
 const loggingPrefix = "MetricsCollector";
 
+type MetricsCollectorInstance = InstanceType<typeof MetricsCollector>;
 interface MetricNames {
   errors: string;
   alerts: string;
@@ -60,15 +61,15 @@ export class MetricsCollector {
     this.addGauge(this.#metricNames.resyncFailureCount, "Number of failures per resync operation", ["count"]);
   }
 
-  #getMetricName = (name: string) => `${this.#prefix}_${name}`;
+  #getMetricName = (name: string): string => `${this.#prefix}_${name}`;
 
   #addMetric = <T extends Counter<string> | Gauge<string> | Summary<string>>(
     collection: Map<string, T>,
     MetricType: new (args: MetricArgs) => T,
     name: string,
     help: string,
     labelNames?: string[],
-  ) => {
+  ): void => {
     if (collection.has(this.#getMetricName(name))) {
       Log.debug(`Metric for ${name} already exists`, loggingPrefix);
       return;
@@ -84,80 +85,80 @@ export class MetricsCollector {
     collection.set(this.#getMetricName(name), metric);
   };
 
-  addCounter = (name: string, help: string) => {
+  addCounter = (name: string, help: string): void => {
     this.#addMetric(this.#counters, promClient.Counter, name, help, []);
   };
 
-  addSummary = (name: string, help: string) => {
+  addSummary = (name: string, help: string): void => {
     this.#addMetric(this.#summaries, promClient.Summary, name, help, []);
   };
 
-  addGauge = (name: string, help: string, labelNames?: string[]) => {
+  addGauge = (name: string, help: string, labelNames?: string[]): void => {
     this.#addMetric(this.#gauges, promClient.Gauge, name, help, labelNames);
   };
 
-  incCounter = (name: string) => {
+  incCounter = (name: string): void => {
     this.#counters.get(this.#getMetricName(name))?.inc();
   };
 
-  incGauge = (name: string, labels?: Record<string, string>, value: number = 1) => {
+  incGauge = (name: string, labels?: Record<string, string>, value: number = 1): void => {
     this.#gauges.get(this.#getMetricName(name))?.inc(labels || {}, value);
   };
 
   /**
    * Increments the error counter.
    */
-  error = () => this.incCounter(this.#metricNames.errors);
+  error = (): void => this.incCounter(this.#metricNames.errors);
 
   /**
    * Increments the alerts counter.
    */
-  alert = () => this.incCounter(this.#metricNames.alerts);
+  alert = (): void => this.incCounter(this.#metricNames.alerts);
 
   /**
    * Observes the duration since the provided start time and updates the summary.
    * @param startTime - The start time.
    * @param name - The metrics summary to increment.
    */
-  observeEnd = (startTime: number, name: string = this.#metricNames.mutate) => {
+  observeEnd = (startTime: number, name: string = this.#metricNames.mutate): void => {
     this.#summaries.get(this.#getMetricName(name))?.observe(performance.now() - startTime);
   };
 
   /**
    * Fetches the current metrics from the registry.
    * @returns The metrics.
    */
-  getMetrics = () => this.#registry.metrics();
+  getMetrics = (): Promise<string> => this.#registry.metrics();
 
   /**
    * Returns the current timestamp from performance.now() method. Useful for start timing an operation.
    * @returns The timestamp.
    */
-  static observeStart() {
+  static observeStart(): number {
     return performance.now();
   }
 
   /**
    * Increments the cache miss gauge for a given label.
    * @param label - The label for the cache miss.
    */
-  incCacheMiss = (window: string) => {
+  incCacheMiss = (window: string): void => {
     this.incGauge(this.#metricNames.cacheMiss, { window });
   };
 
   /**
    * Increments the retry count gauge.
    * @param count - The count to increment by.
    */
-  incRetryCount = (count: string) => {
+  incRetryCount = (count: string): void => {
     this.incGauge(this.#metricNames.resyncFailureCount, { count });
   };
 
   /**
    * Initializes the cache miss gauge for a given label.
    * @param label - The label for the cache miss.
    */
-  initCacheMissWindow = (window: string) => {
+  initCacheMissWindow = (window: string): void => {
     this.#rollCacheMissWindows();
     this.#gauges.get(this.#getMetricName(this.#metricNames.cacheMiss))?.set({ window }, 0);
     this.#cacheMissWindows.set(window, 0);
@@ -166,7 +167,7 @@ export class MetricsCollector {
   /**
    * Manages the size of the cache miss gauge map.
    */
-  #rollCacheMissWindows = () => {
+  #rollCacheMissWindows = (): void => {
     const maxCacheMissWindows = process.env.PEPR_MAX_CACHE_MISS_WINDOWS
       ? parseInt(process.env.PEPR_MAX_CACHE_MISS_WINDOWS, 10)
       : undefined;
@@ -181,4 +182,4 @@ export class MetricsCollector {
   };
 }
 
-export const metricsCollector = new MetricsCollector("pepr");
+export const metricsCollector: MetricsCollectorInstance = new MetricsCollector("pepr");