Skip to content

Commit 56f62e7

Browse files
HweinstockHweinstock
andauthored
fix(logger): migrate to winston for simpler executable support (#1808)
* fix(logger): migrate to winston for simpler executable support * refactor(logging): swap logging constants to maintain past 10-days at 5MB caps * fix(root): log to console if logger/io fails to init * refactor(logging): rename flush to end to make it clear the stream is closed --------- Co-authored-by: Hweinstock <hkobew@amazom.com>
1 parent 416fd35 commit 56f62e7

11 files changed

Lines changed: 175 additions & 85 deletions

File tree

.github/workflows/build.yml

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Verifies lint, formatting, types, audit, and build all pass.
1+
# Builds the CLI and compiles it into executables for each platform for a smoke test.
22
name: build
33
on:
44
workflow_call:
@@ -8,28 +8,41 @@ on:
88
type: string
99

1010
jobs:
11-
check:
12-
runs-on: codebuild-agentcore-e2e-${{ github.run_id }}-${{ github.run_attempt }}
11+
build:
12+
name: Build (${{ matrix.name }})
13+
runs-on: ${{ fromJSON(matrix.runner) }}
1314
permissions:
1415
contents: read
16+
strategy:
17+
fail-fast: false
18+
matrix:
19+
include:
20+
- name: Linux
21+
runner: '["codebuild-agentcore-e2e-${{ github.run_id }}-${{ github.run_attempt }}"]'
22+
target: linux-x64
23+
binary: ./dist/bin/agentcore-linux-x64
24+
- name: Windows
25+
runner: '["codebuild-agentcore-e2e-${{ github.run_id }}-${{ github.run_attempt }}", "image:windows-1.0"]'
26+
target: windows-x64
27+
binary: ./dist/bin/agentcore-windows-x64.exe
28+
# CodeBuild does not support macOS.
29+
- name: macOS
30+
runner: '["macos-latest"]'
31+
target: darwin-arm64
32+
binary: ./dist/bin/agentcore-darwin-arm64
1533
steps:
1634
- uses: actions/checkout@v7
1735
with:
1836
ref: ${{ inputs.ref }}
1937
persist-credentials: false
2038
- uses: oven-sh/setup-bun@v2
2139
- run: bun install --frozen-lockfile
22-
- run: bun run lint:check
23-
if: always()
24-
- run: bun run format:check
25-
if: always()
26-
- run: bun run typecheck
27-
if: always()
28-
- run: bun audit
29-
if: always()
40+
3041
- run: bun run build
31-
if: always()
42+
3243
- run: bun pm pack
33-
if: always()
34-
- run: bun run compile
35-
if: always()
44+
45+
- run: bun run compile:${{ matrix.target }}
46+
47+
- name: Smoke test binary
48+
run: ${{ matrix.binary }} --help

.github/workflows/check.yml

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# Verifies lint, formatting, types, audit, and other static checks pass.
2+
name: check
3+
on:
4+
workflow_call:
5+
inputs:
6+
ref:
7+
required: true
8+
type: string
9+
10+
jobs:
11+
check:
12+
runs-on: codebuild-agentcore-e2e-${{ github.run_id }}-${{ github.run_attempt }}
13+
permissions:
14+
contents: read
15+
steps:
16+
- uses: actions/checkout@v7
17+
with:
18+
ref: ${{ inputs.ref }}
19+
persist-credentials: false
20+
- uses: oven-sh/setup-bun@v2
21+
- run: bun install --frozen-lockfile
22+
- run: bun run lint:check
23+
if: always()
24+
- run: bun run format:check
25+
if: always()
26+
- run: bun run typecheck
27+
if: always()
28+
- run: bun audit
29+
if: always()

.github/workflows/ci.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ concurrency:
1313
cancel-in-progress: true
1414

1515
jobs:
16+
check:
17+
uses: ./.github/workflows/check.yml
18+
permissions:
19+
contents: read
20+
with:
21+
ref: ${{ github.event.pull_request.head_sha || github.sha }}
1622
build:
1723
uses: ./.github/workflows/build.yml
1824
permissions:

bun.lock

Lines changed: 59 additions & 27 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@
6161
"react": "^19.2.7",
6262
"react-devtools-core": "^7.0.1",
6363
"react-router": "^8.1.0",
64-
"pino": "^10.3.1",
65-
"pino-roll": "^4.0.0",
64+
"winston": "^3.19.0",
65+
"winston-daily-rotate-file": "^5.0.0",
6666
"zod": "^4.4.3"
6767
}
6868
}

src/index.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,12 @@ process.exit(
5555
await rootHandler.route(argv);
5656
} catch (e) {
5757
const error = e instanceof Error ? e : new Error(String(e));
58-
io.stderr.write(`${error.name}: ${error.message}\n`);
5958
rootLogger
6059
.child({ errorName: error.name, errorMessage: error.message, stack: error.stack ?? "" })
6160
.error();
6261
throw e;
6362
} finally {
64-
await rootLogger.flush();
63+
await rootLogger.end();
6564
}
6665
}),
6766
);

src/logging/fileLogger.tsx

Lines changed: 43 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import pino from "pino";
1+
import winston from "winston";
2+
import DailyRotateFile from "winston-daily-rotate-file";
23
import { type AsyncLogger, type LoggerBindings, type LogLevel } from "./types";
34

