stdlib: Do not progress report dynamically started supervisors

[IngelaAndin]

Closes #8715

[github-actions]

CT Test Results

    2 files     95 suites   35m 7s ⏱️
2 147 tests 2 099 ✅ 48 💤 0 ❌
2 456 runs  2 406 ✅ 50 💤 0 ❌

Results for commit 62ba338.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

• Complete CT logs (Download Logs)
• HTML Documentation (Download HTML Docs)
• Windows Installer

// Erlang/OTP Github Action Bot

[okeuday]

@IngelaAndin It is already possible to disable select supervisors using logger configuration with code like https://github.com/CloudI/CloudI/blob/9c4233246c5214cdf75b1f08d6d021fdea8ffb2a/src/lib/cloudi_core/src/cloudi_logger_kernel.erl#L42-L85
and configuration like https://github.com/CloudI/CloudI/blob/9c4233246c5214cdf75b1f08d6d021fdea8ffb2a/src/rel/files/vm.config.in#L25-L28 .

Preventing output from all dynamically started supervisors in a way that configuration is unable to disable, seems like a bad direction for people just trying to figure out what is happening in their system. The default likely needs to allow dynamically started supervisors by default for consistency with past usage, to provide the least surprise for users.

Making it easier to filter the logger output is another approach, but being able to selectively disable processes reporting could likely make disabling progress report data simpler. A configuration approach, e.g., in the init/1 return value or a supervisor:start_link/4 Options list value, could make the situation simpler.

[IngelaAndin]

@okeuday Yes I am fully aware that you can disable them, and this is what we told people several times when they reported it. However the reports keep coming and it is quite different to get the progress reports just for application start up and for each connection that happens frequently all the time. And this feels like a bad and not intended default behavior.
If progress reports had not been at info level from legacy I think we would have made them debug level reports. Changing that for all progress reports now is very backwards incompatible and probably would create complaints. However, I think you have a point and I changed the implementation to make only the dynamically started supervisor reports being on debug-level.

[okeuday]

@IngelaAndin Having dynamic supervisors more visible might not be a bad thing though, when considering all possible use. Erlang source code benefits from having immutable data (in a way described at https://github.com/okeuday/varpool#readme ) and static supervision hierarchies should have less errors when compared to dynamically started supervisors. From that perspective, it seems bad to disable or reduce logging output regarding dynamically started supervisors. If the setting can be set selectively just when dynamically started supervisors happen to be annoying (and are considered unlikely to be a future problem), that should be best for everyone's use.

[IngelaAndin]

@okeuday The point is they are always annoying unless you are debugging and then you can turn on debug level logging.
For legacy reasons we will not do it for all progress reports, as it is less annoying for the statically started ones and might be expected by users due to legacy. Dynamically start ones I believe are less usual, and when they are used they will typically be used in dynamic contexts such as connections that goes up and down and spamming logs. Such supervisor will not be restarted automatically if the application is restarted and hence are a different kind of supervisor and no I do not think errors are more common in these supervisors
that statically started ones. The errors in supervisors typically happens during development and default logging should not focus on development but rather on deployment.

Also note, we are just talking about progress reports here, that is errors/crashes will still be logged in the same way as before.

[eproxus]

This is already merged but I just wanted to chip in with my experience:

One could of course filter out the reports or part of the reports but it is neither trivial (as can be seen by the amount of configuration and code needed in the example above) nor enabled by default. I struggled many hours trying to solve this in various ways but could never figure out a log filter that stripped the binary certificates from the log output.

I prefer this solution because it somehow follows the principle of least surprise for most users (in my opinion). Now the opposite becomes true that one can get to these reports with extra configuration or code if it is really needed.

@okeuday I'll probably try to steal your filtering for our test runs where we have debug logging enabled (and which produce megabytes of logs each runs for just the certificates).