From 9b6d0e9d56a873952b690f6e2c918cfab73941cf Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Mon, 30 Sep 2024 21:17:30 +0200 Subject: [PATCH] fix(medusa): Replace default http tracer (#9390) --- packages/core/framework/src/http/router.ts | 23 +--------- packages/medusa/src/commands/start.ts | 45 ++++++++++---------- packages/medusa/src/instrumentation/index.ts | 28 ++++++------ 3 files changed, 36 insertions(+), 60 deletions(-) diff --git a/packages/core/framework/src/http/router.ts b/packages/core/framework/src/http/router.ts index 9b00ff9bab..f0fb7f1bf8 100644 --- a/packages/core/framework/src/http/router.ts +++ b/packages/core/framework/src/http/router.ts @@ -180,7 +180,7 @@ function getBodyParserMiddleware(args?: ParserConfigArgs) { // TODO this router would need a proper rework, but it is out of scope right now -class ApiRoutesLoader { +export class ApiRoutesLoader { /** * Map of router path and its descriptor * @private @@ -910,27 +910,6 @@ export class RoutesLoader { */ readonly #sourceDir: string | string[] - static instrument: { - /** - * Instrument middleware function calls by wrapping the original - * middleware handler inside a custom implementation - */ - middleware: (callback: (typeof ApiRoutesLoader)["traceMiddleware"]) => void - - /** - * Instrument route handler function calls by wrapping the original - * middleware handler inside a custom implementation - */ - route: (callback: (typeof ApiRoutesLoader)["traceRoute"]) => void - } = { - middleware(callback) { - ApiRoutesLoader.traceMiddleware = callback - }, - route(callback) { - ApiRoutesLoader.traceRoute = callback - }, - } - constructor({ app, activityId, diff --git a/packages/medusa/src/commands/start.ts b/packages/medusa/src/commands/start.ts index 1d9c03f752..219b45a1be 100644 --- a/packages/medusa/src/commands/start.ts +++ b/packages/medusa/src/commands/start.ts @@ -9,7 +9,7 @@ import { gqlSchemaToTypes, GracefulShutdownServer, } from "@medusajs/framework/utils" -import http, { IncomingMessage, ServerResponse } from "http" +import http from "http" import { logger } from "@medusajs/framework/logger" import loaders from "../loaders" @@ -45,6 +45,13 @@ export async function registerInstrumentation(directory: string) { } } +/** + * Wrap request handler inside custom implementation to enabled + * instrumentation. + */ +// eslint-disable-next-line no-var +export var traceRequestHandler: (...args: any[]) => Promise = void 0 as any + async function start({ port, directory, types }) { async function internalStart() { track("CLI_START") @@ -53,16 +60,20 @@ async function start({ port, directory, types }) { const app = express() const http_ = http.createServer(async (req, res) => { - await start.traceRequestHandler( - async () => { - return new Promise((resolve) => { - res.on("finish", resolve) - app(req, res) - }) - }, - req, - res - ) + if (traceRequestHandler) { + await traceRequestHandler( + async () => { + return new Promise((resolve) => { + res.on("finish", resolve) + app(req, res) + }) + }, + req, + res + ) + } else { + app(req, res) + } }) try { @@ -123,16 +134,4 @@ async function start({ port, directory, types }) { await internalStart() } -/** - * Wrap request handler inside custom implementation to enabled - * instrumentation. - */ -start.traceRequestHandler = async ( - requestHandler: () => Promise, - _: IncomingMessage, - __: ServerResponse -) => { - return await requestHandler() -} - export default start diff --git a/packages/medusa/src/instrumentation/index.ts b/packages/medusa/src/instrumentation/index.ts index 5bcf91e4a6..2ab46362e8 100644 --- a/packages/medusa/src/instrumentation/index.ts +++ b/packages/medusa/src/instrumentation/index.ts @@ -1,6 +1,6 @@ import { snakeCase } from "lodash" import { Query } from "@medusajs/framework" -import { RoutesLoader } from "@medusajs/framework/http" +import { ApiRoutesLoader } from "@medusajs/framework/http" import { Tracer } from "@medusajs/framework/telemetry" import type { SpanExporter } from "@opentelemetry/sdk-trace-node" import type { Instrumentation } from "@opentelemetry/instrumentation" @@ -19,11 +19,11 @@ function shouldExcludeResource(resource: string) { * OpenTelemetry */ export function instrumentHttpLayer() { - const start = require("../commands/start") + const startCommand = require("../commands/start") const HTTPTracer = new Tracer("@medusajs/http", "2.0.0") const { SpanStatusCode } = require("@opentelemetry/api") - start.traceRequestHandler = async (requestHandler, req, res) => { + startCommand.traceRequestHandler = async (requestHandler, req, res) => { if (shouldExcludeResource(req.url!)) { return await requestHandler() } @@ -55,16 +55,14 @@ export function instrumentHttpLayer() { * Instrumenting the route handler to report traces to * OpenTelemetry */ - RoutesLoader.instrument.route((handler) => { - const traceName = `route: ${ - handler.name ? snakeCase(handler.name) : `anonymous` - }` - + ApiRoutesLoader.traceRoute = (handler) => { return async (req, res) => { if (shouldExcludeResource(req.originalUrl)) { return await handler(req, res) } + const traceName = `route: ${req.method} ${req.originalUrl}` + await HTTPTracer.trace(traceName, async (span) => { try { await handler(req, res) @@ -79,22 +77,22 @@ export function instrumentHttpLayer() { } }) } - }) + } /** * Instrumenting the middleware handler to report traces to * OpenTelemetry */ - RoutesLoader.instrument.middleware((handler) => { - const traceName = `middleware: ${ - handler.name ? snakeCase(handler.name) : `anonymous` - }` - + ApiRoutesLoader.traceMiddleware = (handler) => { return async (req, res, next) => { if (shouldExcludeResource(req.originalUrl)) { return handler(req, res, next) } + const traceName = `middleware: ${ + handler.name ? snakeCase(handler.name) : `anonymous` + }` + await HTTPTracer.trace(traceName, async (span) => { return new Promise((resolve, reject) => { const _next = (error?: any) => { @@ -117,7 +115,7 @@ export function instrumentHttpLayer() { .catch(next) .then(next) } - }) + } } /**