From ece3f9e2c18d60320516ccb4ec5cdc1ead787802 Mon Sep 17 00:00:00 2001 From: Romain Lanz Date: Fri, 16 Oct 2026 06:36:10 +0000 Subject: [PATCH] perf(server): skip tracing channels without subscribers Every request called tracePromise/traceSync once for the request, once per global and route middleware, once for the route handler and once for the response serializer, even when nothing listens. Without subscribers Node only forwards the call, but each call still pays for the rest args array and Reflect.apply. Check hasSubscribers at each call site and call the function directly when it is false. The check stays per call, so subscribers attached after boot still receive events. --- src/response.ts | 6 +- src/router/executor.ts | 55 ++++++++-------- src/server/factories/middleware_handler.ts | 11 +++- src/server/main.ts | 30 ++++----- tests/server.spec.ts | 77 ++++++++++++++++++++++ 5 files changed, 135 insertions(+), 44 deletions(-) diff --git a/src/response.ts b/src/response.ts index 70fa144..9ccbc75 100644 --- a/src/response.ts +++ b/src/response.ts @@ -1214,7 +1214,11 @@ export class HttpResponse extends Macroable { * Handle text based response */ if (this.content) { - httpResponseSerializer.traceSync(this.writeBody, undefined, this, ...this.content) + if (httpResponseSerializer.hasSubscribers) { + httpResponseSerializer.traceSync(this.writeBody, undefined, this, ...this.content) + } else { + this.writeBody(...this.content) + } return } diff --git a/src/router/executor.ts b/src/router/executor.ts index 524d0fa..8a558b9 100644 --- a/src/router/executor.ts +++ b/src/router/executor.ts @@ -33,46 +33,49 @@ export function execute( .runner() .errorHandler((error) => errorResponder(error, ctx)) .finalHandler(() => { + /** + * Tracing channels are only used when someone is listening. Otherwise + * the handlers are called directly to skip the tracePromise overhead. + */ if (typeof route.handler === 'function') { - return ( - httpRouteHandler.tracePromise( - ($ctx: HttpContext) => Promise.resolve((route.handler as Function)($ctx)), - httpRouteHandler.hasSubscribers ? { route } : undefined, - undefined, - ctx - ) as unknown as Promise - ).then(useReturnValue(ctx)) + const handler = route.handler + const promise = httpRouteHandler.hasSubscribers + ? httpRouteHandler.tracePromise( + ($ctx: HttpContext) => Promise.resolve(handler($ctx)), + { route }, + undefined, + ctx + ) + : Promise.resolve(handler(ctx)) + + return promise.then(useReturnValue(ctx)) } - return ( - httpRouteHandler.tracePromise( - route.handler.handle, - httpRouteHandler.hasSubscribers ? { route } : undefined, - undefined, - resolver, - ctx - ) as unknown as Promise - ).then(useReturnValue(ctx)) + const promise = httpRouteHandler.hasSubscribers + ? httpRouteHandler.tracePromise(route.handler.handle, { route }, undefined, resolver, ctx) + : route.handler.handle(resolver, ctx) + + return promise.then(useReturnValue(ctx)) }) .run(async (middleware, next) => { if (typeof middleware === 'function') { - return httpMiddleware.tracePromise( - middleware, - httpMiddleware.hasSubscribers ? { middleware } : undefined, - undefined, - ctx, - next - ) as unknown as Promise + if (!httpMiddleware.hasSubscribers) { + return middleware(ctx, next) + } + return httpMiddleware.tracePromise(middleware, { middleware }, undefined, ctx, next) } + if (!httpMiddleware.hasSubscribers) { + return middleware.handle(resolver, ctx, next, middleware.args) + } return httpMiddleware.tracePromise( middleware.handle, - httpMiddleware.hasSubscribers ? { middleware } : undefined, + { middleware }, undefined, resolver, ctx, next, middleware.args - ) as unknown as Promise + ) }) } diff --git a/src/server/factories/middleware_handler.ts b/src/server/factories/middleware_handler.ts index 5e38d6b..0831757 100644 --- a/src/server/factories/middleware_handler.ts +++ b/src/server/factories/middleware_handler.ts @@ -37,9 +37,18 @@ export function middlewareHandler(resolver: ContainerResolver, ctx: HttpCon */ return function (fn: ParsedGlobalMiddleware, next: NextFn) { debug('executing middleware %s', fn.name) + + /** + * Calling the handler directly avoids the variadic tracePromise call + * (rest args array + Reflect.apply) when nobody is listening. + */ + if (!httpMiddleware.hasSubscribers) { + return fn.handle(resolver, ctx, next) + } + return httpMiddleware.tracePromise( fn.handle, - httpMiddleware.hasSubscribers ? { middleware: fn } : undefined, + { middleware: fn }, undefined, resolver, ctx, diff --git a/src/server/main.ts b/src/server/main.ts index db096d9..d30db0b 100644 --- a/src/server/main.ts +++ b/src/server/main.ts @@ -148,6 +148,18 @@ export class Server { ) } + /** + * Handles the request inside the "adonisjs.http.request" tracing channel. + * The channel is skipped when nobody is listening to avoid the + * tracePromise overhead on every request. + */ + #traceRequest(ctx: HttpContext, resolver: ContainerResolver) { + if (!httpRequest.hasSubscribers) { + return this.#handleRequest(ctx, resolver) + } + return httpRequest.tracePromise(this.#handleRequest, { ctx }, this, ctx, resolver) + } + /** * Indicates whether the server has completed its boot process */ @@ -471,23 +483,9 @@ export class Server { * Handle request */ if (this.usingAsyncLocalStorage) { - return asyncLocalStorage.storage!.run(ctx, () => - httpRequest.tracePromise( - this.#handleRequest, - httpRequest.hasSubscribers ? { ctx } : undefined, - this, - ctx, - resolver - ) - ) + return asyncLocalStorage.storage!.run(ctx, () => this.#traceRequest(ctx, resolver)) } - return httpRequest.tracePromise( - this.#handleRequest, - httpRequest.hasSubscribers ? { ctx } : undefined, - this, - ctx, - resolver - ) + return this.#traceRequest(ctx, resolver) } } diff --git a/tests/server.spec.ts b/tests/server.spec.ts index b5d521b..492a65e 100644 --- a/tests/server.spec.ts +++ b/tests/server.spec.ts @@ -15,6 +15,12 @@ import { Emitter } from '@adonisjs/events' import type { NextFn } from '@poppinss/middleware/types' import { AppFactory } from '@adonisjs/application/factories' import { createServer, IncomingMessage, ServerResponse } from 'node:http' +import { + httpRequest, + httpMiddleware, + httpRouteHandler, + httpResponseSerializer, +} from '../src/tracing_channels.ts' import { Router } from '../src/router/main.ts' import { HttpContext } from '../src/http_context/main.ts' @@ -85,6 +91,77 @@ test.group('Server', () => { }) }) +test.group('Server | tracing', () => { + test('emit tracing events when subscribing after the server has booted', async ({ + assert, + cleanup, + }) => { + const app = new AppFactory().create(BASE_URL, () => {}) + const server = new ServerFactory().merge({ app }).create() + const httpServer = createServer(server.handle.bind(server)) + + await app.init() + + server.use([ + async () => { + return { + default: class GlobalMiddleware { + async handle(_: HttpContext, next: NextFn) { + await next() + } + }, + } + }, + ]) + server + .getRouter() + .get('/', async () => 'handled') + .middleware(async (_, next) => { + await next() + }) + + await server.boot() + + /** + * Requests made before subscribing must not prevent events + * from being emitted later on + */ + await supertest(httpServer).get('/').expect(200) + + const events: string[] = [] + const subscribe = (name: string, channel: { subscribe: Function; unsubscribe: Function }) => { + const handlers = { + start: () => events.push(`${name}:start`), + end: () => {}, + asyncStart: () => {}, + asyncEnd: () => events.push(`${name}:asyncEnd`), + error: () => {}, + } + channel.subscribe(handlers) + cleanup(() => channel.unsubscribe(handlers)) + } + + subscribe('request', httpRequest) + subscribe('middleware', httpMiddleware) + subscribe('route', httpRouteHandler) + subscribe('serializer', httpResponseSerializer) + + const { text } = await supertest(httpServer).get('/').expect(200) + assert.equal(text, 'handled') + assert.deepEqual(events, [ + 'request:start', + 'middleware:start', + 'middleware:start', + 'route:start', + 'route:asyncEnd', + 'middleware:asyncEnd', + 'middleware:asyncEnd', + 'serializer:start', + 'request:asyncEnd', + ]) + }) +}) + test.group('Server | Response handling', () => { test('invoke router handler', async ({ assert }) => { const app = new AppFactory().create(BASE_URL, () => {})