Skip to content

Commit

Permalink
feat: Add language to appmap.yml
Browse files Browse the repository at this point in the history
By default, this is set to `javascript`. Any existing configurations
will be migrated automatically.
  • Loading branch information
dustinbyrne committed May 21, 2024
1 parent 68b63cd commit a9f37b3
Show file tree
Hide file tree
Showing 21 changed files with 128 additions and 25 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ You can create `appmap.yml` config file; if not found, a default one will be cre
```yaml
name: application-name # from package.json by default
appmap_dir: tmp/appmap
language: javascript
packages:
- path: . # paths to instrument, relative to appmap.yml location
exclude: # code to exclude from instrumentation
Expand Down
19 changes: 19 additions & 0 deletions src/__tests__/__snapshots__/config.test.ts.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Config migrate does not update an existing config if the language field is already set 1`] = `
"name: test-package
language: ruby
appmap_dir: appmap
packages:
- path: .
"
`;

exports[`Config migrate sets the language field in existing configs 1`] = `
"name: test-package
appmap_dir: appmap
packages:
- path: .
language: javascript
"
`;
39 changes: 39 additions & 0 deletions src/__tests__/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ describe(Config, () => {
packages: new PackageMatcher("/test/app", [
{ path: ".", exclude: ["node_modules", ".yarn"] },
]),
language: "javascript",
});
});

Expand All @@ -41,6 +42,7 @@ describe(Config, () => {
root: fixMacOsTmpPath(dir),
relativeAppmapDir: "tmp/appmap",
appName: basename(dir),
language: "javascript",
});
});

Expand All @@ -53,6 +55,7 @@ describe(Config, () => {
root: fixMacOsTmpPath(dir),
relativeAppmapDir: "tmp/appmap",
appName: "test-package",
language: "javascript",
});
});

