chore: Improve instrumentation loading (#9378)

* chore: Improve instrumentation loading

* fix build

* fix buildgst

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,
+}

packages/medusa/src/commands/__fixtures__/instrumentation-no-register/instrumentation.js

@@ -0,0 +1 @@
+module.exports = {}

packages/medusa/src/commands/__fixtures__/instrumentation.js

@@ -0,0 +1,5 @@
+const registerMock = jest.fn()
+module.exports = {
+  registerMock,
+  register: registerMock,
+}

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)
+    })
+  })
+})

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
-    }
   }
 }