[
https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15030197#comment-15030197
]
Jason Gerlowski commented on SOLR-8330:
---------------------------------------
I think having a source-patterns check is a good idea in general, but it might
not be as useful as we hope.
For example, a number of the violations in Uwe's comment above are (arguably)
false positives.
1.) Several interfaces contain Logger declarations. These declarations can't
be private since interfaces can't contain private members. These Loggers
should probably be removed entirely. I didn't do this in my initial patch
because I was making an effort to be conservative about what I changed, but I
really don't think there's a reason to keep them around. This affects
{{SolrEventListener}}, and {{SolrCache}}.
2.) Some Java files contain two non-nested classes which share loggers. For
example:
{code:java}
//All code in file A.java
class A {
static final Logger log = ....;
}
class B {
static final Logger log = B.log;
}
{code}
Because the classes aren't nested, the logger can't be a private member. The
classes can be made to use separate loggers, but this would break Mike Drob's
point that:
bq: As somebody that looks at customer logs for the majority of my interaction
with Solr, I would want Loggers to correspond to file names every single time.
This corner case affects {{SurroundQParserPlugin}},
{{IgnoreCommitOptimizeUpdateProcessorFactory}}, and
{{LogUpdateProcessorFactory}}.
3.) Some of the Java files reported above are actually testing log4j
configuration/setup. Often, these tests declare Loggers in non-standard ways.
For example {{TestLogWatcher}} declares a Logger as a local variable in a test
method.
To clarify my point a bit, I'm not saying that a we shouldn't have a
source-analysis check for this. But the fact that there are already corner
cases/edge cases probably means this shouldn't be used as pass/fail criteria
for {{ant validate}}. And unfortunately it might mean that the check will be
ignored more often than not (since it doesn't really have any "teeth").
> 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-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
