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

Question about default logging on service crash #22

Open
kevgeni opened this issue Dec 2, 2021 · 2 comments
Open

Question about default logging on service crash #22

kevgeni opened this issue Dec 2, 2021 · 2 comments
Labels
question Further information is requested

Comments

@kevgeni
Copy link

kevgeni commented Dec 2, 2021

Question : The current default implementation of CriticalBackgroundService.OnError log to the console. Is this a design choice ?

I'm using your library in one of my windows service application (from a worker service) and a few days ago it crashed. In my log file, I only had "INFO ... Microsoft.Hosting.Lifetime - Application is shutting down...". No reason why. From the lines before the shutdown, I could guess that it crashed during an operation inside the CriticalBackgroundService and no log/exception was written.
I found out that the CriticalBackgroundService.OnError will not log to an ILogger, but to the console. Obviously the console is not redirected to my log file. I'm using Microsoft ILogger, and my ILogger is configured to output to log4net, which log to a file and also the console.
I could think that you're using console because you're targetting a basic program, without any ilogger and all the DI stuff. However that can't be the case because you're already registering your services with the microsoft DI in the readme file and the HostedService you're trying to fix are from asp.net / worker.


I've implemented the virtual OnError in all my background service to log to my ILogger, so i'm fine with the rest and I don't require any modification from your part. I was kinda not expecting it to log to console, but instead to log to ILogger by default.

Is it by design that CriticalBackgroundService log to console, while PeriodicTaskRunnerBackgroundService use the ILogger facility ?

When I check the rest of your project, PeriodicTaskRunnerBackgroundService seems to use ILogger to report crash, but CriticalBackgroundService only use the console. Perhaps it's because it's critical ?

@kevgeni kevgeni added the question Further information is requested label Dec 2, 2021
@GeeWee
Copy link
Owner

GeeWee commented Dec 2, 2021

That's a good question. No I think it's honestly just a design oversight that I didn't log to an ILogger - probably because I wasn't sure if it would be flushed in time when the application exits less-than-gracefully. I'm still not sure it will for all implementations

If you want, I'd be happy to accept a PR that logs to both the ILogger and the Console interface.

@kevgeni
Copy link
Author

kevgeni commented Dec 20, 2021

I do think that you should use the built-in ILogger. By default, when you create a new project, this ILogger output to console, thus the user should still be able to see it if they look at the console output, or the docker output. Given that the .net host also use the ILogger abstraction to writer logs, I do think it's used by everyone.
Those like me that customized the ILogger to output elsewhere will also able to see those messages in their log file or in their remote logging service. I don't think there much of a user base for those that disable all logging of the microsoft ILogger and only output to console using a different logger.

I've created a pull request, feel free to edit it if you wish to have the console back.
#23

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants