Skip to content

Commit d13f07b

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 d13f07b

10 files changed

Lines changed: 214 additions & 166 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',

packages/node/src/integrations/tracing/koa/vendored/instrumentation.ts

Lines changed: 65 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -17,25 +17,43 @@
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
* - Minor TypeScript strictness adjustments for this repository's compiler settings
20+
* - Span creation migrated to the @sentry/core API; op/origin/name and transaction name folded into
21+
* span creation (previously set via a Sentry requestHook)
2022
*/
21-
/* eslint-disable */
2223

2324
import * as api from '@opentelemetry/api';
25+
import { isWrapped, InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation';
26+
27+
import { KoaLayerType, type KoaInstrumentationConfig } from './types';
2428
import {
25-
isWrapped,
26-
InstrumentationBase,
27-
InstrumentationNodeModuleDefinition,
28-
safeExecuteInTheMiddle,
29-
} from '@opentelemetry/instrumentation';
30-
31-
import { KoaLayerType, KoaInstrumentationConfig } from './types';
32-
import { SDK_VERSION } from '@sentry/core';
29+
debug,
30+
getDefaultIsolationScope,
31+
getIsolationScope,
32+
SDK_VERSION,
33+
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
34+
startSpan,
35+
} from '@sentry/core';
36+
import { ATTR_HTTP_ROUTE } from '@opentelemetry/semantic-conventions';
3337
import { getMiddlewareMetadata, isLayerIgnored } from './utils';
3438
import { setHttpServerSpanRouteAttribute } from '../../../../utils/setHttpServerSpanRouteAttribute';
35-
import { Next, kLayerPatched, KoaContext, KoaMiddleware, KoaPatchedMiddleware } from './internal-types';
39+
import { DEBUG_BUILD } from '../../../../debug-build';
40+
import { AttributeNames } from './enums/AttributeNames';
41+
import {
42+
kLayerPatched,
43+
type Next,
44+
type KoaContext,
45+
type KoaMiddleware,
46+
type KoaPatchedMiddleware,
47+
} from './internal-types';
3648

3749
const PACKAGE_NAME = '@sentry/instrumentation-koa';
3850

