Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the JSON log payload format for @ozmap/logger by allowing both application-wide (global) attributes and per-logger-instance attributes to be included in every structured JSON log record.
Changes:
- Bumped package version to
0.2.9-beta. - Added
Logger.setGlobalAttributes(...)and plumbed per-instanceattributesthroughcreateLogger(...)into the JSON formatter. - Updated JSON formatter to merge global + instance attributes into the final structured payload.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| package.json | Version bump for the new logging payload capability. |
| lib/format/json.ts | Merges global/instance attributes into JSON output structure. |
| lib/format/index.ts | Threads attributes into the JSON formatter via getLogWrapper. |
| lib/Logger.ts | Introduces global attributes API and createLogger instance attributes option. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/Logger.ts
Outdated
| static globalAttributes: Record<string, string | number>; | ||
|
|
||
| /** | ||
| * Global attributes to show in every log | ||
| * @param data object containing attributes, needs to be k = string, v = string | ||
| */ | ||
| static setGlobalAttributes(data: Record<string, string | number>): void { | ||
| Logger.globalAttributes = data; | ||
| } |
There was a problem hiding this comment.
Logger.globalAttributes is declared but never initialized. In json formatting it gets spread into the structured payload; if setGlobalAttributes() was never called, spreading undefined will throw at runtime. Initialize it to {} (or Object.create(null)) and/or guard the spread with a default empty object.
| tag, | ||
| level /** @deprecated Use 'severityText' instead. */, | ||
| severityText: level, | ||
| severityNumber: LogLevels[logLevelKey], | ||
| body: data |
There was a problem hiding this comment.
This change removes the previously emitted data field (which was marked deprecated). Even if deprecated, removing it is a breaking change for any consumers still reading it. Consider keeping data as an alias of body for one more release (or explicitly document the breaking change / bump major version).
| public constructor( | ||
| opts: { tag?: string; client?: AbstractLogger; noServer?: boolean } = {} | ||
| opts: { | ||
| tag?: string; | ||
| client?: AbstractLogger; | ||
| noServer?: boolean; | ||
| attributes?: LogContext['attributes']; | ||
| } = {} | ||
| ) { | ||
| this.logger = getLogWrapper.call( | ||
| this, | ||
| output(), | ||
| opts.client ?? console, | ||
| opts.tag | ||
| opts.tag, | ||
| opts.attributes | ||
| ); |
There was a problem hiding this comment.
New behavior (global attributes + per-instance attributes passed into the JSON formatter) isn’t covered by existing tests. Consider adding a Jest test using a custom client that captures logger.log(...), then JSON.parse the payload to assert the attributes are present and that logging works when setGlobalAttributes() isn’t called.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
lib/format/json.ts:102
- The function currently returns an
asynclog wrapper and also contains a second nestedreturn (level, ...args) => { ... }block (duplicated wrapper) without properly closing the first one. This makes the file syntactically invalid and also violatesLogWrapper(expects a sync function returningvoid, not aPromise). Remove the accidental nested return/asyncand keep a single wrapper that logs the payload built with the newattributesparameter.
return async (level: LevelTag, ...args: unknown[]) => {
const payload = toStructuredJsonLog.call(
this,
level,
now,
tag,
attributes
);
return (level: LevelTag, ...args: unknown[]) => {
const payload = toStructuredJsonLog.call(this, level, now, tag);
for (const arg of args) {
payload.push(normalize(arg));
}
logger.log(paint[level](payload.toString()));
};
}
lib/Logger.ts:503
- The
createLoggerJSDoc block has duplicated@paramentries (tag/client/noServer repeated) and contains typos likeinstace. Please clean this up so the public API docs reflect the actual signature and newattributesoption only once.
* @param tag Tag with which the logger is being created.
* @param opts Optional attributes to add context to logger
* @param opts.client Underlying abstract logger to override console.
* @param opts.noServer Disable the embedded http server for runtime actions.
* @param opts.attributes Adds fields with static value for every log
* @param tag Tag with which the logger is being created.
* @param opts.client Underlying abstract logger to override console.
* @param opts.noServer Disable the embedded http server for runtime actions.
* @param opts.allowExit Allow process to exit naturally (uses server.unref()).
* @param opts.timerTTL TTL for timers in ms (default: 10min). Set to 0 to disable cleanup.
* @returns Logger instace
*/
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const structuredData = { | ||
| ...timestamp(), | ||
| ...this.getContext(), | ||
| ...Logger.globalAttributes, | ||
| ...attributes, | ||
| tag, | ||
| level /** @deprecated Use 'severityText' instead. */, | ||
| severityText: level, | ||
| severityNumber: LogLevels[logLevelKey], | ||
| body: data | ||
| }; |
There was a problem hiding this comment.
New behavior adds per-instance attributes and global attributes into JSON output, but the existing JSON formatter tests don’t assert these fields. Add tests covering: (1) instance attributes passed to createLogger appear on the root of the JSON payload, and (2) Logger.setGlobalAttributes() adds fields without breaking logs when unset.
lib/Logger.ts
Outdated
| static globalAttributes: Record<string, string | number>; | ||
|
|
||
| /** | ||
| * Global attributes to show in every log | ||
| * @param data object containing attributes, needs to be k = string, v = string | ||
| */ | ||
| static setGlobalAttributes(data: Record<string, string | number>): void { | ||
| Logger.globalAttributes = data; | ||
| } |
There was a problem hiding this comment.
Logger.globalAttributes is declared but never initialized. Since the JSON formatter spreads it into every log, this can crash at runtime unless setGlobalAttributes is called early. Initialize it to an empty object (and consider keeping the property always defined) so default usage is safe.
lib/Logger.ts
Outdated
| * @param opts.tag Tag with which the logger is being created. | ||
| * @param opts.client Underlying abstract logger to override console. | ||
| * @param opts.noServer Disable the embedded http server for runtime actions. | ||
| * @param opts.customFiels Add extra fields with fixed value. |
There was a problem hiding this comment.
Typo in the constructor JSDoc: opts.customFiels should be opts.customFields (and the code uses attributes, not customFiels). This can confuse consumers reading generated docs.
| * @param opts.customFiels Add extra fields with fixed value. | |
| * @param opts.attributes Additional attributes to include in the log context. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/format/json.ts
Outdated
| return async (level: LevelTag, ...args: unknown[]) => { | ||
| const payload = toStructuredJsonLog.call( | ||
| this, | ||
| level, | ||
| now, |
There was a problem hiding this comment.
In json(), the returned function starts as async and the block isn’t closed before the subsequent return (level...) below, which leaves a stray nested return / mismatched braces and will fail to compile. Remove the unintended extra return, and keep a single wrapper implementation (also ensure the wrapper passes attributes through when calling toStructuredJsonLog).
lib/format/json.ts
Outdated
| return (level: LevelTag, ...args: unknown[]) => { | ||
| const payload = toStructuredJsonLog.call(this, level, now, tag); | ||
|
|
There was a problem hiding this comment.
This return (level: LevelTag, ...args) branch appears to be leftover from before the attributes change. After fixing the wrapper, make sure there is only one returned function and that it calls toStructuredJsonLog with tag and attributes (currently this call drops attributes).
| const structuredData = { | ||
| ...timestamp(), | ||
| ...this.getContext(), | ||
| ...(Logger.globalAttributes ?? {}), | ||
| ...(attributes ?? {}), | ||
| tag, |
There was a problem hiding this comment.
attributes are spread into structuredData without normalization. If a consumer passes a non-JSON-serializable value (e.g. BigInt/symbol) in attributes, JSON.stringify will throw; the catch block only replaces body and then stringifies again, which will still throw for the same bad attribute. Consider normalizing/sanitizing attributes (and/or Logger.globalAttributes) before spreading, or in the catch block fall back to a minimal guaranteed-serializable payload (e.g. omit attributes/context when serialization fails).
lib/Logger.ts
Outdated
| * Global attributes to show in every log | ||
| * @param data object containing attributes, needs to be k = string, v = string |
There was a problem hiding this comment.
The JSDoc for setGlobalAttributes says the values must be string, but the actual type allows string | number. Please align the doc with the public signature (and consider documenting whether attributes are merged or replaced on subsequent calls).
| * Global attributes to show in every log | |
| * @param data object containing attributes, needs to be k = string, v = string | |
| * Global attributes to show in every log. | |
| * Subsequent calls replace any previously set global attributes rather than merging them. | |
| * @param data Object containing attributes, with string keys and values of type string or number. |
lib/Logger.ts
Outdated
| * @param tag Tag with which the logger is being created. | ||
| * @param opts Optional attributes to add context to logger | ||
| * @param opts.client Underlying abstract logger to override console. | ||
| * @param opts.noServer Disable the embedded http server for runtime actions. | ||
| * @param opts.attributes Adds fields with static value for every log |
There was a problem hiding this comment.
The createLogger JSDoc has duplicated @param entries (tag, opts, opts.client, etc.) which makes the generated docs confusing. Please remove the duplicate block and keep a single, accurate parameter list (including the new opts.attributes).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/logger-core.test.ts
Outdated
| test('should throw when timer ID already exists', () => { | ||
| logger.time('duplicate'); | ||
| expect(() => logger.time('duplicate')).toThrow( | ||
| 'Identifier duplicate is in use' | ||
| ); | ||
| expect(() => logger.time('duplicate')).not.toThrow(); | ||
| }); |
lib/Logger.ts
Outdated
| * Factory function to create tagged Logger instance. | ||
| * | ||
| * @param tag Tag with which the logger is being created. | ||
| * @param opts.client Underlying abstract logger to override console. | ||
| * @param opts.noServer Disable the embedded http server for runtime actions. | ||
| * @param opts.allowExit Allow process to exit naturally (uses server.unref()). | ||
| * @param opts.timerTTL TTL for timers in ms (default: 10min). Set to 0 to disable cleanup. | ||
| * @returns Logger instace | ||
| * @param tag Tag with which the logger is being created. | ||
| * @param opts Optional attributes to add context to logger | ||
| * @param opts.client Underlying abstract logger to override console. | ||
| * @param opts.noServer Disable the embedded http server for runtime actions. | ||
| * @param opts.attributes Adds fields with static value for every log | ||
| * @param tag Tag with which the logger is being created. | ||
| * @param opts Optional attributes to add context to logger | ||
| * @param opts.client Underlying abstract logger to override console. | ||
| * @param opts.noServer Disable the embedded http server for runtime actions. | ||
| * @param opts.attributes Adds fields with static value for every log | ||
| * @returns Logger instance |
lib/Logger.ts
Outdated
| opts: { | ||
| client?: AbstractLogger; | ||
| noServer?: boolean; | ||
| attributes?: Record<string, string | number>; |
| export function getLogWrapper<TScope extends Logger>( | ||
| this: TScope, | ||
| output: (typeof Outputs)[number], | ||
| logger: AbstractLogger, | ||
| tag?: string | ||
| tag?: string, | ||
| attributes?: LogContext['attributes'] | ||
| ): LogWrapper { | ||
| switch (output) { | ||
| case 'text': | ||
| return text.call(this, logger, tag); | ||
|
|
||
| case 'json': | ||
| return json.call(this, logger, tag); | ||
| return json.call(this, logger, tag, attributes); | ||
|
|
| - name: install node v22 | ||
| uses: actions/setup-node@v2 | ||
| with: | ||
| registry-url: 'https://registry.npmjs.org' | ||
| node-version: 22 |
| on: | ||
| push: | ||
| tags: | ||
| - '*.*.*' |
| - uses: actions/checkout@v2 | ||
| - name: 'cat package.json' | ||
| run: cat ./package.json | ||
| - name: install node v22 | ||
| uses: actions/setup-node@v2 |
| - uses: actions/checkout@v2 | ||
| - name: 'cat package.json' | ||
| run: cat ./package.json | ||
| - name: install node v22 | ||
| uses: actions/setup-node@v2 |
| - name: install node v22 | ||
| uses: actions/setup-node@v2 | ||
| with: | ||
| registry-url: 'https://registry.npmjs.org' | ||
| node-version: 22 |
| toString: () => { | ||
| const structuredData = { | ||
| ...timestamp(), | ||
| ...this.getContext(), | ||
| ...(Logger.globalAttributes ?? {}), | ||
| ...(attributes ?? {}), | ||
| tag, |
…JSDoc, add text attributes support, add attribute tests
Code Review - Correções aplicadas (commit 4da30e3)Todos os comentários de revisão foram endereçados neste commit: 1. Tipagem inconsistente de
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| on: | ||
| push: | ||
| tags: | ||
| - '*.*.*' |
| - uses: actions/checkout@v2 | ||
| - name: 'cat package.json' | ||
| run: cat ./package.json | ||
| - name: install node v22 | ||
| uses: actions/setup-node@v2 | ||
| with: | ||
| registry-url: 'https://registry.npmjs.org' | ||
| node-version: 22 |
| { | ||
| "name": "@ozmap/logger", | ||
| "version": "0.2.8", | ||
| "version": "0.3.0-beta.5", |
| - uses: actions/checkout@v2 | ||
| - name: 'cat package.json' | ||
| run: cat ./package.json | ||
| - name: install node v22 | ||
| uses: actions/setup-node@v2 | ||
| with: | ||
| registry-url: 'https://registry.npmjs.org' | ||
| node-version: 22 |
| on: | ||
| push: | ||
| branches: | ||
| - main | ||
|
|
||
| jobs: | ||
| build: | ||
| name: tsc | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v2 | ||
| - name: 'cat package.json' | ||
| run: cat ./package.json | ||
| - name: install node v22 | ||
| uses: actions/setup-node@v2 | ||
| with: | ||
| registry-url: 'https://registry.npmjs.org' | ||
| node-version: 22 | ||
| - name: npm install | ||
| run: npm install | ||
| - name: tsc | ||
| run: npm run build | ||
| - name: publish | ||
| uses: JS-DevTools/npm-publish@v1 | ||
| with: | ||
| token: ${{ secrets.NPM_TOKEN }} | ||
| access: public |
| on: | ||
| push: | ||
| branches: | ||
| - main | ||
|
|
||
| jobs: | ||
| build: | ||
| name: tsc | ||
| runs-on: ubuntu-latest |
lib/Logger.ts
Outdated
| * @param opts.allowExit Allow process to exit naturally (uses server.unref()). | ||
| * @param opts.timerTTL TTL for timers in ms (default: 10min). Set to 0 to disable cleanup. |
…ributes, restore data field, fix lockfile, improve error fallback
Code Review - Segunda rodada de correções (commit b42d302)Todos os comentários de revisão restantes foram endereçados: 1.
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return (level: LevelTag, ...args: unknown[]) => { | ||
| const data = args.map((arg) => stringify(arg)).join(' '); | ||
| const labels = formatTextAttributes(attributes); | ||
| const message = [`${now()}[${level}]`, tag ?? '', labels, data] | ||
| .filter(Boolean) | ||
| .join(' '); | ||
|
|
||
| logger.log(paint[level](`${now()}[${level}] ${tag ?? ''} ${data}`)); | ||
| logger.log(paint[level](message)); |
| afterEach(() => { | ||
| delete process.env.OZLOGGER_OUTPUT; | ||
| delete process.env.OZLOGGER_LEVEL; | ||
| Logger.globalAttributes = {}; |
| branches: | ||
| - main |
| - uses: actions/checkout@v2 | ||
| - name: 'cat package.json' | ||
| run: cat ./package.json | ||
| - name: install node v22 | ||
| uses: actions/setup-node@v2 | ||
| with: | ||
| registry-url: 'https://registry.npmjs.org' | ||
| node-version: 22 | ||
| - name: npm install | ||
| run: npm install |
| on: | ||
| push: | ||
| tags: | ||
| - '*.*.*' |
| - uses: actions/checkout@v2 | ||
| - name: 'cat package.json' | ||
| run: cat ./package.json | ||
| - name: install node v22 | ||
| uses: actions/setup-node@v2 | ||
| with: | ||
| registry-url: 'https://registry.npmjs.org' | ||
| node-version: 22 | ||
| - name: npm install | ||
| run: npm install |
| const structuredData = { | ||
| ...timestamp(), | ||
| ...this.getContext(), | ||
| ...(Logger.globalAttributes ?? {}), | ||
| ...(attributes ?? {}), | ||
| tag, |
| } catch (e) { | ||
| structuredData.data = structuredData.body = | ||
| '[OZLogger internal] - Unable to serialize log data'; | ||
| return JSON.stringify(structuredData, getCircularReplacer()); | ||
| const fallback = { | ||
| ...timestamp(), | ||
| tag, | ||
| severityText: level, | ||
| severityNumber: LogLevels[logLevelKey], | ||
| body: '[OZLogger internal] - Unable to serialize log data' | ||
| }; | ||
| return JSON.stringify(fallback); | ||
| } |


Melhoria gerais no formato do log:
Exemplo de aplicação de propriedade estática(OZMAP):
Todos os logs da aplicação irão com essas propriedades.
Exemplo de aplicação de propriedades na instância do logger:
Formato de output final:
{ "pid":91596, "ppid":91515, "tenant_id":"3efd49e8-cde1-40fa-881f-e7f3d7587a4e", "tenant":"http://localhost", "job":"history", "tag":"/home/nestor/devoz/ozmap/src/db/model/0History.ts", "level":"AUDIT", "severityText":"AUDIT", "severityNumber":12, "body":{ "0":{ "id": 1, "kind": "a" } } }