honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-03-24 17:09:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

Disallow logger methods with Object parameter (#28969)

Browse Source 
Log4j2 provides a wide range of logging methods. Our code typically only uses a subset of them. In particular, uses of the methods trace|debug|info|warn|error|fatal(Object) or trace|debug|info|warn|error|fatal(Object, Throwable) have all been wrong, leading to not properly logging the provided message. To prevent these issues in the future, the corresponding Logger methods have been blacklisted.
This commit is contained in:
Yannick Welsch 2018-03-12 03:05:24 -07:00 committed by GitHub
parent 7afe5ad943
commit be7f5dde24
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 36 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
buildSrc/src/main/resources/forbidden/es-server-signatures.txt
View File

@ -133,3 +133,17 @@ java.time.OffsetDateTime#withYear(int)
java.time.zone.ZoneRules#getStandardOffset(java.time.Instant)
java.time.zone.ZoneRules#getDaylightSavings(java.time.Instant)
java.time.zone.ZoneRules#isDaylightSavings(java.time.Instant)
@defaultMessage Use logger methods with non-Object parameter
org.apache.logging.log4j.Logger#trace(java.lang.Object)
org.apache.logging.log4j.Logger#trace(java.lang.Object, java.lang.Throwable)
org.apache.logging.log4j.Logger#debug(java.lang.Object)
org.apache.logging.log4j.Logger#debug(java.lang.Object, java.lang.Throwable)
org.apache.logging.log4j.Logger#info(java.lang.Object)
org.apache.logging.log4j.Logger#info(java.lang.Object, java.lang.Throwable)
org.apache.logging.log4j.Logger#warn(java.lang.Object)
org.apache.logging.log4j.Logger#warn(java.lang.Object, java.lang.Throwable)
org.apache.logging.log4j.Logger#error(java.lang.Object)
org.apache.logging.log4j.Logger#error(java.lang.Object, java.lang.Throwable)
org.apache.logging.log4j.Logger#fatal(java.lang.Object)
org.apache.logging.log4j.Logger#fatal(java.lang.Object, java.lang.Throwable)

2
server/src/main/java/org/elasticsearch/common/util/concurrent/QueueResizingEsThreadPoolExecutor.java
View File

@ -197,7 +197,7 @@ public final class QueueResizingEsThreadPoolExecutor extends EsThreadPoolExecuto
}
} catch (ArithmeticException e) {
// There was an integer overflow, so just log about it, rather than adjust the queue size
logger.warn((Supplier<?>) () -> new ParameterizedMessage(
logger.warn(() -> new ParameterizedMessage(
"failed to calculate optimal queue size for [{}] thread pool, " +
"total frame time [{}ns], tasks [{}], task execution time [{}ns]",
getName(), totalRuntime, tasksPerFrame, totalNanos),

8
server/src/main/java/org/elasticsearch/index/IndexService.java
View File

@ -448,7 +448,7 @@ public class IndexService extends AbstractIndexComponent implements IndicesClust
}
} catch (Exception e) {
logger.warn(
(Supplier<?>) () -> new ParameterizedMessage(
() -> new ParameterizedMessage(
"[{}] failed to close store on shard removal (reason: [{}])", shardId, reason), e);
}
}
@ -467,7 +467,7 @@ public class IndexService extends AbstractIndexComponent implements IndicesClust
} catch (IOException e) {
shardStoreDeleter.addPendingDelete(lock.getShardId(), indexSettings);
logger.debug(
(Supplier<?>) () -> new ParameterizedMessage(
() -> new ParameterizedMessage(
"[{}] failed to delete shard content - scheduled a retry", lock.getShardId().id()), e);
}
}
@ -623,7 +623,7 @@ public class IndexService extends AbstractIndexComponent implements IndicesClust
shard.onSettingsChanged();
} catch (Exception e) {
logger.warn(
(Supplier<?>) () -> new ParameterizedMessage(
() -> new ParameterizedMessage(
"[{}] failed to notify shard about setting change", shard.shardId().id()), e);
}
}
@ -832,7 +832,7 @@ public class IndexService extends AbstractIndexComponent implements IndicesClust
if (lastThrownException == null || sameException(lastThrownException, ex) == false) {
// prevent the annoying fact of logging the same stuff all the time with an interval of 1 sec will spam all your logs
indexService.logger.warn(
(Supplier<?>) () -> new ParameterizedMessage(
() -> new ParameterizedMessage(
"failed to run task {} - suppressing re-occurring exceptions unless the exception changes",
toString()),
ex);

18
server/src/main/java/org/elasticsearch/indices/IndicesService.java
View File

@ -311,7 +311,7 @@ public class IndicesService extends AbstractLifecycleComponent
}
} catch (IllegalIndexShardStateException | AlreadyClosedException e) {
// we can safely ignore illegal state on ones that are closing for example
logger.trace((Supplier<?>) () -> new ParameterizedMessage("{} ignoring shard stats", indexShard.shardId()), e);
logger.trace(() -> new ParameterizedMessage("{} ignoring shard stats", indexShard.shardId()), e);
}
}
}
@ -561,7 +561,7 @@ public class IndicesService extends AbstractLifecycleComponent
deleteIndexStore(extraInfo, indexService.index(), indexSettings);
}
} catch (Exception e) {
logger.warn((Supplier<?>) () -> new ParameterizedMessage("failed to remove index {} ([{}][{}])", index, reason, extraInfo), e);
logger.warn(() -> new ParameterizedMessage("failed to remove index {} ([{}][{}])", index, reason, extraInfo), e);
}
}
@ -617,7 +617,7 @@ public class IndicesService extends AbstractLifecycleComponent
}
deleteIndexStore(reason, metaData, clusterState);
} catch (Exception e) {
logger.warn((Supplier<?>) () -> new ParameterizedMessage("[{}] failed to delete unassigned index (reason [{}])", metaData.getIndex(), reason), e);
logger.warn(() -> new ParameterizedMessage("[{}] failed to delete unassigned index (reason [{}])", metaData.getIndex(), reason), e);
}
}
}
@ -669,9 +669,9 @@ public class IndicesService extends AbstractLifecycleComponent
}
success = true;
} catch (LockObtainFailedException ex) {
logger.debug((Supplier<?>) () -> new ParameterizedMessage("{} failed to delete index store - at least one shards is still locked", index), ex);
logger.debug(() -> new ParameterizedMessage("{} failed to delete index store - at least one shards is still locked", index), ex);
} catch (Exception ex) {
logger.warn((Supplier<?>) () -> new ParameterizedMessage("{} failed to delete index", index), ex);
logger.warn(() -> new ParameterizedMessage("{} failed to delete index", index), ex);
} finally {
if (success == false) {
addPendingDelete(index, indexSettings);
@ -774,7 +774,7 @@ public class IndicesService extends AbstractLifecycleComponent
try {
metaData = metaStateService.loadIndexState(index);
} catch (Exception e) {
logger.warn((Supplier<?>) () -> new ParameterizedMessage("[{}] failed to load state file from a stale deleted index, folders will be left on disk", index), e);
logger.warn(() -> new ParameterizedMessage("[{}] failed to load state file from a stale deleted index, folders will be left on disk", index), e);
return null;
}
final IndexSettings indexSettings = buildIndexSettings(metaData);
@ -783,7 +783,7 @@ public class IndicesService extends AbstractLifecycleComponent
} catch (Exception e) {
// we just warn about the exception here because if deleteIndexStoreIfDeletionAllowed
// throws an exception, it gets added to the list of pending deletes to be tried again
logger.warn((Supplier<?>) () -> new ParameterizedMessage("[{}] failed to delete index on disk", metaData.getIndex()), e);
logger.warn(() -> new ParameterizedMessage("[{}] failed to delete index on disk", metaData.getIndex()), e);
}
return metaData;
}
@ -960,7 +960,7 @@ public class IndicesService extends AbstractLifecycleComponent
nodeEnv.deleteIndexDirectoryUnderLock(index, indexSettings);
iterator.remove();
} catch (IOException ex) {
logger.debug((Supplier<?>) () -> new ParameterizedMessage("{} retry pending delete", index), ex);
logger.debug(() -> new ParameterizedMessage("{} retry pending delete", index), ex);
}
} else {
assert delete.shardId != -1;
@ -970,7 +970,7 @@ public class IndicesService extends AbstractLifecycleComponent
deleteShardStore("pending delete", shardLock, delete.settings);
iterator.remove();
} catch (IOException ex) {
logger.debug((Supplier<?>) () -> new ParameterizedMessage("{} retry pending delete", shardLock.getShardId()), ex);
logger.debug(() -> new ParameterizedMessage("{} retry pending delete", shardLock.getShardId()), ex);
}
} else {
logger.warn("{} no shard lock for pending delete", delete.shardId);

2
server/src/main/java/org/elasticsearch/rest/RestController.java
View File

@ -177,7 +177,7 @@ public class RestController extends AbstractComponent implements HttpServerTrans
channel.sendResponse(new BytesRestResponse(channel, e));
} catch (Exception inner) {
inner.addSuppressed(e);
logger.error((Supplier<?>) () ->
logger.error(() ->
new ParameterizedMessage("failed to send failure response for uri [{}]", request.uri()), inner);
}
}

