Skip to content
This repository has been archived by the owner on Mar 15, 2024. It is now read-only.

Commit

Permalink
feat: provide feedback if the stack overflows
Browse files Browse the repository at this point in the history
Currenlty, trace post-processing relies on recursion. This code is hard to understand and will even harder to understand if we use loops. So let's provide useful feedback to the user to help reduce appmap size. Even if the stack does not overflow we still log a summary of the events at the info level.
  • Loading branch information
lachrist committed Oct 20, 2022
1 parent b673238 commit 7a4e2b6
Show file tree
Hide file tree
Showing 4 changed files with 192 additions and 11 deletions.
107 changes: 104 additions & 3 deletions components/trace/appmap/index.mjs
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
const { URL, Error, Set } = globalThis;
const {
URL,
Error,
Set,
Map,
Array: { from: toArray },
String,
Math: { round },
RangeError,
} = globalThis;

const { search: __search } = new URL(import.meta.url);

const { hasOwnProperty } = await import(`../../util/index.mjs${__search}`);
const { logDebug } = await import(`../../log/index.mjs${__search}`);
const { logDebug, logInfo } = await import(`../../log/index.mjs${__search}`);
const { expect } = await import(`../../expect/index.mjs${__search}`);
const { validateAppmap } = await import(
`../../validate-appmap/index.mjs${__search}`
);
Expand All @@ -19,6 +29,36 @@ const { orderEventArray } = await import(`./ordering/index.mjs${__search}`);

const VERSION = "1.8.0";

const summary_template =
"Received %j raw events.\n\nEvent Repartition:\n%f\n\nApply Repartition:\n%f\n";

const stackoverflow_template = `We cannot process your appmap because it has too many (nested) events.
There is three ways to solve this issue:
* You could tweak the \`appmap.yml\` configuration file to record fewer events:
\`\`\`yaml
# disable asynchronous jump recording
ordering: chronological
# exclude anonymous functions
anonymous-name-separator: '-'
exclude:
- name: '-'
# exclude functions in dependencies
packages:
- glob: 'node_modules/**/*'
enabled: false
\`\`\`
* You could reduce the scope of the recording.
For instance, by splitting test files or reducing the time span of remote recording.
* You could increase the callstack size of this node process.
This can be done via the node \`--stack-size\` cli option.
\`\`\`
> node --stack-size=5000 node_modules/bin/appmap-agent-js -- npm run test
\`\`\`
NB: Unfortunately, \`--stack-size\` cannot be set via the \`NODE_OPTIONS\` environment variable.
${summary_template}`;

