From 7484cef0faa568a7c30a53139652b23d24d3c21d Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Mon, 30 Sep 2024 11:22:23 +0200 Subject: [PATCH] chore: Improve instrumentation loading (#9378) * chore: Improve instrumentation loading * fix build * fix buildgst --- .../instrumentation.js | 7 ++ .../instrumentation.js | 1 + .../commands/__fixtures__/instrumentation.js | 5 + .../src/commands/__tests__/start.spec.ts | 92 +++++++++++++++++++ packages/medusa/src/commands/start.ts | 34 +++---- 5 files changed, 123 insertions(+), 16 deletions(-) create mode 100644 packages/medusa/src/commands/__fixtures__/instrumentation-failure/instrumentation.js create mode 100644 packages/medusa/src/commands/__fixtures__/instrumentation-no-register/instrumentation.js create mode 100644 packages/medusa/src/commands/__fixtures__/instrumentation.js create mode 100644 packages/medusa/src/commands/__tests__/start.spec.ts diff --git a/packages/medusa/src/commands/__fixtures__/instrumentation-failure/instrumentation.js b/packages/medusa/src/commands/__fixtures__/instrumentation-failure/instrumentation.js new file mode 100644 index 0000000000..7c1833084d --- /dev/null +++ b/packages/medusa/src/commands/__fixtures__/instrumentation-failure/instrumentation.js @@ -0,0 +1,7 @@ +const registerMock = jest.fn().mockImplementation(() => { + throw new Error("Failed to register instrumentation") +}) +module.exports = { + registerMock, + register: registerMock, +} diff --git a/packages/medusa/src/commands/__fixtures__/instrumentation-no-register/instrumentation.js b/packages/medusa/src/commands/__fixtures__/instrumentation-no-register/instrumentation.js new file mode 100644 index 0000000000..4ba52ba2c8 --- /dev/null +++ b/packages/medusa/src/commands/__fixtures__/instrumentation-no-register/instrumentation.js @@ -0,0 +1 @@ +module.exports = {} diff --git a/packages/medusa/src/commands/__fixtures__/instrumentation.js b/packages/medusa/src/commands/__fixtures__/instrumentation.js new file mode 100644 index 0000000000..f3fbe7b687 --- /dev/null +++ b/packages/medusa/src/commands/__fixtures__/instrumentation.js @@ -0,0 +1,5 @@ +const registerMock = jest.fn() +module.exports = { + registerMock, + register: registerMock, +} diff --git a/packages/medusa/src/commands/__tests__/start.spec.ts b/packages/medusa/src/commands/__tests__/start.spec.ts new file mode 100644 index 0000000000..9bb86367a3 --- /dev/null +++ b/packages/medusa/src/commands/__tests__/start.spec.ts @@ -0,0 +1,92 @@ +import { registerInstrumentation } from "../start" +import * as utils from "@medusajs/framework/utils" +import * as logger from "@medusajs/framework/logger" +import * as instrumentationFixture from "../__fixtures__/instrumentation" +import * as instrumentationFailureFixture from "../__fixtures__/instrumentation-failure/instrumentation" +import path from "path" + +describe("start", () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + describe("registerInstrumentation", () => { + it("should not throw when registering the instrumentation if the file is not ", async () => { + const fsSpy = jest.spyOn( + utils.FileSystem.prototype, + "exists", + "" as never + ) + + await registerInstrumentation(__dirname) + + expect(fsSpy).toHaveBeenCalled() + expect(fsSpy).toHaveBeenCalledWith( + expect.stringContaining("instrumentation.js") + ) + }) + + it("should log an info message if the file is present but not register function is found", async () => { + const fsSpy = jest.spyOn( + utils.FileSystem.prototype, + "exists", + "" as never + ) + const loggerSpy = jest.spyOn(logger.logger, "info", "" as never) + + await registerInstrumentation( + path.join(__dirname, "../__fixtures__/instrumentation-no-register") + ) + + expect(fsSpy).toHaveBeenCalled() + expect(fsSpy).toHaveBeenCalledWith( + expect.stringContaining("instrumentation.js") + ) + + expect(loggerSpy).toHaveBeenCalled() + expect(loggerSpy).toHaveBeenCalledWith( + "Skipping instrumentation registration. No register function found." + ) + }) + + it("should register the instrumentation if the file is present and exports a register function", async () => { + const fsSpy = jest.spyOn( + utils.FileSystem.prototype, + "exists", + "" as never + ) + + instrumentationFixture.registerMock.mockReturnValue(true) + + await registerInstrumentation(path.join(__dirname, "../__fixtures__")) + + expect(fsSpy).toHaveBeenCalled() + expect(instrumentationFixture.registerMock).toHaveBeenCalled() + + expect(fsSpy).toHaveBeenCalledWith( + expect.stringContaining("instrumentation.js") + ) + }) + + it("should throw if the instrumentation file exists but cannot be imported", async () => { + const fsSpy = jest.spyOn( + utils.FileSystem.prototype, + "exists", + "" as never + ) + + const err = await registerInstrumentation( + path.join(__dirname, "../__fixtures__/instrumentation-failure") + ).catch((e) => e) + + expect(fsSpy).toHaveBeenCalled() + expect(instrumentationFailureFixture.registerMock).toHaveBeenCalled() + + expect(fsSpy).toHaveBeenCalledWith( + expect.stringContaining("instrumentation.js") + ) + + expect(err).toBeInstanceOf(Error) + }) + }) +}) diff --git a/packages/medusa/src/commands/start.ts b/packages/medusa/src/commands/start.ts index 1d1ee76ae4..1d9c03f752 100644 --- a/packages/medusa/src/commands/start.ts +++ b/packages/medusa/src/commands/start.ts @@ -5,6 +5,7 @@ import { scheduleJob } from "node-schedule" import { dynamicImport, + FileSystem, gqlSchemaToTypes, GracefulShutdownServer, } from "@medusajs/framework/utils" @@ -16,6 +17,7 @@ import { MedusaModule } from "@medusajs/framework/modules-sdk" const EVERY_SIXTH_HOUR = "0 */6 * * *" const CRON_SCHEDULE = EVERY_SIXTH_HOUR +const INSTRUMENTATION_FILE = "instrumentation.js" /** * Imports the "instrumentation.js" file from the root of the @@ -23,23 +25,23 @@ const CRON_SCHEDULE = EVERY_SIXTH_HOUR * of this file is optional, hence we ignore "ENOENT" * errors. */ -async function registerInstrumentation(directory: string) { - try { - const instrumentation = await dynamicImport( - path.join(directory, "instrumentation.js") +export async function registerInstrumentation(directory: string) { + const fileSystem = new FileSystem(directory) + const exists = await fileSystem.exists(INSTRUMENTATION_FILE) + if (!exists) { + return + } + + const instrumentation = await dynamicImport( + path.join(directory, INSTRUMENTATION_FILE) + ) + if (typeof instrumentation.register === "function") { + logger.info("OTEL registered") + instrumentation.register() + } else { + logger.info( + "Skipping instrumentation registration. No register function found." ) - if (typeof instrumentation.register === "function") { - logger.info("OTEL registered") - instrumentation.register() - } - } catch (error) { - if ( - !["ENOENT", "MODULE_NOT_FOUND", "ERR_MODULE_NOT_FOUND"].includes( - error.code - ) - ) { - throw error - } } }