Skip to content

Commit 0326731

Browse files
committed
chore(config): invoke key-value parsing directly in config
chore(instrumentation): use cached singleton vs. global metrics
1 parent e166140 commit 0326731

File tree

4 files changed

+60
-21
lines changed

4 files changed

+60
-21
lines changed

src/config.test.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

55
import { describe, it, expect } from "@jest/globals";
6-
import { getEnvEnum, getEnvInt, getEnvString, parseKVList } from "./config";
6+
import {
7+
getEnvEnum,
8+
getEnvInt,
9+
getEnvKVList,
10+
getEnvString,
11+
parseKVList,
12+
} from "./config";
713

814
describe("getEnvString", () => {
915
it("throws an error when attempting to access an undefined environment variable", () => {
@@ -116,6 +122,27 @@ describe("getEnvEnum", () => {
116122
});
117123
});
118124

125+
describe("getEnvKVList", () => {
126+
it("returns the parsed record when defined", () => {
127+
process.env.TEST_KEYV_VAR = "key=value";
128+
129+
expect(getEnvKVList("TEST_KEYV_VAR")).toStrictEqual({ key: "value" });
130+
131+
delete process.env.TEST_KEYV_VAR;
132+
});
133+
it("throws an error when attempting to access an undefined environment variable", () => {
134+
expect(() => getEnvKVList("UNDEFINED_ENV_VAR")).toThrow(
135+
"Variable $UNDEFINED_ENV_VAR is not defined in `.env`, `.env.test`, `.env.local` and `.env.test.local`, nor as an environment variable.",
136+
);
137+
});
138+
139+
it("returns the fallback value when attempting to access an undefined environment variable and one is given", () => {
140+
expect(
141+
getEnvKVList("UNDEFINED_ENV_VAR", { fallbackValue: { key: "value" } }),
142+
).toStrictEqual({ key: "value" });
143+
});
144+
});
145+
119146
describe("parseKVList", () => {
120147
it("returns empty object for undefined", () => {
121148
expect(parseKVList(undefined)).toEqual({});

src/config.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,9 @@ export const config = {
131131
fallbackValue: "http/protobuf",
132132
},
133133
),
134-
resourceAttributes: process.env.OTEL_RESOURCE_ATTRIBUTES,
134+
resourceAttributes: getEnvKVList("OTEL_RESOURCE_ATTRIBUTES", {
135+
fallbackValue: {},
136+
}),
135137
serviceName: getEnvString("OTEL_SERVICE_NAME", {
136138
fallbackValue: "monitor",
137139
}),
@@ -228,9 +230,28 @@ export function getEnvString(
228230

229231
return value;
230232
}
233+
/**
234+
* Parse comma-separated key=value list from env var into dictionary.
235+
* */
236+
export function getEnvKVList(
237+
key: string,
238+
options: Partial<{ fallbackValue: Record<string, string> }> = {},
239+
): Record<string, string> {
240+
const value = process.env[key];
241+
if (typeof value !== "string" || value === "") {
242+
if ("fallbackValue" in options && options.fallbackValue !== undefined) {
243+
return options.fallbackValue;
244+
}
245+
throw new Error(
246+
`Variable $${key} is not defined in \`.env\`, \`.env.${process.env.NODE_ENV}\`, \`.env.local\` and \`.env.${process.env.NODE_ENV}.local\`, nor as an environment variable.`,
247+
);
248+
}
249+
return parseKVList(value);
250+
}
231251

232252
/**
233253
* Parse comma-separated key=value list into dictionary.
254+
* Undefined or empty values will return an empty record.
234255
* Segments without '=' are ignored (a=1,b,c=2 -> {a: 1, c: 2}
235256
* (e.g. used by OTEL_RESOURCE_ATTRIBUTES env var:
236257
* 'k8s.container.name=monitor-api-endpoint,k8s.namespace.name=monitor-stage...')

src/instrumentation.node.ts

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ import {
1010
ConsoleMetricExporter,
1111
PeriodicExportingMetricReader,
1212
} from "@opentelemetry/sdk-metrics";
13-
import { detectResources } from "@opentelemetry/resources";
14-
import { gcpDetector } from "@opentelemetry/resource-detector-gcp";
1513
import { resourceFromAttributes } from "@opentelemetry/resources";
1614
import { NodeSDK } from "@opentelemetry/sdk-node";
1715
import {
@@ -42,7 +40,7 @@ import {
4240
} from "@sentry/opentelemetry";
4341
import * as Sentry from "@sentry/nextjs";
4442
import { AsyncLocalStorageContextManager } from "@opentelemetry/context-async-hooks";
45-
import { config as appConfig, parseKVList } from "./config";
43+
import { config as appConfig } from "./config";
4644

4745
// Note: console.log use is intentional because this step is done as early
4846
// as possible, before setting up winston logging
@@ -101,7 +99,7 @@ export async function nodeSDKBuilder() {
10199
const sdk = new NodeSDK({
102100
resource: resourceFromAttributes({
103101
[ATTR_SERVICE_NAME]: appConfig.otel.serviceName,
104-
...parseKVList(appConfig.otel.resourceAttributes),
102+
...appConfig.otel.resourceAttributes,
105103
}),
106104
metricReaders: [
107105
new PeriodicExportingMetricReader({
@@ -170,21 +168,18 @@ export async function nodeSDKBuilder() {
170168
console.log("[INSTRUMENTATION] Otel setup is valid");
171169
}
172170

173-
// Prometheus setup
174171
type MetricsState = {
175172
hibpNotifyRequestsTotal: Counter<Attributes>;
176173
hibpNotifyRequestFailuresTotal: Counter<Attributes>;
177174
};
178175

179-
declare global {
180-
var __metrics: Readonly<MetricsState> | undefined;
181-
}
176+
let metricsState: MetricsState | undefined;
182177

183178
// NOTE: OTEL must be initialized before calling any metrics
184179
// or else it will be a No-Op metric
185-
function getOrInitMetrics(): MetricsState {
180+
export function getOrInitMetrics(): MetricsState {
186181
// Return cached state
187-
if (globalThis.__metrics !== undefined) return globalThis.__metrics;
182+
if (metricsState !== undefined) return metricsState;
188183
// Get the pre-registered metrics provider
189184
// Will be a no-op if otel has not been initialized
190185
const meter = metrics.getMeter(appConfig.otel.serviceName);
@@ -204,14 +199,6 @@ function getOrInitMetrics(): MetricsState {
204199
hibpNotifyRequestFailuresTotal,
205200
hibpNotifyRequestsTotal,
206201
};
207-
208-
// Make it readonly
209-
Object.defineProperty(globalThis, "__metrics", {
210-
value: state,
211-
writable: false,
212-
configurable: false,
213-
enumerable: false,
214-
});
215202
return state;
216203
}
217204

src/instrumentation.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,13 @@ export async function register() {
3131
// Disable Sentry's automatic OTEL instrumentation since we're managing it ourselves
3232
skipOpenTelemetrySetup: true,
3333
});
34-
const { nodeSDKBuilder } = await import("./instrumentation.node");
34+
const { nodeSDKBuilder, getOrInitMetrics } = await import(
35+
"./instrumentation.node"
36+
);
3537
console.log("[INSTRUMENTATION] Initializing OTEL...");
3638
await nodeSDKBuilder();
39+
// Ensure metrics are initialized after OTEL
40+
getOrInitMetrics();
3741
}
3842
}
3943

0 commit comments

Comments
 (0)