feat(console monitor): Log level can be set via program arg

[rafaelmag110]

What this PR changes/adds

This allows to set the log level of the ConsoleMonitor via program args (i.e. --log-level=INFO).

Why it does that

It was not possible to set the level.

Linked Issue(s)

Closes #4392

[github-actions]

We are always happy to welcome new contributors ❤️ To make things easier for everyone, please make sure to follow our contribution guidelines, check if you have already signed the ECA, and relate this pull request to an existing issue or discussion.

[jimmarino]

Setting the log level should not be done like this, particularly having BaseRuntime downcast to ConsoleMonitor. The log level can be set via a program argument. This PR can be simplified only to change ExtensionLoader.loadMonitor(..) to handle this case.

[jimmarino]

See my comment about redoing this PR.

[rafaelmag110]

So you suggest removing the config option?

[jimmarino]

It should be set the same way the color arg is done. Only one class needs to be modified for this.

[rafaelmag110]

Thanks for your reviews. I do understand your points.
I'd still like the option to set the default monitor log level via a system / env property since changing the run command might not be ideal in certain situations.

I have two suggestions:
1 - Simply read from the system or environment properties using propOrEnv() on the constructor. (function link)
2 - Have a constructor that reads default monitor configs from a dedicated config file (similar to what the jdk-logger permits).

With option 2 we can easily add new configs later if we need them (i.e. like a formatter option).

We can also change the color option to be configurable that way.

[jimmarino]

Thanks for your reviews. I do understand your points. I'd still like the option to set the default monitor log level via a system / env property since changing the run command might not be ideal in certain situations.

I have two suggestions: 1 - Simply read from the system or environment properties using propOrEnv() on the constructor. (function link) 2 - Have a constructor that reads default monitor configs from a dedicated config file (similar to what the jdk-logger permits).

With option 2 we can easily add new configs later if we need them (i.e. like a formatter option).

We can also change the color option to be configurable that way.

As I mentioned, this PR is too complex for what it needs to do. Program arguments can be sourced from environment variables for people who want to create their Docker images that way. There's no need to introduce this behavior in the codebase, and we should never introduce a dedicated configuration source such as a file. Let's keep this as simple as possible and try to only modify the single class.

[rafaelmag110]

@jimmarino @paullatzelsperger @ndr-brt
Adjusted the PR as per discussed. Please review.

[ndr-brt]

a pair of nits, ready to go otherwise

[paullatzelsperger]

small nits, OK for me otherwise

[paullatzelsperger]

I would also test various combinations of spaces, upper, lower case etc.:

"--log-level = INFO", "--log-level= INFO", "--log-level =INFO", "--log-level=info",...

This could also be done with a ParamterizedTest and a @ValueSource.

Not saying all of these variants should get accepted, IMO it's fine to only accept "--log-level=XYZ", but to have a definitive test and to guard against regression.

[paullatzelsperger]

we are getting there.