You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Uwe Schindler (JIRA)" <ji...@apache.org> on 2015/11/27 20:00:12 UTC
[jira] [Comment Edited] (SOLR-8330) Restrict logger visibility
throughout the codebase to private so that only the file that declares it
can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15030152#comment-15030152 ]
Uwe Schindler edited comment on SOLR-8330 at 11/27/15 6:59 PM:
---------------------------------------------------------------
Hi,
I wrote a pattern detector, basically it does the following: If the java file contains "org.slf4j.LoggerFactory", it will look for some pattern matching 3 occurences of either "private", "static", "final", followed by "LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());"
Without this patch, it reports 433 errors, when the patch posted here is applied it still detects some violations:
{noformat}
[source-patterns] invalid logging pattern [not private static final, uses static class name]: solr/core/src/java/org/apache/solr/core/SolrEventListener.java
[source-patterns] invalid logging pattern [not private static final, uses static class name]: solr/core/src/java/org/apache/solr/search/SolrCache.java
[source-patterns] invalid logging pattern [not private static final, uses static class name]: solr/core/src/java/org/apache/solr/search/SurroundQParserPlugin.java
[source-patterns] invalid logging pattern [not private static final, uses static class name]: solr/core/src/java/org/apache/solr/servlet/CheckLoggingConfiguration.java
[source-patterns] invalid logging pattern [not private static final, uses static class name]: solr/core/src/java/org/apache/solr/update/processor/IgnoreCommitOptimizeUpdateProcessorFactory.java
[source-patterns] invalid logging pattern [not private static final, uses static class name]: solr/core/src/java/org/apache/solr/update/processor/LogUpdateProcessorFactory.java
[source-patterns] invalid logging pattern [not private static final, uses static class name]: solr/core/src/test/org/apache/solr/logging/TestLogWatcher.java
{noformat}
Maybe these are false positives, but the pattern looks quite good, although it might not be able to correctly handle crazy Eclipse newline formatting.
was (Author: thetaphi):
Hi,
I wrote a pattern detector, basically it does the following: If the java file contains "org.slf4j.LoggerFactory", it will look for some pattern matching 3 occurences of either "private", "static", "final", followed by "LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());"
Without this patch, it reports 433 errors, when the patch posted here is applied it still detects some violations:
{noformat}
[source-patterns] invalid logging pattern [private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());]: solr/core/src/java/org/apache/solr/core/SolrEventListener.java
[source-patterns] invalid logging pattern [private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());]: solr/core/src/java/org/apache/solr/search/SolrCache.java
[source-patterns] invalid logging pattern [private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());]: solr/core/src/java/org/apache/solr/search/SurroundQParserPlugin.java
[source-patterns] invalid logging pattern [private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());]: solr/core/src/java/org/apache/solr/servlet/CheckLoggingConfiguration.java
[source-patterns] invalid logging pattern [private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());]: solr/core/src/java/org/apache/solr/update/processor/IgnoreCommitOptimizeUpdateProcessorFactory.java
[source-patterns] invalid logging pattern [private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());]: solr/core/src/java/org/apache/solr/update/processor/LogUpdateProcessorFactory.java
[source-patterns] invalid logging pattern [private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());]: solr/core/src/test/org/apache/solr/logging/TestLogWatcher.java
{noformat}
Maybe these are false positives, but the pattern looks quite good, although it might not be able to correctly handle crazy Eclipse newline formatting.
> Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
> ---------------------------------------------------------------------------------------------------------------
>
> Key: SOLR-8330
> URL: https://issues.apache.org/jira/browse/SOLR-8330
> Project: Solr
> Issue Type: Sub-task
> Affects Versions: Trunk
> Reporter: Jason Gerlowski
> Assignee: Anshum Gupta
> Labels: logging
> Fix For: Trunk
>
> Attachments: SOLR-8330-detector.patch, SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch
>
>
> As Mike Drob pointed out in Solr-8324, many loggers in Solr are unintentionally shared between classes. Many instances of this are caused by overzealous copy-paste. This can make debugging tougher, as messages appear to come from an incorrect location.
> As discussed in the comments on SOLR-8324, there also might be legitimate reasons for sharing loggers between classes. Where any ambiguity exists, these instances shouldn't be touched.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org