Skip to content

Commit e65ed79

Browse files
RulaKhaledclaude
andauthored
test(e2e): Address review on the node-flue app
- Drop the explicit `dataloaderIntegration()`; it is in `getTracingIntegrations()` now, so Node registers it by default when spans are enabled. - Always run under orchestrion rather than keeping it as a variant, since that is how the SDK is meant to be set up. Removes the `*:orchestrion` scripts, the `USE_ORCHESTRION` plumbing and the `test.fail()` branch in the dataloader test, which now simply asserts the span lands under `execute_tool`. - Drop the node-eve reference from `vite.config.ts`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 1eb2e97 commit e65ed79

5 files changed

Lines changed: 16 additions & 51 deletions

File tree

‎dev-packages/e2e-tests/test-applications/node-flue/package.json‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,13 @@
44
"private": true,
55
"type": "module",
66
"scripts": {
7-
"dev": "vite dev --port 3030",
7+
"dev": "NODE_OPTIONS='--import=@sentry/node/import' vite dev --port 3030",
88
"build": "vite build",
9-
"start": "PORT=3030 node dist/server.mjs",
10-
"dev:orchestrion": "NODE_OPTIONS='--import=@sentry/node/import' pnpm dev",
11-
"start:orchestrion": "NODE_OPTIONS='--import=@sentry/node/import' pnpm start",
9+
"start": "NODE_OPTIONS='--import=@sentry/node/import' PORT=3030 node dist/server.mjs",
1210
"clean": "npx rimraf node_modules dist pnpm-lock.yaml",
1311
"test:build": "pnpm install && pnpm build",
14-
"test:build-orchestrion": "USE_ORCHESTRION=1 pnpm test:build",
1512
"test:build-latest": "pnpm install && pnpm add @flue/runtime@latest @flue/vite@latest @flue/cli@latest && pnpm build",
1613
"test:assert": "pnpm test:prod && pnpm test:dev",
17-
"test:assert-orchestrion": "USE_ORCHESTRION=1 pnpm test:assert",
1814
"test:prod": "OPENROUTER_API_KEY=$E2E_OPENROUTER_API_KEY TEST_ENV=production playwright test",
1915
"test:dev": "OPENROUTER_API_KEY=$E2E_OPENROUTER_API_KEY TEST_ENV=development playwright test"
2016
},
@@ -48,11 +44,6 @@
4844
{
4945
"build-command": "pnpm test:build-latest",
5046
"label": "node-flue (latest)"
51-
},
52-
{
53-
"build-command": "pnpm test:build-orchestrion",
54-
"assert-command": "pnpm test:assert-orchestrion",
55-
"label": "node-flue (orchestrion)"
5647
}
5748
]
5849
}

‎dev-packages/e2e-tests/test-applications/node-flue/playwright.config.mjs‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,15 @@
11
import { getPlaywrightConfig } from '@sentry-internal/test-utils';
22

33
const testEnv = process.env.TEST_ENV;
4-
const useOrchestrion = process.env.USE_ORCHESTRION === '1';
54

65
if (!testEnv) {
76
throw new Error('No test env defined');
87
}
98

10-
let startCommand = testEnv === 'development' ? 'pnpm dev' : 'pnpm start';
11-
12-
if (useOrchestrion) {
13-
startCommand = `${startCommand}:orchestrion`;
14-
}
15-
169
const config = getPlaywrightConfig(
17-
{ startCommand },
18-
// Each agent turn is a real OpenRouter tool-calling round trip (two model calls) followed by a
19-
// span flush, which does not fit the default 30s timeout when the provider is slow.
10+
{ startCommand: testEnv === 'development' ? 'pnpm dev' : 'pnpm start' },
11+
// Each test drives a real OpenRouter tool-calling turn and then waits for the spans to flush,
12+
// which does not fit the default 30s timeout when the provider is slow.
2013
{ timeout: 90_000 },
2114
);
2215

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,13 @@
11
import { instrument } from '@flue/runtime';
22
import * as Sentry from '@sentry/node';
33

4-
// Imported for its side effects as the first line of `src/app.ts`, which is how a Flue app is
5-
// expected to set Sentry up: there is no framework-owned instrumentation hook to auto-discover.
4+
// Imported for its side effects as the first line of `src/app.ts`, which is how a Flue app sets
5+
// Sentry up: there is no framework-owned instrumentation hook to auto-discover.
66
Sentry.init({
77
environment: 'qa',
88
dsn: process.env.E2E_TEST_DSN,
99
tunnel: 'http://localhost:3031/', // proxy server
1010
tracesSampleRate: 1.0,
11-
// Not a default integration. It only produces spans in the "orchestrion" test variant, where the
12-
// server starts with `NODE_OPTIONS=--import=@sentry/node/import` so the module transform is
13-
// registered before `dataloader` is loaded.
14-
integrations: [Sentry.dataloaderIntegration()],
1511
});
1612

1713
instrument(Sentry.createFlueInstrumentation());

‎dev-packages/e2e-tests/test-applications/node-flue/tests/dataloader.test.ts‎

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,44 +3,29 @@ import { collectStreamedSpans, getSpanOp } from '@sentry-internal/test-utils';
33
import { runAgentTurn } from './utils';
44

55
const APP = 'node-flue';
6-
const useOrchestrion = process.env.USE_ORCHESTRION === '1';
76

87
const isDataloaderSpan = (span: { attributes?: Record<string, { value?: unknown }> }): boolean =>
98
span.attributes?.['sentry.origin']?.value === 'auto.db.dataloader';
109

1110
/**
12-
* `dataloaderIntegration` relies on orchestrion, a module transform, so it only emits spans when the
13-
* server starts with `NODE_OPTIONS=--import=@sentry/node/import` (the `node-flue (orchestrion)`
14-
* variant). The integration installs and subscribes either way, so the absence of a span is the only
15-
* thing that distinguishes the two — hence `test.fail(!useOrchestrion)`.
16-
*
17-
* Flue needs no build configuration for this: a Flue node build leaves dependencies as bare
18-
* specifiers, so `dataloader` stays a real module the transform can hook. If Flue ever switches to
19-
* a bundled server output, this test is what catches it.
11+
* `dataloader` is instrumented through orchestrion, a module transform, so it only produces spans
12+
* with the loader registered at process start. A Flue node build needs no externals config for
13+
* that: dependencies stay bare specifiers, so `dataloader` is still a real module to hook. If Flue
14+
* ever switches to a bundled server output, this is what catches it.
2015
*
2116
* The loader is called from inside a tool so its span lands in the agent's trace, beside the AI
2217
* spans, rather than in a trace of its own.
2318
*/
2419
test('captures orchestrion-instrumented dataloader spans in the same trace as the AI spans', async ({ baseURL }) => {
25-
test.fail(!useOrchestrion, 'orchestrion module instrumentation needs NODE_OPTIONS=--import=@sentry/node/import');
26-
27-
// With orchestrion, wait for the dataloader span itself. Without it that span never arrives, so
28-
// anchor on the always-present tool span and let the assertion below fail fast rather than time
29-
// the test out.
30-
const spansPromise = collectStreamedSpans(APP, spansOfTrace =>
31-
useOrchestrion
32-
? spansOfTrace.some(isDataloaderSpan)
33-
: spansOfTrace.some(span => getSpanOp(span) === 'gen_ai.execute_tool'),
34-
);
20+
const spansPromise = collectStreamedSpans(APP, spansOfTrace => spansOfTrace.some(isDataloaderSpan));
3521

3622
await runAgentTurn(baseURL!, 'dataloader-conversation', 'Please call count_items to count the items.');
3723

3824
const spans = await spansPromise;
3925
const executeTool = spans.find(span => span.attributes?.['gen_ai.tool.name']?.value === 'count_items');
4026
const dataloaderSpan = spans.find(isDataloaderSpan);
4127

42-
expect(dataloaderSpan?.attributes?.['sentry.origin']?.value).toBe('auto.db.dataloader');
43-
// Same trace as the AI spans, and underneath the tool that triggered it.
28+
expect(getSpanOp(dataloaderSpan!)).toBe('cache.get');
4429
expect(dataloaderSpan?.trace_id).toBe(executeTool?.trace_id);
4530
expect(dataloaderSpan?.parent_span_id).toBe(executeTool?.span_id);
4631
});
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { flue } from '@flue/vite';
22
import { defineConfig } from 'vite';
33

4-
// Unmodified from what `flue init` scaffolds. In particular there is no externals config: a Flue
5-
// node build leaves dependencies as bare specifiers, so orchestrion's module transform still sees
6-
// them as real modules. (eve needs `externalDependencies` because it emits a bundled server.)
4+
// Unmodified from what `flue init` scaffolds. No externals config is needed: a Flue node build
5+
// already leaves dependencies as bare specifiers, so orchestrion's module transform still sees them
6+
// as real modules.
77
export default defineConfig({
88
plugins: [flue()],
99
});

0 commit comments

Comments
 (0)