export const compileTrace = (configuration, messages) => {
logDebug(
"Trace:\n configuration = %j\n messages = %j",
Expand Down Expand Up @@ -93,13 +133,74 @@ export const compileTrace = (configuration, messages) => {
routes.add(event.payload.function);
}
}
const printEventRepartition = () => {
const counters = new Map();
for (const { payload } of events) {
const { type } = payload;
counters.set(type, (counters.has(type) ? counters.get(type) : 0) + 1);
}
const { length: total } = events;
return toArray(counters.keys())
.map(
(key) =>
` - ${key}: ${String(counters.get(key))} [${String(
round((100 * counters.get(key)) / total),
)}%]`,
)
.join("\n");
};
const printApplyRepartition = () => {
const counters = new Map();
let total = 0;
for (const { payload } of events) {
if (payload.type === "apply") {
total += 1;
const { function: location } = payload;
counters.set(
location,
(counters.has(location) ? counters.get(location) : 0) + 1,
);
}
}
return toArray(counters.keys())
.sort((key1, key2) => counters.get(key1) - counters.get(key2))
.slice(0, 20)
.map(
(key) =>
` - ${key}: ${String(counters.get(key))} [${String(
round((100 * counters.get(key)) / total),
)}%]`,
)
.join("\n");
};
let digested_events = null;
/* c8 ignore start */
try {
digested_events = digestEventTrace(orderEventArray(events), classmap);
} catch (error) {
expect(
!(error instanceof RangeError),
stackoverflow_template,
events.length,
printEventRepartition,
printApplyRepartition,
);
throw error;
}
/* c8 ignore stop */
const appmap = {
version: VERSION,
metadata: compileMetadata(configuration, errors, status),
classMap: compileClassmap(classmap, routes),
events: digestEventTrace(orderEventArray(events), classmap),
events: digested_events,
};
validateAppmap(appmap);
logInfo(
summary_template,
events.length,
printEventRepartition,
printApplyRepartition,
);
return {
head: configuration,
body: appmap,
Expand Down
82 changes: 74 additions & 8 deletions components/trace/appmap/index.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,21 @@ assertDeepEqual(
payload: {
type: "apply",
function: location,
this: { type: "string", print: "THIS-PRINT" },
arguments: [{ type: "string", print: "ARG-PRINT" }],
this: { type: "string", print: "THIS-PRINT-1" },
arguments: [{ type: "string", print: "ARG-PRINT-1" }],
},
},
{
type: "event",
site: "begin",
tab: tabs.event2,
group: 0,
time: 0,
payload: {
type: "apply",
function: location,
this: { type: "string", print: "this-print-2" },
arguments: [{ type: "string", print: "arg-print-2" }],
},
},
{
Expand All @@ -85,6 +98,21 @@ assertDeepEqual(
child: 1,
description: "description",
},
{
type: "event",
site: "end",
tab: tabs.event2,
time: 0,
group: 0,
payload: {
type: "return",
function: location,
result: {
type: "string",
print: "result-print-2",
},
},
},
{
type: "event",
site: "end",
Expand All @@ -96,7 +124,7 @@ assertDeepEqual(
function: location,
result: {
type: "string",
print: "result-print",
print: "result-print-1",
},
},
},
Expand All @@ -107,8 +135,8 @@ assertDeepEqual(
payload: {
type: "apply",
function: location,
this: { type: "string", print: "this-print" },
arguments: [{ type: "string", print: "arg-print" }],
this: { type: "string", print: "this-print-1" },
arguments: [{ type: "string", print: "arg-print-1" }],
},
},
{
Expand Down Expand Up @@ -188,19 +216,57 @@ assertDeepEqual(
name: "this",
class: "string",
object_id: null,
value: "this-print",
value: "this-print-1",
},
parameters: [
{
name: "x",
class: "string",
object_id: null,
value: "arg-print",
value: "arg-print-1",
},
],
},
{
id: 2,
event: "call",
thread_id: 0,
defined_class: "f",
method_id: "$",
path: "filename.js",
lineno: 1,
static: false,
receiver: {
name: "this",
class: "string",
object_id: null,
value: "this-print-2",
},
parameters: [
{
name: "x",
class: "string",
object_id: null,
value: "arg-print-2",
},
],
},
{
id: 3,
event: "return",
thread_id: 0,
parent_id: 2,
elapsed: 0,
return_value: {
name: "return",
class: "string",
object_id: null,
value: "result-print-2",
},
exceptions: null,
},
{
id: 4,
event: "return",
thread_id: 0,
parent_id: 1,
Expand All @@ -209,7 +275,7 @@ assertDeepEqual(
name: "return",
class: "string",
object_id: null,
value: "result-print",
value: "result-print-1",
},
exceptions: null,
},
Expand Down
5 changes: 5 additions & 0 deletions components/util/default/format.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ export const format = (template, values) => {
assert(typeof value === "string", "expected a string for format");
return value;
}
if (marker === "f") {
const print = value();
assert(typeof print === "string", "expected a string as result");
return print;
}
if (marker === "j") {
return stringifyJSON(value);
}
Expand Down
9 changes: 9 additions & 0 deletions components/util/default/format.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ assertThrow(() =>

assertEqual(format("%%", []), "%");

// %f //

assertEqual(format("%f", [() => "foo"]), "foo");

assertThrow(
() => format("%f", [() => 123]),
/^AssertionError: expected a string as result/u,
);

// %s //

assertEqual(format("%s", ["foo"]), "foo");
Expand Down

0 comments on commit 7a4e2b6

Please sign in to comment.