From 80593fb23c37b62aff205dfd0a61471f198e7151 Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Tue, 10 Oct 2017 09:07:33 +0200 Subject: [PATCH] Watcher: Add execution state to watch status (elastic/x-pack-elasticsearch#2699) The execution state is kind of a global indicator if a watch has been running successfully and is used by the watcher UI. However this field is only stored in the watch history but not part of the watch status, thus it is not available everywhere. In order to simplify the watcher UI this commit also adds the field to the watch status which is stored together with the watch. It is stored under the `status.execution_state` field as `status.state` is already taken. This is also reflects with the name of the java class. The WatchStatus class does not contain serialization checks, as this is intended to be backported to 6.x, where those checks will be added. Once the backport is done, the old execution state field can be fully deleted from the master branch in another commit (syncing with Kibana folks required). relates elastic/x-pack-elasticsearch#2385 * fix doc tests Original commit: elastic/x-pack-elasticsearch@26e8f995717b1216c61fd1a2563c23bc919a37bf --- docs/en/rest-api/watcher/ack-watch.asciidoc | 3 ++ .../rest-api/watcher/execute-watch.asciidoc | 1 + .../execution/WatchExecutionContext.java | 11 +++-- .../xpack/watcher/history/WatchRecord.java | 2 +- .../xpack/watcher/watch/WatchStatus.java | 43 ++++++++++++++++--- .../execution/ExecutionServiceTests.java | 2 + .../test/watcher/execute_watch/10_basic.yml | 3 ++ .../watcher/execute_watch/20_transform.yml | 3 ++ .../watcher/execute_watch/30_throttled.yml | 3 ++ .../execute_watch/40_ignore_condition.yml | 1 + .../watcher/execute_watch/50_action_mode.yml | 2 + ...0_put_watch_with_index_action_using_id.yml | 4 ++ 12 files changed, 68 insertions(+), 10 deletions(-) diff --git a/docs/en/rest-api/watcher/ack-watch.asciidoc b/docs/en/rest-api/watcher/ack-watch.asciidoc index cd04b047f7b..8f246661f45 100644 --- a/docs/en/rest-api/watcher/ack-watch.asciidoc +++ b/docs/en/rest-api/watcher/ack-watch.asciidoc @@ -148,6 +148,7 @@ and the action is now in `ackable` state: } }, "state": ..., + "execution_state": "executed", "last_checked": ..., "last_met_condition": ... }, @@ -195,6 +196,7 @@ GET _xpack/watcher/watch/my_watch } }, "state": ..., + "execution_state": "executed", "last_checked": ..., "last_met_condition": ... }, @@ -260,6 +262,7 @@ The response looks like a get watch response, but only contains the status: } } }, + "execution_state": "executed", "version": 2 } } diff --git a/docs/en/rest-api/watcher/execute-watch.asciidoc b/docs/en/rest-api/watcher/execute-watch.asciidoc index 09b40c43d68..bb9f9c5b9ee 100644 --- a/docs/en/rest-api/watcher/execute-watch.asciidoc +++ b/docs/en/rest-api/watcher/execute-watch.asciidoc @@ -185,6 +185,7 @@ This is an example of the output: "state": "executed", "status": { "version": 1, + "execution_state": "executed", "state": { "active": true, "timestamp": "2015-06-02T23:17:55.111Z" diff --git a/plugin/src/main/java/org/elasticsearch/xpack/watcher/execution/WatchExecutionContext.java b/plugin/src/main/java/org/elasticsearch/xpack/watcher/execution/WatchExecutionContext.java index 04ef1ed44eb..0b58649ab4f 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/watcher/execution/WatchExecutionContext.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/watcher/execution/WatchExecutionContext.java @@ -202,25 +202,27 @@ public abstract class WatchExecutionContext { return Collections.unmodifiableMap(actionsResults); } - public WatchRecord abortBeforeExecution(ExecutionState state, String message) { + WatchRecord abortBeforeExecution(ExecutionState state, String message) { assert !phase.sealed(); phase = ExecutionPhase.ABORTED; return new WatchRecord.MessageWatchRecord(id, triggerEvent, state, message, getNodeId()); } - public WatchRecord abortFailedExecution(String message) { + WatchRecord abortFailedExecution(String message) { assert !phase.sealed(); phase = ExecutionPhase.ABORTED; long executionTime = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - relativeStartTime); WatchExecutionResult result = new WatchExecutionResult(this, executionTime); + watch().status().setExecutionState(WatchRecord.getState(result)); return new WatchRecord.MessageWatchRecord(this, result, message); } - public WatchRecord abortFailedExecution(Exception e) { + WatchRecord abortFailedExecution(Exception e) { assert !phase.sealed(); phase = ExecutionPhase.ABORTED; long executionTime = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - relativeStartTime); WatchExecutionResult result = new WatchExecutionResult(this, executionTime); + watch().status().setExecutionState(WatchRecord.getState(result)); return new WatchRecord.ExceptionWatchRecord(this, result, e); } @@ -229,10 +231,11 @@ public abstract class WatchExecutionContext { phase = ExecutionPhase.FINISHED; long executionTime = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - relativeStartTime); WatchExecutionResult result = new WatchExecutionResult(this, executionTime); + watch().status().setExecutionState(WatchRecord.getState(result)); return new WatchRecord.MessageWatchRecord(this, result); } - public WatchExecutionSnapshot createSnapshot(Thread executionThread) { + WatchExecutionSnapshot createSnapshot(Thread executionThread) { return new WatchExecutionSnapshot(this, executionThread.getStackTrace()); } } diff --git a/plugin/src/main/java/org/elasticsearch/xpack/watcher/history/WatchRecord.java b/plugin/src/main/java/org/elasticsearch/xpack/watcher/history/WatchRecord.java index 7a1553f59f9..fdb29b2a5f0 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/watcher/history/WatchRecord.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/watcher/history/WatchRecord.java @@ -95,7 +95,7 @@ public abstract class WatchRecord implements ToXContentObject { context.watch().condition(), context.watch().metadata(), context.watch(), executionResult, context.getNodeId()); } - private static ExecutionState getState(WatchExecutionResult executionResult) { + public static ExecutionState getState(WatchExecutionResult executionResult) { if (executionResult == null || executionResult.conditionResult() == null) { return ExecutionState.FAILED; } diff --git a/plugin/src/main/java/org/elasticsearch/xpack/watcher/watch/WatchStatus.java b/plugin/src/main/java/org/elasticsearch/xpack/watcher/watch/WatchStatus.java index 75289d2de51..bf980afa910 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/watcher/watch/WatchStatus.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/watcher/watch/WatchStatus.java @@ -17,6 +17,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.xpack.watcher.actions.Action; import org.elasticsearch.xpack.watcher.actions.ActionStatus; import org.elasticsearch.xpack.watcher.actions.throttler.AckThrottler; +import org.elasticsearch.xpack.watcher.execution.ExecutionState; import org.elasticsearch.xpack.watcher.support.xcontent.WatcherXContentParser; import org.joda.time.DateTime; @@ -41,6 +42,7 @@ public class WatchStatus implements ToXContentObject, Streamable { private State state; + @Nullable private ExecutionState executionState; @Nullable private DateTime lastChecked; @Nullable private DateTime lastMetCondition; @Nullable private long version; @@ -51,19 +53,21 @@ public class WatchStatus implements ToXContentObject, Streamable { } public WatchStatus(DateTime now, Map actions) { - this(-1, new State(true, now), null, null, actions); + this(-1, new State(true, now), null, null, null, actions); } public WatchStatus(WatchStatus other) { - this(other.version, other.state, other.lastChecked, other.lastMetCondition, other.actions); + this(other.version, other.state, other.executionState, other.lastChecked, other.lastMetCondition, other.actions); } - private WatchStatus(long version, State state, DateTime lastChecked, DateTime lastMetCondition, Map actions) { + private WatchStatus(long version, State state, ExecutionState executionState, DateTime lastChecked, DateTime lastMetCondition, + Map actions) { this.version = version; this.lastChecked = lastChecked; this.lastMetCondition = lastMetCondition; this.actions = actions; this.state = state; + this.executionState = executionState; } public State state() { @@ -90,6 +94,14 @@ public class WatchStatus implements ToXContentObject, Streamable { this.version = version; } + public void setExecutionState(ExecutionState executionState) { + this.executionState = executionState; + } + + public ExecutionState getExecutionState() { + return executionState; + } + @Override public boolean equals(Object o) { if (this == o) return true; @@ -100,12 +112,13 @@ public class WatchStatus implements ToXContentObject, Streamable { return Objects.equals(lastChecked, that.lastChecked) && Objects.equals(lastMetCondition, that.lastMetCondition) && Objects.equals(version, that.version) && + Objects.equals(executionState, that.executionState) && Objects.equals(actions, that.actions); } @Override public int hashCode() { - return Objects.hash(lastChecked, lastMetCondition, actions, version); + return Objects.hash(lastChecked, lastMetCondition, actions, version, executionState); } /** @@ -185,6 +198,10 @@ public class WatchStatus implements ToXContentObject, Streamable { } out.writeBoolean(state.active); writeDate(out, state.timestamp); + out.writeBoolean(executionState != null); + if (executionState != null) { + out.writeString(executionState.id()); + } } @Override @@ -199,6 +216,10 @@ public class WatchStatus implements ToXContentObject, Streamable { } this.actions = unmodifiableMap(actions); state = new State(in.readBoolean(), readDate(in, UTC)); + boolean executionStateExists = in.readBoolean(); + if (executionStateExists) { + executionState = ExecutionState.resolve(in.readString()); + } } public static WatchStatus read(StreamInput in) throws IOException { @@ -226,12 +247,16 @@ public class WatchStatus implements ToXContentObject, Streamable { } builder.endObject(); } + if (executionState != null) { + builder.field(Field.EXECUTION_STATE.getPreferredName(), executionState.id()); + } builder.field(Field.VERSION.getPreferredName(), version); return builder.endObject(); } public static WatchStatus parse(String watchId, XContentParser parser, Clock clock) throws IOException { State state = null; + ExecutionState executionState = null; DateTime lastChecked = null; DateTime lastMetCondition = null; Map actions = null; @@ -270,6 +295,13 @@ public class WatchStatus implements ToXContentObject, Streamable { throw new ElasticsearchParseException("could not parse watch status for [{}]. expecting field [{}] to hold a date " + "value, found [{}] instead", watchId, currentFieldName, token); } + } else if (Field.EXECUTION_STATE.match(currentFieldName)) { + if (token.isValue()) { + executionState = ExecutionState.resolve(parser.text()); + } else { + throw new ElasticsearchParseException("could not parse watch status for [{}]. expecting field [{}] to hold a string " + + "value, found [{}] instead", watchId, currentFieldName, token); + } } else if (Field.ACTIONS.match(currentFieldName)) { actions = new HashMap<>(); if (token == XContentParser.Token.START_OBJECT) { @@ -296,7 +328,7 @@ public class WatchStatus implements ToXContentObject, Streamable { } actions = actions == null ? emptyMap() : unmodifiableMap(actions); - return new WatchStatus(version, state, lastChecked, lastMetCondition, actions); + return new WatchStatus(version, state, executionState, lastChecked, lastMetCondition, actions); } public static class State implements ToXContentObject { @@ -354,5 +386,6 @@ public class WatchStatus implements ToXContentObject, Streamable { ParseField LAST_MET_CONDITION = new ParseField("last_met_condition"); ParseField ACTIONS = new ParseField("actions"); ParseField VERSION = new ParseField("version"); + ParseField EXECUTION_STATE = new ParseField("execution_state"); } } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/watcher/execution/ExecutionServiceTests.java b/plugin/src/test/java/org/elasticsearch/xpack/watcher/execution/ExecutionServiceTests.java index 81588fcf56e..4a9fbdbab93 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/watcher/execution/ExecutionServiceTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/watcher/execution/ExecutionServiceTests.java @@ -813,6 +813,8 @@ public class ExecutionServiceTests extends ESTestCase { public void testThatTriggeredWatchDeletionWorksOnExecutionRejection() throws Exception { Watch watch = mock(Watch.class); when(watch.id()).thenReturn("foo"); + WatchStatus status = new WatchStatus(DateTime.now(UTC), Collections.emptyMap()); + when(watch.status()).thenReturn(status); GetResponse getResponse = mock(GetResponse.class); when(getResponse.isExists()).thenReturn(true); when(getResponse.getId()).thenReturn("foo"); diff --git a/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/10_basic.yml b/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/10_basic.yml index 5053fdbb360..0e46035bbf4 100644 --- a/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/10_basic.yml +++ b/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/10_basic.yml @@ -58,6 +58,7 @@ teardown: - match: { watch_record.trigger_event.triggered_time: "2012-12-12T12:12:12.120Z" } - match: { watch_record.trigger_event.manual.schedule.scheduled_time: "2000-12-12T12:12:12.120Z" } - match: { watch_record.state: "executed" } + - match: { watch_record.status.execution_state: "executed" } - match: { watch_record.status.state.active: true } - is_true: watch_record.node - match: { watch_record.status.actions.indexme.ack.state: "ackable" } @@ -97,6 +98,7 @@ teardown: - match: { watch_record.watch_id: "_inlined_" } - match: { watch_record.trigger_event.type: "manual" } - match: { watch_record.state: "executed" } + - match: { watch_record.status.execution_state: "executed" } - match: { watch_record.status.state.active: true } - match: { watch_record.status.actions.indexme.ack.state: "ackable" } @@ -149,6 +151,7 @@ teardown: - match: { watch_record.watch_id: "test_watch" } - match: { watch_record.state: "executed" } + - match: { watch_record.status.execution_state: "executed" } - match: { watch_record.status.state.active: true } - is_true: watch_record.node - is_false: watch_record.result.input.payload.foo diff --git a/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/20_transform.yml b/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/20_transform.yml index 8958c02a727..c4ad40658ea 100644 --- a/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/20_transform.yml +++ b/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/20_transform.yml @@ -58,6 +58,7 @@ setup: } - match: { watch_record.trigger_event.type: "manual" } - match: { watch_record.state: "executed" } + - match: { watch_record.status.execution_state: "executed" } - match: { watch_record.result.transform.status: "success" } - do: @@ -131,6 +132,7 @@ setup: } - match: { watch_record.trigger_event.type: "manual" } - match: { watch_record.state: "executed" } + - match: { watch_record.status.execution_state: "executed" } - match: { watch_record.result.transform.status: "success" } - do: @@ -191,6 +193,7 @@ setup: - match: { watch_record.trigger_event.type: "manual" } - match: { watch_record.state: "executed" } + - match: { watch_record.status.execution_state: "executed" } - match: { watch_record.result.transform.status: "failure" } - match: { watch_record.result.transform.reason: "no [query] registered for [does_not_exist]" } - is_true: watch_record.result.transform.error diff --git a/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/30_throttled.yml b/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/30_throttled.yml index f33b33ca136..0d2497fed79 100644 --- a/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/30_throttled.yml +++ b/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/30_throttled.yml @@ -56,6 +56,7 @@ teardown: - match: { watch_record.trigger_event.triggered_time: "2012-12-12T12:12:12.120Z" } - match: { watch_record.trigger_event.manual.schedule.scheduled_time: "2000-12-12T12:12:12.120Z" } - match: { watch_record.state: "execution_not_needed" } + - match: { watch_record.status.execution_state: "execution_not_needed" } - match: { watch_record.status.state.active: true } - do: @@ -99,6 +100,7 @@ teardown: - match: { watch_record.watch_id: "test_watch" } - match: { watch_record.state: "executed" } + - match: { watch_record.status.execution_state: "executed" } - do: xpack.watcher.execute_watch: @@ -114,5 +116,6 @@ teardown: - match: { watch_record.watch_id: "test_watch" } - match: { watch_record.state: "throttled" } + - match: { watch_record.status.execution_state: "throttled" } diff --git a/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/40_ignore_condition.yml b/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/40_ignore_condition.yml index 7d66e3c09f2..5c835f7d692 100644 --- a/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/40_ignore_condition.yml +++ b/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/40_ignore_condition.yml @@ -52,6 +52,7 @@ teardown: - match: { watch_record.input.simple.foo: "bar" } - match: { watch_record.trigger_event.type: "manual" } - match: { watch_record.state: "executed" } + - match: { watch_record.status.execution_state: "executed" } - match: { watch_record.status.state.active: true } - match: { watch_record.status.actions.logging.ack.state: "ackable" } - is_true: watch_record.condition.never diff --git a/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/50_action_mode.yml b/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/50_action_mode.yml index d3c7a8d7aab..3f6303b4d47 100644 --- a/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/50_action_mode.yml +++ b/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/50_action_mode.yml @@ -50,6 +50,7 @@ teardown: - match: { watch_record.watch_id: "test_watch" } - match: { watch_record.trigger_event.type: "manual" } - match: { watch_record.state: "executed" } + - match: { watch_record.status.execution_state: "executed" } - match: { watch_record.result.actions.0.id: "logging" } - match: { watch_record.result.actions.0.status: "simulated" } @@ -66,6 +67,7 @@ teardown: - match: { watch_record.watch_id: "test_watch" } - match: { watch_record.trigger_event.type: "manual" } - match: { watch_record.state: "executed" } + - match: { watch_record.status.execution_state: "executed" } - match: { watch_record.result.actions.0.id: "logging" } - match: { watch_record.result.actions.0.status: "simulated" } diff --git a/plugin/src/test/resources/rest-api-spec/test/watcher/put_watch/70_put_watch_with_index_action_using_id.yml b/plugin/src/test/resources/rest-api-spec/test/watcher/put_watch/70_put_watch_with_index_action_using_id.yml index 4a1eaa11f51..7d025489c79 100644 --- a/plugin/src/test/resources/rest-api-spec/test/watcher/put_watch/70_put_watch_with_index_action_using_id.yml +++ b/plugin/src/test/resources/rest-api-spec/test/watcher/put_watch/70_put_watch_with_index_action_using_id.yml @@ -57,6 +57,7 @@ teardown: xpack.watcher.execute_watch: id: "my_watch" - match: { watch_record.state: "executed" } + - match: { watch_record.status.execution_state: "executed" } - match: { watch_record.result.actions.0.index.response.id: "test_id1" } --- @@ -105,6 +106,7 @@ teardown: xpack.watcher.execute_watch: id: "my_watch" - match: { watch_record.state: "executed" } + - match: { watch_record.status.execution_state: "executed" } - match: { watch_record.result.actions.0.index.response.id: "test_id2" } --- @@ -163,6 +165,7 @@ teardown: xpack.watcher.execute_watch: id: "my_watch" - match: { watch_record.state: "executed" } + - match: { watch_record.status.execution_state: "executed" } - match: { watch_record.result.actions.0.index.response.0.id: "test_id3" } - match: { watch_record.result.actions.0.index.response.1.id: "test_id4" } @@ -220,4 +223,5 @@ teardown: xpack.watcher.execute_watch: id: "my_watch" - match: { watch_record.state: "executed" } + - match: { watch_record.status.execution_state: "executed" } - match: { watch_record.result.actions.0.index.response.1.id: "test_id6" }