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

Commit

Permalink
fix: Add debugging information for sourcemapper (#977)
Browse files Browse the repository at this point in the history 
* fix: Add debugging information for sourcemapper

* Fix lint issue

* Separate the debugging info related test cases in sourcemapper

* Use match instead of indexOf, use normalize for windows in tests

* Use indexof for windows in test
  • Loading branch information
@Louis-Ye
Louis-Ye committed Jun 18, 2021
1 parent afc5e3a commit b647106
Show file tree
Hide file tree
Showing 16 changed files with 243 additions and 223 deletions.
8 changes: 4 additions & 4 deletions src/agent/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,10 +359,10 @@ export interface ResolvedDebugAgentConfig extends GoogleAuthOptions {
apiUrl?: string;

/**
* Number of times the V8 pauses (could be breakpoint hits) before the
* debugging session is reset. This is to release the memory usage held by V8
* engine for each breakpoint hit to prevent the memory leak. The default
* value is specified in defaultConfig.
* Number of times of the V8 breakpoint hits events before resetting the
* breakpoints. This is to release the memory usage held by V8 engine for each
* breakpoint hit to prevent the memory leak. The default value is specified
* in defaultConfig.
*/
resetV8DebuggerThreshold: number;
}
Expand Down
3 changes: 2 additions & 1 deletion src/agent/debuglet.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,12 +396,13 @@ export class Debuglet extends EventEmitter {

let mapper;
try {
mapper = await SourceMapper.create(findResults.mapFiles);
mapper = await SourceMapper.create(findResults.mapFiles, that.logger);
} catch (err3) {
that.logger.error('Error processing the sourcemaps.', err3);
that.emit('initError', err3);
return;
}

that.v8debug = debugapi.create(
that.logger,
that.config,
Expand Down
46 changes: 35 additions & 11 deletions src/agent/io/sourcemapper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import * as path from 'path';
import {promisify} from 'util';
import * as sourceMap from 'source-map';

import {Logger} from '../config';
import {findScriptsFuzzy} from '../util/utils';

const CONCURRENCY = 10;
Expand Down Expand Up @@ -167,17 +168,10 @@ async function processSourcemap(
}

export class SourceMapper {
/** Maps each original source path to the corresponding source map info. */
infoMap: Map<string, MapInfoInput>;

/**
* @param {Array.<string>} sourcemapPaths An array of paths to .map sourcemap
* files that should be processed. The paths should be relative to the
* current process's current working directory
* @param {Logger} logger A logger that reports errors that occurred while
* processing the given sourcemap files
* @constructor
*/
constructor() {
constructor(readonly logger: Logger) {
this.infoMap = new Map();
}

Expand Down Expand Up @@ -211,6 +205,8 @@ export class SourceMapper {
Array.from(this.infoMap.keys())
);

this.logger.debug(`sourcemapper fuzzy matches: ${matches}`);

if (matches.length === 1) {
return this.infoMap.get(matches[0]) as MapInfoInput;
}
Expand Down Expand Up @@ -246,6 +242,8 @@ export class SourceMapper {
colNumber: number,
entry: MapInfoInput
): MapInfoOutput | null {
this.logger.debug(`sourcemapper inputPath: ${inputPath}`);

inputPath = path.normalize(inputPath);

const relPath = path
Expand Down Expand Up @@ -273,6 +271,8 @@ export class SourceMapper {
column: colNumber, // to be zero-based
};

this.logger.debug(`sourcemapper sourcePos: ${JSON.stringify(sourcePos)}`);

const allPos = entry.mapConsumer.allGeneratedPositionsFor(sourcePos);
/*
* Based on testing, it appears that the following code is needed to
Expand All @@ -290,6 +290,8 @@ export class SourceMapper {
})
: entry.mapConsumer.generatedPositionFor(sourcePos);

this.logger.debug(`sourcemapper mappedPos: ${JSON.stringify(mappedPos)}`);

return {
file: entry.outputFile,
line: (mappedPos.line ?? 0) - 1, // convert the one-based line numbers returned
Expand All @@ -305,11 +307,32 @@ export class SourceMapper {
// output
};
}

/** Prints the debugging information of the source mapper to the logger. */
debug() {
this.logger.debug('Printing source mapper debugging information ...');
for (const [key, value] of this.infoMap) {
this.logger.debug(` source ${key}:`);
this.logger.debug(` outputFile: ${value.outputFile}`);
this.logger.debug(` mapFile: ${value.mapFile}`);
this.logger.debug(` sources: ${value.sources}`);
}
}
}

export async function create(sourcemapPaths: string[]): Promise<SourceMapper> {
/**
* @param {Array.<string>} sourcemapPaths An array of paths to .map sourcemap
* files that should be processed. The paths should be relative to the
* current process's current working directory
* @param {Logger} logger A logger that reports errors that occurred while
* processing the given sourcemap files
*/
export async function create(
sourcemapPaths: string[],
logger: Logger
): Promise<SourceMapper> {
const limit = pLimit(CONCURRENCY);
const mapper = new SourceMapper();
const mapper = new SourceMapper(logger);
const promises = sourcemapPaths.map(path =>
limit(() => processSourcemap(mapper.infoMap, path))
);
Expand All @@ -320,5 +343,6 @@ export async function create(sourcemapPaths: string[]): Promise<SourceMapper> {
'An error occurred while processing the sourcemap files' + err
);
}
mapper.debug();
return mapper;
}
130 changes: 101 additions & 29 deletions src/agent/v8/inspector-debugapi.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,32 @@ import * as utils from '../util/utils';
import * as debugapi from './debugapi';
import {V8Inspector} from './v8inspector';

/**
* An interface that describes options that set behavior when interacting with
* the V8 Inspector API.
*/
interface InspectorOptions {
/**
* Whether to add a 'file://' prefix to a URL when setting breakpoints.
*/
useWellFormattedUrl: boolean;
}

/** Data related to the v8 inspector. */
interface V8Data {
session: inspector.Session;
// Options for behavior when interfacing with the Inspector API.
inspectorOptions: InspectorOptions;
inspector: V8Inspector;
// Store the v8 setBreakpoint parameters for each v8 breakpoint so that later
// the recorded parameters can be used to reset the breakpoints.
setBreakpointsParams: {
[
v8BreakpointId: string
]: inspector.Debugger.SetBreakpointByUrlParameterType;
};
}

/**
* In older versions of Node, the script source as seen by the Inspector
* backend is wrapped in `require('module').wrapper`, and in new versions
Expand Down Expand Up @@ -73,9 +99,8 @@ export class InspectorDebugApi implements debugapi.DebugApi {
// stackdriver breakpoint id.
breakpointMapper: {[id: string]: stackdriver.BreakpointId[]} = {};
numBreakpoints = 0;

// The wrapper class that interacts with the V8 debugger.
inspector: V8Inspector;
numBreakpointHitsBeforeReset = 0;
v8: V8Data;

constructor(
logger: consoleLogLevel.Logger,
Expand All @@ -87,28 +112,36 @@ export class InspectorDebugApi implements debugapi.DebugApi {
this.config = config;
this.fileStats = jsFiles;
this.sourcemapper = sourcemapper;
this.inspector = new V8Inspector(
/* logger=*/ logger,
/*useWellFormattedUrl=*/ utils.satisfies(process.version, '>10.11.0'),
/*resetV8DebuggerThreshold=*/ this.config.resetV8DebuggerThreshold,
/*onScriptParsed=*/
scriptParams => {
this.scriptMapper[scriptParams.scriptId] = scriptParams;
},
/*onPaused=*/
messageParams => {
try {
this.handleDebugPausedEvent(messageParams);
} catch (error) {
this.logger.error(error);
}
}
);
this.scriptMapper = {};
this.v8 = this.createV8Data();
}

/** Disconnects and marks the current V8 data as not connected. */
disconnect(): void {
this.inspector.detach();
/** Creates a new V8 Debugging session and the related data. */
private createV8Data(): V8Data {
const session = new inspector.Session();
session.connect();
session.on('Debugger.scriptParsed', script => {
this.scriptMapper[script.params.scriptId] = script.params;
});
session.post('Debugger.enable');
session.post('Debugger.setBreakpointsActive', {active: true});
session.on('Debugger.paused', message => {
try {
this.handleDebugPausedEvent(message.params);
} catch (error) {
this.logger.error(error);
}
});

return {
session,
inspectorOptions: {
// Well-Formatted URL is required in Node 10.11.1+.
useWellFormattedUrl: utils.satisfies(process.version, '>10.11.0'),
},
inspector: new V8Inspector(session),
setBreakpointsParams: {},
};
}

set(
Expand Down Expand Up @@ -235,7 +268,8 @@ export class InspectorDebugApi implements debugapi.DebugApi {
if (!this.breakpointMapper[breakpointData.id]) {
// When breakpointmapper does not countain current v8/inspector breakpoint
// id, we should remove this breakpoint from v8.
result = this.inspector.removeBreakpoint(breakpointData.id);
result = this.v8.inspector.removeBreakpoint(breakpointData.id);
delete this.v8.setBreakpointsParams[breakpointData.id];
}
delete this.breakpoints[breakpoint.id];
delete this.listeners[breakpoint.id];
Expand Down Expand Up @@ -312,6 +346,10 @@ export class InspectorDebugApi implements debugapi.DebugApi {
this.listeners[breakpoint.id] = {enabled: true, listener};
}

disconnect(): void {
this.v8.session.disconnect();
}

numBreakpoints_(): number {
// Tracks the number of stackdriver breakpoints.
return Object.keys(this.breakpoints).length;
Expand Down Expand Up @@ -499,7 +537,7 @@ export class InspectorDebugApi implements debugapi.DebugApi {
let v8BreakpointId; // v8/inspector breakpoint id
if (!this.locationMapper[locationStr]) {
// The first time when a breakpoint was set to this location.
const rawUrl = this.inspector.shouldUseWellFormattedUrl()
const rawUrl = this.v8.inspectorOptions.useWellFormattedUrl
? `file://${matchingScript}`
: matchingScript;
// on windows on Node 11+, the url must start with file:///
Expand All @@ -508,17 +546,19 @@ export class InspectorDebugApi implements debugapi.DebugApi {
process.platform === 'win32' && utils.satisfies(process.version, '>=11')
? rawUrl.replace(/^file:\/\//, 'file:///').replace(/\\/g, '/')
: rawUrl;
const res = this.inspector.setBreakpointByUrl({
const params = {
lineNumber: line - 1,
url,
columnNumber: column - 1,
condition: breakpoint.condition || undefined,
});
};
const res = this.v8.inspector.setBreakpointByUrl(params);
if (res.error || !res.response) {
// Error case.
return null;
}
v8BreakpointId = res.response.breakpointId;
this.v8.setBreakpointsParams[v8BreakpointId] = params;

this.locationMapper[locationStr] = [];
this.breakpointMapper[v8BreakpointId] = [];
Expand Down Expand Up @@ -606,7 +646,7 @@ export class InspectorDebugApi implements debugapi.DebugApi {
const evaluatedExpressions = breakpoint.expressions.map(exp => {
// returnByValue is set to true here so that the JSON string of the
// value will be returned to log.
const result = state.evaluate(exp, frame, that.inspector, true);
const result = state.evaluate(exp, frame, that.v8.inspector, true);
if (result.error) {
return result.error;
} else {
Expand All @@ -621,7 +661,7 @@ export class InspectorDebugApi implements debugapi.DebugApi {
breakpoint,
this.config,
this.scriptMapper,
this.inspector
this.v8.inspector
);
if (
breakpoint.location &&
Expand Down Expand Up @@ -655,5 +695,37 @@ export class InspectorDebugApi implements debugapi.DebugApi {
} catch (e) {
this.logger.warn('Internal V8 error on breakpoint event: ' + e);
}

this.tryResetV8Debugger();
}

/**
* Periodically resets breakpoints to prevent memory leaks in V8 (for holding
* contexts of previous breakpoint hits).
*/
private tryResetV8Debugger() {
this.numBreakpointHitsBeforeReset += 1;
if (
this.numBreakpointHitsBeforeReset < this.config.resetV8DebuggerThreshold
) {
return;
}
this.numBreakpointHitsBeforeReset = 0;

const storedParams = this.v8.setBreakpointsParams;

// Re-connect the session to clean the memory usage.
this.disconnect();
this.scriptMapper = {};
this.v8 = this.createV8Data();
this.v8.setBreakpointsParams = storedParams;

// Setting the v8 breakpoints again according to the stored parameters.
for (const params of Object.values(storedParams)) {
const res = this.v8.inspector.setBreakpointByUrl(params);
if (res.error || !res.response) {
this.logger.error('Error upon re-setting breakpoint: ' + res);
}
}
}
}

0 comments on commit b647106

Please sign in to comment.