microsoft / microsoft/aspire

ApplicationExecutor shouldn't conflate `stderr` with LogLevel.Error

Open
#5,094 0 comments 4 reactions 0 assignees View on GitHub
area-app-model
Dominant language
C#
Stars
6.3k
Forks
991
Avg merge
2d 12h
Merged PRs (30d)
201

Description

In the AppHost we read from the DCP stdout and stderr stream and write the information to the `ResourceLoggerService`'s logger here:

https://github.com/dotnet/aspire/blob/96626404baca66e52797e0a41e4a5c4b208c944a/src/Aspire.Hosting/Dcp/ApplicationExecutor.cs#L483-L492

That `enumerable` is a `ResourceLogSource` and comes from:

https://github.com/dotnet/aspire/blob/e3d3cb4c7ff733573940f177484c8213d71ef8aa/src/Aspire.Hosting/Dcp/ResourceLogSource.cs#L74-L87

where `isError` is whether it is `stderr` or `stdout`:

https://github.com/dotnet/aspire/blob/e3d3cb4c7ff733573940f177484c8213d71ef8aa/src/Aspire.Hosting/Dcp/ResourceLogSource.cs#L54-L58

From @karolz-ms:

> We are now capturing container startup logs and they are shown in the dashboard. These are logs from commands like docker build, docker container create and docker start
... and Docker CLI, like any program, is able to write to stdout and stderr. And we capture both in DCP, as separate log streams
Now the dashboard decided to unify everything, "normal" container logs, and startup logs, both stdout and stderr.
Kind of makes sense... but now you might see more logs
And on top of that, for most Unixy programs, including Docker CLI, writing to stderr DOES NOT mean an error occurred
That is not even treated as such by Powershell, albeit starting with PS 7.2 I think
stdout = data stream, to be processed by programs consuming the output
stderr = status messages for the human user
That is the very old Unix convention

See https://www.gnu.org/software/libc/manual/html_node/Standard-Streams.html

> Variable: FILE * stderr
The standard error stream, which is used for error messages and diagnostics issued by the program.

Given this convention, we shouldn't be turning messages written to `stderr` into `LogLevel.Error` messages. `LogLevel.Error` messages should only be actual errors - i.e. a problem occurred. Since Unix convention is to write diagnostic information to `stderr`, just because a message is written to `stderr` doesn't mean it is an actual problem.

@mitchdenny @radical @davidfowl @danegsta

Contributor guide

Open the contributing guide

Research direction

Start with the linked sections of src/Aspire.Hosting/Dcp/ApplicationExecutor.cs and ResourceLogSource.cs, tracing how the isError value from stdout or stderr becomes a logger level. Confirm how actual errors are represented and how stderr diagnostics should be handled. Done means stderr is no longer treated as LogLevel.Error solely because of its stream, while genuine errors remain errors.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
observability
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.