Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logger: output as JSON #159

Merged
merged 19 commits into from
Jan 19, 2023
Merged

Logger: output as JSON #159

merged 19 commits into from
Jan 19, 2023

Conversation

josephjclark
Copy link
Collaborator

@josephjclark josephjclark commented Jan 11, 2023

This PR enables the logger to output JSON objects rather than strings. So instead of logs like:

[CLI] i Loaded adaptor

You get logs like:

{ level: 'info', name: 'CLI', message: ['Loaded adaptor'] }

Objects are shown in-line rather than embedded as a string:

{ level: 'info', name: 'CLI', message: [{ data: {}, configuration: {} ] }

This is useless to the CLI, but dead handy for integration tests and Lightning as we can easily filter logs.

This also fixes #157, where deeply nested objects will be truncated. We now stringify all output objects, which has the added bonus of supporting circular structures. Note that properties with function values are ignored, which I think is appropriate.

@josephjclark josephjclark marked this pull request as draft January 12, 2023 12:20
Base automatically changed from release/cli-0.0.27 to main January 16, 2023 11:08
@josephjclark josephjclark marked this pull request as ready for review January 17, 2023 12:12
@josephjclark josephjclark changed the base branch from main to release/cli-0.0.28 January 17, 2023 12:12
Copy link
Member

@stuartc stuartc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work here @josephjclark, it did take me a little while to get my head around the 'transparency' of this implementation.

I was trying to work out what the behaviour is when JSON logging is enabled and you log a string. But if my understanding is correct, the logger instance is created with a namespace already and when logging a string it emits JSON with the namespace attached right?

I'm happy to merge this but could you take a look at my comment around the logger using an env var vs the CLI explicitly being in charge of that (via a flag or env, either or)?

I can imagine some situations where your approach is useful down the line - but want to make sure I'm understanding your approach here.

packages/logger/test/options.test.ts Outdated Show resolved Hide resolved
@josephjclark
Copy link
Collaborator Author

Thanks Stu - let me sort out the env var thing, you're dead right about that

I was trying to work out what the behaviour is when JSON logging is enabled and you log a string

logger.log(`hello world`) // { level: 'info', message: 'hello world' }

the logger instance is created with a namespace already and when logging a string it emits JSON with the namespace attached right

💯

@josephjclark
Copy link
Collaborator Author

@stuartc All done!

@stuartc stuartc merged commit ffcdf8a into release/cli-0.0.28 Jan 19, 2023
@stuartc stuartc deleted the log-json branch January 19, 2023 06:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants