Skip to content

Commit c9edda4

Browse files
fix: instrument both builds of a dual-package provider SDK (#26)
1 parent 4345922 commit c9edda4

4 files changed

Lines changed: 85 additions & 34 deletions

File tree

package-lock.json

Lines changed: 52 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@
5959
"@opentelemetry/semantic-conventions": "^1.43.0"
6060
},
6161
"peerDependencies": {
62-
"@arizeai/openinference-instrumentation-anthropic": "^0.1.20",
62+
"@arizeai/openinference-instrumentation-anthropic": "^0.2.1",
6363
"@arizeai/openinference-instrumentation-langchain": "^4.0.17",
64-
"@arizeai/openinference-instrumentation-openai": "^4.1.8",
64+
"@arizeai/openinference-instrumentation-openai": "^4.2.1",
6565
"@arizeai/openinference-vercel": "^3.1.4",
6666
"@langchain/core": "^1.0.0 || ^0.3.0",
6767
"@modelcontextprotocol/sdk": "^1.30.0",
@@ -89,9 +89,9 @@
8989
},
9090
"devDependencies": {
9191
"@anthropic-ai/sdk": "^0.65.0",
92-
"@arizeai/openinference-instrumentation-anthropic": "0.1.20",
92+
"@arizeai/openinference-instrumentation-anthropic": "0.2.1",
9393
"@arizeai/openinference-instrumentation-langchain": "4.0.17",
94-
"@arizeai/openinference-instrumentation-openai": "^4.1.8",
94+
"@arizeai/openinference-instrumentation-openai": "^4.2.1",
9595
"@arizeai/openinference-vercel": "^3.1.4",
9696
"@biomejs/biome": "^1.9.0",
9797
"@langchain/core": "^1.2.5",

src/instrumentation.ts

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -183,18 +183,27 @@ export function cachedCjsExports(
183183
* Patch the build of a dual-package provider SDK that this process is actually
184184
* using. `openai` and `@anthropic-ai/sdk` ship separate CJS and ESM builds
185185
* with separate class objects, and the OpenInference require hook only ever
186-
* sees the CJS one, so a pure-ESM app would silently get no spans. Their
187-
* `patch()` is also guarded by a module-global flag
188-
* (github.com/Arize-ai/openinference/issues/3557), so only ONE build can be
189-
* patched per process and the choice matters:
186+
* sees the CJS one, so a pure-ESM app would silently get no spans:
190187
*
191188
* - CJS build already in `require.cache` → the app requires it, patch that
192189
* copy. This also covers a require that happened BEFORE init(), which the
193190
* require hook alone never repairs.
194191
* - Otherwise → import the ESM build and patch it. In an ESM app every static
195-
* import already ran before init(), so this is the copy in use. The one
196-
* pattern this trades away is a CJS app whose only require of the provider
197-
* comes after init(): it gets the ESM build patched instead (documented).
192+
* import already ran before init(), so this is the copy in use.
193+
*
194+
* Picking one build used to be forced on us: `patch()` was guarded by a
195+
* module-global flag (github.com/Arize-ai/openinference/issues/3557) that let
196+
* only ONE build be patched per process, so a CJS app whose only require came
197+
* after init() got the ESM build patched and nothing else. That flag is now
198+
* scoped to the patched class (instrumentation-anthropic >= 0.2.1,
199+
* -openai >= 4.2.1, the floor package.json pins), so the require hook can
200+
* still patch a later CJS require on top of whatever we patched here — the
201+
* case above is covered without this function choosing differently.
202+
*
203+
* What remains uncovered is narrower: a HYBRID app that has already loaded
204+
* both builds before init(). The cached CJS copy wins here and the ESM one
205+
* goes unpatched. Patching both is now permitted and would close it, at the
206+
* cost of importing a build the app may never use.
198207
*
199208
* The patchable is a plain-object wrapper, never the ESM namespace itself:
200209
* `patch()` writes an `openInferencePatched` marker onto what it receives, and

tests/instrumentationEsmPatch.test.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ describe("the openai entry in a pure-ESM consumer", () => {
9494
});
9595

9696
describe("the anthropic entry in a pure-ESM consumer", () => {
97-
it("patches the ESM build, and the flag leaves a later CJS require unpatched", async () => {
97+
it("patches the ESM build, and a later CJS require is patched too", async () => {
9898
const { provider } = recordingProvider();
9999
const enabled = await enableInstrumentations(makeSink(), provider, ["anthropic"]);
100100
expect(enabled).toContain("anthropic");
@@ -106,20 +106,24 @@ describe("the anthropic entry in a pure-ESM consumer", () => {
106106
};
107107
expect(isWrapped(EsmAnthropic.Messages.prototype.create)).toBe(true);
108108

109-
// Documents the residual gap, not an aspiration: OpenInference's global
110-
// patched flag (Arize-ai/openinference#3557) means the CJS build of the
111-
// same package can no longer be patched in this process. A CJS require
112-
// that only happens after init() therefore stays uninstrumented. If this
113-
// assertion starts failing after an OpenInference upgrade, the upstream
114-
// fix landed: patch BOTH builds unconditionally and drop the
115-
// cache-checking heuristic.
109+
// This used to assert `false`, documenting the gap left by OpenInference's
110+
// module-global patched flag (Arize-ai/openinference#3557): whichever build
111+
// was patched first blocked the other, so a CJS require arriving after
112+
// init() stayed uninstrumented. The guard is now scoped to the patched
113+
// class (instrumentation-anthropic >= 0.2.1, -openai >= 4.2.1), so the
114+
// require hook is free to patch the CJS build even though we already
115+
// patched the ESM one. Two DIFFERENT class objects, both wrapped.
116+
//
117+
// Keep asserting it: this is what proves the floor in package.json is
118+
// doing its job. Against an older instrumentation it fails, which is the
119+
// signal we want if the range is ever loosened.
116120
const cjs = createRequire(import.meta.url)("@anthropic-ai/sdk") as {
117121
default?: { Messages: { prototype: { create: unknown } } };
118122
Anthropic?: { Messages: { prototype: { create: unknown } } };
119123
};
120124
const CjsAnthropic = cjs.default ?? cjs.Anthropic;
121125
expect(CjsAnthropic).toBeDefined();
122126
expect(EsmAnthropic).not.toBe(CjsAnthropic);
123-
expect(isWrapped(CjsAnthropic?.Messages.prototype.create)).toBe(false);
127+
expect(isWrapped(CjsAnthropic?.Messages.prototype.create)).toBe(true);
124128
});
125129
});

0 commit comments

Comments
 (0)