Cleanup Watcher Logging (#83343)

Log errors including the cause instead of sometimes dropping the cause. Use a dedicated class logger instead of the deprecated one for the transport action. Align error log usage to always use the same patter of parameterized message + cause, no need to use a supplier at error level.
elastic · Feb 1, 2022 · b67152a · b67152a
1 parent a062bdf
commit b67152a
Show file tree

Hide file tree

Showing 13 changed files with 30 additions and 32 deletions.
diff --git a/...plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/WatcherIndexingListener.java b/...plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/WatcherIndexingListener.java
@@ -222,7 +222,7 @@ public void clusterChanged(ClusterChangedEvent event) {
                     checkWatchIndexHasChanged(metadata, event);
                 }
             } catch (IllegalStateException e) {
-                logger.error("error loading watches index: [{}]", e.getMessage());
+                logger.error("error loading watches index", e);
                 configuration = INACTIVE;
             }
         }

diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/WatcherService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/WatcherService.java
@@ -385,13 +385,7 @@ private Collection<Watch> loadWatches(ClusterState clusterState) {
                             watches.add(watch);
                         }
                     } catch (Exception e) {
-                        logger.error(
-                            (org.apache.logging.log4j.util.Supplier<?>) () -> new ParameterizedMessage(
-                                "couldn't load watch [{}], ignoring it...",
-                                id
-                            ),
-                            e
-                        );
+                        logger.error(new ParameterizedMessage("couldn't load watch [{}], ignoring it...", id), e);
                     }
                 }
                 SearchScrollRequest request = new SearchScrollRequest(response.getScrollId());

diff --git a/...er/src/main/java/org/elasticsearch/xpack/watcher/execution/AsyncTriggerEventConsumer.java b/...er/src/main/java/org/elasticsearch/xpack/watcher/execution/AsyncTriggerEventConsumer.java
@@ -9,7 +9,6 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.message.ParameterizedMessage;
-import org.apache.logging.log4j.util.Supplier;
 import org.elasticsearch.xpack.core.watcher.trigger.TriggerEvent;
 
 import java.util.function.Consumer;
