GH-1970: fix logger name in SPARQLProtocolSession #1971
Conversation
The SparqlProtocolSession uses this.getClass() as a name for the logger. When having an extension of the class, this leads to misleading log output. Particularly the logging levels are also harder to control (w.r.t namespacing and packaging). The general recommendation for logger names should be to use the class name, at least it should adhere to the rdf4j package structure.
| @@ -148,7 +148,7 @@ | |||
| */ | |||
| private final int maximumUrlLength; | |||
|
|
|||
| final Logger logger = LoggerFactory.getLogger(this.getClass()); | |||
| final Logger logger = LoggerFactory.getLogger(SPARQLProtocolSession.class); | |||
There was a problem hiding this comment.
Btw. I prefer final static and possibly private for loggers.
There was a problem hiding this comment.
I agree with preferring private: as this example shows, inheriting loggers is just a recipe for confusion. Unfortunately, this was designed for this purpose in the past, so we're stuck with it at least until the next major release.
I'm not sure why you prefer static though. We use a LoggerFactory so there is very little overhead. And there may be issues caused by static logger references in some applications (see https://cwiki.apache.org/confluence/display/COMMONS/Logging+StaticLog ).
There was a problem hiding this comment.
I wasn't aware of that issue. I mostly like static loggers because you can reference them from static context too.
The SparqlProtocolSession uses this.getClass() as a name for the logger.
When having an extension of the class, this leads to misleading log
output. Particularly the logging levels are also harder to control
(w.r.t namespacing and packaging).
The general recommendation for logger names should be to use the class
name, at least it should adhere to the rdf4j package structure.
GitHub issue resolved: #1970