Generate messages with same content as console when dispatching notifications

6 files changed, 360 insertions, 37 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/FormattedNotification.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/FormattedNotification.java
new file mode 100644
index 00000000000..8a6243a7224
--- /dev/null
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/FormattedNotification.java
@@ -0,0 +1,40 @@
+package com.yahoo.vespa.hosted.controller.notification;
+
+import java.net.URI;
+import java.util.Objects;
+
+/**
+ * Contains formatted text that can be displayed to a user to give extra information and pointers for a given
+ * Notification.
+ *
+ * @author enygaard
+ */
+public class FormattedNotification {
+    private final String prettyType;
+    private final String messagePrefix;
+    private final URI uri;
+    private final Notification notification;
+
+    public FormattedNotification(Notification notification, String prettyType, String messagePrefix, URI uri) {
+        this.prettyType = Objects.requireNonNull(prettyType);
+        this.messagePrefix = Objects.requireNonNull(messagePrefix);
+        this.uri = Objects.requireNonNull(uri);
+        this.notification = Objects.requireNonNull(notification);
+    }
+
+    public String prettyType() {
+        return prettyType;
+    }
+
+    public String messagePrefix() {
+        return messagePrefix;
+    }
+
+    public URI uri() {
+        return uri;
+    }
+
+    public Notification notification() {
+        return notification;
+    }
+}
\ No newline at end of file
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/MissingOptionalException.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/MissingOptionalException.java
new file mode 100644
index 00000000000..1379ab4654f
--- /dev/null
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/MissingOptionalException.java
@@ -0,0 +1,18 @@
+package com.yahoo.vespa.hosted.controller.notification;
+
+/**
+ * Used to signal that an expected value was not present when creating NotificationContent
+ *
+ * @author enygaard
+ */
+class MissingOptionalException extends RuntimeException {
+    private final String field;
+    public MissingOptionalException(String field) {
+        super(field + " was expected but not present");
+        this.field = field;
+    }
+
+    public String field() {
+        return field;
+    }
+}
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationFormatter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationFormatter.java
new file mode 100644
index 00000000000..d2b12ab6edc
--- /dev/null
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationFormatter.java
@@ -0,0 +1,190 @@
+package com.yahoo.vespa.hosted.controller.notification;
+
+import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.Environment;
+import com.yahoo.text.Text;
+import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
+import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry;
+import org.apache.http.client.utils.URIBuilder;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Function;
+
+/**
+ * Created a NotificationContent for a given Notification.
+ *
+ * The formatter will create specific summary, message start and URI for a given Notification.
+ *
+ * @author enygaard
+ */
+public class NotificationFormatter {
+    private final ZoneRegistry zoneRegistry;
+
+    public NotificationFormatter(ZoneRegistry zoneRegistry) {
+        this.zoneRegistry = Objects.requireNonNull(zoneRegistry);
+    }
+
+    public FormattedNotification format(Notification n) {
+        switch (n.type()) {
+            case applicationPackage:
+            case submission:
+                return applicationPackage(n);
+            case deployment:
+                return deployment(n);
+            case testPackage:
+                return testPackage(n);
+            case reindex:
+                return reindex(n);
+            case feedBlock:
+                return feedBlock(n);
+            default:
+                return new FormattedNotification(n, n.type().name(), "", zoneRegistry.dashboardUrl(n.source().tenant()));
+        }
+    }
+
+    private FormattedNotification applicationPackage(Notification n) {
+        var source = n.source();
+        var application = requirePresent(source.application(), "application");
+        var instance = requirePresent(source.instance(), "instance");
+        var message = Text.format("Application package for %s.%s has %s",
+                application,
+                instance,
+                levelText(n.level(), n.messages().size()));
+        var uri = zoneRegistry.dashboardUrl(ApplicationId.from(source.tenant(), application, instance));
+        return new FormattedNotification(n, "Application package", message, uri);
+    }
+
+    private FormattedNotification deployment(Notification n) {
+        var source = n.source();
+        var message = Text.format("%s for %s.%s has %s",
+                jobText(source),
+                requirePresent(source.application(), "application"),
+                requirePresent(source.instance(), "instance"),
+                levelText(n.level(), n.messages().size()));
+        return new FormattedNotification(n,"Deployment", message, jobLink(n.source()));
+    }
+
+    private FormattedNotification testPackage(Notification n) {
+        var source = n.source();
+        var application = requirePresent(source.application(), "application");
+        var message = Text.format("There %s with tests for %s%s",
+                n.messages().size() > 1 ? "are problems" : "is a problem",
+                application,
+                source.instance().map(i -> "."+i).orElse(""));
+        var uri = zoneRegistry.dashboardUrl(source.tenant(), application);
+        return new FormattedNotification(n, "Test package", message, uri);
+    }
+
+    private FormattedNotification reindex(Notification n) {
+        var message = Text.format("%s is reindexing", clusterInfo(n.source()));
+        var source = n.source();
+        var application = requirePresent(source.application(), "application");
+        var instance = requirePresent(source.instance(), "instance");
+        var clusterId = requirePresent(source.clusterId(), "clusterId");
+        var zone = requirePresent(source.zoneId(), "zoneId");
+        var instanceURI = zoneRegistry.dashboardUrl(ApplicationId.from(source.tenant(), application, instance));
+        try {
+            var uri = new URIBuilder(instanceURI)
+                    .setParameter(
+                            String.format("%s.%s.%s", instance, zone.environment(), zone.region()),
+                            String.format("clusters,%s=status", clusterId.value()))
+                    .build();
+            return new FormattedNotification(n, "Reindex", message, uri);
+        } catch (URISyntaxException e) {
+            throw new IllegalArgumentException(e);
+        }
+    }
+
+    private FormattedNotification feedBlock(Notification n) {
+        String type;
+        if (n.level() == Notification.Level.warning) {
+            type = "Nearly feed blocked";
+        } else {
+            type = "Feed blocked";
+        }
+        var message = Text.format("%s is %s", clusterInfo(n.source()), type.toLowerCase());
+        var source = n.source();
+        var application = requirePresent(source.application(), "application");
+        var instance = requirePresent(source.instance(), "instance");
+        var clusterId = requirePresent(source.clusterId(), "clusterId");
+        var zone = requirePresent(source.zoneId(), "zoneId");
+        var instanceURI = zoneRegistry.dashboardUrl(ApplicationId.from(source.tenant(), application, instance));
+        try {
+            var uri = new URIBuilder(instanceURI)
+                    .setParameter(
+                            String.format("%s.%s.%s", instance, zone.environment(), zone.region()),
+                            String.format("clusters,%s", clusterId.value()))
+                    .build();
+            return new FormattedNotification(n, type, message, uri);
+        } catch (URISyntaxException e) {
+            throw new IllegalArgumentException(e);
+        }
+    }
+
+    private URI jobLink(NotificationSource source) {
+        var application = requirePresent(source.application(), "application");
+        var instance = requirePresent(source.instance(), "instance");
+        var jobType = requirePresent(source.jobType(), "jobType");
+        var runNumber = source.runNumber().orElseThrow(() -> new MissingOptionalException("runNumber"));
+        var applicationId = ApplicationId.from(source.tenant(), application, instance);
+        Function<Environment, URI> link = (Environment env) -> zoneRegistry.dashboardUrl(new RunId(applicationId, jobType, runNumber));
+        var environment = jobType.zone().environment();
+        switch (environment) {
+            case dev:
+            case perf:
+                return link.apply(environment);
+            default:
+                return link.apply(Environment.prod);
+        }
+    }
+
+    private String jobText(NotificationSource source) {
+        var jobType = requirePresent(source.jobType(), "jobType");
+        var zone = jobType.zone();
+        var runNumber = source.runNumber().orElseThrow(() -> new MissingOptionalException("runNumber"));
+        switch (zone.environment().value()) {
+            case "production":
+                return Text.format("Deployment job #%d to %s", runNumber, zone.region());
+            case "test":
+                return Text.format("Test job #%d to %s", runNumber, zone.region());
+            case "dev":
+            case "perf":
+                return Text.format("Deployment job #%d to %s.%s", runNumber, zone.environment().value(), zone.region().value());
+        }
+        switch (jobType.jobName()) {
+            case "system-test":
+            case "staging-test":
+        }
+        return Text.format("%s #%d", jobType.jobName(), runNumber);
+    }
+
+    private String levelText(Notification.Level level, int count) {
+        switch (level) {
+            case error:
+                return "failed";
+            case warning:
+                return count > 1 ? Text.format("%d warnings", count) : "a warning";
+            default:
+                return count > 1 ? Text.format("%d messages", count) : "a message";
+        }
+    }
+
+    private String clusterInfo(NotificationSource source) {
+        var application = requirePresent(source.application(), "application");
+        var instance = requirePresent(source.instance(), "instance");
+        var zone = requirePresent(source.zoneId(), "zoneId");
+        var clusterId = requirePresent(source.clusterId(), "clusterId");
+        return Text.format("Cluster %s in %s.%s for %s.%s",
+                clusterId.value(),
+                zone.environment(), zone.region(),
+                application, instance);
+    }
+
+
+    private static <T> T requirePresent(Optional<T> optional, String field) {
+        return optional.orElseThrow(() -> new MissingOptionalException(field));
+    }
+}
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notifier.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notifier.java
index b098b779dbd..5a5188da37f 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notifier.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notifier.java
@@ -1,13 +1,11 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.notification;
 