@@ -30,9 +29,9 @@ public void accept(Iterable<TriggerEvent> events) {
             executionService.processEventsAsync(events);
         } catch (Exception e) {
             logger.error(
-                (Supplier<?>) () -> new ParameterizedMessage(
+                new ParameterizedMessage(
                     "failed to process triggered events [{}]",
-                    (Object) stream(events.spliterator(), false).toArray(size -> new TriggerEvent[size])
+                    (Object) stream(events.spliterator(), false).toArray(TriggerEvent[]::new)
                 ),
                 e
             );

diff --git a/...gin/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/ExecutionService.java b/...gin/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/ExecutionService.java
@@ -9,7 +9,6 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.message.ParameterizedMessage;
-import org.apache.logging.log4j.util.Supplier;
 import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.ResourceNotFoundException;
 import org.elasticsearch.action.ActionListener;
@@ -291,7 +290,10 @@ private void executeTriggeredWatches(
         for (int i = 0; i < response.getItems().length; i++) {
             BulkItemResponse itemResponse = response.getItems()[i];
             if (itemResponse.isFailed()) {
-                logger.error("could not store triggered watch with id [{}]: [{}]", itemResponse.getId(), itemResponse.getFailureMessage());
+                logger.error(
+                    new ParameterizedMessage("could not store triggered watch with id [{}]", itemResponse.getId()),
+                    itemResponse.getFailure().getCause()
+                );
             } else {
                 executeAsync(watchesAndContext.v2().get(i), watchesAndContext.v1().get(i));
             }
@@ -360,7 +362,7 @@ record = createWatchRecord(record, ctx, e);
                             historyStore.put(record);
                         }
                     } catch (Exception e) {
-                        logger.error((Supplier<?>) () -> new ParameterizedMessage("failed to update watch record [{}]", ctx.id()), e);
+                        logger.error(new ParameterizedMessage("failed to update watch record [{}]", ctx.id()), e);
                         // TODO log watch record in logger, when saving in history store failed, otherwise the info is gone!
                     }
                 }
@@ -419,7 +421,7 @@ private WatchRecord createWatchRecord(WatchRecord existingRecord, WatchExecution
     private void logWatchRecord(WatchExecutionContext ctx, Exception e) {
         // failed watches stack traces are only logged in debug, otherwise they should be checked out in the history
         if (logger.isDebugEnabled()) {
-            logger.debug((Supplier<?>) () -> new ParameterizedMessage("failed to execute watch [{}]", ctx.id().watchId()), e);
+            logger.debug(() -> new ParameterizedMessage("failed to execute watch [{}]", ctx.id().watchId()), e);
         } else {
             logger.warn("failed to execute watch [{}]", ctx.id().watchId());
         }
@@ -449,7 +451,7 @@ private void executeAsync(WatchExecutionContext ctx, final TriggeredWatch trigge
                     forcePutHistory(record);
                 } catch (Exception exc) {
                     logger.error(
-                        (Supplier<?>) () -> new ParameterizedMessage(
+                        new ParameterizedMessage(
                             "Error storing watch history record for watch [{}] after thread pool rejection",
                             triggeredWatch.id()
                         ),
@@ -460,7 +462,7 @@ private void executeAsync(WatchExecutionContext ctx, final TriggeredWatch trigge
                     deleteTrigger(triggeredWatch.id());
                 } catch (Exception exc) {
                     logger.error(
-                        (Supplier<?>) () -> new ParameterizedMessage(
+                        new ParameterizedMessage(
                             "Error deleting entry from .triggered_watches for watch [{}] after thread pool rejection",
                             triggeredWatch.id()
                         ),
@@ -505,7 +507,7 @@ private void forcePutHistory(WatchRecord watchRecord) {
             }
         } catch (InterruptedException | ExecutionException | TimeoutException | IOException ioe) {
             final WatchRecord wr = watchRecord;
-            logger.error((Supplier<?>) () -> new ParameterizedMessage("failed to persist watch record [{}]", wr), ioe);
+            logger.error(new ParameterizedMessage("failed to persist watch record [{}]", wr), ioe);
         }
     }
 

diff --git a/...her/src/main/java/org/elasticsearch/xpack/watcher/execution/SyncTriggerEventConsumer.java b/...her/src/main/java/org/elasticsearch/xpack/watcher/execution/SyncTriggerEventConsumer.java
@@ -9,7 +9,6 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.message.ParameterizedMessage;
-import org.apache.logging.log4j.util.Supplier;
 import org.elasticsearch.xpack.core.watcher.trigger.TriggerEvent;
 
 import java.util.function.Consumer;
@@ -31,9 +30,9 @@ public void accept(Iterable<TriggerEvent> events) {
             executionService.processEventsSync(events);
         } catch (Exception e) {
             logger.error(
-                (Supplier<?>) () -> new ParameterizedMessage(
+                new ParameterizedMessage(
                     "failed to process triggered events [{}]",
-                    (Object) stream(events.spliterator(), false).toArray(size -> new TriggerEvent[size])
+                    (Object) stream(events.spliterator(), false).toArray(TriggerEvent[]::new)
                 ),
                 e
             );

diff --git a/...ck/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/history/HistoryStore.java b/...ck/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/history/HistoryStore.java
@@ -9,7 +9,6 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.message.ParameterizedMessage;
-import org.apache.logging.log4j.util.Supplier;
 import org.elasticsearch.action.DocWriteRequest;
 import org.elasticsearch.action.bulk.BulkProcessor;
 import org.elasticsearch.action.index.IndexRequest;
@@ -65,7 +64,7 @@ public void forcePut(WatchRecord watchRecord) {
             bulkProcessor.add(request);
         } catch (IOException ioe) {
             final WatchRecord wr = watchRecord;
-            logger.error((Supplier<?>) () -> new ParameterizedMessage("failed to persist watch record [{}]", wr), ioe);
+            logger.error(new ParameterizedMessage("failed to persist watch record [{}]", wr), ioe);
         }
     }
 

diff --git a/...tcher/src/main/java/org/elasticsearch/xpack/watcher/input/chain/ExecutableChainInput.java b/...tcher/src/main/java/org/elasticsearch/xpack/watcher/input/chain/ExecutableChainInput.java
@@ -8,6 +8,7 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.elasticsearch.core.Tuple;
 import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext;
 import org.elasticsearch.xpack.core.watcher.input.ExecutableInput;
@@ -24,7 +25,7 @@
 public class ExecutableChainInput extends ExecutableInput<ChainInput, ChainInput.Result> {
     private static final Logger logger = LogManager.getLogger(ExecutableChainInput.class);
 
-    private List<Tuple<String, ExecutableInput<?, ?>>> inputs;
+    private final List<Tuple<String, ExecutableInput<?, ?>>> inputs;
 
     public ExecutableChainInput(ChainInput input, List<Tuple<String, ExecutableInput<?, ?>>> inputs) {
         super(input);
@@ -45,7 +46,7 @@ public ChainInput.Result execute(WatchExecutionContext ctx, Payload payload) {
 
             return new ChainInput.Result(results, new Payload.Simple(payloads));
         } catch (Exception e) {
-            logger.error("failed to execute [{}] input for watch [{}], reason [{}]", TYPE, ctx.watch().id(), e.getMessage());
+            logger.error(new ParameterizedMessage("failed to execute [{}] input for watch [{}]", TYPE, ctx.watch().id()), e);
             return new ChainInput.Result(e);
         }
     }

diff --git a/...watcher/src/main/java/org/elasticsearch/xpack/watcher/input/http/ExecutableHttpInput.java b/...watcher/src/main/java/org/elasticsearch/xpack/watcher/input/http/ExecutableHttpInput.java
@@ -8,6 +8,7 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
 import org.elasticsearch.common.xcontent.XContentHelper;
@@ -50,7 +51,7 @@ public HttpInput.Result execute(WatchExecutionContext ctx, Payload payload) {
             request = input.getRequest().render(templateEngine, model);
             return doExecute(ctx, request);
         } catch (Exception e) {
-            logger.error("failed to execute [{}] input for watch [{}], reason [{}]", TYPE, ctx.watch().id(), e.getMessage());
+            logger.error(new ParameterizedMessage("failed to execute [{}] input for watch [{}]", TYPE, ctx.watch().id()), e);
             return new HttpInput.Result(request, e);
         }
     }

diff --git a/...her/src/main/java/org/elasticsearch/xpack/watcher/input/search/ExecutableSearchInput.java b/...her/src/main/java/org/elasticsearch/xpack/watcher/input/search/ExecutableSearchInput.java
@@ -8,6 +8,7 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.elasticsearch.action.search.SearchRequest;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.action.search.SearchType;
@@ -70,7 +71,7 @@ public SearchInput.Result execute(WatchExecutionContext ctx, Payload payload) {
             request = new WatcherSearchTemplateRequest(input.getRequest(), new BytesArray(renderedTemplate));
             return doExecute(ctx, request);
         } catch (Exception e) {
-            logger.error("failed to execute [{}] input for watch [{}], reason [{}]", TYPE, ctx.watch().id(), e.getMessage());
+            logger.error(new ParameterizedMessage("failed to execute [{}] input for watch [{}]", TYPE, ctx.watch().id()), e);
             return new SearchInput.Result(request, e);
         }
     }

diff --git a/...ain/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateService.java b/...ain/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateService.java
@@ -40,7 +40,7 @@ public WatcherSearchTemplateService(ScriptService scriptService, NamedXContentRe
         this.xContentRegistry = xContentRegistry;
     }
 
-    public String renderTemplate(Script source, WatchExecutionContext ctx, Payload payload) throws IOException {
+    public String renderTemplate(Script source, WatchExecutionContext ctx, Payload payload) {
         // Due the inconsistency with templates in ES 1.x, we maintain our own template format.
         // This template format we use now, will become the template structure in ES 2.0
         Map<String, Object> watcherContextParams = Variables.createCtxParamsMap(ctx, payload);

diff --git a/...main/java/org/elasticsearch/xpack/watcher/transform/script/ExecutableScriptTransform.java b/...main/java/org/elasticsearch/xpack/watcher/transform/script/ExecutableScriptTransform.java
@@ -8,7 +8,6 @@
 
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.message.ParameterizedMessage;
-import org.apache.logging.log4j.util.Supplier;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext;
@@ -38,7 +37,7 @@ public ScriptTransform.Result execute(WatchExecutionContext ctx, Payload payload
         try {
             return doExecute(ctx, payload);
         } catch (Exception e) {
-            logger.error((Supplier<?>) () -> new ParameterizedMessage("failed to execute [{}] transform for [{}]", TYPE, ctx.id()), e);
+            logger.error(new ParameterizedMessage("failed to execute [{}] transform for [{}]", TYPE, ctx.id()), e);
             return new ScriptTransform.Result(e);
         }
     }

diff --git a/...main/java/org/elasticsearch/xpack/watcher/transform/search/ExecutableSearchTransform.java b/...main/java/org/elasticsearch/xpack/watcher/transform/search/ExecutableSearchTransform.java
@@ -8,7 +8,6 @@
 
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.message.ParameterizedMessage;
-import org.apache.logging.log4j.util.Supplier;
 import org.elasticsearch.action.search.SearchRequest;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.action.search.SearchType;
@@ -71,7 +70,7 @@ public SearchTransform.Result execute(WatchExecutionContext ctx, Payload payload
             }
             return new SearchTransform.Result(request, new Payload.XContent(resp, params));
         } catch (Exception e) {
-            logger.error((Supplier<?>) () -> new ParameterizedMessage("failed to execute [{}] transform for [{}]", TYPE, ctx.id()), e);
+            logger.error(new ParameterizedMessage("failed to execute [{}] transform for [{}]", TYPE, ctx.id()), e);
             return new SearchTransform.Result(request, e);
         }
     }

diff --git a/...n/java/org/elasticsearch/xpack/watcher/transport/actions/TransportExecuteWatchAction.java b/...n/java/org/elasticsearch/xpack/watcher/transport/actions/TransportExecuteWatchAction.java
@@ -6,6 +6,8 @@
  */
 package org.elasticsearch.xpack.watcher.transport.actions;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.elasticsearch.ResourceNotFoundException;
 import org.elasticsearch.action.ActionListener;
@@ -59,6 +61,8 @@
  */
 public class TransportExecuteWatchAction extends WatcherTransportAction<ExecuteWatchRequest, ExecuteWatchResponse> {
 
+    private static final Logger logger = LogManager.getLogger(TransportExecuteWatchAction.class);
+
     private final ThreadPool threadPool;
     private final ExecutionService executionService;
     private final Clock clock;