feat: add bufferMax to BufferLogAppender #10

Closed
mahseiah_ai wants to merge 9 commits from issue-9-buffer-max into main
4 changed files with 42 additions and 2 deletions
Showing only changes of commit aa14660c2a - Show all commits
+1 -1
View File
@@ -1,6 +1,6 @@
{ {
"name": "@jdbernard/logging", "name": "@jdbernard/logging",
"version": "2.3.3", "version": "2.4.0",
"description": "Simple Javascript logging service.", "description": "Simple Javascript logging service.",
"main": "dist/index.js", "main": "dist/index.js",
"types": "dist/index.d.ts", "types": "dist/index.d.ts",
+10 -1
View File
@@ -4,6 +4,7 @@ import { LogLevel, type LogMessage } from './log-message'
export class BufferLogAppender implements LogAppender { export class BufferLogAppender implements LogAppender {
public threshold: LogLevel public threshold: LogLevel
public buffer: LogMessage[] public buffer: LogMessage[]
public bufferMax?: number
constructor(buffer?: LogMessage[], threshold?: LogLevel) { constructor(buffer?: LogMessage[], threshold?: LogLevel) {
this.buffer = buffer ?? [] this.buffer = buffer ?? []
@@ -12,7 +13,15 @@ export class BufferLogAppender implements LogAppender {
public appendMessage(msg: LogMessage): void { public appendMessage(msg: LogMessage): void {
if (this.threshold && msg.level < this.threshold) return if (this.threshold && msg.level < this.threshold) return
else this.buffer.push(msg)
this.buffer.push(msg)
if (this.bufferMax !== undefined) {
const max = Math.max(0, this.bufferMax)
while (this.buffer.length > max) {
Review

What are the performance characteristics of this pattern? Specifically in two cases:

  1. The common case of one message being added to an already full buffer, leading to one message being removed. If we're logging at high frequency, is this problematic? Is this.buffer.shift() O(1)? O(N)?
  2. What if someone reduces an already full, large buffer by setting bufferMax to a much smaller number?

How does a "slice and re-assign" implementation compare, performance-wise?

What are the performance characteristics of this pattern? Specifically in two cases: 1. The common case of one message being added to an already full buffer, leading to one message being removed. If we're logging at high frequency, is this problematic? Is `this.buffer.shift()` O(1)? O(N)? 2. What if someone reduces an already full, large buffer by setting `bufferMax` to a much smaller number? How does a "slice and re-assign" implementation compare, performance-wise?
Review

Looks like you hit rate limits, can you continue and answer my question above?

Looks like you hit rate limits, can you continue and answer my question above?
this.buffer.shift()
}
}
} }
public clearBuffer(): void { public clearBuffer(): void {
+30
View File
@@ -39,6 +39,36 @@ describe("BufferLogAppender", () => {
expect(appender.buffer[1].msg).toBe("second"); expect(appender.buffer[1].msg).toBe("second");
}); });
test("defaults to an unbounded buffer", () => {
const appender = new BufferLogAppender();
appender.appendMessage(makeMsg({ msg: "first" }));
appender.appendMessage(makeMsg({ msg: "second" }));
appender.appendMessage(makeMsg({ msg: "third" }));
expect(appender.buffer.length).toBe(3);
expect(appender.buffer.map((message) => message.msg)).toEqual([
"first",
"second",
"third",
]);
});
test("trims oldest messages when bufferMax is exceeded", () => {
const appender = new BufferLogAppender();
appender.bufferMax = 2;
appender.appendMessage(makeMsg({ msg: "first" }));
appender.appendMessage(makeMsg({ msg: "second" }));
appender.appendMessage(makeMsg({ msg: "third" }));
expect(appender.buffer.length).toBe(2);
expect(appender.buffer.map((message) => message.msg)).toEqual([
"second",
"third",
]);
});
test("respects threshold", () => { test("respects threshold", () => {
const appender = new BufferLogAppender(undefined, LogLevel.WARN); const appender = new BufferLogAppender(undefined, LogLevel.WARN);
+1
View File
@@ -3,6 +3,7 @@
"module": "commonjs", "module": "commonjs",
"target": "es2016", "target": "es2016",
"declaration": true, "declaration": true,
"rootDir": "./src",
"outDir": "./dist", "outDir": "./dist",
"strict": true, "strict": true,
"skipLibCheck": true "skipLibCheck": true