Skip to content

feat: Ability to limit number of recorded function calls #171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/Recording.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,22 @@ export default class Recording {
public readonly path;
public metadata: AppMap.Metadata;

private callCountPerFunction = new Map<FunctionInfo, number>();
private totalCallCount = 0;

public willExceedFunctionCallLimits(funInfo: FunctionInfo) {
return (
(config().maxRecordedCalls > 0 && this.totalCallCount >= config().maxRecordedCalls) ||
(config().maxRecordedCallsPerFunction &&
(this.callCountPerFunction.get(funInfo) ?? 0) >= config().maxRecordedCallsPerFunction)
);
}

functionCall(funInfo: FunctionInfo, thisArg: unknown, args: unknown[]): AppMap.FunctionCallEvent {
const count = this.callCountPerFunction.get(funInfo) ?? 0;
this.callCountPerFunction.set(funInfo, count + 1);
this.totalCallCount++;

this.functionsSeen.add(funInfo);
const event = makeCallEvent(this.nextId++, funInfo, thisArg, args);
this.emit(event);
Expand Down
29 changes: 29 additions & 0 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ const kResponseBodyMaxLengthEnvar = "APPMAP_RESPONSE_BODY_MAX_LENGTH";
const asyncTrackingTimeoutDefault = 3000;
const kAsyncTrackingTimeoutEnvar = "APPMAP_ASYNC_TRACKING_TIMEOUT";

const maxRecordedCallsDefault = 10_000_000;
const kMaxRecordedCallsEnvar = "APPMAP_MAX_RECORDED_CALLS";

const maxRecordedCallsPerFunctionDefault = 100_000;
const kMaxRecordedCallsPerFunctionEnvar = "APPMAP_MAX_RECORDED_CALLS_PER_FUNCTION";

export class Config {
public readonly relativeAppmapDir: string;
public readonly appName: string;
Expand All @@ -36,6 +42,9 @@ export class Config {
// manipulated. This flag allows easy toggling of the check during testing.
public readonly generateGlobalRecordHookCheck: boolean = true;

public readonly maxRecordedCalls: number;
public readonly maxRecordedCallsPerFunction: number;

private readonly document?: Document;
private migrationPending = false;

Expand Down Expand Up @@ -90,6 +99,16 @@ export class Config {
getNonNegativeIntegerEnvVarValue(kAsyncTrackingTimeoutEnvar) ??
config?.async_tracking_timeout ??
asyncTrackingTimeoutDefault;

this.maxRecordedCalls =
getNonNegativeIntegerEnvVarValue(kMaxRecordedCallsEnvar) ??
config?.max_recorded_calls ??
maxRecordedCallsDefault;

this.maxRecordedCallsPerFunction =
getNonNegativeIntegerEnvVarValue(kMaxRecordedCallsPerFunctionEnvar) ??
config?.max_recorded_calls_per_function ??
maxRecordedCallsPerFunctionDefault;
}

private absoluteAppmapDir?: string;
Expand Down Expand Up @@ -176,6 +195,8 @@ interface ConfigFile {
response_body_max_length?: number;
language?: string;
async_tracking_timeout?: number;
max_recorded_calls?: number;
max_recorded_calls_per_function?: number;
}

// Maintaining the YAML document is important to preserve existing comments and formatting
Expand Down Expand Up @@ -217,6 +238,14 @@ function readConfigFile(document: Document): ConfigFile {
const value = parseInt(String(config.async_tracking_timeout));
result.async_tracking_timeout = value >= 0 ? value : undefined;
}
if ("max_recorded_calls" in config) {
const value = parseInt(String(config.max_recorded_calls));
result.max_recorded_calls = value >= 0 ? value : undefined;
}
if ("max_recorded_calls_per_function" in config) {
const value = parseInt(String(config.max_recorded_calls_per_function));
result.max_recorded_calls_per_function = value >= 0 ? value : undefined;
}

return result;
}
Expand Down
3 changes: 2 additions & 1 deletion src/recorder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ export function record<This, Return>(
funInfo: FunctionInfo,
isLibrary = false,
): Return {
const recordings = getActiveRecordings();
const recordings = getActiveRecordings().filter((r) => !r.willExceedFunctionCallLimits(funInfo));

let pkg;
if (
recordings.length == 0 ||
Expand Down
148 changes: 148 additions & 0 deletions test/__snapshots__/simple.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,154 @@ exports[`mapping a script using async tracking timeout 3000 1`] = `
}
`;

exports[`mapping a script using function call limits 1`] = `
{
"classMap": [
{
"children": [
{
"children": [
{
"location": "callLimits.js:1",
"name": "a",
"static": true,
"type": "function",
},
{
"location": "callLimits.js:5",
"name": "b",
"static": true,
"type": "function",
},
{
"location": "callLimits.js:9",
"name": "c",
"static": true,
"type": "function",
},
],
"name": "callLimits",
"type": "class",
},
],
"name": "simple",
"type": "package",
},
],
"events": [
{
"defined_class": "callLimits",
"event": "call",
"id": 1,
"lineno": 1,
"method_id": "a",
"parameters": [],
"path": "callLimits.js",
"static": true,
"thread_id": 0,
},
{
"elapsed": 31.337,
"event": "return",
"id": 2,
"parent_id": 1,
"thread_id": 0,
},
{
"defined_class": "callLimits",
"event": "call",
"id": 3,
"lineno": 1,
"method_id": "a",
"parameters": [],
"path": "callLimits.js",
"static": true,
"thread_id": 0,
},
{
"elapsed": 31.337,
"event": "return",
"id": 4,
"parent_id": 3,
"thread_id": 0,
},
{
"defined_class": "callLimits",
"event": "call",
"id": 5,
"lineno": 5,
"method_id": "b",
"parameters": [],
"path": "callLimits.js",
"static": true,
"thread_id": 0,
},
{
"elapsed": 31.337,
"event": "return",
"id": 6,
"parent_id": 5,
"thread_id": 0,
},
{
"defined_class": "callLimits",
"event": "call",
"id": 7,
"lineno": 5,
"method_id": "b",
"parameters": [],
"path": "callLimits.js",
"static": true,
"thread_id": 0,
},
{
"elapsed": 31.337,
"event": "return",
"id": 8,
"parent_id": 7,
"thread_id": 0,
},
{
"defined_class": "callLimits",
"event": "call",
"id": 9,
"lineno": 9,
"method_id": "c",
"parameters": [],
"path": "callLimits.js",
"static": true,
"thread_id": 0,
},
{
"elapsed": 31.337,
"event": "return",
"id": 10,
"parent_id": 9,
"thread_id": 0,
},
],
"metadata": {
"app": "simple",
"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",
}
`;

exports[`mapping a script with import attributes/assertions 1`] = `
{
"classMap": [
Expand Down
12 changes: 12 additions & 0 deletions test/simple.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,18 @@ integrationTest("mapping a script with tangled async functions", () => {
expect(readAppmap()).toMatchSnapshot();
});

integrationTest.only("mapping a script using function call limits", () => {
const options = {
env: {
...process.env,
APPMAP_MAX_RECORDED_CALLS: "5",
APPMAP_MAX_RECORDED_CALLS_PER_FUNCTION: "2",
},
};
expect(runAppmapNodeWithOptions(options, "callLimits.js").status).toBe(0);
expect(readAppmap()).toMatchSnapshot();
});

const asyncTimeoutCases = new Map<string, string[]>([
// No async tracking
["0", ["1 task", "2 process", "return 2", "return 1", "5 getMessage", "return 5"]],
Expand Down
23 changes: 23 additions & 0 deletions test/simple/callLimits.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
function a() {
console.log("a");
}

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

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

a();
a();
a();

b();
b();
b();

c();
c();
c();