From 899b1fba4afeed61c343aa0ff5d171bfe6dc5ca8 Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Tue, 7 Jan 2025 14:16:20 +0100 Subject: [PATCH] chore(medusa): Add handler path to the http tracing to be able to group by (#10835) RESOLVES FRMW-2835 In this PR, we trace HTTP requests using the route pattern and not the request URL. This allows us to aggregate all HTTP requests under a single route. In terms of implementation, we have to self find the route for a given request, since there is no API in express to do the same and we begin tracing even before the request is handed over to Express. Since, the route matching happens via RegExp matches, we ensure that this does not add much performance overhead. The matching takes between 0.8ms-1.5ms for various different routes Co-authored-by: Harminder Virk <1706381+thetutlage@users.noreply.github.com> --- .changeset/itchy-walls-call.md | 5 +++ packages/medusa/src/commands/start.ts | 47 +++++++++++++++++++- packages/medusa/src/instrumentation/index.ts | 13 ++++-- 3 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 .changeset/itchy-walls-call.md diff --git a/.changeset/itchy-walls-call.md b/.changeset/itchy-walls-call.md new file mode 100644 index 0000000000..91fff11465 --- /dev/null +++ b/.changeset/itchy-walls-call.md @@ -0,0 +1,5 @@ +--- +"@medusajs/medusa": patch +--- + +chore(medusa): Add handler path to the http tracing to be able to group by diff --git a/packages/medusa/src/commands/start.ts b/packages/medusa/src/commands/start.ts index cbb6145ebe..041233f845 100644 --- a/packages/medusa/src/commands/start.ts +++ b/packages/medusa/src/commands/start.ts @@ -19,6 +19,7 @@ import { logger } from "@medusajs/framework/logger" import loaders from "../loaders" import { MedusaModule } from "@medusajs/framework/modules-sdk" import { MedusaContainer } from "@medusajs/framework/types" +import { parse } from "url" const EVERY_SIXTH_HOUR = "0 */6 * * *" const CRON_SCHEDULE = EVERY_SIXTH_HOUR @@ -88,6 +89,44 @@ function displayAdminUrl({ logger.info(`Admin URL → http://${host || "localhost"}:${port}${adminPath}`) } +type ExpressStack = { + name: string + match: (url: string) => boolean + route: { path: string } + handle: { stack: ExpressStack[] } +} + +/** + * Retrieve the route path from the express stack based on the input url + * @param stack - The express stack + * @param url - The input url + * @returns The route path + */ +function findExpressRoutePath({ + stack, + url, +}: { + stack: ExpressStack[] + url: string +}): string | void { + const stackToProcess = [...stack] + + while (stackToProcess.length > 0) { + const layer = stackToProcess.pop()! + + if (layer.name === "bound dispatch" && layer.match(url)) { + return layer.route.path + } + + // Add nested stack items to be processed if they exist + if (layer.handle?.stack?.length) { + stackToProcess.push(...layer.handle.stack) + } + } + + return undefined +} + async function start(args: { directory: string host?: string @@ -104,15 +143,21 @@ async function start(args: { const app = express() const http_ = http.createServer(async (req, res) => { + const stack = app._router.stack await new Promise((resolve) => { res.on("finish", resolve) if (traceRequestHandler) { + const expressHandlerPath = findExpressRoutePath({ + stack, + url: parse(req.url!, false).pathname!, + }) void traceRequestHandler( async () => { app(req, res) }, req, - res + res, + expressHandlerPath ) } else { app(req, res) diff --git a/packages/medusa/src/instrumentation/index.ts b/packages/medusa/src/instrumentation/index.ts index b9ef9e1348..3fad5a1ca2 100644 --- a/packages/medusa/src/instrumentation/index.ts +++ b/packages/medusa/src/instrumentation/index.ts @@ -28,14 +28,20 @@ export function instrumentHttpLayer() { const HTTPTracer = new Tracer("@medusajs/http", "2.0.0") const { SpanStatusCode } = require("@opentelemetry/api") - startCommand.traceRequestHandler = async (requestHandler, req, res) => { + startCommand.traceRequestHandler = async ( + requestHandler, + req, + res, + handlerPath + ) => { if (shouldExcludeResource(req.url!)) { return await requestHandler() } - const traceName = `${req.method} ${req.url}` + const traceName = handlerPath ?? `${req.method} ${req.url}` await HTTPTracer.trace(traceName, async (span) => { span.setAttributes({ + "http.route": handlerPath, "http.url": req.url, "http.method": req.method, ...req.headers, @@ -66,7 +72,8 @@ export function instrumentHttpLayer() { return await handler(req, res) } - const traceName = `route: ${req.method} ${req.originalUrl}` + const label = req.route?.path ?? `${req.method} ${req.originalUrl}` + const traceName = `route handler: ${label}` await HTTPTracer.trace(traceName, async (span) => { try {