15
server/src/main/java/org/elasticsearch/transport/TransportService.java
View File

@ -249,7 +249,7 @@ public class TransportService extends AbstractLifecycleComponent {
public void onRejection(Exception e) {
// if we get rejected during node shutdown we don't wanna bubble it up
logger.debug(
(Supplier<?>) () -> new ParameterizedMessage(
() -> new ParameterizedMessage(
"failed to notify response handler on rejection, action: {}",
holderToNotify.action()),
e);
@ -257,7 +257,7 @@ public class TransportService extends AbstractLifecycleComponent {
@Override
public void onFailure(Exception e) {
logger.warn(
(Supplier<?>) () -> new ParameterizedMessage(
() -> new ParameterizedMessage(
"failed to notify response handler on exception, action: {}",
holderToNotify.action()),
e);
@ -611,7 +611,7 @@ public class TransportService extends AbstractLifecycleComponent {
public void onRejection(Exception e) {
// if we get rejected during node shutdown we don't wanna bubble it up
logger.debug(
(Supplier<?>) () -> new ParameterizedMessage(
() -> new ParameterizedMessage(
"failed to notify response handler on rejection, action: {}",
holderToNotify.action()),
e);
@ -619,7 +619,7 @@ public class TransportService extends AbstractLifecycleComponent {
@Override
public void onFailure(Exception e) {
logger.warn(
(Supplier<?>) () -> new ParameterizedMessage(
() -> new ParameterizedMessage(
"failed to notify response handler on exception, action: {}",
holderToNotify.action()),
e);
@ -667,8 +667,7 @@ public class TransportService extends AbstractLifecycleComponent {
channel.sendResponse(e);
} catch (Exception inner) {
inner.addSuppressed(e);
logger.warn(
(Supplier<?>) () -> new ParameterizedMessage(
logger.warn(() -> new ParameterizedMessage(
"failed to notify channel of error message for action [{}]", action), inner);
}
}
@ -681,7 +680,7 @@ public class TransportService extends AbstractLifecycleComponent {
} catch (Exception inner) {
inner.addSuppressed(e);
logger.warn(
(Supplier<?>) () -> new ParameterizedMessage(
() -> new ParameterizedMessage(
"failed to notify channel of error message for action [{}]", action), inner);
}
}
@ -1191,7 +1190,7 @@ public class TransportService extends AbstractLifecycleComponent {
handler.handleException(rtx);
} catch (Exception e) {
logger.error(
(Supplier<?>) () -> new ParameterizedMessage(
() -> new ParameterizedMessage(
"failed to handle exception for action [{}], handler [{}]", action, handler), e);
}
}