Add option to configure logging filters

[vpavic]

This PR introduces logging.filters.console and logging.filters.file properties which allow applying logging filters for default console and file appenders.

This is in particular very useful in scenarios where file appender is enabled and users would like to have different things end up in console and file log.

This is related to #4357. Even though that issue was declined, the changes from this PR offer more flexibility and are IMO worth considering.

[rajadilipkolli]

@philwebb, if this is enhancement. In which version we are targetting for??

[philwebb]

@rajadileepkolli There's no current target.

[vpavic]

Any chance this gets considered for 2.0? Thanks.

[snicoll]

Thanks for the PR @vpavic. As I've indicated in a comment, I am not keen to add FQNs in configuration unless we really have to. I think the programmatic route would be better but I don't know how that would work concretely at this point.

Flagging for team attention to get more feedback.

[snicoll]

I am not a huge fan of comma separated list of FQNs in application.properties. I'd assume that it's not something you actually need to change at runtime, is it? How about a programmatic way of doing the same thing?

It can be tricky as the context hasn't been loaded yet though.

[vpavic]

I'd assume that it's not something you actually need to change at runtime, is it?

Correct.

How about a programmatic way of doing the same thing?

That sounds nice, I did consider that at the time I was putting this PR together, however since logging system is initialized very early I didn't see that as a realistic option at the time.

[wilkinsona]

Unless I have missed something, I am not in favour of making this change as it seems to be of limited use. As far as I can tell, the proposed properties will only allow a filter to be used with its default configuration. If your filter requires any further configuration, using logback.xml or logback-spring.xml will still be necessary. An example is the ThresholdFilter from the related issue (#4357) which takes a level. Furthermore, all of the other standard filters described in the Logback documentation require some further configuration.

[vpavic]

I wouldn't agree it is of limited use. The fact that some logging filters are configurable doesn't prevent you from either subclassing or composing them to achieve the desirable filtering functionality. Such filters can then be packaged (together with others that are written from scratch) into an archive that can be shared between the projects and simply referenced through the logging configuration properties.

OTOH going back to using logback.xml or logback-spring.xml simply to use logging filters requires you to redo manually everything Boot offers to configure through configuration properties.

To provide some background and motivation behind this PR, as discussed with @philwebb last month at S1P, I believe that Boot should make it possible to easily configure application's logging system to match what's considered a well behaved Unix/Linux app. By that I mean that app's main log output is a logging file that's managed by the app itself, while stdout contains minimal output that's solely related to the status of the app and does not flood the journal (or whatever the place is where stdout is redirected).

A few examples of what I've described above:

$ service nginx status 
● nginx.service - A high performance web server and a reverse proxy server
   Loaded: loaded (/lib/systemd/system/nginx.service; enabled; vendor preset: enabled)
   Active: active (running) since Thu 2017-12-21 07:52:18 CET; 2 weeks 0 days ago
     Docs: man:nginx(8)
 Main PID: 1142 (nginx)
    Tasks: 5 (limit: 4915)
   Memory: 3.2M
      CPU: 27ms
   CGroup: /system.slice/nginx.service
           ├─1142 nginx: master process /usr/sbin/nginx -g daemon on; master_process on;
           ├─1143 nginx: worker process
           ├─1144 nginx: worker process
           ├─1145 nginx: worker process
           └─1146 nginx: worker process

Dec 21 07:52:18 vedran-nb systemd[1]: Starting A high performance web server and a revers
Dec 21 07:52:18 vedran-nb systemd[1]: Started A high performance web server and a reverse

$ service postgresql status 
● postgresql.service - PostgreSQL RDBMS
   Loaded: loaded (/lib/systemd/system/postgresql.service; enabled; vendor preset: enable
   Active: active (exited) since Thu 2017-12-21 07:52:20 CET; 2 weeks 0 days ago
 Main PID: 1354 (code=exited, status=0/SUCCESS)
    Tasks: 0 (limit: 4915)
   Memory: 0B
      CPU: 0
   CGroup: /system.slice/postgresql.service

Dec 21 07:52:20 vedran-nb systemd[1]: Starting PostgreSQL RDBMS...
Dec 21 07:52:20 vedran-nb systemd[1]: Started PostgreSQL RDBMS.

While the main log files are located in /var/log/nginx and /var/log/postgresql, respectively.

[wilkinsona]

Thanks for the additional insight, @vpavic. Unfortunately, I remain unconvinced that this is the right thing to do.

The fact that some logging filters are configurable doesn't prevent you from either subclassing or composing them to achieve the desirable filtering functionality

In my opinion, the developer experience would be poor with this approach. For example, you may end up having to write multiple subclasses of the same filter to enable various different settings of a particular piece of configuration.

It feels like we're edging towards implementing a properties-based configuration mechanism that's, in part at least, inferior to Logback's native XML.

I'd prefer to pursue @snicoll's suggestion of a programmatic approach. Callbacks that can contribute to the default logging configuration could be loaded via spring.factories and passed the LoggerContext.

[vpavic]

Callbacks that can contribute to the default logging configuration could be loaded via spring.factories and passed the LoggerContext.

That looks like a good solution to the original problem, and one that provides even more power and flexibility.

[snicoll]

@vpavic are you willing to rework this PR in that direction?

[vpavic]

Starting mid next week I should be able to find time to revisit this. I guess that should be inline with current schedule for 2.0.0.RC1.

BTW just to confirm we're at the same page - do you still want this to be focused around configuring logging filters, or should it provide a more general purpose customization mechanism?

[wilkinsona]

Thanks, @vpavic. FWIW, I'd envisaged something that, initially at least, just passing the LoggerContext to the callback and then letting it do whatever it likes. There may be some mileage in a more sophisticated API on top of that (org.springframework.boot.logging.logback.LogbackConfigurator could provide some inspiration) but I don't think we need it right now.

[vpavic]

FWIW, I'd envisaged something that, initially at least, just passing the LoggerContext to the callback and then letting it do whatever it likes.

I think that's not enough to meet the requirement that was the motivation for this PR. IMO the hook points should be provided inside DefaultLogbackConfiguration#consoleAppender and DefaultLogbackConfiguration#fileAppender in order to manipulate the appenders before they get started.

[wilkinsona]

Thanks, @vpavic. It looks like a trade-off between the complexity of the callback API, and having to rework DefaultLogbackConfiguration and LogbackConfigurator so that LoggerContext is configured before all of its components are started.

I'm not strongly in favour of one approach over the other, but a finer-grained callback would probably be less risky. As a first attempt at this approach, I think I'd explore LogbackConfigurator being responsible for managing and calling the callbacks.

[snicoll]

@vpavic any interest on your side to update this one?

[vpavic]

Yes, I'll try to revisit this after Spring I/O.

[snicoll]

@vpavic are you still interested to update this one?

[vpavic]

Sorry for my failure to update this @snicoll - yes, I'm still pretty much interested in having the easy way to establish logging configuration described in this comment.

At the moment I can't promise when, but I'll definitely try to update the PR at some point.

[snicoll]

There has been quite a few back and forth with no resolution so I am going to close this one for now. We can reconsider later if need to be. Thanks for the PR, in any case.