Skip to content

Commit 1a5a6ca

Browse files
committed
Fix circular structure error in OpenTelemetry sink
Very similar to 8821447. In addition to fixing the inspect call, I also needed to track objects seen in the convertValueToAnyValue recursion as it would otherwise cause an infinite recursion in some cases. Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
1 parent 0e5c5dd commit 1a5a6ca

8 files changed

Lines changed: 155 additions & 26 deletions

File tree

packages/otel/deno.json

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,35 @@
55
"exports": {
66
".": "./src/mod.ts"
77
},
8+
"imports": {
9+
"#util": "src/util.deno.ts"
10+
},
811
"tasks": {
912
"build": "pnpm build",
10-
"test": "deno test --allow-net --allow-env"
11-
}
13+
"test": "deno test",
14+
"test:node": {
15+
"dependencies": [
16+
"build"
17+
],
18+
"command": "node --experimental-transform-types --test"
19+
},
20+
"test:bun": {
21+
"dependencies": [
22+
"build"
23+
],
24+
"command": "bun test"
25+
},
26+
"test-all": {
27+
"dependencies": [
28+
"test",
29+
"test:node",
30+
"test:bun"
31+
]
32+
}
33+
},
34+
"exclude": [
35+
"dist/",
36+
"npm/",
37+
"node_modules/"
38+
]
1239
}