Expand All @@ -61,6 +64,7 @@ describe(Config, () => {
"appmap.yml",
YAML.stringify({
name: "test-package",
language: "javascript",
appmap_dir: "appmap",
packages: [{ path: ".", exclude: ["excluded"] }, "../lib"],
}),
Expand All @@ -71,6 +75,7 @@ describe(Config, () => {
expect(new Config()).toMatchObject({
root: fixMacOsTmpPath(dir),
relativeAppmapDir: "appmap",
language: "javascript",
appName: "test-package",
packages: new PackageMatcher(fixMacOsTmpPath(dir), [
{ path: ".", exclude: ["excluded"] },
Expand All @@ -84,6 +89,7 @@ describe(Config, () => {
"appmap.yml",
YAML.stringify({
name: "test-package",
language: "javascript",
appmap_dir: "appmap",
packages: [{ regexp: "foo", enabled: false }],
}),
Expand All @@ -95,6 +101,7 @@ describe(Config, () => {
root: fixMacOsTmpPath(dir),
relativeAppmapDir: "appmap",
appName: "test-package",
language: "javascript",
packages: new PackageMatcher(fixMacOsTmpPath(dir), [
{ path: ".", exclude: ["node_modules", ".yarn"] },
]),
Expand All @@ -112,6 +119,38 @@ describe(Config, () => {
expect(readFileSync("appmap.yml").toString()).toEqual(malformedAppMapFileContent);
});

describe("migrate", () => {
it("sets the language field in existing configs", () => {
writeFileSync(
"appmap.yml",
YAML.stringify({
name: "test-package",
appmap_dir: "appmap",
packages: [{ path: "." }],
}),
);
const config = new Config();
config.migrate();

expect(readFileSync("appmap.yml").toString()).toMatchSnapshot();
});

it("does not update an existing config if the language field is already set", () => {
const appmapYml = YAML.stringify({
name: "test-package",
language: "ruby",
appmap_dir: "appmap",
packages: [{ path: "." }],
});

writeFileSync("appmap.yml", appmapYml);
const config = new Config();
config.migrate();

expect(readFileSync("appmap.yml").toString()).toMatchSnapshot();
});
});

let dir: string;
beforeEach(() => {
chdir((dir = tmp.dirSync().name));
Expand Down
1 change: 1 addition & 0 deletions src/bin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export function main() {
info("Writing default config to %s", config.configPath);
writeFileSync(config.configPath, YAML.stringify(config));
} else info("Using config file %s", config.configPath);
config.migrate();
config.export();

// FIXME: Probably there should be a way to remove this altogether
Expand Down
55 changes: 41 additions & 14 deletions src/config.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import assert from "node:assert";
import { readFileSync } from "node:fs";
import { readFileSync, writeFileSync } from "node:fs";
import { basename, join, resolve } from "node:path";
import { cwd } from "node:process";

import { PackageJson } from "type-fest";
import YAML from "yaml";
import YAML, { Document } from "yaml";

import { warn } from "./message";
import { info, warn } from "./message";
import PackageMatcher, { Package, parsePackages } from "./PackageMatcher";
import locateFileUp from "./util/findFileUp";
import lazyOpt from "./util/lazyOpt";
import tryOr from "./util/tryOr";
import { isNativeError } from "node:util/types";

const responseBodyMaxLengthDefault = 10000;
const kResponseBodyMaxLengthEnvar = "APPMAP_RESPONSE_BODY_MAX_LENGTH";
Expand All @@ -24,6 +23,10 @@ export class Config {
public readonly default: boolean;
public readonly packages: PackageMatcher;
public readonly responseBodyMaxLength: number;
public readonly language: string;

private readonly document?: Document;
private migrationPending = false;

constructor(pwd = cwd()) {
const configDir = locateFileUp("appmap.yml", process.env.APPMAP_ROOT ?? pwd);
Expand All @@ -35,13 +38,17 @@ export class Config {
const root = (this.root = process.env.APPMAP_ROOT ?? configDir ?? packageDir() ?? pwd);

this.configPath = join(root, "appmap.yml");
const config = readConfigFile(this.configPath);
this.default = !config;
this.document = loadConfigDocument(this.configPath);
const config = this.document ? readConfigFile(this.document) : undefined;
this.default = !this.document;

this.relativeAppmapDir = config?.appmap_dir ?? "tmp/appmap";

this.appName = config?.name ?? targetPackage()?.name ?? basename(root);

this.language = config?.language ?? "javascript";
this.migrationPending ||= !!config && config.language === undefined;

this.packages = new PackageMatcher(
root,
config?.packages ?? [
Expand Down Expand Up @@ -115,20 +122,38 @@ export class Config {
toJSON(): ConfigFile {
return {
name: this.appName,
language: this.language,
appmap_dir: this.relativeAppmapDir,
packages: this.packages,
};
}

migrate() {
if (!this.migrationPending) return;

if (this.document) {
this.document.set("language", this.language);
}

info("appmap.yml requires migration, changes will be automatically applied.");
writeFileSync(
this.configPath,
YAML.stringify(this.document ?? this, { keepSourceTokens: true }),
);
}
}

interface ConfigFile {
appmap_dir?: string;
name?: string;
packages?: Package[];
response_body_max_length?: number;
language?: string;
}

function readConfigFile(path: string | undefined): ConfigFile | undefined {
// Maintaining the YAML document is important to preserve existing comments and formatting
// in the original file.
function loadConfigDocument(path: string | undefined): Document | undefined {
if (!path) return;

let fileContent: string;
Expand All @@ -139,22 +164,24 @@ function readConfigFile(path: string | undefined): ConfigFile | undefined {
throw exn;
}

let config;
try {
config = YAML.parse(fileContent) as unknown;
} catch (exn) {
assert(isNativeError(exn));
const document = YAML.parseDocument(fileContent, { keepSourceTokens: true });
if (document.errors.length > 0) {
const errorMessage = document.errors.map((e) => `${e.name}: ${e.message}`).join("\n");
throw new Error(
`Error parsing config file at ${path}: ${exn.message}\nYou can remove the file to use the default configuration.`,
`Failed to parse config file at ${path}\n${errorMessage}\nYou can remove the file to use the default configuration.`,
);
}
if (!config) return;
return document;
}

function readConfigFile(document: Document): ConfigFile {
const config = document.toJSON() as Record<string, unknown>;
const result: ConfigFile = {};
assert(typeof config === "object");
if ("name" in config) result.name = String(config.name);
if ("appmap_dir" in config) result.appmap_dir = String(config.appmap_dir);
if ("packages" in config) result.packages = parsePackages(config.packages);
if ("language" in config) result.language = String(config.language);
if ("response_body_max_length" in config) {
const value = parseInt(String(config.response_body_max_length));
result.response_body_max_length = value >= 0 ? value : undefined;
Expand Down
1 change: 1 addition & 0 deletions test/__snapshots__/simple.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

exports[`creating a default config file 1`] = `
"name: test
language: javascript
appmap_dir: tmp/appmap
packages:
- path: .
Expand Down
3 changes: 2 additions & 1 deletion test/codeBlock/appmap.yml
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
name: codeBlock
appmap_dir: tmp/appmap
appmap_dir: tmp/appmap
language: javascript
7 changes: 4 additions & 3 deletions test/functionLabels/appmap.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: function-labels-appmap-node-test
appmap_dir: tmp/appmap
packages:
packages:
- path: .
- module: node:console
shallow: true
Expand All @@ -12,6 +12,7 @@ packages:
- name: log
label: logging
- name: debug
labels:
labels:
- logging
- debugging
- debugging
language: javascript
1 change: 1 addition & 0 deletions test/httpClient/appmap.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
name: http-client-appmap-node-test
appmap_dir: tmp/appmap
response_body_max_length: 50
language: javascript
1 change: 1 addition & 0 deletions test/httpServer/appmap.yml
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
name: http-server-appmap-node-test
appmap_dir: tmp/appmap
language: javascript
1 change: 1 addition & 0 deletions test/jest/appmap.yml
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
name: jest-appmap-node-test
appmap_dir: tmp/appmap
language: javascript
5 changes: 3 additions & 2 deletions test/libraryCalls/appmap.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ packages:
shallow: true
- module: node:console # works with or without node: qualifier
shallow: true
exclude:
exclude:
- warn
- assert # exclude because this is captured only in windows
- assert # exclude because this is captured only in windows
language: javascript
1 change: 1 addition & 0 deletions test/mocha/appmap.yml
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
name: mocha-appmap-node-test
appmap_dir: tmp/appmap
language: javascript
1 change: 1 addition & 0 deletions test/mysql/appmap.yml
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
name: mysql-appmap-node-test
appmap_dir: tmp/appmap
language: javascript
1 change: 1 addition & 0 deletions test/next/appmap.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ packages:
exclude:
- node_modules
- .yarn
language: javascript
3 changes: 2 additions & 1 deletion test/prisma/appmap.yml
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
name: prisma-appmap-node-test
appmap_dir: tmp/appmap
appmap_dir: tmp/appmap
language: javascript
9 changes: 5 additions & 4 deletions test/simple/appmap.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
name: simple
appmap_dir: tmp/appmap
packages:
- path: .
exclude:
- skipped
- A.skippedMethod
- path: .
exclude:
- skipped
- A.skippedMethod
language: javascript
1 change: 1 addition & 0 deletions test/sqlite/appmap.yml
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
name: sqlite-appmap-node-test
appmap_dir: tmp/appmap
language: javascript
1 change: 1 addition & 0 deletions test/typescript-esm/appmap.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ packages:
exclude:
- node_modules
- .yarn
language: javascript
1 change: 1 addition & 0 deletions test/typescript/appmap.yml
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
name: appmap-node
appmap_dir: tmp/appmap
language: javascript
1 change: 1 addition & 0 deletions test/vitest/appmap.yml
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
name: vitest-appmap-node-test
appmap_dir: tmp/appmap
language: javascript

0 comments on commit a9f37b3

Please sign in to comment.