azkaban-aplcache

Fixes to flow notification e-mails - Removed misleading

7/19/2016 6:42:59 AM

Details

diff --git a/azkaban-common/src/main/java/azkaban/executor/ExecutorManager.java b/azkaban-common/src/main/java/azkaban/executor/ExecutorManager.java
index cffabe7..7df342c 100644
--- a/azkaban-common/src/main/java/azkaban/executor/ExecutorManager.java
+++ b/azkaban-common/src/main/java/azkaban/executor/ExecutorManager.java
@@ -1434,10 +1434,7 @@ public class ExecutorManager extends EventHandler implements
       if (options.getFailureEmails() != null
           && !options.getFailureEmails().isEmpty()) {
         try {
-          mailAlerter
-              .alertOnError(
-                  flow,
-                  "Executor no longer seems to be running this execution. Most likely due to executor bounce.");
+          mailAlerter.alertOnError(flow);
         } catch (Exception e) {
           logger.error(e);
         }
@@ -1447,10 +1444,7 @@ public class ExecutorManager extends EventHandler implements
         Alerter alerter = alerters.get(alertType);
         if (alerter != null) {
           try {
-            alerter
-                .alertOnError(
-                    flow,
-                    "Executor no longer seems to be running this execution. Most likely due to executor bounce.");
+            alerter.alertOnError(flow);
           } catch (Exception e) {
             // TODO Auto-generated catch block
             e.printStackTrace();
diff --git a/azkaban-common/src/main/java/azkaban/executor/mail/DefaultMailCreator.java b/azkaban-common/src/main/java/azkaban/executor/mail/DefaultMailCreator.java
index c58d774..bb144a7 100644
--- a/azkaban-common/src/main/java/azkaban/executor/mail/DefaultMailCreator.java
+++ b/azkaban-common/src/main/java/azkaban/executor/mail/DefaultMailCreator.java
@@ -67,7 +67,7 @@ public class DefaultMailCreator implements MailCreator {
     if (emailList != null && !emailList.isEmpty()) {
       message.addAllToAddress(emailList);
       message.setMimeType("text/html");
-      message.setSubject("Flow '" + flow.getFlowId() + "' has failed on "
+      message.setSubject("Flow '" + flow.getFlowId() + "' has encountered a failure on "
           + azkabanName);
 
       message.println("<h2 style=\"color:#FF0000\"> Execution '"
@@ -93,6 +93,7 @@ public class DefaultMailCreator implements MailCreator {
       message.println("<tr><td>Duration</td><td>"
           + Utils.formatDuration(flow.getStartTime(), flow.getEndTime())
           + "</td></tr>");
+      message.println("<tr><td>Status</td><td>" + flow.getStatus() + "</td></tr>");
       message.println("</table>");
       message.println("");
       String executionUrl =
@@ -144,6 +145,7 @@ public class DefaultMailCreator implements MailCreator {
       message.println("<tr><td>Duration</td><td>"
           + Utils.formatDuration(flow.getStartTime(), flow.getEndTime())
           + "</td></tr>");
+      message.println("<tr><td>Status</td><td>" + flow.getStatus() + "</td></tr>");
       message.println("</table>");
       message.println("");
       String executionUrl =
@@ -197,6 +199,7 @@ public class DefaultMailCreator implements MailCreator {
       message.println("<tr><td>Duration</td><td>"
           + Utils.formatDuration(flow.getStartTime(), flow.getEndTime())
           + "</td></tr>");
+      message.println("<tr><td>Status</td><td>" + flow.getStatus() + "</td></tr>");
       message.println("</table>");
       message.println("");
       String executionUrl =
diff --git a/azkaban-common/src/main/java/azkaban/utils/EmailMessage.java b/azkaban-common/src/main/java/azkaban/utils/EmailMessage.java
index 8384c4b..0c4e208 100644
--- a/azkaban-common/src/main/java/azkaban/utils/EmailMessage.java
+++ b/azkaban-common/src/main/java/azkaban/utils/EmailMessage.java
@@ -287,4 +287,13 @@ public class EmailMessage {
 
     return this;
   }
+
+  public String getBody() {
+    return _body.toString();
+  }
+
+  public String getSubject() {
+    return _subject;
+  }
+
 }
diff --git a/azkaban-common/src/main/java/azkaban/utils/Utils.java b/azkaban-common/src/main/java/azkaban/utils/Utils.java
index 138b80f..9cdeaaf 100644
--- a/azkaban-common/src/main/java/azkaban/utils/Utils.java
+++ b/azkaban-common/src/main/java/azkaban/utils/Utils.java
@@ -35,6 +35,7 @@ import java.util.zip.ZipFile;
 import java.util.zip.ZipOutputStream;
 
 import org.apache.commons.io.IOUtils;
+import org.joda.time.DateTime;
 import org.joda.time.Days;
 import org.joda.time.DurationFieldType;
 import org.joda.time.Hours;
@@ -298,7 +299,7 @@ public class Utils {
 
     long durationMS;
     if (endTime == -1) {
-      durationMS = System.currentTimeMillis() - startTime;
+      durationMS = DateTime.now().getMillis() - startTime;
     } else {
       durationMS = endTime - startTime;
     }
diff --git a/azkaban-common/src/test/java/azkaban/executor/mail/DefaultMailCreatorTest.java b/azkaban-common/src/test/java/azkaban/executor/mail/DefaultMailCreatorTest.java
new file mode 100644
index 0000000..b412d41
--- /dev/null
+++ b/azkaban-common/src/test/java/azkaban/executor/mail/DefaultMailCreatorTest.java
@@ -0,0 +1,128 @@
+package azkaban.executor.mail;
+
+import azkaban.executor.ExecutableFlow;
+import azkaban.executor.ExecutionOptions;
+import azkaban.executor.Status;
+import azkaban.flow.Flow;
+import azkaban.flow.Node;
+import azkaban.project.Project;
+import azkaban.utils.EmailMessage;
+import com.google.common.base.Charsets;
+import com.google.common.collect.ImmutableList;
+import org.apache.commons.io.IOUtils;
+import org.joda.time.DateTimeUtils;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.InputStream;
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+public class DefaultMailCreatorTest {
+
+  // 2016/07/17 11:54:11 EEST
+  public static final long START_TIME_MILLIS = 1468745651608L;
+  // 2016/07/17 11:54:16 EEST (START_TIME_MILLIS + 5 seconds)
+  public static final long END_TIME_MILLIS = START_TIME_MILLIS + 5_000L;
+  // 2016/07/17 11:54:21 EEST (START_TIME_MILLIS + 10 seconds)
+  public static final long FIXED_CURRENT_TIME_MILLIS = START_TIME_MILLIS + 10_000L;
+
+  private DefaultMailCreator mailCreator;
+
+  private ExecutableFlow executableFlow;
+  private Flow flow;
+  private Project project;
+  private ExecutionOptions options;
+  private EmailMessage message;
+  private String azkabanName;
+  private String scheme;
+  private String clientHostname;
+  private String clientPortNumber;
+  private TimeZone defaultTz;
+
+  @Before
+  public void setUp() throws Exception {
+    defaultTz = TimeZone.getDefault();
+    assertNotNull(defaultTz);
+    // EEST
+    TimeZone.setDefault(TimeZone.getTimeZone("Europe/Helsinki"));
+    DateTimeUtils.setCurrentMillisFixed(FIXED_CURRENT_TIME_MILLIS);
+
+    mailCreator = new DefaultMailCreator();
+
+    flow = new Flow("mail-creator-test");
+    project = new Project(1, "test-project");
+    options = new ExecutionOptions();
+    message = new EmailMessage();
+
+    azkabanName = "unit-tests";
+    scheme = "http";
+    clientHostname = "localhost";
+    clientPortNumber = "8081";
+
+    Node failedNode = new Node("test-job");
+    failedNode.setType("noop");
+    flow.addNode(failedNode);
+
+    executableFlow = new ExecutableFlow(project, flow);
+    executableFlow.setExecutionOptions(options);
+    executableFlow.setStartTime(START_TIME_MILLIS);
+
+    options.setFailureEmails(ImmutableList.of("test@example.com"));
+    options.setSuccessEmails(ImmutableList.of("test@example.com"));
+  }
+
+  @After
+  public void tearDown() throws Exception {
+    if (defaultTz != null) {
+      TimeZone.setDefault(defaultTz);
+    }
+    DateTimeUtils.setCurrentMillisSystem();
+  }
+
+  private void setJobStatus(Status status) {
+    executableFlow.getExecutableNodes().get(0).setStatus(status);
+  }
+
+  @Test
+  public void createErrorEmail() throws Exception {
+    setJobStatus(Status.FAILED);
+    executableFlow.setEndTime(END_TIME_MILLIS);
+    executableFlow.setStatus(Status.FAILED);
+    assertTrue(mailCreator.createErrorEmail(
+        executableFlow, message, azkabanName, scheme, clientHostname, clientPortNumber));
+    assertEquals("Flow 'mail-creator-test' has failed on unit-tests", message.getSubject());
+    assertEquals(read("errorEmail.html"), message.getBody());
+  }
+
+  @Test
+  public void createFirstErrorMessage() throws Exception {
+    setJobStatus(Status.FAILED);
+    executableFlow.setStatus(Status.FAILED_FINISHING);
+    assertTrue(mailCreator.createFirstErrorMessage(
+        executableFlow, message, azkabanName, scheme, clientHostname, clientPortNumber));
+    assertEquals("Flow 'mail-creator-test' has encountered a failure on unit-tests", message.getSubject());
+    assertEquals(read("firstErrorMessage.html"), message.getBody());
+  }
+
+  @Test
+  public void createSuccessEmail() throws Exception {
+    setJobStatus(Status.SUCCEEDED);
+    executableFlow.setEndTime(END_TIME_MILLIS);
+    executableFlow.setStatus(Status.SUCCEEDED);
+    assertTrue(mailCreator.createSuccessEmail(
+        executableFlow, message, azkabanName, scheme, clientHostname, clientPortNumber));
+    assertEquals("Flow 'mail-creator-test' has succeeded on unit-tests", message.getSubject());
+    assertEquals(read("successEmail.html"), message.getBody());
+  }
+
+  private String read(String file) throws Exception {
+    InputStream is = DefaultMailCreatorTest.class.getResourceAsStream(file);
+    return IOUtils.toString(is, Charsets.UTF_8).trim();
+  }
+
+}
diff --git a/azkaban-common/src/test/resources/azkaban/executor/mail/errorEmail.html b/azkaban-common/src/test/resources/azkaban/executor/mail/errorEmail.html
new file mode 100644
index 0000000..833eb8c
--- /dev/null
+++ b/azkaban-common/src/test/resources/azkaban/executor/mail/errorEmail.html
@@ -0,0 +1 @@
+<h2 style="color:#FF0000"> Execution '-1' of flow 'mail-creator-test' has failed on unit-tests</h2><table><tr><td>Start Time</td><td>2016/07/17 11:54:11 EEST</td></tr><tr><td>End Time</td><td>2016/07/17 11:54:16 EEST</td></tr><tr><td>Duration</td><td>5 sec</td></tr><tr><td>Status</td><td>FAILED</td></tr></table><a href="http://localhost:8081/executor?execid=-1">mail-creator-test Execution Link</a><h3>Reason</h3><ul><li><a href="http://localhost:8081/executor?execid=-1&job=test-job">Failed job 'test-job' Link</a></li></ul>
diff --git a/azkaban-common/src/test/resources/azkaban/executor/mail/firstErrorMessage.html b/azkaban-common/src/test/resources/azkaban/executor/mail/firstErrorMessage.html
new file mode 100644
index 0000000..643b2ea
--- /dev/null
+++ b/azkaban-common/src/test/resources/azkaban/executor/mail/firstErrorMessage.html
@@ -0,0 +1 @@
+<h2 style="color:#FF0000"> Execution '-1' of flow 'mail-creator-test' has encountered a failure on unit-tests</h2>This flow is set to complete all currently running jobs before stopping.<table><tr><td>Start Time</td><td>2016/07/17 11:54:11 EEST</td></tr><tr><td>End Time</td><td>N/A</td></tr><tr><td>Duration</td><td>10 sec</td></tr><tr><td>Status</td><td>FAILED_FINISHING</td></tr></table><a href="http://localhost:8081/executor?execid=-1">mail-creator-test Execution Link</a><h3>Reason</h3><ul><li><a href="http://localhost:8081/executor?execid=-1&job=test-job">Failed job 'test-job' Link</a></li></ul>
diff --git a/azkaban-common/src/test/resources/azkaban/executor/mail/successEmail.html b/azkaban-common/src/test/resources/azkaban/executor/mail/successEmail.html
new file mode 100644
index 0000000..8b00fea
--- /dev/null
+++ b/azkaban-common/src/test/resources/azkaban/executor/mail/successEmail.html
@@ -0,0 +1 @@
+<h2> Execution '-1' of flow 'mail-creator-test' has succeeded on unit-tests</h2><table><tr><td>Start Time</td><td>2016/07/17 11:54:11 EEST</td></tr><tr><td>End Time</td><td>2016/07/17 11:54:16 EEST</td></tr><tr><td>Duration</td><td>5 sec</td></tr><tr><td>Status</td><td>SUCCEEDED</td></tr></table><a href="http://localhost:8081/executor?execid=-1">mail-creator-test Execution Link</a>