Skip to content

Disallow logger methods with Object parameter #28969

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 12, 2018
Merged

Disallow logger methods with Object parameter #28969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e00699e
Disallow logger methods with Object parameter
ywelsch Mar 9, 2018
05a981e
Fix logger usages
ywelsch Mar 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions buildSrc/src/main/resources/forbidden/es-server-signatures.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 changes: 1 addition & 1 deletion ...main/java/org/elasticsearch/common/util/concurrent/QueueResizingEsThreadPoolExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ protected void afterExecute(Runnable r, Throwable t) {
}
} 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),
Expand Down
8 changes: 4 additions & 4 deletions server/src/main/java/org/elasticsearch/index/IndexService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ private void closeShard(String reason, ShardId sId, IndexShard indexShard, Store
}
} catch (Exception e) {
logger.warn(
(Supplier<?>) () -> new ParameterizedMessage(
() -> new ParameterizedMessage(
"[{}] failed to close store on shard removal (reason: [{}])", shardId, reason), e);
}
}
Expand All @@ -467,7 +467,7 @@ private void onShardClose(ShardLock lock) {
} 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);
}
}
Expand Down Expand Up @@ -623,7 +623,7 @@ public synchronized void updateMetaData(final IndexMetaData metadata) {
shard.onSettingsChanged();
} catch (Exception e) {
logger.warn(
(Supplier<?>) () -> new ParameterizedMessage(
() -> new ParameterizedMessage(
"[{}] failed to notify shard about setting change", shard.shardId().id()), e);
}
}
Expand Down Expand Up @@ -832,7 +832,7 @@ public final void run() {
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);
Expand Down
18 changes: 9 additions & 9 deletions server/src/main/java/org/elasticsearch/indices/IndicesService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ Map<Index, List<IndexShardStats>> statsByShard(final IndicesService indicesServi
}
} 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);
}
}
}
Expand Down Expand Up @@ -561,7 +561,7 @@ public void removeIndex(final Index index, final IndexRemovalReason reason, fina
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);
}
}

Expand Down Expand Up @@ -617,7 +617,7 @@ public void deleteUnassignedIndex(String reason, IndexMetaData metaData, Cluster
}
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);
}
}
}
Expand Down Expand Up @@ -669,9 +669,9 @@ private void deleteIndexStoreIfDeletionAllowed(final String reason, final Index
}
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);
Expand Down Expand Up @@ -774,7 +774,7 @@ public IndexMetaData verifyIndexIsDeleted(final Index index, final ClusterState
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);
Expand All @@ -783,7 +783,7 @@ public IndexMetaData verifyIndexIsDeleted(final Index index, final ClusterState
} 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;
}
Expand Down Expand Up @@ -960,7 +960,7 @@ public void processPendingDeletes(Index index, IndexSettings indexSettings, Time
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;
Expand All @@ -970,7 +970,7 @@ public void processPendingDeletes(Index index, IndexSettings indexSettings, Time
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);
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/rest/RestController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ public void dispatchRequest(RestRequest request, RestChannel channel, ThreadCont
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);
}
}
Expand Down
15 changes: 7 additions & 8 deletions server/src/main/java/org/elasticsearch/transport/TransportService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,15 +249,15 @@ protected void doStop() {
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);
}
@Override
public void onFailure(Exception e) {
logger.warn(
(Supplier<?>) () -> new ParameterizedMessage(
() -> new ParameterizedMessage(
"failed to notify response handler on exception, action: {}",
holderToNotify.action()),
e);
Expand Down Expand Up @@ -611,15 +611,15 @@ private <T extends TransportResponse> void sendRequestInternal(final Transport.C
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);
}
@Override
public void onFailure(Exception e) {
logger.warn(
(Supplier<?>) () -> new ParameterizedMessage(
() -> new ParameterizedMessage(
"failed to notify response handler on exception, action: {}",
holderToNotify.action()),
e);
Expand Down Expand Up @@ -667,8 +667,7 @@ public void onFailure(Exception e) {
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);
}
}
Expand All @@ -681,7 +680,7 @@ public void onFailure(Exception e) {
} catch (Exception inner) {
inner.addSuppressed(e);
logger.warn(
(Supplier<?>) () -> new ParameterizedMessage(
() -> new ParameterizedMessage(
"failed to notify channel of error message for action [{}]", action), inner);
}
}
Expand Down Expand Up @@ -1191,7 +1190,7 @@ protected void processException(final TransportResponseHandler handler, final Re
handler.handleException(rtx);
} catch (Exception e) {
logger.error(
(Supplier<?>) () -> new ParameterizedMessage(
() -> new ParameterizedMessage(
"failed to handle exception for action [{}], handler [{}]", action, handler), e);
}
}
Expand Down