diff --git a/CHANGELOG.md b/CHANGELOG.md index ddc0717d..74e1b0e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## [Unreleased] + +- Fixed an issue where decorators were changing `this` values for the methods they'd be wrapping, breaking them. +- Improved how `getModulePath` utility works, passing stack trace as structured data, and making it more robust. + ## [v0.5.3] - 2023-06-26 ### Changed diff --git a/packages/lib/src/utils.ts b/packages/lib/src/utils.ts index a06de9cd..48880fbb 100644 --- a/packages/lib/src/utils.ts +++ b/packages/lib/src/utils.ts @@ -20,13 +20,20 @@ export function getRuntime(): Runtime { // HACK: this entire function is a hacky way to acquire the module name for a // given function e.g.: dist/index.js export function getModulePath(): string | undefined { - Error.stackTraceLimit = 5; - const stack = new Error()?.stack?.split("\n"); + const defaultPrepareStackTrace = Error.prepareStackTrace; + Error.prepareStackTrace = (_, stack) => stack; + const { stack: stackConstructor } = new Error() as Error & { + stack: NodeJS.CallSite[]; + }; + Error.prepareStackTrace = defaultPrepareStackTrace; // we have to make sure to reset this to normal + + const stack = stackConstructor.map((callSite) => ({ + name: callSite.getFunctionName(), + file: callSite.getFileName(), + })); let rootDir: string; - const runtime = getRuntime(); - if (runtime === "browser") { rootDir = ""; } else if (runtime === "deno") { @@ -45,30 +52,27 @@ export function getModulePath(): string | undefined { } /** + * Finds the original wrapped function, first it checks if it's a decorator, + * and returns that filename or gets the 3th item of the stack trace: * * 0: Error - * 1: at getModulePath() ... - * 2: at autometrics() ... - * 3: at ... -> 4th line is always the original caller + * 1: at getModulePath ... + * 2: at autometrics ... + * 3: at ... -> 4th line is always the original wrapped function */ - const originalCaller = 3 as const; - - // The last element in this array will have the full path - const fullPath = stack[originalCaller].split(" ").pop(); + const wrappedFunctionPath = + stack.find((call) => { + if (call.name === "__decorateClass") return true; + })?.file ?? stack[2]?.file; - const containsFileProtocol = fullPath.includes("file://"); + const containsFileProtocol = wrappedFunctionPath.includes("file://"); // We split away everything up to the root directory of the project, // if the path contains file:// we need to remove it - let modulePath = fullPath.replace( + return wrappedFunctionPath.replace( containsFileProtocol ? `file://${rootDir}` : rootDir, "", ); - - // We split away the line and column numbers index.js:14:6 - modulePath = modulePath.substring(0, modulePath.indexOf(":")); - - return modulePath; } type ALSContext = { diff --git a/packages/lib/src/wrappers.ts b/packages/lib/src/wrappers.ts index a2b9ea17..93e28fa5 100644 --- a/packages/lib/src/wrappers.ts +++ b/packages/lib/src/wrappers.ts @@ -250,7 +250,7 @@ export function autometrics( function instrumentedFunction() { try { - const result = fn(...params); + const result = fn.apply(this, params); if (isPromise>(result)) { return result .then((res: Awaited>) => { diff --git a/packages/lib/tests/integration.test.ts b/packages/lib/tests/integration.test.ts index d8c010d6..c30c8029 100644 --- a/packages/lib/tests/integration.test.ts +++ b/packages/lib/tests/integration.test.ts @@ -1,4 +1,4 @@ -import { autometrics, init } from "../src"; +import { Autometrics, autometrics, init } from "../src"; import { describe, test, expect, beforeAll, afterEach } from "vitest"; import { AggregationTemporality, @@ -57,4 +57,39 @@ describe("Autometrics integration test", () => { expect(serialized).toMatch(errorCountMetric); }); + + test("class method", async () => { + const callCountMetric = + /function_calls_count_total\{\S*function="helloWorld"\S*module="\/packages\/lib\/tests\/integration.test.ts"\S*\} 2/gm; + const durationMetric = + /function_calls_duration_bucket\{\S*function="helloWorld"\S*module="\/packages\/lib\/tests\/integration.test.ts"\S*\}/gm; + + // @Autometrics decorator is likely to be used along-side other decorators + // this tests for any conflicts + function Bar() { + return function Bar( + target: Object, + propertyKey: string, + descriptor: PropertyDescriptor, + ) { + const originalMethod = descriptor.value; + descriptor.value = originalMethod; + }; + } + + @Autometrics() + class Foo { + @Bar() + helloWorld() {} + } + + const foo = new Foo(); + foo.helloWorld(); + foo.helloWorld(); + + const serialized = await collectAndSerialize(exporter); + + expect(serialized).toMatch(callCountMetric); + expect(serialized).toMatch(durationMetric); + }); });