Skip to content

Commit

Permalink
Refactor getModulePath and fix decorator errors (#87)
Browse files Browse the repository at this point in the history
* fix undefined "this" error for decorators

* refactor getModulePath so it doesn't require string parsing and gets file names for decorators correctly as well

* add tests

* update changelog

* use typecasting instead of ts-ignore

* return values immediately
  • Loading branch information
keturiosakys authored Jul 13, 2023
1 parent 622d7bd commit 2eecb01
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 20 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
40 changes: 22 additions & 18 deletions packages/lib/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand All @@ -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 = {
Expand Down
2 changes: 1 addition & 1 deletion packages/lib/src/wrappers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ export function autometrics<F extends FunctionSig>(

function instrumentedFunction() {
try {
const result = fn(...params);
const result = fn.apply(this, params);
if (isPromise<ReturnType<F>>(result)) {
return result
.then((res: Awaited<ReturnType<typeof result>>) => {
Expand Down
37 changes: 36 additions & 1 deletion packages/lib/tests/integration.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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);
});
});

0 comments on commit 2eecb01

Please sign in to comment.