From 688598734484d087e5d8aa343cc895f6ed813e59 Mon Sep 17 00:00:00 2001 From: zermelo-wisen Date: Tue, 13 Aug 2024 21:02:41 +0300 Subject: [PATCH] feat: Ability to limit number of recorded function calls Added the ability to limit the number of recorded function calls, both total and per function. This approach ensures no orphaned calls are recorded, as the check is made at the start of the function call recording. The limits can be configured via environment variables and configuration file, with default values of 10,000,000 total calls and 100,000 calls per function. Introduced a new test case that generates an AppMap with enforced total and per-function call limits. --- src/Recording.ts | 15 +++ src/config.ts | 29 +++++ src/recorder.ts | 3 +- test/__snapshots__/simple.test.ts.snap | 148 +++++++++++++++++++++++++ test/simple.test.ts | 12 ++ test/simple/callLimits.js | 23 ++++ 6 files changed, 229 insertions(+), 1 deletion(-) create mode 100644 test/simple/callLimits.js diff --git a/src/Recording.ts b/src/Recording.ts index 40078c5..b76468c 100644 --- a/src/Recording.ts +++ b/src/Recording.ts @@ -39,7 +39,22 @@ export default class Recording { public readonly path; public metadata: AppMap.Metadata; + private callCountPerFunction = new Map(); + 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); diff --git a/src/config.ts b/src/config.ts index 73b244d..da9dd0e 100644 --- a/src/config.ts +++ b/src/config.ts @@ -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; @@ -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; @@ -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; @@ -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 @@ -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; } diff --git a/src/recorder.ts b/src/recorder.ts index cbeee58..1446171 100644 --- a/src/recorder.ts +++ b/src/recorder.ts @@ -68,7 +68,8 @@ export function record( funInfo: FunctionInfo, isLibrary = false, ): Return { - const recordings = getActiveRecordings(); + const recordings = getActiveRecordings().filter((r) => !r.willExceedFunctionCallLimits(funInfo)); + let pkg; if ( recordings.length == 0 || diff --git a/test/__snapshots__/simple.test.ts.snap b/test/__snapshots__/simple.test.ts.snap index ce37efb..c4062a6 100644 --- a/test/__snapshots__/simple.test.ts.snap +++ b/test/__snapshots__/simple.test.ts.snap @@ -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": [ diff --git a/test/simple.test.ts b/test/simple.test.ts index 33ad478..3c67342 100644 --- a/test/simple.test.ts +++ b/test/simple.test.ts @@ -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([ // No async tracking ["0", ["1 task", "2 process", "return 2", "return 1", "5 getMessage", "return 5"]], diff --git a/test/simple/callLimits.js b/test/simple/callLimits.js new file mode 100644 index 0000000..81343b3 --- /dev/null +++ b/test/simple/callLimits.js @@ -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();