-import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.text.Text;
 import com.yahoo.vespa.flags.FetchVector;
 import com.yahoo.vespa.flags.FlagSource;
 import com.yahoo.vespa.flags.Flags;
-import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.Mail;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.Mailer;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.MailerException;
@@ -16,7 +14,6 @@ import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 import com.yahoo.vespa.hosted.controller.tenant.CloudTenant;
 import com.yahoo.vespa.hosted.controller.tenant.TenantContacts;
 
-import java.net.URI;
 import java.util.Collection;
 import java.util.List;
 import java.util.Objects;
@@ -31,17 +28,17 @@ import java.util.stream.Collectors;
  */
 public class Notifier {
     private final CuratorDb curatorDb;
-    private final ZoneRegistry zoneRegistry;
     private final Mailer mailer;
     private final FlagSource flagSource;
+    private final NotificationFormatter formatter;
 
     private static final Logger log = Logger.getLogger(Notifier.class.getName());
 
     public Notifier(CuratorDb curatorDb, ZoneRegistry zoneRegistry, Mailer mailer, FlagSource flagSource) {
         this.curatorDb = Objects.requireNonNull(curatorDb);
-        this.zoneRegistry = Objects.requireNonNull(zoneRegistry);
         this.mailer = Objects.requireNonNull(mailer);
         this.flagSource = Objects.requireNonNull(flagSource);
+        this.formatter = new NotificationFormatter(zoneRegistry);
     }
 
     public void dispatch(List<Notification> notifications, NotificationSource source) {
@@ -64,6 +61,10 @@ public class Notifier {
         });
     }
 
