Skip to content

Commit 92409bd

Browse files
committed
test(node): Add unit tests + changelog for LangChain callback fix
1 parent e2b872a commit 92409bd

3 files changed

Lines changed: 178 additions & 41 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
## Unreleased
44

5+
- fix(node): Preserve `CallbackManager` children when augmenting LangChain callbacks
56
- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
67

78
## 10.53.1

packages/node/src/integrations/tracing/langchain/instrumentation.ts

Lines changed: 77 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import {
44
type InstrumentationModuleDefinition,
55
InstrumentationNodeModuleDefinition,
66
InstrumentationNodeModuleFile,
7-
} from '@opentelemetry/instrumentation';
8-
import type { LangChainOptions } from '@sentry/core';
7+
} from "@opentelemetry/instrumentation";
8+
import type { LangChainOptions } from "@sentry/core";
99
import {
1010
_INTERNAL_skipAiProviderWrapping,
1111
ANTHROPIC_AI_INTEGRATION_NAME,
@@ -14,9 +14,9 @@ import {
1414
instrumentLangChainEmbeddings,
1515
OPENAI_INTEGRATION_NAME,
1616
SDK_VERSION,
17-
} from '@sentry/core';
17+
} from "@sentry/core";
1818

19-
const supportedVersions = ['>=0.1.0 <2.0.0'];
19+
const supportedVersions = [">=0.1.0 <2.0.0"];
2020

2121
type LangChainInstrumentationOptions = InstrumentationConfig & LangChainOptions;
2222

@@ -37,13 +37,22 @@ function isCallbackManager(value: unknown): value is {
3737
copy: () => unknown;
3838
handlers?: unknown[];
3939
} {
40-
if (!value || typeof value !== 'object') {
40+
if (!value || typeof value !== "object") {
4141
return false;
4242
}
4343
const candidate = value as { addHandler?: unknown; copy?: unknown };
44-
return typeof candidate.addHandler === 'function' && typeof candidate.copy === 'function';
44+
return (
45+
typeof candidate.addHandler === "function" &&
46+
typeof candidate.copy === "function"
47+
);
4548
}
4649

50+
/**
51+
* Exported for testing.
52+
* @internal
53+
*/
54+
export const _INTERNAL_augmentCallbackHandlers = augmentCallbackHandlers;
55+
4756
/**
4857
* Augments a callback handler list with Sentry's handler if not already present.
4958
*
@@ -65,7 +74,10 @@ function isCallbackManager(value: unknown): value is {
6574
* mutate the caller's manager across invocations) and register Sentry's
6675
* handler as an inheritable child via `.addHandler()`.
6776
*/
68-
function augmentCallbackHandlers(handlers: unknown, sentryHandler: unknown): unknown {
77+
function augmentCallbackHandlers(
78+
handlers: unknown,
79+
sentryHandler: unknown,
80+
): unknown {
6981
// Handle null/undefined - return array with just our handler
7082
if (!handlers) {
7183
return [sentryHandler];
@@ -122,14 +134,17 @@ function wrapRunnableMethod(
122134
let options = args[optionsIndex] as Record<string, unknown> | undefined;
123135

124136
// If options don't exist or aren't an object, create them
125-
if (!options || typeof options !== 'object' || Array.isArray(options)) {
137+
if (!options || typeof options !== "object" || Array.isArray(options)) {
126138
options = {};
127139
args[optionsIndex] = options;
128140
}
129141

130142
// Inject our callback handler into options.callbacks (request time callbacks)
131143
const existingCallbacks = options.callbacks;
132-
const augmentedCallbacks = augmentCallbackHandlers(existingCallbacks, sentryHandler);
144+
const augmentedCallbacks = augmentCallbackHandlers(
145+
existingCallbacks,
146+
sentryHandler,
147+
);
133148
options.callbacks = augmentedCallbacks;
134149

135150
// Call original method with augmented options
@@ -143,7 +158,7 @@ function wrapRunnableMethod(
143158
*/
144159
export class SentryLangChainInstrumentation extends InstrumentationBase<LangChainInstrumentationOptions> {
145160
public constructor(config: LangChainInstrumentationOptions = {}) {
146-
super('@sentry/instrumentation-langchain', SDK_VERSION, config);
161+
super("@sentry/instrumentation-langchain", SDK_VERSION, config);
147162
}
148163

149164
/**
@@ -153,17 +168,19 @@ export class SentryLangChainInstrumentation extends InstrumentationBase<LangChai
153168
* We hook into provider packages (@langchain/anthropic, @langchain/openai, etc.)
154169
* because @langchain/core is often bundled and not loaded as a separate module
155170
*/
156-
public init(): InstrumentationModuleDefinition | InstrumentationModuleDefinition[] {
171+
public init():
172+
| InstrumentationModuleDefinition
173+
| InstrumentationModuleDefinition[] {
157174
const modules: InstrumentationModuleDefinition[] = [];
158175

159176
// Hook into common LangChain provider packages
160177
const providerPackages = [
161-
'@langchain/anthropic',
162-
'@langchain/openai',
163-
'@langchain/google-genai',
164-
'@langchain/mistralai',
165-
'@langchain/google-vertexai',
166-
'@langchain/groq',
178+
"@langchain/anthropic",
179+
"@langchain/openai",
180+
"@langchain/google-genai",
181+
"@langchain/mistralai",
182+
"@langchain/google-vertexai",
183+
"@langchain/groq",
167184
];
168185

169186
for (const packageName of providerPackages) {
@@ -176,13 +193,13 @@ export class SentryLangChainInstrumentation extends InstrumentationBase<LangChai
176193
packageName,
177194
supportedVersions,
178195
this._patch.bind(this),
179-
exports => exports,
196+
(exports) => exports,
180197
[
181198
new InstrumentationNodeModuleFile(
182199
`${packageName}/dist/index.cjs`,
183200
supportedVersions,
184201
this._patch.bind(this),
185-
exports => exports,
202+
(exports) => exports,
186203
),
187204
],
188205
),
@@ -192,17 +209,17 @@ export class SentryLangChainInstrumentation extends InstrumentationBase<LangChai
192209
// Hook into main 'langchain' package to catch initChatModel (v1+)
193210
modules.push(
194211
new InstrumentationNodeModuleDefinition(
195-
'langchain',
212+
"langchain",
196213
supportedVersions,
197214
this._patch.bind(this),
198-
exports => exports,
215+
(exports) => exports,
199216
[
200217
// To catch the CJS build that contains ConfigurableModel / initChatModel for v1
201218
new InstrumentationNodeModuleFile(
202-
'langchain/dist/chat_models/universal.cjs',
219+
"langchain/dist/chat_models/universal.cjs",
203220
supportedVersions,
204221
this._patch.bind(this),
205-
exports => exports,
222+
(exports) => exports,
206223
),
207224
],
208225
),
@@ -215,7 +232,9 @@ export class SentryLangChainInstrumentation extends InstrumentationBase<LangChai
215232
* Core patch logic - patches chat model and embedding methods
216233
* This is called when a LangChain provider package is loaded
217234
*/
218-
private _patch(exports: PatchedLangChainExports): PatchedLangChainExports | void {
235+
private _patch(
236+
exports: PatchedLangChainExports,
237+
): PatchedLangChainExports | void {
219238
// Skip AI provider wrapping now that LangChain is actually being used
220239
// This prevents duplicate spans from Anthropic/OpenAI/GoogleGenAI standalone integrations
221240
_INTERNAL_skipAiProviderWrapping([
@@ -244,22 +263,30 @@ export class SentryLangChainInstrumentation extends InstrumentationBase<LangChai
244263
* Patches chat model methods (invoke, stream, batch) to inject Sentry callbacks
245264
* Finds a chat model class from the provider package exports and patches its prototype methods
246265
*/
247-
private _patchRunnableMethods(exports: PatchedLangChainExports, sentryHandler: unknown): void {
266+
private _patchRunnableMethods(
267+
exports: PatchedLangChainExports,
268+
sentryHandler: unknown,
269+
): void {
248270
// Known chat model class names for each provider
249271
const knownChatModelNames = [
250-
'ChatAnthropic',
251-
'ChatOpenAI',
252-
'ChatGoogleGenerativeAI',
253-
'ChatMistralAI',
254-
'ChatVertexAI',
255-
'ChatGroq',
256-
'ConfigurableModel',
272+
"ChatAnthropic",
273+
"ChatOpenAI",
274+
"ChatGoogleGenerativeAI",
275+
"ChatMistralAI",
276+
"ChatVertexAI",
277+
"ChatGroq",
278+
"ConfigurableModel",
257279
];
258280

259-
const exportsToPatch = (exports.universal_exports ?? exports) as Record<string, unknown>;
281+
const exportsToPatch = (exports.universal_exports ?? exports) as Record<
282+
string,
283+
unknown
284+
>;
260285

261-
const chatModelClass = Object.values(exportsToPatch).find(exp => {
262-
return typeof exp === 'function' && knownChatModelNames.includes(exp.name);
286+
const chatModelClass = Object.values(exportsToPatch).find((exp) => {
287+
return (
288+
typeof exp === "function" && knownChatModelNames.includes(exp.name)
289+
);
263290
}) as { prototype: unknown; name: string } | undefined;
264291

265292
if (!chatModelClass) {
@@ -277,11 +304,11 @@ export class SentryLangChainInstrumentation extends InstrumentationBase<LangChai
277304

278305
// Patch the methods (invoke, stream, batch)
279306
// All chat model instances will inherit these patched methods
280-
const methodsToPatch = ['invoke', 'stream', 'batch'] as const;
307+
const methodsToPatch = ["invoke", "stream", "batch"] as const;
281308

282309
for (const methodName of methodsToPatch) {
283310
const method = targetProto[methodName];
284-
if (typeof method === 'function') {
311+
if (typeof method === "function") {
285312
targetProto[methodName] = wrapRunnableMethod(
286313
method as (...args: unknown[]) => unknown,
287314
sentryHandler,
@@ -299,15 +326,24 @@ export class SentryLangChainInstrumentation extends InstrumentationBase<LangChai
299326
*
300327
* Instruments any exported class whose prototype has both embedQuery and embedDocuments as functions.
301328
*/
302-
private _patchEmbeddingsMethods(exports: PatchedLangChainExports, options: LangChainOptions): void {
303-
const exportsToPatch = (exports.universal_exports ?? exports) as Record<string, unknown>;
329+
private _patchEmbeddingsMethods(
330+
exports: PatchedLangChainExports,
331+
options: LangChainOptions,
332+
): void {
333+
const exportsToPatch = (exports.universal_exports ?? exports) as Record<
334+
string,
335+
unknown
336+
>;
304337

305338
for (const exp of Object.values(exportsToPatch)) {
306-
if (typeof exp !== 'function' || !exp.prototype) {
339+
if (typeof exp !== "function" || !exp.prototype) {
307340
continue;
308341
}
309342
const proto = exp.prototype as Record<string, unknown>;
310-
if (typeof proto.embedQuery !== 'function' || typeof proto.embedDocuments !== 'function') {
343+
if (
344+
typeof proto.embedQuery !== "function" ||
345+
typeof proto.embedDocuments !== "function"
346+
) {
311347
continue;
312348
}
313349
if (proto.__sentry_patched__) {
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import { describe, expect, test, vi } from "vitest";
2+
import { _INTERNAL_augmentCallbackHandlers } from "../../../src/integrations/tracing/langchain/instrumentation";
3+
4+
const sentryHandler = { name: "SentryCallbackHandler" };
5+
6+
/**
7+
* Minimal `CallbackManager` stand-in. We only need the duck-typed shape
8+
* (`addHandler` + `copy`) for the production code to recognize this as a
9+
* `CallbackManager` rather than fall through to the "unknown" branch.
10+
*/
11+
function makeFakeCallbackManager(existingHandlers: unknown[] = []) {
12+
const manager = {
13+
handlers: [...existingHandlers],
14+
addHandler: vi.fn(function (
15+
this: any,
16+
handler: unknown,
17+
_inherit?: boolean,
18+
) {
19+
this.handlers.push(handler);
20+
}),
21+
copy: vi.fn(function (this: any) {
22+
return makeFakeCallbackManager(this.handlers);
23+
}),
24+
};
25+
return manager;
26+
}
27+
28+
describe("augmentCallbackHandlers", () => {
29+
test("wraps Sentry handler in an array when no callbacks are configured", () => {
30+
const result = _INTERNAL_augmentCallbackHandlers(undefined, sentryHandler);
31+
expect(result).toEqual([sentryHandler]);
32+
});
33+
34+
test("appends Sentry handler when callbacks is already an array", () => {
35+
const other = { name: "OtherHandler" };
36+
const result = _INTERNAL_augmentCallbackHandlers([other], sentryHandler);
37+
expect(result).toEqual([other, sentryHandler]);
38+
});
39+
40+
test("is idempotent when Sentry handler is already in the array", () => {
41+
const result = _INTERNAL_augmentCallbackHandlers(
42+
[sentryHandler],
43+
sentryHandler,
44+
);
45+
expect(result).toEqual([sentryHandler]);
46+
});
47+
48+
test("preserves inheritable handlers when callbacks is a CallbackManager", () => {
49+
// Reproduces the LangGraph `streamMode: ['messages']` setup: a
50+
// CallbackManager carrying a StreamMessagesHandler is passed via
51+
// options.callbacks. Without this fix, the manager would be wrapped as
52+
// `[manager, sentryHandler]`, dropping all its inheritable children.
53+
const streamMessagesHandler = {
54+
name: "StreamMessagesHandler",
55+
lc_prefer_streaming: true,
56+
};
57+
const manager = makeFakeCallbackManager([streamMessagesHandler]);
58+
59+
const result = _INTERNAL_augmentCallbackHandlers(
60+
manager,
61+
sentryHandler,
62+
) as {
63+
handlers: unknown[];
64+
};
65+
66+
// The result is a manager (object), not a wrapping array.
67+
expect(Array.isArray(result)).toBe(false);
68+
// The original child handler is still there alongside Sentry's.
69+
expect(result.handlers).toEqual([streamMessagesHandler, sentryHandler]);
70+
});
71+
72+
test("copies the manager rather than mutating the caller-supplied one", () => {
73+
// If we mutated the original manager, repeated invocations would
74+
// accumulate Sentry handlers (and tracers from prior runs would leak
75+
// into subsequent unrelated runs).
76+
const manager = makeFakeCallbackManager([]);
77+
_INTERNAL_augmentCallbackHandlers(manager, sentryHandler);
78+
expect(manager.copy).toHaveBeenCalledTimes(1);
79+
expect(manager.handlers).toEqual([]);
80+
});
81+
82+
test("does not double-register Sentry handler when copy already contains it", () => {
83+
const manager = makeFakeCallbackManager([sentryHandler]);
84+
const result = _INTERNAL_augmentCallbackHandlers(
85+
manager,
86+
sentryHandler,
87+
) as {
88+
handlers: unknown[];
89+
addHandler: ReturnType<typeof vi.fn>;
90+
};
91+
expect(result.handlers).toEqual([sentryHandler]);
92+
expect(result.addHandler).not.toHaveBeenCalled();
93+
});
94+
95+
test("returns the value unchanged when it is neither an array nor a CallbackManager", () => {
96+
const opaque = { name: "NotAManager" };
97+
const result = _INTERNAL_augmentCallbackHandlers(opaque, sentryHandler);
98+
expect(result).toBe(opaque);
99+
});
100+
});

0 commit comments

Comments
 (0)