Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions dev-packages/e2e-tests/test-applications/node-eve/agent/agent.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createOpenRouter } from '@openrouter/ai-sdk-provider';
import { getInstrumentedModuleNames } from '@sentry/node';
import { defineAgent } from 'eve';

// We call OpenRouter directly (rather than the default Vercel AI Gateway) so the
Expand All @@ -19,17 +20,18 @@ export default defineAgent({
// Only configure externals for orchestrion mode, to ensure everything else works without it
...(useOrchestrion
? {
// `dataloader` is instrumented by Sentry via orchestrion (a module
// transform). Keep it external so it stays a real module the transform can
// hook; if eve inlined it into the server bundle it could never be
// instrumented. (The Vercel AI SDK needs none of this — it uses a native
// diagnostics channel.)
// Keep every package Sentry instruments via orchestrion (a module transform) external, so
// it stays a real module the transform can hook rather than being inlined into eve's server
// bundle (an inlined module never reaches the transform's `onLoad`). Rather than hardcode
// the set, ask the SDK for it — this app exercises `dataloader`, and the rest are no-ops
// when the app doesn't use them. (The Vercel AI SDK needs none of this; it uses a native
// diagnostics channel and `ai` v7 is registration-only under the transform.)
//
// Do NOT add `@sentry/server-runtime-injection` here: the `--import`
// loader instruments regardless (so the "bundled ... uninstrumented"
// warning is a false positive), and externalizing it makes eve's dev
// host fail to resolve its `/register` subpath (`eve dev` only).
externalDependencies: ['dataloader'],
externalDependencies: getInstrumentedModuleNames(),
}
: {}),
},
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/index.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ export {
withStreamedSpan,
metrics,
eveConversationHook,
getInstrumentedModuleNames,
} from '@sentry/node';

export { init } from './server/sdk';
Expand Down
1 change: 1 addition & 0 deletions packages/aws-serverless/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ export {
// oxlint-disable-next-line typescript/no-deprecated
withStreamedSpan,
eveConversationHook,
getInstrumentedModuleNames,
} from '@sentry/node';

export {
Expand Down
1 change: 1 addition & 0 deletions packages/bun/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ export {
// oxlint-disable-next-line typescript/no-deprecated
withStreamedSpan,
eveConversationHook,
getInstrumentedModuleNames,
} from '@sentry/node';

export {
Expand Down
1 change: 1 addition & 0 deletions packages/cloudflare/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ export {
instrumentCreateReactAgent,
vercelAIIntegration,
eveConversationHook,
getInstrumentedModuleNames,
} from '@sentry/server-utils';

export { instrumentWorkflowWithSentry } from './workflows';
Expand Down
1 change: 1 addition & 0 deletions packages/deno/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ export {
postgresJsIntegration,
tediousIntegration,
eveConversationHook,
getInstrumentedModuleNames,
} from '@sentry/server-utils';
export { openTelemetryIntegration, getOtlpTracesEndpoint } from '@sentry/server-utils/no-diagnostic-channels';
// Deprecated aliases kept for back-compat. Each forwards to the shared
Expand Down
1 change: 1 addition & 0 deletions packages/google-cloud-serverless/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ export {
// oxlint-disable-next-line typescript/no-deprecated
withStreamedSpan,
eveConversationHook,
getInstrumentedModuleNames,
} from '@sentry/node';

export {
Expand Down
2 changes: 1 addition & 1 deletion packages/node/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ export { makeNodeTransport } from './transports';
export { createGetModuleFromFilename } from './utils/module';

export { SENTRY_SEGMENT_NAME_SOURCE } from '@sentry/conventions/attributes';
export { eveConversationHook } from '@sentry/server-utils';
export { eveConversationHook, getInstrumentedModuleNames } from '@sentry/server-utils';
export { httpServerIntegration } from './integrations/http/httpServerIntegration';
export { httpServerSpansIntegration } from './integrations/http/httpServerSpansIntegration';
export { processSessionIntegration } from './integrations/processSession';
Expand Down
1 change: 1 addition & 0 deletions packages/server-utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export type { InstrumentationConfig } from './orchestrion/apmTypes';
// helper with no orchestrion build-time dependency.
export { orchestrionModuleInjected } from './utils/moduleInjected';
export { eveConversationHook } from './eve';
export { getInstrumentedModuleNames } from './orchestrion/config';
export {
fastifyIntegration,
// oxlint-disable-next-line typescript/no-deprecated
Expand Down
15 changes: 15 additions & 0 deletions packages/server-utils/src/orchestrion/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,21 @@ export function instrumentedModuleNames(instrumentations: InstrumentationConfig[
/** The instrumented module names from the default Sentry config, with no custom additions. */
export const INSTRUMENTED_MODULE_NAMES: string[] = instrumentedModuleNames();

/**
* The package names the SDK instruments through the orchestrion module transform (its
* diagnostics-channel injection). Pass these to a server bundler's "keep external" option so the
* packages load through Node's module loader — the only path the transform can hook — instead of
* being inlined into the server bundle. A framework that has no Sentry bundler plugin (e.g. eve, via
* `build.externalDependencies`) is the main caller; a listed package the app doesn't use is simply
* ignored by the bundler.
*
* Unlike {@link INSTRUMENTED_MODULE_NAMES}, this is the plain instrumented set with no bundler-only
* additions — those force a helper package to be *bundled*, the opposite of keeping it external.
*/
export function getInstrumentedModuleNames(): string[] {
return uniq(SENTRY_INSTRUMENTATIONS.map(instrumentation => instrumentation.module.name));
}

/**
* Returns `external` with any instrumented packages removed, so a bundler that
* uses an "external" denylist (esbuild, Bun, Rollup) still bundles — and thus
Expand Down
27 changes: 27 additions & 0 deletions packages/server-utils/test/orchestrion/config.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { InstrumentationConfig } from '@apm-js-collab/code-transformer-bundler-plugins/core';
import { describe, expect, it } from 'vitest';
import {
getInstrumentedModuleNames,
INSTRUMENTED_MODULE_NAMES,
instrumentedModuleNames,
SENTRY_INSTRUMENTATIONS,
Expand All @@ -21,6 +22,32 @@ describe('orchestrion config — scoped @hapi/hapi module', () => {
});
});

describe('getInstrumentedModuleNames', () => {
it('returns the instrumented package names', () => {
const names = getInstrumentedModuleNames();

for (const name of ['dataloader', 'ai', 'express', 'pg', 'redis']) {
expect(names).toContain(name);
}
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test loop covers multiple cases

Low Severity

This test loops over several package names and asserts each one is present. The review guidelines ask for (it)|(test).each instead of a loop that covers multiple scenarios in one test, so a single missing name does not hide the rest. Flagged because that testing convention is in the review rules file.

Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

Reviewed by Cursor Bugbot for commit 1036b89. Configure here.


it('has no duplicates', () => {
const names = getInstrumentedModuleNames();

expect(names.length).toBe(new Set(names).size);
});

it('is the plain instrumented set, without the bundler-only additions in INSTRUMENTED_MODULE_NAMES', () => {
// `INSTRUMENTED_MODULE_NAMES` adds packages that must be force-bundled (e.g. `@remix-run/node`),
// which is the opposite of what a "keep external" caller wants.
expect(getInstrumentedModuleNames()).not.toContain('@remix-run/node');
expect(INSTRUMENTED_MODULE_NAMES).toContain('@remix-run/node');
expect(new Set(getInstrumentedModuleNames())).toEqual(
new Set(SENTRY_INSTRUMENTATIONS.map(instrumentation => instrumentation.module.name)),
);
});
});

describe('orchestrion config — channel-subscriber coverage', () => {
// The subscribe injection rides the real channel configs (the `tracingChannelImport`
// override only runs on instrumented files), so a subscriber definition whose module is
Expand Down
Loading