Skip to content

Commit

Permalink
feat: Allow excluding functions by name from instrumentation
Browse files Browse the repository at this point in the history
  • Loading branch information
dividedmind committed Jan 6, 2024
1 parent fad5ae5 commit 1009386
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 20 deletions.
9 changes: 6 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@ name: application-name # from package.json by default
appmap_dir: tmp/appmap
packages:
- path: . # paths to instrument, relative to appmap.yml location
exclude:
- node_modules
- .yaml
exclude: # code to exclude from instrumentation
- node_modules # these paths are excluded by default
- .yaml # if you create your own config file, you probably want to add them too
# You can also exclude methods and functions by name:
# - functionName
# - Klass.method
```

## Limitations
Expand Down
2 changes: 2 additions & 0 deletions src/PackageMatcher.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { resolve } from "node:path";
import { fileURLToPath } from "node:url";

export default class PackageMatcher extends Array<Package> {
constructor(
Expand All @@ -16,6 +17,7 @@ export default class PackageMatcher extends Array<Package> {
}

match(path: string): Package | undefined {
if (path.startsWith("file:")) path = fileURLToPath(path);
const pkg = this.find((pkg) => path.startsWith(this.resolve(pkg.path)));
return pkg?.exclude?.find((ex) => path.includes(ex)) ? undefined : pkg;
}
Expand Down
17 changes: 17 additions & 0 deletions src/hooks/instrument.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import assert from "node:assert";
import { fileURLToPath } from "node:url";
import { debuglog } from "node:util";

import { ancestor as walk } from "acorn-walk";
import { ESTree, parse } from "meriyah";
Expand All @@ -21,6 +22,8 @@ import {
import { FunctionInfo, SourceLocation, createFunctionInfo, createMethodInfo } from "../registry";
import findLast from "../util/findLast";

const debug = debuglog("appmap:instrument");

const __appmapFunctionRegistryIdentifier = identifier("__appmapFunctionRegistry");
export const transformedFunctionInfos: FunctionInfo[] = [];

Expand All @@ -31,12 +34,15 @@ function addTransformedFunctionInfo(fi: FunctionInfo): number {

export function transform(program: ESTree.Program, sourceMap?: SourceMapConsumer): ESTree.Program {
transformedFunctionInfos.splice(0);
const source = program.loc?.source;
const pkg = source ? config.packages.match(source) : undefined;

const locate = makeLocator(sourceMap);

walk(program, {
FunctionDeclaration(fun: ESTree.FunctionDeclaration) {
if (!hasIdentifier(fun)) return;
if (pkg?.exclude?.includes(fun.id.name)) return;

const location = locate(fun);
if (!location) return; // don't instrument generated code
Expand All @@ -51,9 +57,20 @@ export function transform(program: ESTree.Program, sourceMap?: SourceMapConsumer
const klass = findLast(ancestors, isNamedClass);
if (!klass) return;

const { name } = method.key;
const qname = [klass.id.name, name].join(".");

// Not sure why eslint complains here, ?? is the wrong operator
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
if (pkg?.exclude?.includes(name) || pkg?.exclude?.includes(qname)) {
debug(`Excluding ${qname}`);
return;
}

const location = locate(method);
if (!location) return; // don't instrument generated code

debug(`Instrumenting ${qname}`);
method.value.body = wrapWithRecord(
{ ...method.value },
createMethodInfo(method, klass, location),
Expand Down
34 changes: 17 additions & 17 deletions test/__snapshots__/simple.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,13 @@ exports[`mapping a simple script 1`] = `
{
"children": [
{
"location": "./index.js:14",
"location": "./index.js:15",
"name": "immediatePromise",
"static": true,
"type": "function",
},
{
"location": "./index.js:18",
"location": "./index.js:19",
"name": "throws",
"static": true,
"type": "function",
Expand All @@ -234,7 +234,7 @@ exports[`mapping a simple script 1`] = `
"type": "function",
},
{
"location": "./index.js:8",
"location": "./index.js:9",
"name": "promised",
"static": true,
"type": "function",
Expand Down Expand Up @@ -286,7 +286,7 @@ exports[`mapping a simple script 1`] = `
"defined_class": "index",
"event": "call",
"id": 1,
"lineno": 18,
"lineno": 19,
"method_id": "throws",
"parameters": [],
"path": "./index.js",
Expand Down Expand Up @@ -367,7 +367,7 @@ exports[`mapping a simple script 1`] = `
"defined_class": "index",
"event": "call",
"id": 7,
"lineno": 8,
"lineno": 9,
"method_id": "promised",
"parameters": [],
"path": "./index.js",
Expand All @@ -390,7 +390,7 @@ exports[`mapping a simple script 1`] = `
"defined_class": "index",
"event": "call",
"id": 9,
"lineno": 8,
"lineno": 9,
"method_id": "promised",
"parameters": [
{
Expand Down Expand Up @@ -419,7 +419,7 @@ exports[`mapping a simple script 1`] = `
"defined_class": "index",
"event": "call",
"id": 11,
"lineno": 14,
"lineno": 15,
"method_id": "immediatePromise",
"parameters": [],
"path": "./index.js",
Expand All @@ -442,7 +442,7 @@ exports[`mapping a simple script 1`] = `
"defined_class": "index",
"event": "call",
"id": 13,
"lineno": 18,
"lineno": 19,
"method_id": "throws",
"parameters": [],
"path": "./index.js",
Expand Down Expand Up @@ -691,37 +691,37 @@ exports[`mapping js class methods and constructors containing super keyword 1`]
{
"children": [
{
"location": "./class.js:16",
"location": "./class.js:2",
"name": "constructor",
"static": false,
"type": "function",
},
{
"location": "./class.js:21",
"location": "./class.js:7",
"name": "m1",
"static": false,
"type": "function",
},
],
"name": "B",
"name": "A",
"type": "class",
},
{
"children": [
{
"location": "./class.js:2",
"location": "./class.js:25",
"name": "constructor",
"static": false,
"type": "function",
},
{
"location": "./class.js:6",
"location": "./class.js:30",
"name": "m1",
"static": false,
"type": "function",
},
],
"name": "A",
"name": "B",
"type": "class",
},
],
Expand Down Expand Up @@ -752,7 +752,7 @@ exports[`mapping js class methods and constructors containing super keyword 1`]
"defined_class": "B",
"event": "call",
"id": 3,
"lineno": 16,
"lineno": 25,
"method_id": "constructor",
"parameters": [],
"path": "./class.js",
Expand Down Expand Up @@ -788,7 +788,7 @@ exports[`mapping js class methods and constructors containing super keyword 1`]
"defined_class": "B",
"event": "call",
"id": 7,
"lineno": 21,
"lineno": 30,
"method_id": "m1",
"parameters": [],
"path": "./class.js",
Expand All @@ -804,7 +804,7 @@ exports[`mapping js class methods and constructors containing super keyword 1`]
"defined_class": "A",
"event": "call",
"id": 8,
"lineno": 6,
"lineno": 7,
"method_id": "m1",
"parameters": [],
"path": "./class.js",
Expand Down
5 changes: 5 additions & 0 deletions test/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ export function integrationTest(name: string, fn?: jest.ProvidesCallback, timeou
test(name, fn, timeout);
}

integrationTest.only = function (name: string, fn?: jest.ProvidesCallback, timeout?: number): void {
testDir(caller().replace(/\.test\.[tj]s$/, "/"));
test.only(name, fn, timeout);
};

type AppMap = object & Record<"events", unknown>;

export function readAppmap(path?: string): AppMap.AppMap {
Expand Down
10 changes: 10 additions & 0 deletions test/simple/class.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
class A {
constructor() {
this.skippedMethod();
console.log("A.constructor()");
}

Expand All @@ -10,6 +11,14 @@ class A {
console.log("A.m2()");
return "return m2";
}

skippedMethod() {
console.log("A.skippedMethod()");
}

static skipped() {
console.log("A.skipped()");
}
}

class B extends A {
Expand All @@ -21,6 +30,7 @@ class B extends A {
m1() {
super.m1();
console.log("B.m1()");
A.skipped();
}
}

Expand Down
5 changes: 5 additions & 0 deletions test/simple/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
const { setTimeout } = require("timers/promises");

function foo(x) {
skipped();
return x * 2;
}

Expand All @@ -20,6 +21,10 @@ function throws() {
throw new Error("throws intentionally");
}

function skipped() {
console.log("skipped");
}

try {
throws();
console.log(foo(43));
Expand Down

0 comments on commit 1009386

Please sign in to comment.