+    public void dispatch(Notification notification) {
+        dispatch(List.of(notification), notification.source());
+    }
+
     private boolean dispatchEnabled(NotificationSource source) {
         return Flags.NOTIFICATION_DISPATCH_FLAG.bindTo(flagSource)
                 .with(FetchVector.Dimension.TENANT_ID, source.tenant().value())
@@ -80,10 +81,6 @@ public class Notifier {
         return false;
     }
 
-    public void dispatch(Notification notification) {
-        dispatch(List.of(notification), notification.source());
-    }
-
     private void dispatch(Notification notification, TenantContacts.Type type, Collection<? extends TenantContacts.Contact> contacts) {
         switch (type) {
             case EMAIL:
@@ -96,21 +93,24 @@ public class Notifier {
 
     private void dispatch(Notification notification, Collection<TenantContacts.EmailContact> contacts) {
         try {
-            mailer.send(mailOf(notification, contacts.stream().map(c -> c.email()).collect(Collectors.toList())));
+            var content = formatter.format(notification);
+            mailer.send(mailOf(content, contacts.stream().map(c -> c.email()).collect(Collectors.toList())));
         } catch (MailerException e) {
             log.log(Level.SEVERE, "Failed sending email", e);
+        } catch (MissingOptionalException e) {
+            log.log(Level.WARNING, "Missing value in required field '" + e.field() + "' for notification type: " + notification.type(), e);
         }
     }
 
-    private Mail mailOf(Notification n, Collection<String> recipients) {
-        var source = n.source();
-        var subject = Text.format("[%s] %s Vespa Notification for %s", n.level().toString().toUpperCase(), n.type().name(), applicationIdSource(source));
+    private Mail mailOf(FormattedNotification content, Collection<String> recipients) {
+        var notification = content.notification();
+        var subject = Text.format("[%s] %s Vespa Notification for %s", notification.level().toString().toUpperCase(), content.prettyType(), applicationIdSource(notification.source()));
         var body = new StringBuilder();
-        body.append("Source: ").append(n.source().toString()).append("\n")
-                .append("\n")
-                .append(String.join("\n", n.messages()))
+        body.append(content.messagePrefix()).append("\n\n")
+                .append(notification.messages().stream().map(m -> " * " + m).collect(Collectors.joining("\n"))).append("\n")
                 .append("\n")
-                .append(url(source).toString());
+                .append("Vespa Console link:\n")
+                .append(content.uri().toString());
         return new Mail(recipients, subject, body.toString());
     }
 
@@ -122,22 +122,5 @@ public class Notifier {
         return sb.toString();
     }
 
-    private URI url(NotificationSource source) {
-        if (source.application().isPresent()) {
-            if (source.instance().isPresent()) {
-                if (source.jobType().isPresent() && source.runNumber().isPresent()) {
-                    return zoneRegistry.dashboardUrl(
-                            new RunId(ApplicationId.from(source.tenant(),
-                                    source.application().get(),
-                                    source.instance().get()),
-                                    source.jobType().get(),
-                                    source.runNumber().getAsLong()));
-                }
-                return zoneRegistry.dashboardUrl(ApplicationId.from(source.tenant(), source.application().get(), source.instance().get()));
-            }
-            return zoneRegistry.dashboardUrl(source.tenant(), source.application().get());
-        }
-        return zoneRegistry.dashboardUrl(source.tenant());
-    }
 
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationFormatterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationFormatterTest.java
new file mode 100644
index 00000000000..c643a612f00
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationFormatterTest.java
@@ -0,0 +1,92 @@
+package com.yahoo.vespa.hosted.controller.notification;
+
+import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.ApplicationName;
+import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.InstanceName;
+import com.yahoo.config.provision.RegionName;
+import com.yahoo.config.provision.SystemName;
+import com.yahoo.config.provision.TenantName;
+import com.yahoo.config.provision.zone.ZoneId;
+import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
+import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
+import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
+import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
+import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock;
+import org.junit.Test;
+
+import java.time.Instant;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author enygaard
+ */
+public class NotificationFormatterTest {
+    private final TenantName tenant = TenantName.from("scoober");
+    private final ApplicationName application = ApplicationName.from("myapp");
+    private final InstanceName instance = InstanceName.from("beta");
+    private final ApplicationId applicationId = ApplicationId.from(tenant, application, instance);
+    private final DeploymentId deploymentId = new DeploymentId(applicationId, ZoneId.defaultId());
+    private final ClusterSpec.Id cluster = new ClusterSpec.Id("content");
+    private final ZoneRegistryMock zoneRegistry = new ZoneRegistryMock(SystemName.Public);
+
+    private final NotificationFormatter formatter = new NotificationFormatter(zoneRegistry);
+
+    @Test
+    public void applicationPackage() {
+        var notification = new Notification(Instant.now(), Notification.Type.applicationPackage, Notification.Level.warning, NotificationSource.from(applicationId), List.of("1", "2"));
+        var content = formatter.format(notification);
+        assertEquals("Application package", content.prettyType());
+        assertEquals("Application package for myapp.beta has 2 warnings", content.messagePrefix());
+        assertEquals("https://dashboard.tld/scoober.myapp.beta", content.uri().toString());
+    }
+
+    @Test
+    public void deployment() {
+        var runId = new RunId(applicationId, JobType.prod(RegionName.defaultName()), 1001);
+        var notification = new Notification(Instant.now(), Notification.Type.deployment, Notification.Level.warning, NotificationSource.from(runId), List.of("1"));
+        var content = formatter.format(notification);
+        assertEquals("Deployment", content.prettyType());
+        assertEquals("production-default #1001 for myapp.beta has a warning", content.messagePrefix());
+        assertEquals("https://dashboard.tld/scoober.myapp.beta/production-default/1001", content.uri().toString());
+    }
+
+    @Test
+    public void deploymentError() {
+        var runId = new RunId(applicationId, JobType.prod(RegionName.defaultName()), 1001);
+        var notification = new Notification(Instant.now(), Notification.Type.deployment, Notification.Level.error, NotificationSource.from(runId), List.of("1"));
+        var content = formatter.format(notification);
+        assertEquals("Deployment", content.prettyType());
+        assertEquals("production-default #1001 for myapp.beta has failed", content.messagePrefix());
+        assertEquals("https://dashboard.tld/scoober.myapp.beta/production-default/1001", content.uri().toString());
+    }
+
+    @Test
+    public void testPackage() {
+        var notification = new Notification(Instant.now(), Notification.Type.testPackage, Notification.Level.warning, NotificationSource.from(TenantAndApplicationId.from(applicationId)), List.of("1"));
+        var content = formatter.format(notification);
+        assertEquals("Test package", content.prettyType());
+        assertEquals("There is a problem with tests for myapp", content.messagePrefix());
+        assertEquals("https://dashboard.tld/scoober/myapp", content.uri().toString());
+    }
+
+    @Test
+    public void reindex() {
+        var notification = new Notification(Instant.now(), Notification.Type.reindex, Notification.Level.info, NotificationSource.from(deploymentId, cluster), List.of("1"));
+        var content = formatter.format(notification);
+        assertEquals("Reindex", content.prettyType());
+        assertEquals("Cluster content in prod.default for myapp.beta is reindexing", content.messagePrefix());
+        assertEquals("https://dashboard.tld/scoober.myapp.beta?beta.prod.default=clusters%2Ccontent%3Dstatus", content.uri().toString());
+    }
+
+    @Test
+    public void feedBlock() {
+        var notification = new Notification(Instant.now(), Notification.Type.feedBlock, Notification.Level.warning, NotificationSource.from(deploymentId, cluster), List.of("1"));
+        var content = formatter.format(notification);
+        assertEquals("Nearly feed blocked", content.prettyType());
+        assertEquals("Cluster content in prod.default for myapp.beta is nearly feed blocked", content.messagePrefix());
+        assertEquals("https://dashboard.tld/scoober.myapp.beta?beta.prod.default=clusters%2Ccontent", content.uri().toString());
+    }
+}
\ No newline at end of file
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java
index edbee6e3900..75dbebe96ff 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java
@@ -104,9 +104,9 @@ public class NotificationsDbTest {
     @Test
     public void notifier_test() {
         Notification notification1 = notification(12345, Type.deployment, Level.warning, NotificationSource.from(ApplicationId.from(tenant.value(), "app2", "instance2")), "instance msg #2");
-        Notification notification2 = notification(12345, Type.deployment, Level.error,   NotificationSource.from(ApplicationId.from(tenant.value(), "app3", "instance2")), "instance msg #3");
-        Notification notification3 = notification(12345, Type.reindex, Level.warning, NotificationSource.from(ApplicationId.from(tenant.value(), "app2", "instance2")), "instance msg #2");
-
+        Notification notification2 = notification(12345, Type.applicationPackage, Level.error,   NotificationSource.from(ApplicationId.from(tenant.value(), "app3", "instance2")), "instance msg #3");
+        Notification notification3 = notification(12345, Type.reindex, Level.warning, NotificationSource.from(new DeploymentId(ApplicationId.from(tenant.value(), "app2", "instance2"), ZoneId.defaultId()), new ClusterSpec.Id("content")), "instance msg #2");
+;
         var a = notifications.get(0);
         notificationsDb.setNotification(a.source(), a.type(), a.level(), a.messages());
         assertEquals(0, mailer.inbox(email).size());