docs: update logger coding guidelines

[sdirix]

What it does

Recommend named logger usage and specify a naming convention.

Follow-ups

Update all logging use cases in Theia framework

Breaking changes

• This PR introduces breaking changes and requires careful review. If yes, the breaking changes section in the changelog has been updated.

Attribution

Review checklist

• As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• As a reviewer, I agree to behave in accordance with the review guidelines

[msujew]

Fantastic idea, I just have one comment on the naming convention:

It seems a bit verbose to me. Usually, I'd like loggers to be feature based not necessarily file based (does that make sense?). I.e. I've already started logging all notebook related logs to notebook, even if they're in the @theia/plugin-ext package. I'm not against updating my code, but keep in mind that the name of the logger gets printed to every log being performed in the console. That sounds like a lot of noise to me. It also makes deactivating/activating certain loggers in the config more cumbersome.

What do the others think about this?

[tsmaeder]

I think we need to think logger names from the goals we have with them: in the end, the goal is to turn sections of logging on and off in a convenient way. That requires two things:

1. We have to be able to identify loggers uniquely
2. We have to conveniently turn groups of loggers on and off as intended by the user.

If we name loggers based on filenames (or some other "fully qualified classname" 😉 , we can infer the "feature", if we can match rules by something like a regex or glob e.g.

*/packages/**/notebook/* = debug

But generally, we should just imagine log names as a set of properties we can match against. On node, would this work?

@inject() named(__filename)

Also: if we need logging in non-inversify contexts, can we make the inversify stuff generally defer log management to some global service? Kindof like we use contributions to register stuff with a service?

[MatthewKhouzam]

LGTM, I would add for future sprints, wrapping the logger in the trace event format https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/edit?pli=1&tab=t.0#heading=h.yr4qxyxotyw would help make it readable in trace compass, chrome and others. Here's a reference java implementation. https://github.com/eclipse-tracecompass/trace-event-logger

[sdirix]

@tsmaeder @msujew If I understand you both correctly, then we have a conflict of interest: Whether to name the loggers after their location or the purpose of the code. Both of these make sense to me.

• In practice, purpose naming is more difficult than just using a location based name, however it could be more useful and easier to use.
• Location based names will produce so many different names, that we likely need a (simplified) regex-configuration support to make configuring them more simple

The approaches could also be combined using a scheme like this:

[optional-purpose]<location>

The location could be all kind of schemes like

<package-name-without-@theia>:ClassName
<full-package-name>:<file-location-relative-to-src>

For example

[notebook]plugin-ext:PluginHost
vsx-registry:VSXExtensionResolver
remote:SSHIdentityFileCollector
[authentication]core:AuthenticationServiceImpl

If we then also support regex configuring, then it's easy to configure for purpose and/or location

What are your thoughts?

[tsmaeder]

wrapping the logger in the trace event format
IMO, the precise log format should be part of the output configuration of the log system.

[tsmaeder]

+1 on doing a purpose + location logger naming and filtering on regex. Again, if we're trying to allow for precise filtering, we could add more loggers with suffixes (for example, with the class name as a suffix) if that component produces output that needs to be either addressed precisely or filtered out. So for me a naming strategy of
[purpose]:location[#suffix] Would make sense, but only if we do have regex support for filtering.