Skip to content

Commit

Permalink
fix: Instrument based on source map paths (ESM)
Browse files Browse the repository at this point in the history
  • Loading branch information
zermelo-wisen committed Jul 24, 2024
1 parent 41b14d5 commit 1fdfb52
Show file tree
Hide file tree
Showing 12 changed files with 611 additions and 16 deletions.
6 changes: 3 additions & 3 deletions src/hooks/libraries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export default function librariesHook(mod: unknown, id: string) {
// "node:console" and "console", for example, spreading around.
const moduleId = id.replace(/^node:/, "");
function wrapFunction(owner: FunctionOwner, key: string, klass?: string) {
if (config.getPackage(moduleId, true)?.exclude?.includes(key)) return;
if (config().getPackage(moduleId, true)?.exclude?.includes(key)) return;

const f = owner[key] as F;
if (!proxiedFunctions.has(f)) {
Expand Down Expand Up @@ -57,7 +57,7 @@ function createProxy(f: F, moduleId: string, klass?: string) {
}

librariesHook.applicable = function (id: string) {
return config.getPackage(id, true) != undefined;
return config().getPackage(id, true) != undefined;
};

const functionInfos = new Map<string, FunctionInfo>();
Expand Down Expand Up @@ -85,7 +85,7 @@ function getFunctionInfo(f: F, moduleId: string, klass?: string) {
location: { path: `${moduleId}`, lineno: ++functionCount },
klassOrFile: moduleId + (klass ? "." + klass : ""),
static: false,
labels: config.getFunctionLabels(config.getPackage(moduleId, true), f.name),
labels: config().getFunctionLabels(config().getPackage(moduleId, true), f.name),
};
functionInfos.set(key, info);
}
Expand Down
15 changes: 9 additions & 6 deletions src/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { NodeLoaderHooksAPI2 } from "ts-node";

import config from "./config";
import { warn } from "./message";
import transform, { findHook } from "./transform";
import transform, { findHook, getSourceMap, shouldMatchOriginalSourcePaths } from "./transform";
import { readPkgUp } from "./util/readPkgUp";
import { forceRequire } from "./register";

Expand All @@ -21,7 +21,7 @@ export const resolve: NodeLoaderHooksAPI2["resolve"] = async function resolve(
// their import name here (i.e. url: "json5"). Then it gets resolved
// to a path (i.e. result.path: ".../node_modules/json5/lib/index.js")
// and passed to the load function.
if (config.getPackage(url, true) != undefined) forceRequire(url);
if (config().getPackage(url, true) != undefined) forceRequire(url);

return result;
};
Expand All @@ -43,10 +43,13 @@ export const load: NodeLoaderHooksAPI2["load"] = async function load(url, contex
context.format = "commonjs";
}

const hook = findHook(urlObj);
const original = await defaultLoad(url, context, defaultLoad);
const sourceMap =
original.source && shouldMatchOriginalSourcePaths(urlObj)
? getSourceMap(urlObj, original.source.toString())
: undefined;
const hook = findHook(urlObj, sourceMap);
if (hook) {
const original = await defaultLoad(url, context, defaultLoad);

if (original.source) {
const xformed = transform(original.source.toString(), new URL(url));

Expand All @@ -65,5 +68,5 @@ export const load: NodeLoaderHooksAPI2["load"] = async function load(url, contex
if (["node:http", "node:https", "http", "https", ...config().prismaClientModuleIds].includes(url))
forceRequire(url);

return defaultLoad(url, context, defaultLoad);
return original;
};
2 changes: 1 addition & 1 deletion src/recorder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const funToPackage = new WeakMap<FunctionInfo, Package | undefined>();

function getPackage(funInfo: FunctionInfo, isLibrary: boolean) {
if (!funToPackage.has(funInfo))
funToPackage.set(funInfo, config.getPackage(funInfo.location?.path, isLibrary));
funToPackage.set(funInfo, config().getPackage(funInfo.location?.path, isLibrary));
return funToPackage.get(funInfo);
}

Expand Down
2 changes: 1 addition & 1 deletion src/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export class CustomSourceMapConsumer extends SourceMapConsumer {
}
}

function getSourceMap(fileUrl: URL, code: string): CustomSourceMapConsumer | undefined {
export function getSourceMap(fileUrl: URL, code: string): CustomSourceMapConsumer | undefined {
const sourceMappingUrl = code.match(/\/\/# sourceMappingURL=(.*)/)?.[1];
if (!sourceMappingUrl) return;

Expand Down
70 changes: 70 additions & 0 deletions test/__snapshots__/sourceMapPath.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,73 @@ exports[`mapping a bundled script 1`] = `
"version": "1.12",
}
`;

exports[`mapping an bundled ESM script 1`] = `
{
"classMap": [
{
"children": [
{
"children": [
{
"location": "original.mjs:3",
"name": "foo",
"static": true,
"type": "function",
},
],
"name": "original",
"type": "class",
},
],
"name": "appmap-node",
"type": "package",
},
],
"events": [
{
"defined_class": "original",
"event": "call",
"id": 1,
"lineno": 3,
"method_id": "foo",
"parameters": [
{
"class": "String",
"name": "msg",
"value": "'hello'",
},
],
"path": "original.mjs",
"static": true,
"thread_id": 0,
},
{
"elapsed": 31.337,
"event": "return",
"id": 2,
"parent_id": 1,
"thread_id": 0,
},
],
"metadata": {
"app": "appmap-node",
"client": {
"name": "appmap-node",
"url": "https://github.com/getappmap/appmap-node",
"version": "test node-appmap version",
},
"language": {
"engine": "Node.js",
"name": "javascript",
"version": "test node version",
},
"name": "test process recording",
"recorder": {
"name": "process",
"type": "process",
},
},
"version": "1.12",
}
`;
9 changes: 7 additions & 2 deletions test/jest.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { format } from "node:util";
import { format, inspect } from "node:util";

import { integrationTest, readAppmaps, runAppmapNode, runAppmapNodeWithOptions } from "./helpers";

Expand Down Expand Up @@ -29,6 +29,11 @@ integrationTest("mapping Jest tests with process recording active", () => {
([, a]) => a.metadata?.recorder.type == "process",
);
if (processMaps.length != 1)
throw new Error(format("expected one process appmap, got: ", Object.fromEntries(processMaps)));
throw new Error(
format(
"expected one process appmap, got: ",
inspect(Object.fromEntries(processMaps), undefined, Infinity),
),
);
expect(appmapsArray.filter((a) => a.metadata?.recorder.type == "tests").length).toEqual(4);
});
5 changes: 5 additions & 0 deletions test/sourceMapPath.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,8 @@ integrationTest("mapping a bundled script", () => {
expect(runAppmapNode("./built/index.js").status).toBe(0);
expect(readAppmap()).toMatchSnapshot();
});

integrationTest("mapping an bundled ESM script", () => {
expect(runAppmapNode("./built/index-esm.mjs").status).toBe(0);
expect(readAppmap()).toMatchSnapshot();
});
3 changes: 2 additions & 1 deletion test/sourceMapPath/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
To recreate index.js & index.js.map:
To recreate index.js, index.js.map, index-esm.mjs and index-esm.mjs.map:

```
$ yarn esbuild original.mjs --bundle --platform=node --sourcemap --outfile=built/index.js
$ yarn esbuild original.mjs --bundle --platform=node --sourcemap --outfile=built/index-esm.mjs --format=esm
```
3 changes: 2 additions & 1 deletion test/sourceMapPath/appmap.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ packages:
- node_modules
- .yarn
- built
- bar
- bar
language: javascript
2 changes: 1 addition & 1 deletion test/sourceMapPath/built/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"ignorePatterns": ["index.js"]
"ignorePatterns": ["index.js", "index-esm.mjs"]
}
Loading

0 comments on commit 1fdfb52

Please sign in to comment.