Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(hono): Instrument main-app inline middleware spans#20999
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.
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -4,7 +4,11 @@ import { DEBUG_BUILD } from '../debug-build'; | ||
| import { wrapMiddlewareWithSpan } from './wrapMiddlewareSpan'; | ||
| // oxlint-disable-next-line typescript/no-explicit-any | ||
| const patchedInstances = new WeakSet<Hono<any>>(); | ||
| const patchedUseInstances = new WeakSet<Hono<any>>(); | ||
| // oxlint-disable-next-line typescript/no-explicit-any | ||
| const patchedMethodInstances = new WeakSet<Hono<any>>(); | ||
| const HTTP_METHODS = ['get', 'post', 'put', 'delete', 'options', 'patch', 'all'] as const; | ||
| /** | ||
| * Patches `app.use` (instance own property) on a Hono instance to instrument middleware at registration time. | ||
| @@ -13,12 +17,12 @@ const patchedInstances = new WeakSet<Hono<any>>(); | ||
| * Idempotent. | ||
| */ | ||
| export function patchAppUse<E extends Env>(app: Hono<E>): void { | ||
| if (patchedInstances.has(app)) { | ||
| if (patchedUseInstances.has(app)) { | ||
| DEBUG_BUILD && debug.log('[hono] app.use already patched — skipping.'); | ||
| return; | ||
| } | ||
| patchedInstances.add(app); | ||
| patchedUseInstances.add(app); | ||
| app.use = new Proxy(app.use, { | ||
| apply(target: typeof app.use, thisArg: typeof app, args: Parameters<typeof app.use>): ReturnType<typeof app.use> { | ||
| @@ -34,3 +38,54 @@ export function patchAppUse<E extends Env>(app: Hono<E>): void { | ||
| }, | ||
| }); | ||
| } | ||
| /** | ||
| * Patches HTTP method class fields (get, post, put, delete, options, patch, all) to instrument inline middleware at registration time. | ||
| * | ||
| * For `app.get('/path', mw1, mw2, handler)`, all handlers except the last are middleware and get wrapped with spans. | ||
| * The final handler (the route handler) is already covered by the root http.server transaction. | ||
| */ | ||
| export function patchHttpMethodHandlers<E extends Env>(app: Hono<E>): void { | ||
| if (patchedMethodInstances.has(app)) { | ||
| DEBUG_BUILD && debug.log('[hono] HTTP method handlers already patched - skipping.'); | ||
| return; | ||
| } | ||
| patchedMethodInstances.add(app); | ||
| for (const method of HTTP_METHODS) { | ||
| app[method] = new Proxy(app[method], { | ||
| apply(target, thisArg, args: unknown[]) { | ||
| return Reflect.apply(target, thisArg, wrapInlineMiddleware(args)); | ||
| }, | ||
| }); | ||
| } | ||
s1gr1d marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| app.on = new Proxy(app.on, { | ||
| apply(target, thisArg, args: unknown[]) { | ||
| // .on(method, path, ...handlers) — first two args are method and path | ||
| const [method, path, ...handlers] = args; | ||
| return Reflect.apply(target, thisArg, [method, path, ...wrapInlineMiddleware(handlers)]); | ||
| }, | ||
| }); | ||
| } | ||
| /** | ||
| * Given `[path?, ...handlers]` or `[...handlers]`, wraps all handlers except the last one with spans. | ||
| * The last handler is the route handler and is left as-is. | ||
| */ | ||
| function wrapInlineMiddleware(args: unknown[]): unknown[] { | ||
| const hasPathPrefix = typeof args[0] === 'string'; | ||
| const handlersStart = hasPathPrefix ? 1 : 0; | ||
| const handlers = args.slice(handlersStart) as MiddlewareHandler[]; | ||
| if (handlers.length <= 1) { | ||
| return args; | ||
| } | ||
| const wrapped = [...args]; | ||
| for (let i = handlersStart; i < wrapped.length - 1; i++) { | ||
| wrapped[i] = wrapMiddlewareWithSpan(wrapped[i] as MiddlewareHandler); | ||
| } | ||
| return wrapped; | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l: For
patchAppUseyou prevented double wrapping with a check at this pointUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We check for double wrapping in
wrapMiddlewareWithSpanin line 24. The function is called bypatchHttpMethodHandlers. But I'll add a test so we can be sure it produces only one span.But for tidiness it's probably worth monitoring what was already patched, so we don't accumulate multiple proxy layers (even though those would still just create one span).