chore(config): invoke key-value parsing directly in config

chore(instrumentation): use cached singleton vs. global metrics

Author: kschelonka

src/config.test.ts

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import { describe, it, expect } from "@jest/globals";
-import { getEnvEnum, getEnvInt, getEnvString, parseKVList } from "./config";
+import { getEnvEnum, getEnvInt, getEnvString, getKVList } from "./config";
 
 describe("getEnvString", () => {
   it("throws an error when attempting to access an undefined environment variable", () => {
@@ -118,26 +118,26 @@ describe("getEnvEnum", () => {
 
 describe("parseKVList", () => {
   it("returns empty object for undefined", () => {
-    expect(parseKVList(undefined)).toEqual({});
+    expect(getKVList(undefined)).toEqual({});
   });
 
   it("returns empty object for empty string", () => {
-    expect(parseKVList("")).toEqual({});
+    expect(getKVList("")).toEqual({});
   });
 
   it("returns empty object for whitespace-only string", () => {
-    expect(parseKVList("   \n\t  ")).toEqual({});
+    expect(getKVList("   \n\t  ")).toEqual({});
   });
 
   it("parses a single key=value pair", () => {
-    expect(parseKVList("service.name=monitor")).toEqual({
+    expect(getKVList("service.name=monitor")).toEqual({
       "service.name": "monitor",
     });
   });
 
   it("parses multiple key=value pairs separated by commas", () => {
     expect(
-      parseKVList(
+      getKVList(
         "k8s.namespace.name=monitor-stage,k8s.pod.name=pod-123,service.version=123abcd",
       ),
     ).toEqual({
@@ -148,47 +148,47 @@ describe("parseKVList", () => {
   });
 
   it("trims whitespace around segments but preserves whitespace inside values", () => {
-    expect(parseKVList("  a=1  ,  b=hello world  ,   c=3 ")).toEqual({
+    expect(getKVList("  a=1  ,  b=hello world  ,   c=3 ")).toEqual({
       a: "1",
       b: "hello world",
       c: "3",
     });
   });
 
   it("ignores empty segments from consecutive commas", () => {
-    expect(parseKVList("a=1,,b=2,,,c=3,")).toEqual({
+    expect(getKVList("a=1,,b=2,,,c=3,")).toEqual({
       a: "1",
       b: "2",
       c: "3",
     });
   });
 
   it("ignores segments without '='", () => {
-    expect(parseKVList("flag,service.name=monitor")).toEqual({
+    expect(getKVList("flag,service.name=monitor")).toEqual({
       "service.name": "monitor",
     });
   });
 
   it("ignores keys with empty values (key=)", () => {
-    expect(parseKVList("a=,b=2")).toEqual({
+    expect(getKVList("a=,b=2")).toEqual({
       b: "2",
     });
   });
 
   it("ignores empty keys (=value)", () => {
-    expect(parseKVList("=value,ok=yes")).toEqual({
+    expect(getKVList("=value,ok=yes")).toEqual({
       ok: "yes",
     });
   });
   it("properly preserves values with '='", () => {
-    expect(parseKVList("auth=Bearer abc=def==,x=1")).toEqual({
+    expect(getKVList("auth=Bearer abc=def==,x=1")).toEqual({
       auth: "Bearer abc=def==",
       x: "1",
     });
   });
   it("allows keys containing dots/slashes and other characters", () => {
     expect(
-      parseKVList(
+      getKVList(
         "http.request.header.user_agent=curl/8.7.1,service.namespace=monitor-stage",
       ),
     ).toEqual({
@@ -197,11 +197,11 @@ describe("parseKVList", () => {
     });
   });
   it("handles '=' string alone", () => {
-    expect(parseKVList("=")).toEqual({});
+    expect(getKVList("=")).toEqual({});
   });
 
   it("handles mixed malformed and well-formed segments", () => {
-    expect(parseKVList("a=1, ,flag,b=2,=nope,c=3")).toEqual({
+    expect(getKVList("a=1, ,flag,b=2,=nope,c=3")).toEqual({
       a: "1",
       b: "2",
       c: "3",

src/config.ts

@@ -131,7 +131,7 @@ export const config = {
         fallbackValue: "http/protobuf",
       },
     ),
-    resourceAttributes: process.env.OTEL_RESOURCE_ATTRIBUTES,
+    resourceAttributes: getKVList(process.env.OTEL_RESOURCE_ATTRIBUTES),
     serviceName: getEnvString("OTEL_SERVICE_NAME", {
       fallbackValue: "monitor",
     }),
@@ -231,11 +231,12 @@ export function getEnvString(
 
 /**
  * Parse comma-separated key=value list into dictionary.
+ * Undefined or empty values will return an empty object.
  * Segments without '=' are ignored (a=1,b,c=2 -> {a: 1, c: 2}
  * (e.g. used by OTEL_RESOURCE_ATTRIBUTES env var:
  * 'k8s.container.name=monitor-api-endpoint,k8s.namespace.name=monitor-stage...')
  * */
-export function parseKVList(str?: string): Record<string, string> {
+export function getKVList(str?: string): Record<string, string> {
   if (str === undefined) return {};
   return str
     .split(",")

src/instrumentation.node.ts

@@ -10,8 +10,6 @@ import {
   ConsoleMetricExporter,
   PeriodicExportingMetricReader,
 } from "@opentelemetry/sdk-metrics";
-import { detectResources } from "@opentelemetry/resources";
-import { gcpDetector } from "@opentelemetry/resource-detector-gcp";
 import { resourceFromAttributes } from "@opentelemetry/resources";
 import { NodeSDK } from "@opentelemetry/sdk-node";
 import {
@@ -42,7 +40,7 @@ import {
 } from "@sentry/opentelemetry";
 import * as Sentry from "@sentry/nextjs";
 import { AsyncLocalStorageContextManager } from "@opentelemetry/context-async-hooks";
-import { config as appConfig, parseKVList } from "./config";
+import { config as appConfig } from "./config";
 
 // Note: console.log use is intentional because this step is done as early
 // as possible, before setting up winston logging
@@ -101,7 +99,7 @@ export async function nodeSDKBuilder() {
   const sdk = new NodeSDK({
     resource: resourceFromAttributes({
       [ATTR_SERVICE_NAME]: appConfig.otel.serviceName,
-      ...parseKVList(appConfig.otel.resourceAttributes),
+      ...appConfig.otel.resourceAttributes,
     }),
     metricReaders: [
       new PeriodicExportingMetricReader({
@@ -170,21 +168,18 @@ export async function nodeSDKBuilder() {
   console.log("[INSTRUMENTATION] Otel setup is valid");
 }
 
-// Prometheus setup
 type MetricsState = {
   hibpNotifyRequestsTotal: Counter<Attributes>;
   hibpNotifyRequestFailuresTotal: Counter<Attributes>;
 };
 
-declare global {
-  var __metrics: Readonly<MetricsState> | undefined;
-}
+let metricsState: MetricsState | undefined;
 
 // NOTE: OTEL must be initialized before calling any metrics
 // or else it will be a No-Op metric
-function getOrInitMetrics(): MetricsState {
+export function getOrInitMetrics(): MetricsState {
   // Return cached state
-  if (globalThis.__metrics !== undefined) return globalThis.__metrics;
+  if (metricsState !== undefined) return metricsState;
   // Get the pre-registered metrics provider
   // Will be a no-op if otel has not been initialized
   const meter = metrics.getMeter(appConfig.otel.serviceName);
@@ -204,14 +199,6 @@ function getOrInitMetrics(): MetricsState {
     hibpNotifyRequestFailuresTotal,
     hibpNotifyRequestsTotal,
   };
-
-  // Make it readonly
-  Object.defineProperty(globalThis, "__metrics", {
-    value: state,
-    writable: false,
-    configurable: false,
-    enumerable: false,
-  });
   return state;
 }
 

src/instrumentation.ts

@@ -31,9 +31,13 @@ export async function register() {
       // Disable Sentry's automatic OTEL instrumentation since we're managing it ourselves
       skipOpenTelemetrySetup: true,
     });
-    const { nodeSDKBuilder } = await import("./instrumentation.node");
+    const { nodeSDKBuilder, getOrInitMetrics } = await import(
+      "./instrumentation.node"
+    );
     console.log("[INSTRUMENTATION] Initializing OTEL...");
     await nodeSDKBuilder();
+    // Ensure metrics are initialized after OTEL
+    getOrInitMetrics();
   }
 }