packages/otel/package.json

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,30 @@
4040
},
4141
"./package.json": "./package.json"
4242
},
43+
"imports": {
44+
"#util": {
45+
"types": {
46+
"import": "./dist/util.d.ts",
47+
"require": "./dist/util.d.cts"
48+
},
49+
"browser": {
50+
"import": "./dist/util.js",
51+
"require": "./dist/util.cjs"
52+
},
53+
"node": {
54+
"import": "./dist/util.node.js",
55+
"require": "./dist/util.node.cjs"
56+
},
57+
"bun": {
58+
"import": "./dist/util.node.js",
59+
"require": "./dist/util.node.cjs"
60+
},
61+
"deno": "./dist/util.deno.js",
62+
"import": "./dist/util.js",
63+
"require": "./dist/util.cjs",
64+
"default": "./dist/util.js"
65+
}
66+
},
4367
"sideEffects": false,
4468
"files": [
4569
"dist/"

packages/otel/src/mod.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ import {
1717
type OpenTelemetrySinkProviderOptions,
1818
} from "./mod.ts";
1919

20+
// Resolved per runtime so expectations match the sink's own formatting
21+
// (util.inspect on Node/Bun, Deno.inspect on Deno, JSON.stringify elsewhere).
22+
import { inspect } from "#util";
23+
2024
// Helper to create a mock log record
2125
function createMockLogRecord(overrides: Partial<LogRecord> = {}): LogRecord {
2226
return {
@@ -468,6 +472,35 @@ test("sink handles array values in properties", () => {
468472
]);
469473
});
470474

475+
test("sink handles circular references in interpolated message values", () => {
476+
const { provider, emittedRecords } = createMockLoggerProvider();
477+
const sink = getOpenTelemetrySink({
478+
loggerProvider: provider as never,
479+
objectRenderer: "inspect",
480+
});
481+
482+
// A circular value (resembling a Response with a back-reference) used as a
483+
// message interpolation value. Previously this hit JSON.stringify's circular
484+
// structure error inside getParameterizedString(), which the sink swallowed,
485+
// so captureMessage was never reached. Now inspect() (util.inspect /
486+
// Deno.inspect) renders it instead.
487+
const circular: Record<string, unknown> = { body: "ok" };
488+
circular.self = circular;
489+
490+
sink(createMockLogRecord({
491+
level: "error",
492+
message: ["Saw error: ", circular, ""],
493+
rawMessage: "Saw error: {error}",
494+
properties: { error: circular },
495+
}));
496+
497+
assert.strictEqual(emittedRecords.length, 1);
498+
499+
const body = emittedRecords[0].body as string;
500+
assert.ok(body.startsWith("Saw error: "));
501+
assert.ok(body.length > "Saw error: ".length);
502+
});
503+
471504
test("sink handles Date objects in properties", () => {
472505
const { provider, emittedRecords } = createMockLoggerProvider();
473506
const sink = getOpenTelemetrySink({

packages/otel/src/mod.ts

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ import {
2525
} from "@opentelemetry/sdk-logs";
2626
import { ATTR_SERVICE_NAME } from "@opentelemetry/semantic-conventions";
2727
import metadata from "../deno.json" with { type: "json" };
28+
// Cross-runtime inspect: Deno.inspect / util.inspect (handles circular
29+
// references); falls back to JSON.stringify in browsers. Resolved via the
30+
// `#util` import map per runtime.
31+
import { inspect } from "#util";
2832

2933
/**
3034
* Gets an environment variable value across different JavaScript runtimes.
@@ -606,6 +610,7 @@ function convertValueToAnyValue(
606610
value: unknown,
607611
objectRenderer: ObjectRenderer,
608612
exceptionMode: ExceptionAttributeMode,
613+
seenObjects: Set<any> = new Set(),
609614
): AnyValue | null {
610615
// Handle null/undefined
611616
if (value == null) return null;
@@ -618,6 +623,11 @@ function convertValueToAnyValue(
618623
return value;
619624
}
620625

626+
if (seenObjects.has(value)) {
627+
return null;
628+
}
629+
seenObjects.add(value);
630+
621631
// Handle arrays - recursively convert elements
622632
if (Array.isArray(value)) {
623633
// Check if it's a homogeneous array of primitives (OTel spec prefers these)
@@ -654,6 +664,7 @@ function convertValueToAnyValue(
654664
item,
655665
objectRenderer,
656666
exceptionMode,
667+
seenObjects,
657668
);
658669
// Skip null items but preserve the structure
659670
if (convertedItem !== null) {
@@ -677,6 +688,7 @@ function convertValueToAnyValue(
677688
val,
678689
objectRenderer,
679690
exceptionMode,
691+
seenObjects,
680692
);
681693
if (convertedVal !== null) {
682694
converted[key] = convertedVal;
@@ -700,6 +712,7 @@ function convertValueToAnyValue(
700712
val,
701713
objectRenderer,
702714
exceptionMode,
715+
seenObjects,
703716
);
704717
if (convertedVal !== null) {
705718
converted[key] = convertedVal;
@@ -869,29 +882,6 @@ function convertMessageToCustomBodyFormat(
869882
return bodyFormatter(body);
870883
}
871884

872-
/**
873-
* A platform-specific inspect function. In Deno, this is {@link Deno.inspect},
874-
* and in Node.js/Bun it is {@link util.inspect}. If neither is available, it
875-
* falls back to {@link JSON.stringify}.
876-
*
877-
* @param value The value to inspect.
878-
* @returns The string representation of the value.
879-
*/
880-
const inspect: (value: unknown) => string =
881-
// @ts-ignore: Deno global
882-
"Deno" in globalThis && "inspect" in globalThis.Deno &&
883-
// @ts-ignore: Deno global
884-
typeof globalThis.Deno.inspect === "function"
885-
// @ts-ignore: Deno global
886-
? globalThis.Deno.inspect
887-
// @ts-ignore: Node.js global
888-
: "util" in globalThis && "inspect" in globalThis.util &&
889-
// @ts-ignore: Node.js global
890-
globalThis.util.inspect === "function"
891-
// @ts-ignore: Node.js global
892-
? globalThis.util.inspect
893-
: JSON.stringify;
894-
895885
class DiagLoggerAdaptor implements DiagLogger {
896886
logger: Logger;
897887

packages/otel/src/util.deno.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
export interface InspectOptions {
2+
colors?: boolean;
3+
depth?: number | null;
4+
compact?: boolean;
5+
[key: string]: unknown;
6+
}
7+
8+
export function inspect(obj: unknown, options?: InspectOptions): string {
9+
if ("Deno" in globalThis) {
10+
return Deno.inspect(obj, {
11+
colors: options?.colors,
12+
depth: options?.depth ?? undefined,
13+
compact: options?.compact ?? true,
14+
});
15+
} else {
16+
const indent = options?.compact === true ? undefined : 2;
17+
return JSON.stringify(obj, null, indent);
18+
}
19+
}

packages/otel/src/util.node.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import util from "node:util";
2+
3+
export interface InspectOptions {
4+
colors?: boolean;
5+
depth?: number | null;
6+
compact?: boolean;
7+
[key: string]: unknown;
8+
}
9+
10+
export function inspect(obj: unknown, options?: InspectOptions): string {
11+
return util.inspect(obj, options);
12+
}

packages/otel/src/util.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
export interface InspectOptions {
2+
colors?: boolean;
3+
depth?: number | null;
4+
compact?: boolean;
5+
[key: string]: unknown;
6+
}
7+
8+
export function inspect(obj: unknown, options?: InspectOptions): string {
9+
const indent = options?.compact === true ? undefined : 2;
10+
return JSON.stringify(obj, null, indent);
11+
}

packages/otel/tsdown.config.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,24 @@
11
import { defineConfig } from "tsdown";
22

33
export default defineConfig({
4-
entry: "src/mod.ts",
4+
entry: ["src/mod.ts", "src/util.ts", "src/util.deno.ts", "src/util.node.ts"],
55
dts: {
66
sourcemap: true,
77
},
88
format: ["esm", "cjs"],
99
platform: "neutral",
1010
unbundle: true,
11+
inputOptions: {
12+
onLog(level, log, defaultHandler) {
13+
if (
14+
level === "warn" && log.code === "UNRESOLVED_IMPORT" &&
15+
["node:util", "#util"].includes(
16+
log.exporter ?? "",
17+
)
18+
) {
19+
return;
20+
}
21+
defaultHandler(level, log);
22+
},
23+
},
1124
});

0 commit comments

Comments
 (0)