-
Notifications
You must be signed in to change notification settings - Fork 11
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
Minor logging additions #176
Conversation
nulltoken
commented
Jan 22, 2025
•
edited
Loading
edited
- Favor UTC date format when logging (Follow-up to fix: Use better time format and fix sample app (fixes #173) #174)
- Enrich StartupJobManager with logs
Codecov ReportAll modified and coverable lines are covered by tests ✅
|
@@ -50,4 +57,10 @@ public async Task ProcessStartupJobs(CancellationToken stopToken) | |||
|
|||
private async Task CreateExecutionTask(JobRun job, CancellationToken stopToken) => | |||
await jobProcessor.ProcessJobAsync(job, stopToken).ConfigureAwait(false); | |||
|
|||
[LoggerMessage(LogLevel.Information, "Triggering startup jobs execution at {at:o}")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As they're only going to be logged once, I figured using a Information log level would work. Thoughts?
@linkdotnet BTW, those links don't seem to work anymore. All I got are 404s. |
Same here. I hope that is just a bug and they fix that. It does work on main |
@@ -195,13 +195,13 @@ private void HandleUpdate(object? sender, NotifyCollectionChangedEventArgs e) | |||
case NotifyCollectionChangedAction.Add: | |||
foreach (JobRun job in e.NewItems!) | |||
{ | |||
LogJobAddedToQueue(job.JobDefinition.Type.Name, job.RunAt?.LocalDateTime); | |||
LogJobAddedToQueue(job.JobDefinition.Type.Name, job.RunAt); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good catch