51+
interface KoaModuleExports {
52+
prototype: { use: KoaMiddleware };
53+
}
54+
55+
type KoaModule = KoaModuleExports & { [Symbol.toStringTag]?: string; default?: KoaModuleExports };
56+
3957
/** Koa instrumentation for OpenTelemetry */
4058
export class KoaInstrumentation extends InstrumentationBase<KoaInstrumentationConfig> {
4159
constructor(config: KoaInstrumentationConfig = {}) {
@@ -46,11 +64,8 @@ export class KoaInstrumentation extends InstrumentationBase<KoaInstrumentationCo
4664
return new InstrumentationNodeModuleDefinition(
4765
'koa',
4866
['>=2.0.0 <4'],
49-
(module: any) => {
50-
const moduleExports: any =
51-
module[Symbol.toStringTag] === 'Module'
52-
? module.default // ESM
53-
: module; // CommonJS
67+
(module: KoaModule) => {
68+
const moduleExports = module[Symbol.toStringTag] === 'Module' ? module.default : module;
5469
if (moduleExports == null) {
5570
return moduleExports;
5671
}
@@ -60,12 +75,9 @@ export class KoaInstrumentation extends InstrumentationBase<KoaInstrumentationCo
6075
this._wrap(moduleExports.prototype, 'use', this._getKoaUsePatch.bind(this));
6176
return module;
6277
},
63-
(module: any) => {
64-
const moduleExports: any =
65-
module[Symbol.toStringTag] === 'Module'
66-
? module.default // ESM
67-
: module; // CommonJS
68-
if (isWrapped(moduleExports.prototype.use)) {
78+
(module: KoaModule) => {
79+
const moduleExports = module[Symbol.toStringTag] === 'Module' ? module.default : module;
80+
if (moduleExports && isWrapped(moduleExports.prototype.use)) {
6981
this._unwrap(moduleExports.prototype, 'use');
7082
}
7183
},
@@ -77,15 +89,13 @@ export class KoaInstrumentation extends InstrumentationBase<KoaInstrumentationCo
7789
* middleware layer which is introduced
7890
* @param {KoaMiddleware} middleware - the original middleware function
7991
*/
80-
private _getKoaUsePatch(original: (middleware: KoaMiddleware) => any) {
81-
const plugin = this;
82-
return function use(this: any, middlewareFunction: KoaMiddleware) {
83-
let patchedFunction: KoaMiddleware;
84-
if (middlewareFunction.router) {
85-
patchedFunction = plugin._patchRouterDispatch(middlewareFunction);
86-
} else {
87-
patchedFunction = plugin._patchLayer(middlewareFunction, false);
88-
}
92+
private _getKoaUsePatch(original: (middleware: KoaMiddleware) => unknown) {
93+
const patchRouterDispatch = this._patchRouterDispatch.bind(this);
94+
const patchLayer = this._patchLayer.bind(this);
95+
return function use(this: unknown, middlewareFunction: KoaMiddleware) {
96+
const patchedFunction = middlewareFunction.router
97+
? patchRouterDispatch(middlewareFunction)
98+
: patchLayer(middlewareFunction, false);
8999
return original.apply(this, [patchedFunction]);
90100
};
91101
}
@@ -107,9 +117,9 @@ export class KoaInstrumentation extends InstrumentationBase<KoaInstrumentationCo
107117
const path = pathLayer.path;
108118
// Type cast needed: router.stack comes from @types/koa@2.x but we use @types/koa@3.x
109119
// See internal-types.ts for full explanation
110-
const pathStack = pathLayer.stack as any;
120+
const pathStack = pathLayer.stack as KoaMiddleware[];
111121
for (let j = 0; j < pathStack.length; j++) {
112-
const routedMiddleware: KoaMiddleware = pathStack[j];
122+
const routedMiddleware = pathStack[j] as KoaMiddleware;
113123
pathStack[j] = this._patchLayer(routedMiddleware, true, path);
114124
}
115125
}
@@ -146,49 +156,40 @@ export class KoaInstrumentation extends InstrumentationBase<KoaInstrumentationCo
146156
middlewareLayer[kLayerPatched] = true;
147157

148158
api.diag.debug('patching Koa middleware layer');
149-
return async (context: KoaContext, next: Next) => {
159+
return (context: KoaContext, next: Next) => {
150160
const parent = api.trace.getSpan(api.context.active());
151161
if (parent === undefined) {
152162
return middlewareLayer(context, next);
153163
}
154164
const metadata = getMiddlewareMetadata(context, middlewareLayer, isRouter, layerPath);
155-
const span = this.tracer.startSpan(metadata.name, {
156-
attributes: metadata.attributes,
157-
});
158165

159166
if (context._matchedRoute) {
160167
setHttpServerSpanRouteAttribute(context._matchedRoute.toString());
161168
}
162169

163-
const { requestHook } = this.getConfig();
164-
if (requestHook) {
165-
safeExecuteInTheMiddle(
166-
() =>
167-
requestHook(span, {
168-
context,
169-
middlewareLayer,
170-
layerType,
171-
}),
172-
e => {
173-
if (e) {
174-
api.diag.error('koa instrumentation: request hook failed', e);
175-
}
176-
},
177-
true,
178-
);
179-
}
170+
const koaName = metadata.attributes[AttributeNames.KOA_NAME];
171+
const name = typeof koaName === 'string' ? koaName || '< unknown >' : metadata.name;
180172

181-
const newContext = api.trace.setSpan(api.context.active(), span);
182-
return api.context.with(newContext, async () => {
183-
try {
184-
return await middlewareLayer(context, next);
185-
} catch (err: any) {
186-
span.recordException(err);
187-
throw err;
188-
} finally {
189-
span.end();
190-
}
191-
});
173+
return startSpan(
174+
{
175+
name,
176+
op: `${layerType}.koa`,
177+
attributes: {
178+
...metadata.attributes,
179+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.koa',
180+
},
181+
},
182+
() => {
183+
const route = metadata.attributes[ATTR_HTTP_ROUTE];
184+
if (getIsolationScope() === getDefaultIsolationScope()) {
185+
DEBUG_BUILD && debug.warn('Isolation scope is default isolation scope - skipping setting transactionName');
186+
} else if (route) {
187+
const method = (context.request as { method?: string } | undefined)?.method?.toUpperCase() || 'GET';
188+
getIsolationScope().setTransactionName(`${method} ${route}`);
189+
}
190+
return middlewareLayer(context, next);
191+
},
192+
);
192193
};
193194
}
194195
}

0 commit comments

Comments
 (0)