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

Always output entire log messages (#11) #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ecampidoglio
Copy link

@ecampidoglio ecampidoglio commented Oct 19, 2023

This PR ensures that log messages are printed to the console's stdout in their entirety by flushing the internal buffer of the StreamWriter after every write operation.

Resolves #11

This commit ensures that log messages are printed to the console's
stdout in their entirety by flushing the internal buffer of the
`StreamWriter` after every write operation.
@JFFby
Copy link

JFFby commented Nov 22, 2023

It looks like this behavior should be configurable and disabled by default, because it may hit performance.

@ecampidoglio
Copy link
Author

ecampidoglio commented Nov 22, 2023

It looks like this behavior should be configurable and disabled by default, because it may hit performance.

I see your point, but correctness is more important than performance. The current implementation causes incomplete log lines to be sent to stdout (see #11). If you have an external log collector like Filebeat, this will prevent the affected lines from being parsed correctly.

@sungam3r
Copy link
Contributor

sungam3r commented Dec 8, 2023

@ecampidoglio Your suggestion in understandable but I'm sure it's not applicable due to the design of this repo. Just look at the name - it contains word fast. AutoFlush = true goes against it and breaks all sense. The core mission of this repo is performance. Devs end up here when they need performance. If they need correctness then they just use well-known standard console sink for serilog.

So I would not merge this. At least, I would make it configurable, but again - such setting will look strange here.

@github-staff github-staff deleted a comment from mehdi-dev97 May 27, 2024
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.

Incomplete log output
3 participants