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

Update Alog Petty Print to prepend Metadata #451

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

HonakerM
Copy link
Contributor

Description

This PR updates the pretty print alog formatter to prepend the metadata instead of appending it e.g.:

2024-09-17T13:57:48.463604 [CHNL:INFO:123146149339136] <CRE00499128I>Processing has started {"id": "61515949-313b-49be-9f89-d71c38b22b3a"}

to

2024-09-17T13:57:48.463604 [CHNL:INFO:123146149339136] {"id": "61515949-313b-49be-9f89-d71c38b22b3a"} <CRE00499128I>Processing has started 

This makes it easier to read the message while retaining all of the metadata information

Changes

What changes are included in this PR?

Testing

How did you test this PR?

Related Issue(s)

List any issues related to this PR

Copy link
Collaborator

@gabe-l-hart gabe-l-hart left a comment

Choose a reason for hiding this comment

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

Thanks for the feature suggestion Casey! Since this library is used across a bunch of different projects, I'd like to approach this kind of change carefully. I think the best approach would be to add a new variable to alog.AlogPrettyFormatter's initializer to toggle this behavior. We can start with it off by default to avoid a breaking change, then toggle on in a future release. In the meantime, individual projects are free to call configure with a custom-constructed instance of the formatter.

@gabe-l-hart
Copy link
Collaborator

@HonakerM What are your thoughts on this one? I think it's a nice improvement, but I'd like to approach it with compatibility in mind.

@HonakerM
Copy link
Contributor Author

HonakerM commented Oct 8, 2024

@gabe-l-hart sorry, I forgot about this because I agree with your original comment and I didn't want to do more work 😂 😅 . I'm game to add a config option, let me push a comment

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