Skip to content

Commit

Permalink
feat: Multiple recording support (process always active)
Browse files Browse the repository at this point in the history
_ Added always active process recording mode with an env var:
  APPMAP_RECORDER_PROCESS_ALWAYS.
- Removed the global recording: Recording instance and encapsulated
  new Recording instances (processRecording and nonProcessRecording)
  inside recorder.ts.
- Changed related hooks to work with multiple possible recordings
  using getActiveRecordings().
- Added tests for test and remote recording for always active
  process recording mode: jest.test.ts, mocha.test.ts and
  httpServer.test.ts.
  • Loading branch information
zermelo-wisen committed Apr 4, 2024
1 parent 5cfd49d commit ca759db
Show file tree
Hide file tree
Showing 20 changed files with 1,911 additions and 223 deletions.
16 changes: 9 additions & 7 deletions src/__tests__/recorder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,14 @@ describe(recorder.record, () => {
});
});

describe(recorder.fixReturnEventIfPromiseResult, () => {
describe(recorder.fixReturnEventsIfPromiseResult, () => {
it("records a fix up after the promise resolves", async () => {
const promise = Promise.resolve("resolved");
const result = recorder.fixReturnEventIfPromiseResult(
const result = recorder.fixReturnEventsIfPromiseResult(
recorder.getActiveRecordings().slice(0, 1),
promise,
returnEvent,
callEvent,
[returnEvent],
[callEvent],
getTime(),
);

Expand All @@ -76,10 +77,11 @@ describe(recorder.fixReturnEventIfPromiseResult, () => {

it("records a fix up after the promise rejects", async () => {
const promise = Promise.reject(new Error("test"));
const result = recorder.fixReturnEventIfPromiseResult(
const result = recorder.fixReturnEventsIfPromiseResult(
recorder.getActiveRecordings().slice(0, 1),
promise,
returnEvent,
callEvent,
[returnEvent],
[callEvent],
getTime(),
);
await expect(result).rejects.toThrowError("test");
Expand Down
15 changes: 8 additions & 7 deletions src/facade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@ import { isPromise } from "node:util/types";
import type * as AppMap from "./AppMap";
import { exceptionMetadata } from "./metadata";

import { recording, start } from "./recorder";
import { getActiveRecordings, getCodeBlockRecording, startCodeBlockRecording } from "./recorder";
import {
disableGlobalRecording,
startCodeBlockRecording,
stopCodeBlockRecording,
setCodeBlockRecordingActive,
unsetCodeBlockRecordingActive,
} from "./recorderControl";
import Recording from "./Recording";

// Since _this_ module is loaded, we'll do code block recording only.
recording.abandon();
// We ignore APPMAP_RECORDER_PROCESS_ALWAYS environment variable in this mode.
getActiveRecordings().forEach((r) => r.abandon());
disableGlobalRecording();

function isInstrumented() {
Expand All @@ -30,8 +30,8 @@ export function record(
if (!isInstrumented())
throw Error("Code is not instrumented. Please run the project with appmap-node.");

start(new Recording("block", "block", new Date().toISOString()));
startCodeBlockRecording();
setCodeBlockRecordingActive();
try {
const result = block();
if (isPromise(result)) return result.then(() => finishRecording(), finishRecording);
Expand All @@ -42,7 +42,8 @@ export function record(
}

function finishRecording(exn?: unknown): AppMap.AppMap | undefined {
stopCodeBlockRecording();
unsetCodeBlockRecordingActive();
const recording = getCodeBlockRecording();
if (!recording.finish()) return;

const appmap = recording.readAppMap();
Expand Down
149 changes: 74 additions & 75 deletions src/hooks/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,16 @@ import { URL } from "node:url";

import type * as AppMap from "../AppMap";
import config from "../config";
import Recording from "../Recording";
import { info, warn } from "../message";
import { parameter } from "../parameter";
import { recording, start } from "../recorder";
import {
getActiveRecordings,
getRemoteRecording,
getRequestRecording,
startProcessRecording,
startRemoteRecording,
startRequestRecording,
} from "../recorder";
import { getTime } from "../util/getTime";

type HTTP = typeof http | typeof https;
Expand Down Expand Up @@ -92,19 +98,21 @@ function handleClientRequest(request: http.ClientRequest) {
const startTime = getTime();
request.on("finish", () => {
const url = extractRequestURL(request);
const recordings = getActiveRecordings();
// recording may have finished at this point
if (!recording.running) {
if (recordings.length == 0) {
warnRecordingNotRunning(url);
return;
}

// Setting port to the default port for the protocol makes it empty string.
// See: https://nodejs.org/api/url.html#urlport
url.port = request.socket?.remotePort + "";
const clientRequestEvent = recording.httpClientRequest(
request.method,
`${url.protocol}//${url.host}${url.pathname}`,
normalizeHeaders(request.getHeaders()),

const urlString = `${url.protocol}//${url.host}${url.pathname}`;
const headers = normalizeHeaders(request.getHeaders());
const clientRequestEvents = recordings.map((r) =>
r.httpClientRequest(request.method, urlString, headers),
);

request.on("response", (response) => {
Expand All @@ -114,11 +122,27 @@ function handleClientRequest(request: http.ClientRequest) {
});

response.once("end", () => {
if (!recording.running) {
const elapsed = getTime() - startTime;
const headers = normalizeHeaders(response.headers);
const returnValue = capture.createReturnValue(
response.headers["content-type"]?.startsWith("application/json") ?? false,
);

if (getActiveRecordings().length == 0) {
warnRecordingNotRunning(url);
return;
}
handleClientResponse(clientRequestEvent, startTime, response, capture);

recordings.forEach((recording, idx) => {
assert(response.statusCode != undefined);
recording.httpClientResponse(
clientRequestEvents[idx].id,
elapsed,
response.statusCode,
headers,
returnValue,
);
});
});
});
});
Expand All @@ -135,24 +159,6 @@ function extractRequestURL(request: ClientRequest): URL {
return new URL(`${protocol}//${host}${request.path}`);
}

function handleClientResponse(
requestEvent: AppMap.HttpClientRequestEvent,
startTime: number,
response: http.IncomingMessage,
capture: BodyCapture,
): void {
assert(response.statusCode != undefined);
recording.httpClientResponse(
requestEvent.id,
getTime() - startTime,
response.statusCode,
normalizeHeaders(response.headers),
capture.createReturnValue(
response.headers["content-type"]?.startsWith("application/json") ?? false,
),
);
}

let remoteRunning = false;

// TODO: return ![next, ...].some(h => h.shouldIgnoreRequest?.(request))
Expand Down Expand Up @@ -256,22 +262,52 @@ function handleRequest(request: http.IncomingMessage, response: http.ServerRespo
if (shouldIgnoreRequest(request)) return;

const url = new URL(request.url, "http://example");
const testRunning = recording.metadata.recorder.type == "tests";
const testRunning = getActiveRecordings().some((r) => r.metadata.recorder.type == "tests");
const timestamp = remoteRunning || testRunning ? undefined : startRequestRecording(url.pathname);
const requestEvent = recording.httpRequest(
request.method,
url.pathname,
`HTTP/${request.httpVersion}`,
normalizeHeaders(request.headers),
url.searchParams,

const method = request.method;
const headers = normalizeHeaders(request.headers);
const protocol = `HTTP/${request.httpVersion}`;
const recordings = getActiveRecordings();
const requestEvents = recordings.map((r) =>
r.httpRequest(method, url.pathname, protocol, headers, url.searchParams),
);

const capture = new BodyCapture();
captureResponseBody(response, capture);

const startTime = getTime();
response.once("finish", () => {
handleResponse(request, requestEvent, startTime, timestamp, response, capture);
const contentType = response.getHeader("Content-Type");
const isJson = typeof contentType == "string" && contentType.startsWith("application/json");

const elapsed = getTime() - startTime;
const headers = normalizeHeaders(response.getHeaders());
const returnValue = capture.createReturnValue(isJson);

recordings.forEach((recording, idx) => {
if (fixupEvent(request, requestEvents[idx])) recording.fixup(requestEvents[idx]);

recording.httpResponse(
requestEvents[idx].id,
elapsed,
response.statusCode,
headers,
returnValue,
);
});

// If there is a test or remote recording we don't create a separate request recording
// appmap, so we don't have to finish it.
const testRunning = recordings.some((r) => r.metadata.recorder.type == "tests");
if (remoteRunning || testRunning) return;

const recording = getRequestRecording();
const { request_method, path_info } = requestEvents[0].http_server_request;
recording.metadata.name = `${request_method} ${path_info} (${response.statusCode}) — ${timestamp}`;
recording.finish();
info("Wrote %s", recording.path);
startProcessRecording(); // just so there's always a recording running
});
}

Expand Down Expand Up @@ -332,42 +368,6 @@ function normalizeHeaders(
return result;
}

function handleResponse(
request: http.IncomingMessage,
requestEvent: AppMap.HttpServerRequestEvent,
startTime: number,
timestamp: string | undefined,
response: http.ServerResponse<http.IncomingMessage>,
capture: BodyCapture,
): void {
if (fixupEvent(request, requestEvent)) recording.fixup(requestEvent);

const contentType = response.getHeader("Content-Type");
const isJson = typeof contentType == "string" && contentType.startsWith("application/json");

recording.httpResponse(
requestEvent.id,
getTime() - startTime,
response.statusCode,
normalizeHeaders(response.getHeaders()),
capture.createReturnValue(isJson),
);
if (remoteRunning || recording.metadata.recorder.type == "tests") return;

const { request_method, path_info } = requestEvent.http_server_request;
recording.metadata.name = `${request_method} ${path_info} (${response.statusCode}) — ${timestamp}`;
recording.finish();
info("Wrote %s", recording.path);
start(); // just so there's always a recording running
}

function startRequestRecording(pathname: string): string {
recording.abandon();
const timestamp = new Date().toISOString();
start(new Recording("requests", "requests", [timestamp, pathname].join(" ")));
return timestamp;
}

function capitalize(str: string): string {
return str[0].toUpperCase() + str.slice(1).toLowerCase();
}
Expand All @@ -387,22 +387,21 @@ function handleRemoteRecording(
res.writeHead(200);
remoteRunning = true;
res.end("Recording started");
info("Remote recording started");
recording.abandon();
start(new Recording("remote", "remote", new Date().toISOString()));
startRemoteRecording();
}
break;
case "DELETE":
if (remoteRunning) {
remoteRunning = false;
const recording = getRemoteRecording();
if (recording.finish()) {
res.writeHead(200);
const { path } = recording;
res.end(readFileSync(path));
rmSync(path);
} else res.writeHead(200).end("{}");
info("Remote recording finished");
start(); // just so there's always a recording running
startProcessRecording(); // just so there's always a recording running
} else res.writeHead(404).end("No recording is in progress");
break;
default:
Expand Down
22 changes: 10 additions & 12 deletions src/hooks/jest.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
import assert from "node:assert";
import { pathToFileURL } from "node:url";

import type { Circus } from "@jest/types";
import { simple as walk } from "acorn-walk";
import type { ESTree } from "meriyah";

import { expressionFor, wrap } from ".";
import Recording from "../Recording";
import { assignment, call_, identifier, memberId } from "../generate";
import { info } from "../message";
import { exceptionMetadata } from "../metadata";
import { recording, start } from "../recorder";
import {
abandonProcessRecordingIfNotAlwaysActive,
getTestRecording,
startTestRecording,
} from "../recorder";
import genericTranform from "../transform";
import { isId } from "../util/isId";

Expand Down Expand Up @@ -44,7 +46,7 @@ const passGlobals: ESTree.Statement[] = ["AppMap", "AppMapRecordHook"].map((name
);

export function patchCircus(program: ESTree.Program): ESTree.Program {
recording.abandon();
abandonProcessRecordingIfNotAlwaysActive();
info("Detected Jest. Tests will be automatically recorded.");
program.body.push({
type: "ExpressionStatement",
Expand All @@ -54,17 +56,18 @@ export function patchCircus(program: ESTree.Program): ESTree.Program {
}

function eventHandler(event: Circus.Event) {
let recording;
switch (event.name) {
case "test_fn_start":
start(createRecording(event.test));
startTestRecording("tests", "jest", ...testNames(event.test));
break;
case "test_fn_failure":
assert(recording.metadata.recorder.type == "tests");
recording = getTestRecording();
recording.metadata.test_status = "failed";
recording.metadata.exception = exceptionMetadata(event.error);
return recording.finish();
case "test_fn_success":
assert(recording.metadata.recorder.type == "tests");
recording = getTestRecording();
recording.metadata.test_status = "succeeded";
return recording.finish();
}
Expand All @@ -84,8 +87,3 @@ function testNames(test: Circus.TestEntry): string[] {
for (let block = test.parent; block.parent; block = block.parent) names.push(block.name);
return names.reverse();
}

function createRecording(test: Circus.TestEntry): Recording {
const recording = new Recording("tests", "jest", ...testNames(test));
return recording;
}
Loading

0 comments on commit ca759db

Please sign in to comment.