-
Notifications
You must be signed in to change notification settings - Fork 115
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
Migrate to an external logger provider #302
Comments
Hi @TrabacchinLuigi , |
the cool part of using a third party lib is that you don't have to work on it... |
Also i've pushed some modifications to my branch, it's not complete, but you can give it a spin and have a look at the serilog provided ilogger interface |
Are you talking about master...TrabacchinLuigi:feature-spans ? |
Yes
Also yes, they also provide adapters to common other loggers. The only thing is the dependency on serilog package... which i don't think is something your userbase would dislike much, since it would provide better performace
That would take a bit of time anyway |
The scope of this library isn't providing a logger, so using some third party abstractions is a sensible way to move away from maintain something we aren't focused on.
The actual implementation is also allocating memory when it shouldn't, and doesn't "enforce" good logging practices (like one call per log: see DokanOperationProxy where multiple log.Debug are called to write about just one event, nor using a template string instead of a variable string)
I suggest migrating to serilog, just because i know it's high performance and can be adapted to all other major loggers, or custom ones
I think also microsoft.extensions.logging.abstractions could be a good choice, but i'm not sure about performance which are quite relevant in a project like this one
The text was updated successfully, but these errors were encountered: