AMBARI-9597 - Current Alerts Do Not Reflect the Name Change of an Alert Definition (jonathanhurley)

ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java

@@ -213,6 +213,9 @@ public class AlertDefinitionResourceProvider extends AbstractControllerResourceP
     }
   }
 
+  /**
+   * {@inheritDoc}
+   */
   @Override
   public Set<Resource> getResources(Request request, Predicate predicate)
       throws SystemException, UnsupportedPropertyException,
@@ -255,6 +258,9 @@ public class AlertDefinitionResourceProvider extends AbstractControllerResourceP
     return results;
   }
 
+  /**
+   * {@inheritDoc}
+   */
   @Override
   public RequestStatus updateResources(Request request, Predicate predicate)
       throws SystemException, UnsupportedPropertyException,
@@ -317,6 +323,9 @@ public class AlertDefinitionResourceProvider extends AbstractControllerResourceP
     return getRequestStatus(null);
   }
 
+  /**
+   * {@inheritDoc}
+   */
   @Override
   public RequestStatus deleteResources(Predicate predicate)
       throws SystemException, UnsupportedPropertyException,

ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java

@@ -195,8 +195,10 @@ public class AlertResourceProvider extends ReadOnlyResourceProvider {
    */
   private Resource toResource(boolean isCollection, String clusterName,
       AlertCurrentEntity entity, Set<String> requestedIds) {
-    Resource resource = new ResourceImpl(Resource.Type.Alert);
+    AlertHistoryEntity history = entity.getAlertHistory();
+    AlertDefinitionEntity definition = history.getAlertDefinition();
 
+    Resource resource = new ResourceImpl(Resource.Type.Alert);
     setResourceProperty(resource, ALERT_CLUSTER_NAME, clusterName, requestedIds);
     setResourceProperty(resource, ALERT_ID, entity.getAlertId(), requestedIds);
     setResourceProperty(resource, ALERT_LATEST_TIMESTAMP, entity.getLatestTimestamp(), requestedIds);
@@ -204,15 +206,14 @@ public class AlertResourceProvider extends ReadOnlyResourceProvider {
     setResourceProperty(resource, ALERT_ORIGINAL_TIMESTAMP, entity.getOriginalTimestamp(), requestedIds);
     setResourceProperty(resource, ALERT_TEXT, entity.getLatestText(), requestedIds);
 
-    AlertHistoryEntity history = entity.getAlertHistory();
     setResourceProperty(resource, ALERT_INSTANCE, history.getAlertInstance(), requestedIds);
-    setResourceProperty(resource, ALERT_LABEL, history.getAlertLabel(), requestedIds);
+    setResourceProperty(resource, ALERT_LABEL, definition.getLabel(), requestedIds);
     setResourceProperty(resource, ALERT_STATE, history.getAlertState(), requestedIds);
     setResourceProperty(resource, ALERT_COMPONENT, history.getComponentName(), requestedIds);
     setResourceProperty(resource, ALERT_HOST, history.getHostName(), requestedIds);
     setResourceProperty(resource, ALERT_SERVICE, history.getServiceName(), requestedIds);
 
-    AlertDefinitionEntity definition = history.getAlertDefinition();
+
     setResourceProperty(resource, ALERT_DEFINITION_ID, definition.getDefinitionId(),requestedIds);
     setResourceProperty(resource, ALERT_DEFINITION_NAME, definition.getDefinitionName(), requestedIds);
     setResourceProperty(resource, ALERT_SCOPE, definition.getScope(), requestedIds);

ambari-server/src/main/java/org/apache/ambari/server/events/AlertDefinitionChangedEvent.java

@@ -38,8 +38,7 @@ public class AlertDefinitionChangedEvent extends ClusterEvent {
    * @param definition
    *          the alert definition that was changed.
    */
-  public AlertDefinitionChangedEvent(long clusterId,
- AlertDefinition definition) {
+  public AlertDefinitionChangedEvent(long clusterId, AlertDefinition definition) {
     super(AmbariEventType.ALERT_DEFINITION_CHANGED, clusterId);
     m_definition = definition;
   }

ambari-server/src/main/java/org/apache/ambari/server/events/listeners/alerts/AlertLifecycleListener.java

@@ -17,6 +17,7 @@
  */
 package org.apache.ambari.server.events.listeners.alerts;
 
+import java.util.List;
 import java.util.Set;
 
 import org.apache.ambari.server.EagerSingleton;
@@ -25,10 +26,14 @@ import org.apache.ambari.server.events.AlertDefinitionDeleteEvent;
 import org.apache.ambari.server.events.AlertDefinitionRegistrationEvent;
 import org.apache.ambari.server.events.AlertHashInvalidationEvent;
 import org.apache.ambari.server.events.publishers.AmbariEventPublisher;
+import org.apache.ambari.server.orm.dao.AlertsDAO;
+import org.apache.ambari.server.orm.entities.AlertCurrentEntity;
+import org.apache.ambari.server.orm.entities.AlertHistoryEntity;
 import org.apache.ambari.server.state.alert.AggregateDefinitionMapping;
 import org.apache.ambari.server.state.alert.AlertDefinition;
 import org.apache.ambari.server.state.alert.AlertDefinitionHash;
 import org.apache.ambari.server.state.alert.SourceType;
+import org.apache.commons.lang.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -69,6 +74,12 @@ public class AlertLifecycleListener {
   @Inject
   private AmbariEventPublisher m_eventPublisher;
 
+  /**
+   * Used for querying current alerts when an alert definition changes.
+   */
+  @Inject
+  private AlertsDAO m_alertsDao;
+
   /**
    * Constructor.
    *
@@ -106,6 +117,7 @@ public class AlertLifecycleListener {
    * tasks:
    * <ul>
    * <li>Updating definition with {@link AggregateDefinitionMapping}</li>
+   * <li>Updating current alerts with definition label</li>
    * </ul>
    *
    * @param event
@@ -121,6 +133,18 @@ public class AlertLifecycleListener {
     if (definition.getSource().getType() == SourceType.AGGREGATE) {
       m_aggregateMapping.registerAggregate(event.getClusterId(), definition);
     }
+
+    // update any current alerts
+    List<AlertCurrentEntity> currentAlerts = m_alertsDao.findCurrentByDefinitionId(definition.getDefinitionId());
+    for (AlertCurrentEntity current : currentAlerts) {
+      AlertHistoryEntity history = current.getAlertHistory();
+
+      // if the definition label changed, update the current alerts
+      if (!StringUtils.equals(definition.getLabel(), history.getAlertLabel())) {
+        history.setAlertLabel(definition.getLabel());
+        m_alertsDao.merge(history);
+      }
+    }
   }
 
   /**

ambari-server/src/main/java/org/apache/ambari/server/events/listeners/alerts/AlertReceivedListener.java

@@ -156,14 +156,15 @@ public class AlertReceivedListener {
     } else if (alert.getState() == current.getAlertHistory().getAlertState()) {
       current.setLatestTimestamp(alert.getTimestamp());
       current.setLatestText(alert.getText());
-
       current = m_alertsDao.merge(current);
     } else {
-      LOG.debug(
-          "Alert State Changed: CurrentId {}, CurrentTimestamp {}, HistoryId {}, HistoryState {}",
-          current.getAlertId(), current.getLatestTimestamp(),
-          current.getAlertHistory().getAlertId(),
-          current.getAlertHistory().getAlertState());
+      if (LOG.isDebugEnabled()) {
+        LOG.debug(
+            "Alert State Changed: CurrentId {}, CurrentTimestamp {}, HistoryId {}, HistoryState {}",
+            current.getAlertId(), current.getLatestTimestamp(),
+            current.getAlertHistory().getAlertId(),
+            current.getAlertHistory().getAlertState());
+      }
 
       AlertHistoryEntity oldHistory = current.getAlertHistory();
       AlertState oldState = oldHistory.getAlertState();
@@ -183,11 +184,13 @@ public class AlertReceivedListener {
 
       current = m_alertsDao.merge(current);
 
-      LOG.debug(
-          "Alert State Merged: CurrentId {}, CurrentTimestamp {}, HistoryId {}, HistoryState {}",
-          current.getAlertId(), current.getLatestTimestamp(),
-          current.getAlertHistory().getAlertId(),
-          current.getAlertHistory().getAlertState());
+      if (LOG.isDebugEnabled()) {
+        LOG.debug(
+            "Alert State Merged: CurrentId {}, CurrentTimestamp {}, HistoryId {}, HistoryState {}",
+            current.getAlertId(), current.getLatestTimestamp(),
+            current.getAlertHistory().getAlertId(),
+            current.getAlertHistory().getAlertState());
+      }
 
       // broadcast the alert changed event for other subscribers
       AlertStateChangeEvent alertChangedEvent = new AlertStateChangeEvent(
@@ -317,8 +320,8 @@ public class AlertReceivedListener {
       AlertDefinitionEntity definition, Alert alert) {
     AlertHistoryEntity history = new AlertHistoryEntity();
     history.setAlertDefinition(definition);
+    history.setAlertLabel(definition.getLabel());
     history.setAlertInstance(alert.getInstance());
-    history.setAlertLabel(alert.getLabel());
     history.setAlertState(alert.getState());
     history.setAlertText(alert.getText());
     history.setAlertTimestamp(Long.valueOf(alert.getTimestamp()));

ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java

@@ -292,10 +292,29 @@ public class AlertsDAO {
     return entityManagerProvider.get().find(AlertCurrentEntity.class, alertId);
   }
 
+  /**
+   * Gets the current alerts for the specified definition ID.
+   *
+   * @param definitionId
+   *          the ID of the definition to retrieve current alerts for.
+   * @return the current alerts for the definition or an empty list if none
+   *         exist (never {@code null}).
+   */
+  @RequiresSession
+  public List<AlertCurrentEntity> findCurrentByDefinitionId(long definitionId) {
+    TypedQuery<AlertCurrentEntity> query = entityManagerProvider.get().createNamedQuery(
+        "AlertCurrentEntity.findByDefinitionId", AlertCurrentEntity.class);
+
+    query.setParameter("definitionId", Long.valueOf(definitionId));
+    query = setQueryRefreshHint(query);
+
+    return daoUtils.selectList(query);
+  }
+
   /**
    * Gets the current alerts for a given cluster.
    *
-   * @return the current alerts for the given clusteror an empty list if none
+   * @return the current alerts for the given cluster or an empty list if none
    *         exist (never {@code null}).
    */
   @RequiresSession

ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertCurrentEntity.java

@@ -48,6 +48,7 @@ import org.apache.ambari.server.state.MaintenanceState;
 @NamedQueries({
     @NamedQuery(name = "AlertCurrentEntity.findAll", query = "SELECT alert FROM AlertCurrentEntity alert"),
     @NamedQuery(name = "AlertCurrentEntity.findByCluster", query = "SELECT alert FROM AlertCurrentEntity alert WHERE alert.alertHistory.clusterId = :clusterId"),
+    @NamedQuery(name = "AlertCurrentEntity.findByDefinitionId", query = "SELECT alert FROM AlertCurrentEntity alert WHERE alert.alertDefinition.definitionId = :definitionId"),
     @NamedQuery(name = "AlertCurrentEntity.findByService", query = "SELECT alert FROM AlertCurrentEntity alert WHERE alert.alertHistory.clusterId = :clusterId AND alert.alertHistory.serviceName = :serviceName AND alert.alertHistory.alertDefinition.scope IN :inlist"),
     @NamedQuery(name = "AlertCurrentEntity.findByHost", query = "SELECT alert FROM AlertCurrentEntity alert WHERE alert.alertHistory.clusterId = :clusterId AND alert.alertHistory.hostName = :hostName AND alert.alertHistory.alertDefinition.scope IN :inlist"),
     @NamedQuery(name = "AlertCurrentEntity.findByHostAndName", query = "SELECT alert FROM AlertCurrentEntity alert WHERE alert.alertHistory.clusterId = :clusterId AND alert.alertHistory.alertDefinition.definitionName = :definitionName AND alert.alertHistory.hostName = :hostName"),

ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinition.java

@@ -37,6 +37,7 @@ import com.google.gson.annotations.SerializedName;
 public class AlertDefinition {
 
   private long clusterId;
+  private long definitionId;
   private String serviceName = null;
   private String componentName = null;
 
@@ -70,6 +71,21 @@ public class AlertDefinition {
     this.clusterId = clusterId;
   }
 
+  /**
+   * @return the definitionId
+   */
+  public long getDefinitionId() {
+    return definitionId;
+  }
+
+  /**
+   * @param definitionId
+   *          the definitionId to set
+   */
+  public void setDefinitionId(long definitionId) {
+    this.definitionId = definitionId;
+  }
+
   /**
    * @return the service name
    */

ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionFactory.java

@@ -178,6 +178,7 @@ public class AlertDefinitionFactory {
 
     AlertDefinition definition = new AlertDefinition();
     definition.setClusterId(entity.getClusterId());
+    definition.setDefinitionId(entity.getDefinitionId());
     definition.setComponentName(entity.getComponentName());
     definition.setEnabled(entity.getEnabled());
     definition.setHostIgnored(entity.isHostIgnored());

ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java

@@ -225,6 +225,67 @@ public class AlertsDAOTest {
     assertEquals(5, currentAlerts.size());
   }
 
+  /**
+   * Test looking up current alerts by definition ID.
+   */
+  @Test
+  public void testFindCurrentByDefinitionId() throws Exception {
+    // create a host
+    AlertDefinitionEntity definition = new AlertDefinitionEntity();
+    definition.setDefinitionName("Foo Definition");
+    definition.setServiceName("YARN");
+    definition.setComponentName("NODEMANAGER");
+    definition.setClusterId(m_cluster.getClusterId());
+    definition.setHash(UUID.randomUUID().toString());
+    definition.setScheduleInterval(Integer.valueOf(60));
+    definition.setScope(Scope.HOST);
+    definition.setSource("{\"type\" : \"SCRIPT\"}");
+    definition.setSourceType(SourceType.SCRIPT);
+    m_definitionDao.create(definition);
+
+    // history for the definition
+    AlertHistoryEntity history = new AlertHistoryEntity();
+    history.setServiceName(definition.getServiceName());
+    history.setClusterId(m_cluster.getClusterId());
+    history.setAlertDefinition(definition);
+    history.setAlertLabel(definition.getDefinitionName());
+    history.setAlertText(definition.getDefinitionName());
+    history.setAlertTimestamp(Long.valueOf(1L));
+    history.setHostName("h1");
+    history.setAlertState(AlertState.OK);
+
+    // current for the history
+    AlertCurrentEntity current = new AlertCurrentEntity();
+    current.setOriginalTimestamp(1L);
+    current.setLatestTimestamp(2L);
+    current.setAlertHistory(history);
+    m_dao.create(current);
+
+    List<AlertCurrentEntity> currentAlerts = m_dao.findCurrentByDefinitionId(definition.getDefinitionId());
+    assertEquals(1, currentAlerts.size());
+
+    // history for the definition
+    AlertHistoryEntity history2 = new AlertHistoryEntity();
+    history2.setServiceName(definition.getServiceName());
+    history2.setClusterId(m_cluster.getClusterId());
+    history2.setAlertDefinition(definition);
+    history2.setAlertLabel(definition.getDefinitionName());
+    history2.setAlertText(definition.getDefinitionName());
+    history2.setAlertTimestamp(Long.valueOf(1L));
+    history2.setHostName("h2");
+    history2.setAlertState(AlertState.OK);
+
+    // current for the history
+    AlertCurrentEntity current2 = new AlertCurrentEntity();
+    current2.setOriginalTimestamp(1L);
+    current2.setLatestTimestamp(2L);
+    current2.setAlertHistory(history2);
+    m_dao.create(current2);
+
+    currentAlerts = m_dao.findCurrentByDefinitionId(definition.getDefinitionId());
+    assertEquals(2, currentAlerts.size());
+  }
+
   /**
    *
    */

ambari-server/src/test/java/org/apache/ambari/server/state/alerts/AlertEventPublisherTest.java

@@ -35,8 +35,12 @@ import org.apache.ambari.server.orm.InMemoryDefaultTestModule;
 import org.apache.ambari.server.orm.OrmTestHelper;
 import org.apache.ambari.server.orm.dao.AlertDefinitionDAO;
 import org.apache.ambari.server.orm.dao.AlertDispatchDAO;
+import org.apache.ambari.server.orm.dao.AlertsDAO;
+import org.apache.ambari.server.orm.entities.AlertCurrentEntity;
 import org.apache.ambari.server.orm.entities.AlertDefinitionEntity;
 import org.apache.ambari.server.orm.entities.AlertGroupEntity;
+import org.apache.ambari.server.orm.entities.AlertHistoryEntity;
+import org.apache.ambari.server.state.AlertState;
 import org.apache.ambari.server.state.Cluster;
 import org.apache.ambari.server.state.Clusters;
 import org.apache.ambari.server.state.Service;
@@ -67,6 +71,7 @@ public class AlertEventPublisherTest {
 
   private AlertDispatchDAO dispatchDao;
   private AlertDefinitionDAO definitionDao;
+  private AlertsDAO alertsDao;
   private Clusters clusters;
   private Cluster cluster;
   private String clusterName;
@@ -100,6 +105,7 @@ public class AlertEventPublisherTest {
 
     dispatchDao = injector.getInstance(AlertDispatchDAO.class);
     definitionDao = injector.getInstance(AlertDefinitionDAO.class);
+    alertsDao = injector.getInstance(AlertsDAO.class);
     clusters = injector.getInstance(Clusters.class);
     serviceFactory = injector.getInstance(ServiceFactory.class);
     ormHelper = injector.getInstance(OrmTestHelper.class);
@@ -230,6 +236,54 @@ public class AlertEventPublisherTest {
         aggregate.getSource().getReporting().getOk().getText());
   }
 
+  @Test
+  public void testAlertDefinitionNameChangeEvent() throws Exception {
+    installHdfsService();
+    AlertDefinitionEntity definition = definitionDao.findAll().get(0);
+
+    // create 2 historical entries; one will be current
+    AlertHistoryEntity history = new AlertHistoryEntity();
+    history.setServiceName(definition.getServiceName());
+    history.setClusterId(cluster.getClusterId());
+    history.setAlertDefinition(definition);
+    history.setAlertLabel(definition.getLabel());
+    history.setAlertText(definition.getDefinitionName());
+    history.setAlertTimestamp(Long.valueOf(1L));
+    history.setHostName(null);
+    history.setAlertState(AlertState.OK);
+    alertsDao.create(history);
+
+    // this one will be current
+    AlertHistoryEntity history2 = new AlertHistoryEntity();
+    history2.setServiceName(definition.getServiceName());
+    history2.setClusterId(cluster.getClusterId());
+    history2.setAlertDefinition(definition);
+    history2.setAlertLabel(definition.getLabel());
+    history2.setAlertText(definition.getDefinitionName());
+    history2.setAlertTimestamp(Long.valueOf(1L));
+    history2.setHostName(null);
+    history2.setAlertState(AlertState.CRITICAL);
+
+    // current for the history
+    AlertCurrentEntity current = new AlertCurrentEntity();
+    current.setOriginalTimestamp(1L);
+    current.setLatestTimestamp(2L);
+    current.setAlertHistory(history2);
+    alertsDao.create(current);
+
+    // change the definition name
+    definition.setLabel("testAlertDefinitionNameChangeEvent");
+    definitionDao.merge(definition);
+
+    // the older history item will not have the label changed while
+    // the new one will
+    history = alertsDao.findById(history.getAlertId());
+    history2 = alertsDao.findById(history2.getAlertId());
+
+    Assert.assertFalse(definition.getLabel().equals(history.getAlertLabel()));
+    Assert.assertEquals(definition.getLabel(), history2.getAlertLabel());
+  }
+
   /**
    * Tests that {@link AlertDefinitionDeleteEvent} instances are fired when a
    * definition is removed.

ambari-server/src/test/java/org/apache/ambari/server/state/cluster/AlertDataManagerTest.java

@@ -133,6 +133,7 @@ public class AlertDataManagerTest {
       definition.setScope(Scope.SERVICE);
       definition.setSource("{\"type\" : \"SCRIPT\"}");
       definition.setSourceType(SourceType.SCRIPT);
+      definition.setLabel(ALERT_LABEL);
       m_definitionDao.create(definition);
     }
   }