Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// the worker entry named in wrangler's `main`) and imports its default export as
// the options callback for every wrapper it injects.
export default (env: Env) => ({
traceLifecycle: 'static' as const,
dsn: env.E2E_TEST_DSN,
environment: 'qa',
tunnel: 'http://localhost:3031/',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,31 @@
import { expect, test } from '@playwright/test';
import { waitForTransaction } from '@sentry-internal/test-utils';
import { collectStreamedSpans, getSpanOp, waitForStreamedSpan } from '@sentry-internal/test-utils';
import { callRpc } from './agent-socket';

// The worker entry (`src/index.ts`) contains no Sentry calls at all — every
// wrapper below was injected by the Vite auto-instrument plugin at build time.
// Any transaction arriving here therefore proves the injection happened.
// Any span arriving here therefore proves the injection happened.
//
// With span streaming, URL-sourced `http.server` spans are named by method only, so the
// `/plain-do` request segment is identified by its `url.path` attribute.

test('wraps the default export with withSentry (options from instrument.server.ts)', async ({ baseURL }) => {
const transactionPromise = waitForTransaction('cloudflare-autoinstrument', event => {
return event.contexts?.trace?.op === 'http.server' && (event.request?.url ?? '').includes('/plain-do');
});
const spanPromise = waitForStreamedSpan(
'cloudflare-autoinstrument',
span => getSpanOp(span) === 'http.server' && span.is_segment && span.attributes['url.path']?.value === '/plain-do',
);

const res = await fetch(`${baseURL}/plain-do`);
expect(res.status).toBe(200);
await expect(res.json()).resolves.toEqual({ durableObject: true });

const transaction = await transactionPromise;
const span = await spanPromise;

expect(transaction.contexts?.trace?.origin).toBe('auto.http.cloudflare');
expect(span.attributes['sentry.origin']?.value).toBe('auto.http.cloudflare');
// `environment: 'qa'` is only set in `instrument.server.ts`, so seeing it here
// proves the plugin sourced its options callback from that file rather than
// falling back to reading configuration off `env`.
expect(transaction.environment).toBe('qa');
expect(span.attributes['sentry.environment']?.value).toBe('qa');
});

// Each of these three classes is registered in wrangler.jsonc exactly like the
Expand Down Expand Up @@ -49,50 +53,57 @@ for (const { title, binding, agentClass } of [
test(`applies agent instrumentation to ${title}`, async ({ baseURL }) => {
const instance = `${binding}-instance`;

const transactionPromise = waitForTransaction('cloudflare-autoinstrument', event => {
return (
event.transaction === 'webSocketMessage' &&
(event.spans ?? []).some(span => span.op === 'rpc' && span.description === 'greet')
);
});
// The rpc span is a child of the `webSocketMessage` segment span, which ends after it and is
// streamed in a later envelope, so collect until both have arrived.
const spansPromise = collectStreamedSpans(
'cloudflare-autoinstrument',
spans =>
spans.some(
span =>
getSpanOp(span) === 'rpc' &&
span.name === 'greet' &&
Comment on lines +60 to +64

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The waitForStreamedSpan test utility lacks trace isolation, which can cause race conditions and flaky results when tests are run in parallel.
Severity: MEDIUM

Suggested Fix

Modify waitForStreamedSpan to implement trace isolation. It should group spans by trace_id before evaluating them, similar to how collectStreamedSpans is implemented. This will ensure that the function only resolves with a span that belongs to the same trace as the action that triggered it, preventing cross-test contamination during parallel execution.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location:
dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/tests/autoinstrument.test.ts#L60-L64

Potential issue: The `waitForStreamedSpan` test utility function lacks trace isolation.
It filters incoming spans by a timestamp but not by `trace_id`. When tests are run in
parallel, as configured with `workers: '100%'`, a race condition can occur. If two tests
concurrently make requests that generate similar spans, the test calling
`waitForStreamedSpan` may incorrectly capture and validate a span generated by the other
test. This can lead to flaky tests that pass or fail based on timing and not because
their own specific action was correctly instrumented.

String(span.attributes['gen_ai.agent.name']?.value ?? '').includes(agentClass),
) && spans.some(span => span.is_segment && span.name === 'webSocketMessage'),
);

// Each agent's greet() returns a string naming its class, so the reply
// identifies exactly which class handled the call.
const reply = await callRpc(baseURL!, { binding, instance, method: 'greet', args: ['World'] });
expect(reply).toBe(`Hello, World! (from ${agentClass})`);

const transaction = await transactionPromise;
const rpcSpan = (transaction.spans ?? []).find(span => span.op === 'rpc' && span.description === 'greet');
const spans = await spansPromise;
const rpcSpan = spans.find(
Comment thread
sentry[bot] marked this conversation as resolved.
span =>
getSpanOp(span) === 'rpc' &&
span.name === 'greet' &&
String(span.attributes['gen_ai.agent.name']?.value ?? '').includes(agentClass),
)!;

expect(rpcSpan).toEqual(
expect.objectContaining({
op: 'rpc',
description: 'greet',
origin: 'auto.faas.cloudflare.agents',
data: expect.objectContaining({
// Read back off the instance at runtime (`_ParentClass.name`), so it
// confirms the wrapper landed on the user's real class. Matched loosely
// because the transform renames the class it wraps to
// `__SENTRY_ORIGINAL_<name>__` and the bundler infers that name.
'gen_ai.agent.name': expect.stringContaining(agentClass),
}),
}),
);
expect(rpcSpan.attributes['sentry.op']?.value).toBe('rpc');
expect(rpcSpan.attributes['sentry.origin']?.value).toBe('auto.faas.cloudflare.agents');
// Read back off the instance at runtime (`_ParentClass.name`), so it
// confirms the wrapper landed on the user's real class. Matched loosely
// because the transform renames the class it wraps to
// `__SENTRY_ORIGINAL_<name>__` and the bundler infers that name.
expect(rpcSpan.attributes['gen_ai.agent.name']?.value).toContain(agentClass);
});
}

test('applies plain Durable Object instrumentation to a non-Agent class', async ({ baseURL }) => {
const transactionPromise = waitForTransaction('cloudflare-autoinstrument', event => {
return event.contexts?.trace?.op === 'http.server' && (event.request?.url ?? '').includes('/plain-do');
});
const spansPromise = collectStreamedSpans('cloudflare-autoinstrument', spans =>
spans.some(
span =>
getSpanOp(span) === 'http.server' && span.is_segment && span.attributes['url.path']?.value === '/plain-do',
),
);

const res = await fetch(`${baseURL}/plain-do`);
expect(res.status).toBe(200);

const transaction = await transactionPromise;
const spans = await spansPromise;

// A plain Durable Object must NOT pick up agent instrumentation: detection has
// to discriminate, not blanket-upgrade every `durable_objects` binding.
const agentSpans = (transaction.spans ?? []).filter(span => span.origin === 'auto.faas.cloudflare.agents');
const agentSpans = spans.filter(span => span.attributes['sentry.origin']?.value === 'auto.faas.cloudflare.agents');
expect(agentSpans).toEqual([]);
});
Loading