Log exceptions evaluating IValueProvider whilst starting a resource#7032
Log exceptions evaluating IValueProvider whilst starting a resource#7032davidfowl merged 2 commits intomicrosoft:mainfrom
IValueProvider whilst starting a resource#7032Conversation
dotnet-policy-service
Bot
added
the
community-contribution
|
Thanks! Diagnosability is always good. I'll let code owner sign off. |
| catch (Exception ex) | ||
| { | ||
| resourceLogger.LogCritical("Failed to apply arguments. A dependency may have failed to start."); | ||
| _logger.LogDebug(ex, "Failed to apply arguments. A dependency may have failed to start."); |
There was a problem hiding this comment.
Why are we no longer logging to _logger?
There was a problem hiding this comment.
I read this as a basic level of detail being logged as Critical, with the full detail only being logged at Debug. Now the critical level contains the full detail, the debug entry seemed redundant.
The other place in this file that does a resourceLogger.LogCritical(ex, ...) follows this same pattern - only logging the exception to the resourceLogger, and not _logger.
I can put the debug logs back if you wat
There was a problem hiding this comment.
resourceLogger and _logger are separate logs / instances. The first is the logs for the resource that shows up in the dashboard. The 2nd is the log for the AppHost itself (since the AppHost uses the ASP.NET/Microsoft.Extensions.Hosting app model).
It isn't redundant because it goes to 2 separate locations.
There was a problem hiding this comment.
@afscrome Can you add the logs you removed back?
There was a problem hiding this comment.
Added debug logs back, although I did add {ResourceName} into the _logger log entries so you know which resource failed. (I didn't include this on resourceLogger entries since the resource is implied by the resourceLogger instance).
- Added missing logging for failures in container runtime args
- Be more consistent with error message - always including `{ConfigKey}`
- Restore previously removed debug logging, with the addition of `{ResourceName}` to make clear which resource is the problem child.
eerhardt
left a comment
eerhardt
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the contribution!
github-actions
Bot
added
the
area-app-model
4 participants
Description
When an exception happens evaluating
IValueProviderwhen starting a resource, log the exception to the console output to provide more clues as to why the resource failed to start.Makes errors more obvious for #7030 & #6330.
Before:
After:
Checklist
<remarks />and<code />elements on your triple slash comments?breaking-changetemplate):doc-ideatemplate):Microsoft Reviewers: Open in CodeFlow