Azure / Azure/azure-functions-host
Redesign how we log worker process errors
- Dominant language
- C#
- Stars
- 2k
- Forks
- 482
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 38
Description
In the current implementation of how we handle worker process errors, we log a message as an `error` based on if the message contains the words "error", "fail" or "severe", this is fragile and does not cover all cases on when an issue has occurred i.e. exceptions etc.
```csharp
# Method: ParseErrorMessageAndLog
if ((msg.IndexOf("error", StringComparison.OrdinalIgnoreCase) > -1) ||
(msg.IndexOf("fail", StringComparison.OrdinalIgnoreCase) > -1) ||
(msg.IndexOf("severe", StringComparison.OrdinalIgnoreCase) > -1))
{
LogError(msg);
}
```
> [Link to code](https://github.com/Azure/azure-functions-host/blob/64614b7392f36e3871862d22d21e72304df901e9/src/WebJobs.Script/Workers/ProcessManagement/WorkerProcess.cs#L103-L129)
It's worth noting that this method (`ParseErrorMessageAndLog`) is only used by the error callback:
```csharp
private void OnErrorDataReceived(object sender, DataReceivedEventArgs e)
{
if (e.Data != null)
{
ParseErrorMessageAndLog(e.Data);
}
}
```
Maybe it is safe to assume that all messages coming from the worker through the error event listener `Process.ErrorDataReceived += (sender, e) => OnErrorDataReceived(sender, e);` should be logged as errors, or at least warnings?
## Following tasks were identified as next steps to resolve this workitem
- [ ] Function host: Review the option of exposing UseStdErrorStreamForErrorsOnly setting to users under a more descriptive app setting value
- [ ] Extension Authors: Look into the possibility of intercepting the stdError stream, identify log level through language specific constructs and pass that info via a log prefix.
- [ ] Investigation task on dotnet worker ([#1116](https://github.com/Azure/azure-functions-dotnet-worker/issues/1116))
- [ ] Identify what logs we are missing by introducing worker faults at different points of execution
Contributor guide
Assessment
This issue has not been assessed yet.