Complete Elasticsearch logger names #20457
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit modifies the logger names within Elasticsearch to be the fully-qualified class name as opposed removing the org.elasticsearch prefix and dropping the class name. This change separates the root logger from the Elasticsearch loggers (they were equated from the removal of the org.elasticsearch prefix) and enables log levels to be set at the class level (instead of the package level).
jasontedor
added
>enhancement
blocker
review
:Core/Infra/Logging
|
retest this please |
|
LGTM |
|
retest this please |
|
This pull request unearthed an issue that exists in master today regarding the hierarchy of setting logger levels. I opened #20463. |
Today when associating a logger with a Lucene info stream component, we equate all components with lucene.iw except for IFD which we associate with lucene.ifd. This commit modifies this so that these logger names are now derived from the component name in all cases (we simply append the component name to the parent logger name, with a dot prefix).
|
@jasontedor the Lucene logger name commit LGTM |
|
Thank you @abeyad. |
jasontedor
added a commit
that referenced
this pull request
Sep 14, 2016
This commit modifies the logger names within Elasticsearch to be the fully-qualified class name as opposed removing the org.elasticsearch prefix and dropping the class name. This change separates the root logger from the Elasticsearch loggers (they were equated from the removal of the org.elasticsearch prefix) and enables log levels to be set at the class level (instead of the package level). Relates #20457
jasontedor
added a commit
that referenced
this pull request
Sep 14, 2016
This commit modifies the logger names within Elasticsearch to be the fully-qualified class name as opposed removing the org.elasticsearch prefix and dropping the class name. This change separates the root logger from the Elasticsearch loggers (they were equated from the removal of the org.elasticsearch prefix) and enables log levels to be set at the class level (instead of the package level). Relates #20457
mikemccand
reviewed
Sep 19, 2016
| } else { | ||
| return logger; | ||
| } | ||
| return Loggers.getLogger(parentLogger, "." + component); |
There was a problem hiding this comment.
I think this change inadvertently slowed down indexing: see annot AZ at https://benchmarks-old.elastic.co/index.html#indexing
The problem is (confusingly!) this private method is called for every log message that Lucene's IndexWriter makes. Previously, we invoked Loggers.getLogger just twice on creating LoggerInfoStream but now we are calling it for every log message.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit modifies the logger names within Elasticsearch to be the
fully-qualified class name as opposed removing the org.elasticsearch
prefix and dropping the class name. This change separates the root
logger from the Elasticsearch loggers (they were equated from the
removal of the org.elasticsearch prefix) and enables log levels to be
set at the class level (instead of the package level).
Closes #20326