Skip to content

Commit 00d4e40

Browse files
nicohrubecclaude
andcommitted
ref(node): Streamline koa
Migrate koa span creation to the @sentry/core API, fold the op/origin/name/transaction-name requestHook into span creation, and remove the vendored files' eslint-disable + no-explicit-any exemption. Extend the node-koa e2e for RegExp/nested routers, middleware dedup, and errored-span status, and port the isLayerIgnored unit test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 45d9c0a commit 00d4e40

10 files changed

Lines changed: 215 additions & 172 deletions

File tree

.oxlintrc.base.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,6 @@
145145
"**/integrations/fs/vendored/**/*.ts",
146146
"**/integrations/tracing/knex/vendored/**/*.ts",
147147
"**/integrations/tracing/mongo/vendored/**/*.ts",
148-
"**/integrations/tracing/koa/vendored/**/*.ts",
149148
"**/integrations/tracing/mysql2/vendored/**/*.ts",
150149
"**/integration/aws/vendored/**/*.ts",
151150
"**/integrations/tracing/kafka/vendored/**/*.ts",

dev-packages/e2e-tests/test-applications/node-koa/index.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,30 @@ router1.post('/test-post', async ctx => {
115115
ctx.body = { status: 'ok', body: ctx.request.body };
116116
});
117117

118+
// RegExp route - exercises the @koa/router dispatch patch with a non-string layer path.
119+
router1.get(/^\/test-regexp/, ctx => {
120+
ctx.body = { matched: 'regexp' };
121+
});
122+
123+
// Same middleware instance passed twice - the second occurrence must be skipped (kLayerPatched dedup).
124+
const sharedRouteMiddleware = async (ctx, next) => {
125+
await next();
126+
};
127+
router1.get('/test-dedup', sharedRouteMiddleware, sharedRouteMiddleware, ctx => {
128+
ctx.body = { ok: true };
129+
});
130+
118131
app1.use(router1.routes()).use(router1.allowedMethods());
119132

133+
// Nested router - the routed span's http.route is the composed parent + child path.
134+
const nestedRouter = new Router();
135+
nestedRouter.get('/details/:id', ctx => {
136+
ctx.body = { id: ctx.params.id };
137+
});
138+
const outerRouter = new Router();
139+
outerRouter.use('/:first', nestedRouter.routes());
140+
app1.use(outerRouter.routes()).use(outerRouter.allowedMethods());
141+
120142
app1.listen(port1);
121143

122144
const app2 = new Koa();
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
import { expect, test } from '@playwright/test';
2+
import { waitForTransaction } from '@sentry-internal/test-utils';
3+
4+
test('instruments RegExp router routes', async ({ baseURL }) => {
5+
const transactionPromise = waitForTransaction('node-koa', transactionEvent => {
6+
return (
7+
transactionEvent?.contexts?.trace?.op === 'http.server' && !!transactionEvent.transaction?.includes('test-regexp')
8+
);
9+
});
10+
11+
await fetch(`${baseURL}/test-regexp`);
12+
13+
const transactionEvent = await transactionPromise;
14+
15+
expect(transactionEvent.spans).toEqual(
16+
expect.arrayContaining([
17+
expect.objectContaining({
18+
op: 'router.koa',
19+
origin: 'auto.http.otel.koa',
20+
data: expect.objectContaining({
21+
'koa.type': 'router',
22+
'sentry.op': 'router.koa',
23+
'sentry.origin': 'auto.http.otel.koa',
24+
'http.route': '/^\\/test-regexp/',
25+
}),
26+
}),
27+
]),
28+
);
29+
});
30+
31+
test('instruments nested routers with the composed http.route', async ({ baseURL }) => {
32+
const transactionPromise = waitForTransaction('node-koa', transactionEvent => {
33+
return (
34+
transactionEvent?.contexts?.trace?.op === 'http.server' &&
35+
transactionEvent.transaction === 'GET /:first/details/:id'
36+
);
37+
});
38+
39+
await fetch(`${baseURL}/shop/details/1`);
40+
41+
const transactionEvent = await transactionPromise;
42+
43+
expect(transactionEvent.spans).toEqual(
44+
expect.arrayContaining([
45+
expect.objectContaining({
46+
op: 'router.koa',
47+
description: '/:first/details/:id',
48+
data: expect.objectContaining({
49+
'koa.type': 'router',
50+
'http.route': '/:first/details/:id',
51+
'sentry.op': 'router.koa',
52+
'sentry.origin': 'auto.http.otel.koa',
53+
}),
54+
}),
55+
]),
56+
);
57+
});
58+
59+
test('does not instrument the same middleware twice', async ({ baseURL }) => {
60+
const transactionPromise = waitForTransaction('node-koa', transactionEvent => {
61+
return (
62+
transactionEvent?.contexts?.trace?.op === 'http.server' && transactionEvent.transaction === 'GET /test-dedup'
63+
);
64+
});
65+
66+
await fetch(`${baseURL}/test-dedup`);
67+
68+
const transactionEvent = await transactionPromise;
69+
70+
// The route stack is [sharedRouteMiddleware, sharedRouteMiddleware, handler]; the repeated
71+
// middleware instance is skipped, leaving one span for it plus the handler span.
72+
const dedupSpans = transactionEvent.spans?.filter(
73+
span => span.op === 'router.koa' && span.description === '/test-dedup',
74+
);
75+
expect(dedupSpans).toHaveLength(2);
76+
});
77+
78+
test('marks the layer span as errored when a handler throws', async ({ baseURL }) => {
79+
const transactionPromise = waitForTransaction('node-koa', transactionEvent => {
80+
return (
81+
transactionEvent?.contexts?.trace?.op === 'http.server' &&
82+
transactionEvent.transaction === 'GET /test-exception/:id'
83+
);
84+
});
85+
86+
await fetch(`${baseURL}/test-exception/123`);
87+
88+
const transactionEvent = await transactionPromise;
89+
90+
expect(transactionEvent.spans).toEqual(
91+
expect.arrayContaining([
92+
expect.objectContaining({
93+
op: 'router.koa',
94+
origin: 'auto.http.otel.koa',
95+
status: 'internal_error',
96+
}),
97+
]),
98+
);
99+
});

packages/node/src/integrations/tracing/koa/index.ts

Lines changed: 2 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,8 @@
11
import type { KoaInstrumentationConfig, KoaLayerType } from './vendored/types';
22
import { KoaInstrumentation } from './vendored/instrumentation';
3-
import { ATTR_HTTP_ROUTE } from '@opentelemetry/semantic-conventions';
43
import type { IntegrationFn } from '@sentry/core';
5-
import {
6-
captureException,
7-
debug,
8-
defineIntegration,
9-
getDefaultIsolationScope,
10-
getIsolationScope,
11-
SEMANTIC_ATTRIBUTE_SENTRY_OP,
12-
spanToJSON,
13-
} from '@sentry/core';
14-
import { addOriginToSpan, ensureIsWrapped, generateInstrumentOnce } from '@sentry/node-core';
15-
import { DEBUG_BUILD } from '../../../debug-build';
4+
import { captureException, defineIntegration } from '@sentry/core';
5+
import { ensureIsWrapped, generateInstrumentOnce } from '@sentry/node-core';
166

177
interface KoaOptions {
188
/**
@@ -29,36 +19,6 @@ export const instrumentKoa = generateInstrumentOnce(
2919
(options: KoaOptions = {}) => {
3020
return {
3121
ignoreLayersType: options.ignoreLayersType as KoaLayerType[],
32-
requestHook(span, info) {
33-
addOriginToSpan(span, 'auto.http.otel.koa');
34-
35-
const attributes = spanToJSON(span).data;
36-
37-
// this is one of: middleware, router
38-
const type = attributes['koa.type'];
39-
if (type) {
40-
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, `${type}.koa`);
41-
}
42-
43-
// Also update the name
44-
const name = attributes['koa.name'];
45-
if (typeof name === 'string') {
46-
// Somehow, name is sometimes `''` for middleware spans
47-
// See: https://github.com/open-telemetry/opentelemetry-js-contrib/issues/2220
48-
span.updateName(name || '< unknown >');
49-
}
50-
51-
if (getIsolationScope() === getDefaultIsolationScope()) {
52-
DEBUG_BUILD && debug.warn('Isolation scope is default isolation scope - skipping setting transactionName');
53-
return;
54-
}
55-
const route = attributes[ATTR_HTTP_ROUTE];
56-
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
57-
const method = info.context?.request?.method?.toUpperCase() || 'GET';
58-
if (route) {
59-
getIsolationScope().setTransactionName(`${method} ${route}`);
60-
}
61-
},
6222
} satisfies KoaInstrumentationConfig;
6323
},
6424
);

packages/node/src/integrations/tracing/koa/vendored/enums/AttributeNames.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
* - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/15ef7506553f631ea4181391e0c5725a56f0d082/packages/instrumentation-koa
1818
* - Upstream version: @opentelemetry/instrumentation-koa@0.66.0
1919
*/
20-
/* eslint-disable */
2120

2221
export enum AttributeNames {
2322
KOA_TYPE = 'koa.type',

0 commit comments

Comments
 (0)