From b14e965baf1645762c9bd16a57c6be655274ec98 Mon Sep 17 00:00:00 2001 From: Laurent Christophe Date: Fri, 12 Nov 2021 14:17:32 +0100 Subject: [PATCH] feat: improved exclusion mechanism Exclusion by name now acts on a code object and its descendent. Uncalled function pruning now acts on function granularity rather than file granularity. --- .../trace/appmap/classmap/exclusion.mjs | 21 +- .../trace/appmap/classmap/exclusion.test.mjs | 44 +++- components/trace/appmap/classmap/index.mjs | 234 ++++++++++-------- .../trace/appmap/classmap/index.test.mjs | 203 ++++++++++----- components/trace/appmap/event/index.mjs | 106 ++++---- components/trace/appmap/event/index.test.mjs | 39 +++ 6 files changed, 426 insertions(+), 221 deletions(-) diff --git a/components/trace/appmap/classmap/exclusion.mjs b/components/trace/appmap/classmap/exclusion.mjs index 77bcf2c46..ddf36be07 100644 --- a/components/trace/appmap/classmap/exclusion.mjs +++ b/components/trace/appmap/classmap/exclusion.mjs @@ -8,6 +8,24 @@ const regexp = /^(\p{ID_Start}\p{ID_Continue}*[#.])?\p{ID_Start}\p{ID_Continue}*$/u; export default (dependencies) => { + const { + util: { assert }, + } = dependencies; + const getName = (entity, parent) => { + if (entity.type === "class") { + return entity.name; + } + if (entity.type === "function") { + if (parent === null || parent.type === "function") { + return entity.name; + } + if (parent.type === "class") { + return `${parent.name}${entity.static ? "#" : "."}${entity.name}`; + } + assert(false, "getName called on invalid parent entity"); + } + assert(false, "getName called on invalid entity"); + }; return { createExclusion: (inputs) => { const names = new _Set(); @@ -24,7 +42,8 @@ export default (dependencies) => { } return { names, regexps }; }, - isExcluded: ({ names, regexps }, name) => { + isExcluded: ({ names, regexps }, entity, parent) => { + const name = getName(entity, parent); if (names.has(name)) { return true; } diff --git a/components/trace/appmap/classmap/exclusion.test.mjs b/components/trace/appmap/classmap/exclusion.test.mjs index 9ae37ae83..2638979eb 100644 --- a/components/trace/appmap/classmap/exclusion.test.mjs +++ b/components/trace/appmap/classmap/exclusion.test.mjs @@ -2,18 +2,50 @@ import { strict as Assert } from "assert"; import { buildTestDependenciesAsync } from "../../../build.mjs"; import Exclusion from "./exclusion.mjs"; -const { equal: assertEqual } = Assert; +const { equal: assertEqual, throws: assertThrows } = Assert; const { createExclusion, isExcluded } = Exclusion( await buildTestDependenciesAsync(import.meta.url), ); -const exclusion = createExclusion(["foo#bar", "foo#bar", "(qux)$", "(qux)$"]); +const exclusion = createExclusion(["foo#bar", "foo#bar", "^qux$", "^qux$"]); -assertEqual(isExcluded(exclusion, "foo#bar"), true); +for (const excluded of [true, false]) { + assertEqual( + isExcluded( + exclusion, + { type: "function", name: "bar", static: excluded }, + { type: "class", name: "foo" }, + ), + excluded, + ); +} -assertEqual(isExcluded(exclusion, "qux"), true); +assertEqual( + isExcluded( + exclusion, + { type: "class", name: "qux" }, + { type: "class", name: "baz" }, + ), + true, +); + +assertEqual( + isExcluded(exclusion, { type: "function", name: "qux" }, null), + true, +); -assertEqual(isExcluded(exclusion, "foo#qux"), true); +assertEqual( + isExcluded( + exclusion, + { type: "function", name: "qux" }, + { type: "function" }, + ), + true, +); + +assertThrows(() => isExcluded(exclusion, { type: "foo" }, null)); -assertEqual(isExcluded(exclusion, "qux#bar"), false); +assertThrows(() => + isExcluded(exclusion, { type: "function" }, { type: "foo" }), +); diff --git a/components/trace/appmap/classmap/index.mjs b/components/trace/appmap/classmap/index.mjs index d2fddac43..406895073 100644 --- a/components/trace/appmap/classmap/index.mjs +++ b/components/trace/appmap/classmap/index.mjs @@ -5,7 +5,9 @@ import Exclusion from "./exclusion.mjs"; const _Set = Set; const _Map = Map; +const _URL = URL; const _undefined = undefined; +const { from: toArray } = Array; export default (dependencies) => { const { @@ -20,6 +22,7 @@ export default (dependencies) => { } = dependencies; const { createExclusion, isExcluded } = Exclusion(dependencies); const printCommentArray = (comments) => { + /* c8 ignore start */ const { length } = comments; if (length === 0) { return null; @@ -27,85 +30,109 @@ export default (dependencies) => { if (length === 1) { return comments[0]; } - /* c8 ignore start */ return comments.join("\n"); /* c8 ignore stop */ }; const { extractEstreeEntityArray } = Estree(dependencies); - const default_closure_information = { - excluded: true, - shallow: true, - parameters: [], - link: null, - }; - const transformEntity = (entity, parent, context) => { - const { type, name, children } = entity; - const transformChildEntity = (child) => - transformEntity(child, entity, context); - if (type === "class") { - return { - type, - name, - children: children.map(transformChildEntity), - }; + const generateCutContent = + (content) => + ([start, end]) => + content.substring(start, end); + const excludeEntity = (entity, parent, exclusion, excluded) => { + if (isExcluded(exclusion, entity, parent)) { + excluded.push(entity); + return []; } - if (type === "function") { - const { - closures, - cutContent, - placeholder, - shallow, - path, - url, - exclusion, - inline, - } = context; - const { - parameters, - labels, - static: _static, - comments, - range, - line, - column, - } = entity; - closures.set(stringifyLocation(makeLocation(url, line, column)), { - parameters: parameters.map(cutContent), - shallow, - excluded: isExcluded( - exclusion, - parent !== null && parent.type === "class" - ? `${parent.name}${_static ? "#" : "."}${name}` - : name, + return [ + { + ...entity, + children: entity.children.flatMap((child) => + excludeEntity(child, entity, exclusion, excluded), ), - link: { - method_id: placeholder, - path, - lineno: line, - defined_class: name, - static: _static, - }, - }); - return { - type: "class", - name, - children: [ - { - type: "function", - name: placeholder, - location: `${path}:${line}`, + }, + ]; + }; + const registerExcludedEntityArray = (entities, { url }, closures) => { + const registerExcludedEntity = (entity) => { + const { type, children } = entity; + if (type === "function") { + const { line, column } = entity; + closures.set(stringifyLocation(makeLocation(url, line, column)), null); + } + children.forEach(registerExcludedEntity); + }; + entities.forEach(registerExcludedEntity); + }; + const registerEntityArray = ( + entities, + { content, shallow, placeholder, path, url }, + closures, + ) => { + const cutContent = generateCutContent(content); + const registerEntity = (entity) => { + const { type, children } = entity; + if (type === "function") { + const { line, column, parameters, name, static: _static } = entity; + closures.set(stringifyLocation(makeLocation(url, line, column)), { + parameters: parameters.map(cutContent), + shallow: shallow, + link: { + method_id: placeholder, + path: path, + lineno: line, + defined_class: name, static: _static, - source: inline ? cutContent(range) : null, - comment: printCommentArray(comments), - labels, }, - ...children.map(transformChildEntity), - ], - }; - } - /* c8 ignore start */ - assert(false, "invalid entity type"); - /* c8 ignore stop */ + }); + } + children.forEach(registerEntity); + }; + entities.forEach(registerEntity); + }; + + const filterCalledEntityArray = (entities, { url }, callees) => { + const isEntityCalled = (entity) => + entity.type !== "function" || + callees.has( + stringifyLocation(makeLocation(url, entity.line, entity.column)), + ); + const filterCalledEntityChildren = (entity) => ({ + ...entity, + children: entity.children + .filter(isEntityCalled) + .map(filterCalledEntityChildren), + }); + return entities.filter(isEntityCalled).map(filterCalledEntityChildren); + }; + + const compileEntityArray = ( + entities, + { placeholder, path, inline, content }, + ) => { + const cutContent = generateCutContent(content); + const compileEntity = (entity) => + entity.type === "function" + ? { + type: "class", + name: entity.name, + children: [ + { + type: "function", + name: placeholder, + location: `${path}:${entity.line}`, + static: entity.static, + source: inline ? cutContent(entity.range) : null, + comment: printCommentArray(entity.comments), + labels: entity.labels, + }, + ...entity.children.map(compileEntity), + ], + } + : { + ...entity, + children: entity.children.map(compileEntity), + }; + return entities.map(compileEntity); }; return { @@ -131,26 +158,17 @@ export default (dependencies) => { ) => { assert(!urls.has(url), "duplicate source url"); urls.add(url); - let { pathname: path } = new URL(url); - path = toRelativePath(directory, path); - const cutContent = ([start, end]) => content.substring(start, end); - sources.push({ - url, - path, - entities: extractEstreeEntityArray(path, content, naming).map( - (entity) => - transformEntity(entity, null, { - closures, - exclusion: createExclusion(exclude), - shallow, - inline, - url, - path, - placeholder, - cutContent, - }), - ), - }); + const { pathname } = new _URL(url); + const path = toRelativePath(directory, pathname); + const context = { url, path, shallow, inline, content, placeholder }; + const exclusion = createExclusion(exclude); + const excluded = []; + const entities = extractEstreeEntityArray(path, content, naming).flatMap( + (entity) => excludeEntity(entity, null, exclusion, excluded), + ); + registerExcludedEntityArray(excluded, context, closures); + registerEntityArray(entities, context, closures); + sources.push({ context, entities }); }, getClassmapClosure: ({ closures }, url) => { if (closures.has(url)) { @@ -161,27 +179,35 @@ export default (dependencies) => { ); if (closures.has(next_url)) { logDebug( - "Had to increase column by to fetch closure information at %j", + "Had to increase column by one to fetch closure information at %j", url, ); return closures.get(next_url); } - logWarning("Missing file information for closure at %s", url); - return default_closure_information; + logWarning( + "Missing file information for closure at %s, threating it as excluded.", + url, + ); + return null; }, - compileClassmap: ({ sources, pruning }, urls) => { + compileClassmap: ({ sources, pruning, closures }, urls) => { if (pruning) { - const whitelist = new Set(); - for (const url of urls) { - const { protocol, host, pathname } = new URL(url); - whitelist.add(`${protocol}//${host}${pathname}`); - } - sources = sources.filter(({ url }) => whitelist.has(url)); + urls = new Set( + toArray(urls).map((url) => + closures.has(url) + ? url + : stringifyLocation(incrementLocationColumn(parseLocation(url))), + ), + ); + sources = sources.map(({ context, entities }) => ({ + context, + entities: filterCalledEntityArray(entities, context, urls), + })); } const directories = new Set(); const root = []; - for (const { path, entities } of sources.values()) { - const dirnames = path.split("/"); + for (const { context, entities } of sources.values()) { + const dirnames = context.path.split("/"); const filename = dirnames.pop(); let children = root; for (const dirname of dirnames) { @@ -202,7 +228,7 @@ export default (dependencies) => { children.push({ type: "package", name: filename, - children: entities, + children: compileEntityArray(entities, context), }); } return root; diff --git a/components/trace/appmap/classmap/index.test.mjs b/components/trace/appmap/classmap/index.test.mjs index 8e9f6b7d2..e2dd85a2a 100644 --- a/components/trace/appmap/classmap/index.test.mjs +++ b/components/trace/appmap/classmap/index.test.mjs @@ -7,10 +7,7 @@ import Classmap from "./index.mjs"; Error.stackTraceLimit = Infinity; -const { - deepEqual: assertDeepEqual, - // equal: assertEqual, -} = Assert; +const { deepEqual: assertDeepEqual, equal: assertEqual } = Assert; const dependencies = await buildTestDependenciesAsync(import.meta.url); const { createConfiguration, extendConfiguration } = @@ -26,72 +23,118 @@ const cwd = "/cwd"; const placeholder = "$"; -const classmap = createClassmap( - extendConfiguration( - createConfiguration(cwd), - { pruning: true, "function-name-placeholder": placeholder }, - cwd, - ), -); +{ + const classmap = createClassmap( + extendConfiguration( + createConfiguration(cwd), + { pruning: true, "function-name-placeholder": placeholder }, + cwd, + ), + ); -addClassmapSource(classmap, { - url: `file://${cwd}/directory/function.js`, - content: "// comment\n function f (x) {}", - inline: true, - exclude: ["^"], - shallow: true, -}); + addClassmapSource(classmap, { + url: `file://${cwd}/directory/function.js`, + content: "const o = { f: \n function (x) {} , g: \n function (y) {} };", + inline: false, + exclude: ["o.g"], + shallow: true, + }); -getClassmapClosure(classmap, `file://${cwd}/directory/function.js#0-0`); + assertEqual( + getClassmapClosure(classmap, `file://${cwd}/directory/function.js#1-1`), + null, + ); -assertDeepEqual( - getClassmapClosure(classmap, `file://${cwd}/directory/function.js#2-1`), - { - excluded: true, - parameters: ["x"], - shallow: true, - link: { - defined_class: "f", - method_id: placeholder, - path: "directory/function.js", - lineno: 2, - static: false, + assertDeepEqual( + getClassmapClosure(classmap, `file://${cwd}/directory/function.js#2-1`), + { + parameters: ["x"], + shallow: true, + link: { + defined_class: "f", + method_id: placeholder, + path: "directory/function.js", + lineno: 2, + static: false, + }, }, - }, -); + ); -assertDeepEqual( - getClassmapClosure(classmap, `file://${cwd}/directory/function.js#2-0`), - getClassmapClosure(classmap, `file://${cwd}/directory/function.js#2-1`), -); + assertDeepEqual( + getClassmapClosure(classmap, `file://${cwd}/directory/function.js#3-1`), + null, + ); -addClassmapSource(classmap, { - url: `file://${cwd}/directory/class.js`, - content: "class c {static m1 () {}};\nconst o = { m2 () {} };", - inline: false, - exclude: ["c#m1"], - shallow: false, -}); + assertDeepEqual( + getClassmapClosure(classmap, `file://${cwd}/directory/function.js#2-0`), + getClassmapClosure(classmap, `file://${cwd}/directory/function.js#2-1`), + ); -assertDeepEqual( - getClassmapClosure(classmap, `file://${cwd}/directory/class.js#1-19`), - { - excluded: true, - parameters: [], + assertDeepEqual( + compileClassmap( + classmap, + new Set([ + `file://${cwd}/directory/function.js#2-0`, + `file://${cwd}/directory/function.js#2-1`, + ]), + ), + [ + { + type: "package", + name: "directory", + children: [ + { + type: "package", + name: "function.js", + children: [ + { + type: "class", + name: "o", + children: [ + { + type: "class", + name: "f", + children: [ + { + type: "function", + name: placeholder, + comment: null, + labels: [], + source: null, + static: false, + location: "directory/function.js:2", + }, + ], + }, + ], + }, + ], + }, + ], + }, + ], + ); +} + +{ + const classmap = createClassmap( + extendConfiguration( + createConfiguration(cwd), + { pruning: false, "function-name-placeholder": placeholder }, + cwd, + ), + ); + + addClassmapSource(classmap, { + url: `file://${cwd}/directory/function.js`, + content: + "function f () {} /* comment1 */ \n function g () {} /* comment2 */ /* comment3 */ \n function h () {}", + inline: true, + exclude: [], shallow: false, - link: { - defined_class: "m1", - method_id: placeholder, - path: "directory/class.js", - lineno: 1, - static: true, - }, - }, -); + }); -assertDeepEqual( - compileClassmap(classmap, [`file://${cwd}/directory/function.js#2-0`]), - [ + assertDeepEqual(compileClassmap(classmap, new Set([])), [ { type: "package", name: "directory", @@ -107,17 +150,47 @@ assertDeepEqual( { type: "function", name: placeholder, - comment: "// comment", + comment: null, labels: [], - source: "function f (x) {}", + source: "function f () {}", + static: false, + location: "directory/function.js:1", + }, + ], + }, + { + type: "class", + name: "g", + children: [ + { + type: "function", + name: placeholder, + comment: "/* comment1 */", + labels: [], + source: "function g () {}", static: false, location: "directory/function.js:2", }, ], }, + { + type: "class", + name: "h", + children: [ + { + type: "function", + name: placeholder, + comment: "/* comment2 */\n/* comment3 */", + labels: [], + source: "function h () {}", + static: false, + location: "directory/function.js:3", + }, + ], + }, ], }, ], }, - ], -); + ]); +} diff --git a/components/trace/appmap/event/index.mjs b/components/trace/appmap/event/index.mjs index 8c0e1e3d6..89ae00f3e 100644 --- a/components/trace/appmap/event/index.mjs +++ b/components/trace/appmap/event/index.mjs @@ -8,61 +8,77 @@ export default (dependencies) => { const { getClassmapClosure } = Classmap(dependencies); const { compileCallData, compileReturnData } = Data(dependencies); + const isLastShallow = (stack) => { + for (let index = stack.length - 1; index >= 0; index -= 1) { + const { shallow } = stack[index]; + if (shallow !== null) { + return shallow; + } + } + return false; + }; + const compileEventTrace = (events, classmap) => { - const stack = []; + let counter = 0; const digest = []; - const isLastShallow = (stack) => { - for (let index = stack.length - 1; index >= 0; index -= 1) { - const { shallow } = stack[index]; - if (shallow !== null) { - return shallow; - } + const digestCallEvent = (data, options) => { + const id = (counter += 1); + digest.push({ + event: "call", + thread_id: 0, + id, + ...compileCallData(data, options), + }); + return id; + }; + const digestReturnEvent = (time, data, { time: initial_time, id }) => { + if (id !== null) { + digest.push({ + event: "return", + thread_id: 0, + id: (counter += 1), + parent_id: id, + elapsed: (time - initial_time) / 1000, + ...compileReturnData(data, null), + }); } - return false; }; - let counter = 0; + const stack = []; for (const event of events) { const { type, time, data } = event; const { type: data_type } = data; - if (data_type !== "bundle" && data_type !== "jump") { - if (type === "begin" || type === "before") { - let shallow = null; - let skip = false; - let options = null; + if (data_type !== "jump" && data_type !== "bundle") { + if (type === "before" || type === "begin") { if (data_type === "apply") { - const { function: route } = data; - let excluded; - ({ shallow, excluded, ...options } = getClassmapClosure( - classmap, - route, - )); - skip = excluded || (shallow && isLastShallow(stack)); - } - let id = null; - if (!skip) { - id = counter += 1; - digest.push({ - event: "call", - thread_id: 0, - id, - ...compileCallData(data, options), - }); - } - stack.push({ time, shallow, id }); - } else { - assert(type === "after" || type === "end", "invalid event type"); - const { time: initial_time, id } = stack.pop(); - if (id !== null) { - digest.push({ - event: "return", - thread_id: 0, - id: (counter += 1), - parent_id: id, - elapsed: (time - initial_time) / 1000, - ...compileReturnData(data, null), + assert(type === "begin", "invalid envent type for apply data type"); + const { function: location } = data; + const info = getClassmapClosure(classmap, location); + /* c8 ignore start */ if (info === null) { + stack.push({ time, shallow: null, id: null }); + } /* c8 ignore stop */ else { + const { shallow, ...options } = info; + if (shallow && isLastShallow(stack)) { + stack.push({ time, shallow: true, id: null }); + } else { + stack.push({ + time, + shallow: true, + id: digestCallEvent(data, options), + }); + } + } + } else { + stack.push({ + time, + shallow: null, + id: digestCallEvent(data, null), }); } - } + } else if (type === "after" || type === "end") { + digestReturnEvent(time, data, stack.pop()); + } /* c8 ignore start */ else { + assert(false, "invalid event type"); + } /* c8 ignore stop */ } } return digest; diff --git a/components/trace/appmap/event/index.test.mjs b/components/trace/appmap/event/index.test.mjs index bee00af6f..cfe320a17 100644 --- a/components/trace/appmap/event/index.test.mjs +++ b/components/trace/appmap/event/index.test.mjs @@ -83,6 +83,26 @@ const { compileEventTrace } = Event(dependencies); index: 1, time: 10, }, + { + type: "begin", + index: 3, + time: 0, + data: { + type: "query", + database: "database", + version: "version", + sql: "sql", + parameters: [], + }, + }, + { + type: "end", + index: 3, + time: 0, + data: { + type: "query", + }, + }, ], classmap, ), @@ -125,6 +145,25 @@ const { compileEventTrace } = Event(dependencies); }, exceptions: null, }, + { + event: "call", + id: 3, + message: [], + sql_query: { + database_type: "database", + explain_sql: null, + server_version: "version", + sql: "sql", + }, + thread_id: 0, + }, + { + elapsed: 0, + event: "return", + id: 4, + parent_id: 3, + thread_id: 0, + }, ], ); }