45
export interface FileLoggerConfig {
@@ -9,54 +10,62 @@ export interface FileLoggerConfig {
910
logLevel: LogLevel;
1011
}
1112

12-
function wrapPinoLogger(pinoLogger: pino.Logger): AsyncLogger {
13+
function wrapWinstonLogger(
14+
winstonLogger: winston.Logger,
15+
transport: DailyRotateFile,
16+
bindings: LoggerBindings,
17+
): AsyncLogger {
1318
const log =
14-
(level: pino.Level) =>
19+
(level: string) =>
1520
(...args: string[]) =>
16-
pinoLogger[level](args.join(" "));
21+
winstonLogger.log(level, args.join(" "), bindings);
22+
1723
return {
1824
debug: log("debug"),
1925
info: log("info"),
2026
warn: log("warn"),
2127
error: log("error"),
22-
child: (bindings) => wrapPinoLogger(pinoLogger.child(bindings)),
23-
// we convert pino's flush method that accepts a callback into a promise to make it easier to work with.
24-
// Note: we also treat flush as best-effort and swallow errors
25-
flush: () => new Promise<void>((resolve) => pinoLogger.flush(() => resolve())),
28+
child: (childBindings) =>
29+
wrapWinstonLogger(winstonLogger, transport, { ...bindings, ...childBindings }),
30+
end: () =>
31+
new Promise<void>((resolve) => {
32+
transport.on("finish", resolve);
33+
// note: we prefer close over end since close calls end on the stream internally: https://github.com/winstonjs/winston-daily-rotate-file/blob/a1a4668cfea77476cd6a4a11f038c2aac9d10741/daily-rotate-file.js#L201-L207
34+
if (transport.close) transport.close();
35+
}),
2636
};
2737
}
2838

2939
/**
3040
* Creates a logger that writes structured JSON to a rotating file.
3141
*
3242
* @param config - Logger configuration (file path, rotation limits, level).
33-
* @returns A {@link AsyncLogger} that writes to a rotating file via pino.
43+
* @returns A {@link AsyncLogger} that writes to a rotating file via winston.
3444
*/
3545
export function createFileLogger(config: FileLoggerConfig): AsyncLogger {
36-
const maxSizeInMB = config.maxSizeInMB ?? 10;
37-
const maxFileCount = config.maxFileCount ?? 5;
46+
const maxSizeInMB = config.maxSizeInMB ?? 5;
47+
const maxFileCount = config.maxFileCount ?? 10;
3848
const bindings = config.bindings ?? {};
39-
return wrapPinoLogger(
40-
pino({
41-
level: config.logLevel,
42-
base: undefined, // omit pid and hostname
43-
formatters: {
44-
level(label) {
45-
return { level: label };
46-
},
47-
},
48-
transport: {
49-
target: "pino-roll",
50-
options: {
51-
extension: ".log",
52-
dateFormat: "yyyy-MM-dd'T'HH-mm-ss",
53-
// Rotate when file reaches {maxSizeInMB} MB, and start deleting once we have {maxFileCount} files
54-
size: `${maxSizeInMB}m`,
55-
limit: { count: maxFileCount },
56-
file: config.filePath,
57-
mkdir: true,
58-
},
59-
},
60-
}),
61-
).child(bindings);
49+
50+
const transport = new DailyRotateFile({
51+
filename: `${config.filePath}-%DATE%`,
52+
extension: ".log",
53+
datePattern: "YYYY-MM-DD",
54+
maxSize: `${maxSizeInMB}m`,
55+
maxFiles: maxFileCount,
56+
createSymlink: false,
57+
});
58+
59+
const jsonFormat = winston.format.printf((info) => {
60+
const { level, message, ...rest } = info;
61+
return JSON.stringify({ level, msg: message, time: Date.now(), ...rest });
62+
});
63+
64+
const logger = winston.createLogger({
65+
level: config.logLevel,
66+
format: jsonFormat,
67+
transports: [transport],
68+
});
69+
70+
return wrapWinstonLogger(logger, transport, bindings);
6271
}

src/logging/types.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,5 +27,6 @@ export interface Logger {
2727
/** An extension of {@link Logger} that writes logs asynchronously and requires output to be flushed */
2828
export interface AsyncLogger extends Logger {
2929
child: (bindings: LoggerBindings) => AsyncLogger;
30-
flush: () => Promise<void>;
30+
/** Flushes the pending logs and closes the underlying logging streams **/
31+
end: () => Promise<void>;
3132
}

src/middleware/withLogging.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ describe("withLogging", () => {
2121
});
2222

2323
afterEach(async () => {
24-
await logger.flush();
24+
await logger.end();
2525
await rm(tempDir, { recursive: true, force: true });
2626
});
2727

src/runnable/index.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ export async function runWithExitCode(
2727
try {
2828
await fn(argv);
2929
return ExitCode.SUCCESS;
30-
} catch {
30+
} catch (e) {
31+
const error = e instanceof Error ? e : new Error(String(e));
32+
console.error(`${error.name}: ${error.message}`);
3133
return ExitCode.FAILURE;
3234
}
3335
}

0 commit comments

Comments
 (0)