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

What was your use case for "deferred"? #4

Open
matrey opened this issue Sep 24, 2017 · 2 comments
Open

What was your use case for "deferred"? #4

matrey opened this issue Sep 24, 2017 · 2 comments

Comments

@matrey
Copy link

matrey commented Sep 24, 2017

Hi @frqnck

I intend to use this library to write logs to files in JSON format, then have Fluentd / Fluentbit tail these files for further processing.

I'd be interested in the "deferred" option to avoid writing to a file on disk all throughout script execution (seems cleaner to me to make one big write at the end).
However, the current implementation in AbstractLogger, method __destruct seems to just take all messages (strings), throw away the context, and write a single NOTICE entry in the log.

Why is that? I would have expected the deferred method to just process the backlog of log entries and write() them one by one.
Should you maybe clarify the way it works in the README?

Also, is there any technical reason for the "final" keyword on __destruct in AbstractLogger?

Thanks

@frqnck
Copy link
Member

frqnck commented Mar 13, 2018

Hi @matrey

Sorry for the late reply, you are right currently the __destruct method is much as you described. Not sure why that is -- probably at the time of implementation I must have needed it that (lazy) way. This clearly could do with being refactored and to process the backlog of entries one by one as rightly suggested. Should be straight forward to do. Please feel free to PR.

It is marked as "final" so it could not be overloaded i.e. impose the abstract implementation to all the logger types -- it is just an API choice.

Thanks for posting this out.

@8ctopus
Copy link
Contributor

8ctopus commented Feb 13, 2023

That's actually fixed in #11

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

No branches or pull requests

3 participants