Skip to content

fix(deno): Avoid inferring invalid span op from Deno tracer #20128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Lms24 wants to merge 1 commit into
base: develop
Choose a base branch
from
+5 −8
Open

fix(deno): Avoid inferring invalid span op from Deno tracer #20128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion dev-packages/e2e-tests/test-applications/deno/tests/ai.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ test('should create AI pipeline spans with Vercel AI SDK', async ({ baseURL }) =
(span: any) =>
span.op === 'gen_ai.invoke_agent' ||
span.op === 'gen_ai.generate_content' ||
span.op === 'otel.span' ||
!span.op ||
Copy link
Copy Markdown

@cursor cursor bot Apr 9, 2026

Choose a reason for hiding this comment

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

Overly broad filter makes AI test trivially pass

Medium Severity

The !span.op condition in the AI span filter is far too broad — it matches any span without an op, not just AI-related OTel spans. The previous condition span.op === 'otel.span' was specific to spans coming from the custom OTel tracer, but !span.op will match any span in the transaction that happens to lack an op. Combined with the assertion expect(aiSpans.length).toBeGreaterThanOrEqual(1), this means the test can trivially pass even if no AI spans are actually created.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 00ff5b5. Configure here.

span.description?.includes('ai.generateText'),
);

Expand Down
3 changes: 0 additions & 3 deletions dev-packages/e2e-tests/test-applications/deno/tests/transactions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ test('Sends transaction with OTel tracer.startSpan despite pre-existing provider
expect.arrayContaining([
expect.objectContaining({
description: 'test-otel-span',
op: 'otel.span',
Copy link
Copy Markdown

@cursor cursor bot Apr 9, 2026

Choose a reason for hiding this comment

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

Tests don't assert absence of op field

Low Severity

The expect.objectContaining assertions were updated to remove the op: 'otel.span' check, but no assertion was added to verify that op is actually absent or undefined. The whole point of this fix is that op is no longer set for unknown span kinds, but the tests would still pass even if the old op: 'otel.span' value came back. This is flagged because the review rules require flagging relaxed expect.objectContaining assertions where something is expected NOT to be included but isn't asserted.

Additional Locations (2)
Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

Reviewed by Cursor Bugbot for commit 00ff5b5. Configure here.

origin: 'manual',
}),
]),
Expand All @@ -53,7 +52,6 @@ test('Sends transaction with OTel tracer.startActiveSpan', async ({ baseURL }) =
expect.arrayContaining([
expect.objectContaining({
description: 'test-otel-active-span',
op: 'otel.span',
origin: 'manual',
}),
]),
Expand All @@ -77,7 +75,6 @@ test('OTel span appears as child of Sentry span (interop)', async ({ baseURL })
}),
expect.objectContaining({
description: 'otel-child',
op: 'otel.span',
origin: 'manual',
}),
]),
Expand Down
8 changes: 4 additions & 4 deletions packages/deno/src/opentelemetry/tracer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class SentryDenoTracer implements Tracer {
attributes: {
...options?.attributes,
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'manual',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
...(op ? { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: op } : {}),
'sentry.deno_tracer': true,
},
});
Expand Down Expand Up @@ -77,7 +77,7 @@ class SentryDenoTracer implements Tracer {
attributes: {
...opts.attributes,
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'manual',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
...(op ? { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: op } : {}),
'sentry.deno_tracer': true,
},
};
Expand All @@ -96,7 +96,7 @@ class SentryDenoTracer implements Tracer {
return startSpanManual(spanOpts, callback) as ReturnType<F>;
}

private _mapSpanKindToOp(kind?: SpanKind): string {
private _mapSpanKindToOp(kind?: SpanKind): string | undefined {
switch (kind) {
case SpanKind.CLIENT:
return 'http.client';
Expand All @@ -107,7 +107,7 @@ class SentryDenoTracer implements Tracer {
case SpanKind.CONSUMER:
return 'message.consume';
default:
return 'otel.span';
return undefined;
}
}
}
Loading