Log meaningful job names and use template based log messages#15307
Log meaningful job names and use template based log messages#15307Migaroez merged 2 commits intorelease/13.0from
Conversation
| { | ||
| _logger.LogInformation($"Starting background hosted service for {hostedService.GetType().Name}"); | ||
|
|
||
| _logger.LogInformation("Creating background hosted service for {job}", jobName); |
There was a problem hiding this comment.
I think this is a bit too much spam, there's already a "Starting ...", and if it fails a "Failed to start ..." message, there are no other possibilities.
There was a problem hiding this comment.
Creating a job and starting it can have 2 different failure reasons.
With the current code flow, if you see a failed to start message it might be because of the factory failing or the created service not actually starting.
A way to resolve this and reduce messages being logged is to Log a message right after getting the jobName Booting/Initializing/Hosting background job {Jobname}
And wrapping the factory and the start in separate try catches with each having a distinct message template
For Completnes, you could leave the Creating/Starting messages but change their level to debug.
| private readonly List<IRecurringBackgroundJob> _jobs; | ||
| private readonly Func<IRecurringBackgroundJob, IHostedService> _jobFactory; | ||
| private IList<IHostedService> _hostedServices = new List<IHostedService>(); | ||
| private readonly List<(IHostedService HostedService, string JobName)> _hostedServices = new(); |
There was a problem hiding this comment.
Please create a private sub class in the same file to model this data to improve readability
Name suggestion: NamedServiceJob
| { | ||
| _logger.LogInformation($"Starting background hosted service for {hostedService.GetType().Name}"); | ||
|
|
||
| _logger.LogInformation("Creating background hosted service for {job}", jobName); |
There was a problem hiding this comment.
Creating a job and starting it can have 2 different failure reasons.
With the current code flow, if you see a failed to start message it might be because of the factory failing or the created service not actually starting.
A way to resolve this and reduce messages being logged is to Log a message right after getting the jobName Booting/Initializing/Hosting background job {Jobname}
And wrapping the factory and the start in separate try catches with each having a distinct message template
For Completnes, you could leave the Creating/Starting messages but change their level to debug.
3 participants
Prerequisites
If there's an existing issue for this PR then this fixes #15302
Description
As described in the linked issue, the new
RecurringBackgroundJobHostedServiceRunnerhas a little flaw in its logging: It doesn't log the actual names of the jobs it's starting, but the generic type of the hosted service instead:This PR makes the logging a little bit more verbose, in order to log both the creation and the start-up of each individual hosted service - this time using the job names:
While I was here, I also changed all logging to use template based log messages.
Testing this PR
All recurring jobs (hosted services) should start and run as per usual. The log should display the names of the jobs rather than the generic type of the hosted service.