Skip to content

Commit baebb0d

Browse files
committed
Refactor to proposed wrapping helpers
1 parent 72cb950 commit baebb0d

File tree

13 files changed

+319
-150
lines changed

13 files changed

+319
-150
lines changed
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import * as Sentry from '@sentry/node';
2+
import express from 'express';
3+
4+
function startMockAnthropicServer() {
5+
const app = express();
6+
app.use(express.json());
7+
8+
app.post('/v1/messages', (req, res) => {
9+
res.json({
10+
id: 'msg_test123',
11+
type: 'message',
12+
role: 'assistant',
13+
content: [
14+
{
15+
type: 'text',
16+
text: 'Mock response from Anthropic!',
17+
},
18+
],
19+
model: req.body.model,
20+
stop_reason: 'end_turn',
21+
stop_sequence: null,
22+
usage: {
23+
input_tokens: 10,
24+
output_tokens: 15,
25+
},
26+
});
27+
});
28+
29+
return new Promise(resolve => {
30+
const server = app.listen(0, () => {
31+
resolve(server);
32+
});
33+
});
34+
}
35+
36+
async function run() {
37+
const server = await startMockAnthropicServer();
38+
const baseURL = `http://localhost:${server.address().port}`;
39+
40+
await Sentry.startSpan({ op: 'function', name: 'main' }, async () => {
41+
// EDGE CASE: Import and instantiate Anthropic client BEFORE LangChain is imported
42+
// This simulates the timing issue where a user creates an Anthropic client in one file
43+
// before importing LangChain in another file
44+
const { default: Anthropic } = await import('@anthropic-ai/sdk');
45+
const anthropicClient = new Anthropic({
46+
apiKey: 'mock-api-key',
47+
baseURL,
48+
});
49+
50+
// Use the Anthropic client directly - this will be instrumented by the Anthropic integration
51+
await anthropicClient.messages.create({
52+
model: 'claude-3-5-sonnet-20241022',
53+
messages: [{ role: 'user', content: 'Direct Anthropic call' }],
54+
temperature: 0.7,
55+
max_tokens: 100,
56+
});
57+
58+
// NOW import LangChain - at this point it will mark Anthropic to be skipped
59+
// But the client created above is already instrumented
60+
const { ChatAnthropic } = await import('@langchain/anthropic');
61+
62+
// Create a LangChain model - this uses Anthropic under the hood
63+
const langchainModel = new ChatAnthropic({
64+
model: 'claude-3-5-sonnet-20241022',
65+
temperature: 0.7,
66+
maxTokens: 100,
67+
apiKey: 'mock-api-key',
68+
clientOptions: {
69+
baseURL,
70+
},
71+
});
72+
73+
// Use LangChain - this will be instrumented by LangChain integration
74+
await langchainModel.invoke('LangChain Anthropic call');
75+
76+
// Create ANOTHER Anthropic client after LangChain was imported
77+
// This one should NOT be instrumented (skip mechanism works correctly)
78+
const anthropicClient2 = new Anthropic({
79+
apiKey: 'mock-api-key',
80+
baseURL,
81+
});
82+
83+
await anthropicClient2.messages.create({
84+
model: 'claude-3-5-sonnet-20241022',
85+
messages: [{ role: 'user', content: 'Second direct Anthropic call' }],
86+
temperature: 0.7,
87+
max_tokens: 100,
88+
});
89+
});
90+
91+
await Sentry.flush(2000);
92+
server.close();
93+
}
94+
95+
run();

dev-packages/node-integration-tests/suites/tracing/langchain/test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,4 +245,64 @@ describe('LangChain integration', () => {
245245
});
246246
},
247247
);
248+
249+
createEsmAndCjsTests(
250+
__dirname,
251+
'scenario-openai-before-langchain.mjs',
252+
'instrument.mjs',
253+
(createRunner, test) => {
254+
test('demonstrates timing issue with duplicate spans (ESM only)', async () => {
255+
await createRunner()
256+
.ignore('event')
257+
.expect({
258+
transaction: event => {
259+
// This test highlights the limitation: if a user creates an Anthropic client
260+
// before importing LangChain, that client will still be instrumented and
261+
// could cause duplicate spans when used alongside LangChain.
262+
263+
const spans = event.spans || [];
264+
265+
// First call: Direct Anthropic call made BEFORE LangChain import
266+
// This should have Anthropic instrumentation (origin: 'auto.ai.anthropic')
267+
const firstAnthropicSpan = spans.find(
268+
span =>
269+
span.description === 'messages claude-3-5-sonnet-20241022' && span.origin === 'auto.ai.anthropic',
270+
);
271+
272+
// Second call: LangChain call
273+
// This should have LangChain instrumentation (origin: 'auto.ai.langchain')
274+
const langchainSpan = spans.find(
275+
span => span.description === 'chat claude-3-5-sonnet-20241022' && span.origin === 'auto.ai.langchain',
276+
);
277+
278+
// Third call: Direct Anthropic call made AFTER LangChain import
279+
// This should NOT have Anthropic instrumentation (skip works correctly)
280+
// Count how many Anthropic spans we have - should be exactly 1
281+
const anthropicSpans = spans.filter(
282+
span =>
283+
span.description === 'messages claude-3-5-sonnet-20241022' && span.origin === 'auto.ai.anthropic',
284+
);
285+
286+
// Verify the edge case limitation:
287+
// - First Anthropic client (created before LangChain) IS instrumented
288+
expect(firstAnthropicSpan).toBeDefined();
289+
expect(firstAnthropicSpan?.origin).toBe('auto.ai.anthropic');
290+
291+
// - LangChain call IS instrumented by LangChain
292+
expect(langchainSpan).toBeDefined();
293+
expect(langchainSpan?.origin).toBe('auto.ai.langchain');
294+
295+
// - Second Anthropic client (created after LangChain) is NOT instrumented
296+
// This demonstrates that the skip mechanism works for NEW clients
297+
// We should only have ONE Anthropic span (the first one), not two
298+
expect(anthropicSpans).toHaveLength(1);
299+
},
300+
})
301+
.start()
302+
.completed();
303+
});
304+
},
305+
// This test fails on CJS because we use dynamic imports to simulate importing LangChain after the Anthropic client is created
306+
{ failsOnCjs: true },
307+
);
248308
});

packages/core/src/index.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,12 @@ export { initAndBind, setCurrentClient } from './sdk';
5555
export { createTransport } from './transports/base';
5656
export { makeOfflineTransport } from './transports/offline';
5757
export { makeMultiplexedTransport } from './transports/multiplexed';
58+
export { getIntegrationsToSetup, addIntegration, defineIntegration, installedIntegrations } from './integration';
5859
export {
59-
getIntegrationsToSetup,
60-
addIntegration,
61-
defineIntegration,
62-
installedIntegrations,
63-
_markIntegrationsDisabled,
64-
_isIntegrationMarkedDisabled,
65-
_clearDisabledIntegrationsMarks,
66-
} from './integration';
60+
_INTERNAL_skipAiProviderWrapping,
61+
_INTERNAL_shouldSkipAiProviderWrapping,
62+
_INTERNAL_clearAiProviderSkips,
63+
} from './utils/ai/providerSkip';
6764
export { applyScopeDataToEvent, mergeScopeData } from './utils/applyScopeDataToEvent';
6865
export { prepareEvent } from './utils/prepareEvent';
6966
export { createCheckInEnvelope } from './checkin';

packages/core/src/integration.ts

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -8,58 +8,6 @@ import { debug } from './utils/debug-logger';
88

99
export const installedIntegrations: string[] = [];
1010

11-
/**
12-
* Registry to track integrations marked as disabled.
13-
* This is used to prevent duplicate instrumentation when higher-level integrations
14-
* (like LangChain) already instrument the underlying libraries (like OpenAI, Anthropic, etc.)
15-
*/
16-
const MARKED_DISABLED_INTEGRATIONS = new Set<string>();
17-
18-
/**
19-
* Mark one or more integrations as disabled to prevent their instrumentation from being set up.
20-
* This should be called during an integration's setupOnce() phase.
21-
* The marked integrations will be skipped when their own setupOnce() is called.
22-
*
23-
* @internal This is an internal API for coordination between integrations, not for public use.
24-
* @param integrationName The name(s) of the integration(s) to mark as disabled
25-
*/
26-
export function _markIntegrationsDisabled(integrationName: string | string[]): void {
27-
const names = Array.isArray(integrationName) ? integrationName : [integrationName];
28-
names.forEach(name => {
29-
MARKED_DISABLED_INTEGRATIONS.add(name);
30-
DEBUG_BUILD && debug.log(`Integration marked as disabled: ${name}`);
31-
});
32-
}
33-
34-
/**
35-
* Check if an integration has been marked as disabled.
36-
*
37-
* @internal This is an internal API for coordination between integrations, not for public use.
38-
* @param integrationName The name of the integration to check
39-
* @returns true if the integration is marked as disabled
40-
*/
41-
export function _isIntegrationMarkedDisabled(integrationName: string): boolean {
42-
return MARKED_DISABLED_INTEGRATIONS.has(integrationName);
43-
}
44-
45-
/**
46-
* Clear all integration marks and remove marked integrations from the installed list.
47-
* This is automatically called at the start of Sentry.init() to ensure a clean state
48-
* between different client initializations.
49-
*
50-
* This also removes the marked integrations from the global installedIntegrations list,
51-
* allowing them to run setupOnce() again if they're included in a new client.
52-
*
53-
* @internal This is an internal API for coordination between integrations, not for public use.
54-
*/
55-
export function _clearDisabledIntegrationsMarks(): void {
56-
// Remove marked integrations from the installed list so they can setup again
57-
const filtered = installedIntegrations.filter(integration => !MARKED_DISABLED_INTEGRATIONS.has(integration));
58-
installedIntegrations.splice(0, installedIntegrations.length, ...filtered);
59-
60-
MARKED_DISABLED_INTEGRATIONS.clear();
61-
}
62-
6311
/** Map of integrations assigned to a client */
6412
export type IntegrationIndex = {
6513
[key: string]: Integration;
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import { DEBUG_BUILD } from '../../debug-build';
2+
import { debug } from '../debug-logger';
3+
4+
/**
5+
* Registry tracking which AI provider modules should skip instrumentation wrapping.
6+
*
7+
* This prevents duplicate spans when a higher-level integration (like LangChain)
8+
* already instruments AI providers at a higher abstraction level.
9+
*/
10+
const SKIPPED_AI_PROVIDERS = new Set<string>();
11+
12+
/**
13+
* Mark AI provider modules to skip instrumentation wrapping.
14+
*
15+
* This prevents duplicate spans when a higher-level integration (like LangChain)
16+
* already instruments AI providers at a higher abstraction level.
17+
*
18+
* @internal
19+
* @param modules - Array of npm module names to skip (e.g., '@anthropic-ai/sdk', 'openai')
20+
*
21+
* @example
22+
* ```typescript
23+
* // In LangChain integration
24+
* _INTERNAL_skipAiProviderWrapping(['@anthropic-ai/sdk', 'openai', '@google/generative-ai']);
25+
* ```
26+
*/
27+
export function _INTERNAL_skipAiProviderWrapping(modules: string[]): void {
28+
modules.forEach(module => {
29+
SKIPPED_AI_PROVIDERS.add(module);
30+
DEBUG_BUILD && debug.log(`AI provider "${module}" wrapping will be skipped`);
31+
});
32+
}
33+
34+
/**
35+
* Check if an AI provider module should skip instrumentation wrapping.
36+
*
37+
* @internal
38+
* @param module - The npm module name (e.g., '@anthropic-ai/sdk', 'openai')
39+
* @returns true if wrapping should be skipped
40+
*
41+
* @example
42+
* ```typescript
43+
* // In AI provider instrumentation
44+
* if (_INTERNAL_shouldSkipAiProviderWrapping('@anthropic-ai/sdk')) {
45+
* return Reflect.construct(Original, args); // Don't instrument
46+
* }
47+
* ```
48+
*/
49+
export function _INTERNAL_shouldSkipAiProviderWrapping(module: string): boolean {
50+
return SKIPPED_AI_PROVIDERS.has(module);
51+
}
52+
53+
/**
54+
* Clear all AI provider skip registrations.
55+
*
56+
* This is automatically called at the start of Sentry.init() to ensure a clean state
57+
* between different client initializations.
58+
*
59+
* @internal
60+
*/
61+
export function _INTERNAL_clearAiProviderSkips(): void {
62+
SKIPPED_AI_PROVIDERS.clear();
63+
DEBUG_BUILD && debug.log('Cleared AI provider skip registrations');
64+
}

packages/core/test/lib/integration.test.ts

Lines changed: 1 addition & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,6 @@
11
import { afterEach, beforeEach, describe, expect, it, test, vi } from 'vitest';
22
import { getCurrentScope } from '../../src/currentScopes';
3-
import {
4-
_clearDisabledIntegrationsMarks,
5-
_isIntegrationMarkedDisabled,
6-
_markIntegrationsDisabled,
7-
addIntegration,
8-
getIntegrationsToSetup,
9-
installedIntegrations,
10-
setupIntegration,
11-
} from '../../src/integration';
3+
import { addIntegration, getIntegrationsToSetup, installedIntegrations, setupIntegration } from '../../src/integration';
124
import { setCurrentClient } from '../../src/sdk';
135
import type { Integration } from '../../src/types-hoist/integration';
146
import type { CoreOptions } from '../../src/types-hoist/options';
@@ -691,60 +683,3 @@ describe('addIntegration', () => {
691683
expect(logs).toHaveBeenCalledWith('Integration skipped because it was already installed: test');
692684
});
693685
});
694-
695-
describe('Integration marking system', () => {
696-
beforeEach(() => {
697-
_clearDisabledIntegrationsMarks();
698-
installedIntegrations.splice(0, installedIntegrations.length);
699-
});
700-
701-
afterEach(() => {
702-
_clearDisabledIntegrationsMarks();
703-
});
704-
705-
it('marks and checks single integration', () => {
706-
expect(_isIntegrationMarkedDisabled('TestIntegration')).toBe(false);
707-
708-
_markIntegrationsDisabled('TestIntegration');
709-
710-
expect(_isIntegrationMarkedDisabled('TestIntegration')).toBe(true);
711-
});
712-
713-
it('marks and checks multiple integrations', () => {
714-
_markIntegrationsDisabled(['OpenAI', 'Anthropic', 'GoogleGenAI']);
715-
716-
expect(_isIntegrationMarkedDisabled('OpenAI')).toBe(true);
717-
expect(_isIntegrationMarkedDisabled('Anthropic')).toBe(true);
718-
expect(_isIntegrationMarkedDisabled('GoogleGenAI')).toBe(true);
719-
expect(_isIntegrationMarkedDisabled('Other')).toBe(false);
720-
});
721-
722-
it('clears marks and removes from installedIntegrations', () => {
723-
// Simulate scenario: integrations installed, some marked for cleanup
724-
installedIntegrations.push('LangChain', 'OpenAI', 'Anthropic', 'Normal');
725-
_markIntegrationsDisabled(['OpenAI', 'Anthropic']);
726-
727-
_clearDisabledIntegrationsMarks();
728-
729-
// Marks are cleared
730-
expect(_isIntegrationMarkedDisabled('OpenAI')).toBe(false);
731-
expect(_isIntegrationMarkedDisabled('Anthropic')).toBe(false);
732-
733-
// Marked integrations removed from installed list (can setup again in new client)
734-
expect(installedIntegrations).toEqual(['LangChain', 'Normal']);
735-
});
736-
737-
it('handles multi-client scenario correctly', () => {
738-
// First client with LangChain
739-
installedIntegrations.push('LangChain', 'OpenAI');
740-
_markIntegrationsDisabled('OpenAI');
741-
expect(_isIntegrationMarkedDisabled('OpenAI')).toBe(true);
742-
743-
// Second client initialization clears marks
744-
_clearDisabledIntegrationsMarks();
745-
746-
// OpenAI can be used standalone in second client
747-
expect(_isIntegrationMarkedDisabled('OpenAI')).toBe(false);
748-
expect(installedIntegrations).toEqual(['LangChain']); // OpenAI removed, can setup fresh
749-
});
750-
});

0 commit comments

Comments
 (0)