Skip to content

Change logging format#370

Merged
nimakaviani merged 4 commits intocnoe-io:mainfrom
nabuskey:colored-logging
Aug 29, 2024
Merged

Change logging format#370
nimakaviani merged 4 commits intocnoe-io:mainfrom
nabuskey:colored-logging

Conversation

@nabuskey
Copy link
Copy Markdown
Collaborator

@nabuskey nabuskey commented Aug 27, 2024

Currently, log outputs are hard to read. For example:

time=2024-08-26T20:22:19.468Z level=INFO msg="Creating kind cluster" logger=setup cluster=localdev
time=2024-08-26T20:22:36.409Z level=INFO msg="Done creating cluster" logger=setup cluster=localdev
time=2024-08-26T20:22:36.566Z level=INFO msg="Adding CRDs to the cluster" logger=setup
time=2024-08-26T20:22:37.377Z level=INFO msg="Setting up CoreDNS" logger=setup
time=2024-08-26T20:22:37.404Z level=INFO msg="Setting up TLS certificate" logger=setup
time=2024-08-26T20:22:37.425Z level=INFO msg="Creating localbuild resource" logger=setup
time=2024-08-26T20:22:37.425Z level=INFO msg="Starting EventSource" controller=localbuild controllerGroup=idpbuilder.cnoe.io controllerKind=Localbuild source="kind source: *v1alpha1.Localbuild"

There is a lot of repeated texts that adds no value.

With this PR log messages look like:

Aug 27 19:45:47 INFO Creating kind cluster logger=setup
Aug 27 19:45:47 INFO Runtime detected logger=setup provider=docker
Aug 27 19:45:47 INFO Cluster already exists logger=setup cluster=localdev
hi
Aug 27 19:45:47 INFO Adding CRDs to the cluster logger=setup
Aug 27 19:45:47 INFO Setting up CoreDNS logger=setup
Aug 27 19:45:47 INFO Setting up TLS certificate logger=setup
Aug 27 19:45:47 INFO Creating localbuild resource logger=setup
Aug 27 19:45:47 INFO Starting EventSource controller=localbuild controllerGroup=idpbuilder.cnoe.io controllerKind=Localbuild source=kind source: *v1alpha1.Localbuild

This also adds colored output option. Disabled by default.

image

@nabuskey
use our own logger
7630807 
Signed-off-by: Manabu McCloskey <manabu.mccloskey@gmail.com>
@nabuskey
ensure append a space
6229b6a 
Signed-off-by: Manabu McCloskey <manabu.mccloskey@gmail.com>
@nabuskey
remove debug statement
de68ef6 
Signed-off-by: Manabu McCloskey <manabu.mccloskey@gmail.com>
@cmoulliard
Copy link
Copy Markdown
Contributor

cmoulliard commented Aug 28, 2024

Don't forget to document how to enable/disable the colored output ;-)

@nabuskey
Copy link
Copy Markdown
Collaborator Author

nabuskey commented Aug 28, 2024

Don't forget to document how to enable/disable the colored output ;-)

Yep. Plan is to make an issue to track it if this gets merged :)

@tapas4java
Copy link
Copy Markdown
Contributor

tapas4java commented Aug 28, 2024

Any problem if we make colored output default to true ? Anyway it's not going to break anything

@nabuskey
Copy link
Copy Markdown
Collaborator Author

nabuskey commented Aug 28, 2024

Coloring is very subjective and I didn't want to impose what I like to others by default. If others like it over time, we can make it enabled by default imo.

nimakaviani
nimakaviani reviewed Aug 28, 2024
View reviewed changes
@nabuskey
remove C flag
a7e8bd9 
Signed-off-by: Manabu McCloskey <manabu.mccloskey@gmail.com>
@nimakaviani nimakaviani self-requested a review August 29, 2024 18:41
nimakaviani
nimakaviani approved these changes Aug 29, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@nimakaviani nimakaviani left a comment

Choose a reason for hiding this comment

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

LGTM

@nimakaviani nimakaviani merged commit fba981a into cnoe-io:main Aug 29, 2024
@nabuskey nabuskey deleted the colored-logging branch August 29, 2024 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nimakaviani nimakaviani nimakaviani approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nabuskey @cmoulliard @tapas4java @nimakaviani