openstack/monasca-thresh
Code Issues Proposed changes

Change Alarms.enabled to Alarms.actionsEnabled and remove handling that was appropriate for the alarm being disabled rather than just notifications. Will need some more work to handle updated alarms but will do that separately.

Browse Source
This commit is contained in:
Craig Bryant 2014-04-15 16:10:51 -06:00
parent 6b26b3191c
commit feb5433432
8 changed files with 29 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
src/main/java/com/hpcloud/mon/domain/model/Alarm.java
View File

@ -23,14 +23,14 @@ public class Alarm extends AbstractEntity {
private AlarmExpression expression;
private Map<String, SubAlarm> subAlarms;
private AlarmState state;
private boolean enabled = true;
private boolean actionsEnabled = true;
private String stateChangeReason;
public Alarm() {
}
public Alarm(String id, String tenantId, String name, String description, AlarmExpression expression,
List<SubAlarm> subAlarms, AlarmState state, boolean enabled) {
List<SubAlarm> subAlarms, AlarmState state, boolean actionsEnabled) {
this.id = id;
this.tenantId = tenantId;
this.name = name;
@ -38,7 +38,7 @@ public class Alarm extends AbstractEntity {
this.expression = expression;
setSubAlarms(subAlarms);
this.state = state;
this.enabled = enabled;
this.actionsEnabled = actionsEnabled;
}
static String buildStateChangeReason(AlarmState alarmState, List<String> subAlarmExpressions) {
@ -67,6 +67,8 @@ public class Alarm extends AbstractEntity {
return false;
if (state != other.state)
return false;
if (actionsEnabled != other.actionsEnabled)
return false;
if (!compareObjects(subAlarms, other.subAlarms))
return false;
if (!compareObjects(tenantId, other.tenantId))
@ -153,8 +155,8 @@ public class Alarm extends AbstractEntity {
return state;
}
public boolean isEnabled() {
return enabled;
public boolean isActionsEnabled() {
return actionsEnabled;
}
public String getStateChangeReason() {
@ -181,6 +183,7 @@ public class Alarm extends AbstractEntity {
result = prime * result + ((name == null) ? 0 : name.hashCode());
result = prime * result + ((state == null) ? 0 : state.hashCode());
result = prime * result + ((subAlarms == null) ? 0 : subAlarms.hashCode());
result = prime * result + (actionsEnabled ? 1783 : 0);
result = prime * result + ((tenantId == null) ? 0 : tenantId.hashCode());
return result;
}
@ -201,8 +204,8 @@ public class Alarm extends AbstractEntity {
this.state = state;
}
public void setEnabled(boolean enabled) {
this.enabled = enabled;
public void setActionsEnabled(boolean actionsEnabled) {
this.actionsEnabled = actionsEnabled;
}
public void setSubAlarms(List<SubAlarm> subAlarms) {
@ -217,8 +220,8 @@ public class Alarm extends AbstractEntity {
@Override
public String toString() {
return String.format("Alarm [tenantId=%s, name=%s, description=%s, state=%s]", tenantId,
name, description, state);
return String.format("Alarm [tenantId=%s, name=%s, description=%s, state=%s, actionsEnabled=%s]", tenantId,
name, description, state, actionsEnabled);
}
public void updateSubAlarm(SubAlarm subAlarm) {

2
src/main/java/com/hpcloud/mon/infrastructure/persistence/MetricDefinitionDAOImpl.java
View File

@ -23,7 +23,7 @@ import com.hpcloud.mon.domain.service.SubAlarmMetricDefinition;
public class MetricDefinitionDAOImpl implements MetricDefinitionDAO {
private static final String METRIC_DEF_SQL = "select sa.id, a.tenant_id, sa.metric_name, sad.dimensions from alarm as a, sub_alarm as sa "
+ "left join (select sub_alarm_id, group_concat(dimension_name, '=', value) as dimensions from sub_alarm_dimension group by sub_alarm_id) as sad on sa.id = sad.sub_alarm_id "
+ "where a.id = sa.alarm_id and a.enabled=1 and a.deleted_at is null";
+ "where a.id = sa.alarm_id and a.deleted_at is null";
private final DBI db;

4
src/main/java/com/hpcloud/mon/infrastructure/persistence/SubAlarmDAOImpl.java
View File

@ -32,10 +32,10 @@ public class SubAlarmDAOImpl implements SubAlarmDAO {
*/
private static final String FIND_BY_METRIC_DEF_SQL = "select sa.* from sub_alarm sa, alarm a, sub_alarm_dimension d "
+ "join (%s) v on d.dimension_name = v.dimension_name and d.value = v.value "
+ "where sa.id = d.sub_alarm_id and sa.metric_name = :metric_name and a.tenant_id = :tenant_id and a.id = sa.alarm_id and a.enabled=1 and a.deleted_at is null "
+ "where sa.id = d.sub_alarm_id and sa.metric_name = :metric_name and a.tenant_id = :tenant_id and a.id = sa.alarm_id and a.deleted_at is null "
+ "group by d.sub_alarm_id having count(d.sub_alarm_id) = %s";
private static final String FIND_BY_METRIC_DEF_NO_DIMS_SQL = "select sa.* from sub_alarm sa, alarm a where sa.metric_name = :metric_name "
+ "and a.tenant_id = :tenant_id and a.id = sa.alarm_id and a.enabled=1 and a.deleted_at is null and (select count(*) from sub_alarm_dimension where sub_alarm_id = sa.id) = 0";
+ "and a.tenant_id = :tenant_id and a.id = sa.alarm_id and a.deleted_at is null and (select count(*) from sub_alarm_dimension where sub_alarm_id = sa.id) = 0";
private final DBI db;

4
src/main/java/com/hpcloud/mon/infrastructure/thresholding/AlarmThresholdingBolt.java
View File

@ -184,10 +184,6 @@ public class AlarmThresholdingBolt extends BaseRichBolt {
alarm = alarmDAO.findById(alarmId);
if (alarm == null)
LOG.error("Failed to locate alarm for id {}", alarmId);
else if (!alarm.isEnabled()) {
LOG.warn("Ignoring Alarm {} named '{}' because it is disabled", alarmId, alarm.getName());
return null;
}
else {
for (final SubAlarm subAlarm : alarm.getSubAlarms()) {
subAlarm.setNoState(true);

6
src/test/java/com/hpcloud/mon/infrastructure/persistence/AlarmDAOImplTest.java
View File

@ -29,7 +29,7 @@ public class AlarmDAOImplTest {
private static final String ALARM_ID = "123";
private static String ALARM_NAME = "90% CPU";
private static String ALARM_DESCR = "Description for " + ALARM_NAME;
private static Boolean ALARM_ENABLED = Boolean.FALSE;
private static Boolean ALARM_ENABLED = Boolean.TRUE;
private DBI db;
private Handle handle;
@ -55,7 +55,7 @@ public class AlarmDAOImplTest {
handle.execute("truncate table sub_alarm_dimension");
handle.execute("truncate table alarm_action");
String sql = String.format("insert into alarm (id, tenant_id, name, description, expression, state, enabled, created_at, updated_at) "
String sql = String.format("insert into alarm (id, tenant_id, name, description, expression, state, actions_enabled, created_at, updated_at) "
+ "values ('%s', '%s', '%s', '%s', 'avg(hpcs.compute{disk=vda, instance_id=123, metric_name=cpu}) > 10', 'UNDETERMINED', %d, NOW(), NOW())",
ALARM_ID, TENANT_ID, ALARM_NAME, ALARM_DESCR, ALARM_ENABLED ? 1 : 0);
handle.execute(sql);
@ -72,7 +72,7 @@ public class AlarmDAOImplTest {
String expr = "avg(hpcs.compute{disk=vda, instance_id=123, metric_name=cpu}) > 10";
Alarm expected = new Alarm(ALARM_ID, TENANT_ID, ALARM_NAME, ALARM_DESCR, AlarmExpression.of(expr),
Arrays.asList(new SubAlarm("111", ALARM_ID, AlarmSubExpression.of(expr))),
AlarmState.UNDETERMINED, Boolean.FALSE);
AlarmState.UNDETERMINED, Boolean.TRUE);
Alarm alarm = dao.findById(ALARM_ID);

12
src/test/java/com/hpcloud/mon/infrastructure/persistence/MetricDefinitionDAOImplTest.java
View File

@ -50,8 +50,8 @@ public class MetricDefinitionDAOImplTest {
protected void beforeMethod() {
cleanUp();
handle.execute("insert into alarm (id, tenant_id, name, description, expression, state, enabled, created_at, updated_at) "
+ "values ('123', '" + TENANT_ID + "', 'Test Alarm', 'Test Alarm Description', 'Not real expr', 'OK', '1', NOW(), NOW())");
handle.execute("insert into alarm (id, tenant_id, name, description, expression, state, created_at, updated_at) "
+ "values ('123', '" + TENANT_ID + "', 'Test Alarm', 'Test Alarm Description', 'Not real expr', 'OK', NOW(), NOW())");
handle.execute("insert into sub_alarm (id, alarm_id, function, metric_name, operator, threshold, period, periods, state, created_at, updated_at) "
+ "values ('111', '123', 'AVG', 'cpu', 'GT', 10, 60, 1, 'OK', NOW(), NOW())");
@ -99,14 +99,6 @@ public class MetricDefinitionDAOImplTest {
assertTrue(found.contains(toFind), "Did not find " + toFind);
}
public void shouldNotFindDisabledAlarms() {
handle.execute("update alarm set enabled=0 where id in ('123')");
List<SubAlarmMetricDefinition> found = dao.findForAlarms();
for (final SubAlarmMetricDefinition toFind : expected)
assertFalse(found.contains(toFind), "Should not have found " + toFind);
}
public void shouldNotFindDeletedAlarms() {
handle.execute("update alarm set deleted_at=NOW() where id in ('123')");

27
src/test/java/com/hpcloud/mon/infrastructure/persistence/SubAlarmDAOImplTest.java
View File

@ -51,14 +51,14 @@ public class SubAlarmDAOImplTest {
handle.execute("truncate table sub_alarm_dimension");
// These don't have the real Alarm expression because it doesn't matter for this test
handle.execute("insert into alarm (id, tenant_id, name, description, expression, state, enabled, created_at, updated_at) "
+ "values ('123', '" + TENANT_ID + "', 'Test Alarm', 'Test Alarm Description', 'Not real expr', 'OK', '1', NOW(), NOW())");
handle.execute("insert into alarm (id, tenant_id, name, description, expression, state, enabled, created_at, updated_at) "
+ "values ('234', '" + TENANT_ID + "', 'Test Alarm2', 'Test Alarm2 Description', 'Not real expr', 'OK', '1', NOW(), NOW())");
handle.execute("insert into alarm (id, tenant_id, name, description, expression, state, enabled, created_at, updated_at) "
+ "values ('345', '" + TENANT_ID + "', 'Test Alarm3', 'Test Alarm3 Description', 'Not real expr', 'OK', '1', NOW(), NOW())");
handle.execute("insert into alarm (id, tenant_id, name, description, expression, state, enabled, created_at, updated_at) "
+ "values ('456', '" + TENANT_ID + "', 'Test Alarm4', 'Test Alarm4 Description', 'Not real expr', 'OK', '1', NOW(), NOW())");
handle.execute("insert into alarm (id, tenant_id, name, description, expression, state, created_at, updated_at) "
+ "values ('123', '" + TENANT_ID + "', 'Test Alarm', 'Test Alarm Description', 'Not real expr', 'OK', NOW(), NOW())");
handle.execute("insert into alarm (id, tenant_id, name, description, expression, state, created_at, updated_at) "
+ "values ('234', '" + TENANT_ID + "', 'Test Alarm2', 'Test Alarm2 Description', 'Not real expr', 'OK', NOW(), NOW())");
handle.execute("insert into alarm (id, tenant_id, name, description, expression, state, created_at, updated_at) "
+ "values ('345', '" + TENANT_ID + "', 'Test Alarm3', 'Test Alarm3 Description', 'Not real expr', 'OK', NOW(), NOW())");
handle.execute("insert into alarm (id, tenant_id, name, description, expression, state, created_at, updated_at) "
+ "values ('456', '" + TENANT_ID + "', 'Test Alarm4', 'Test Alarm4 Description', 'Not real expr', 'OK', NOW(), NOW())");
handle.execute("insert into sub_alarm (id, alarm_id, function, metric_name, operator, threshold, period, periods, created_at, updated_at) "
+ "values ('111', '123', 'AVG', 'cpu', 'GT', 10, 60, 1, NOW(), NOW())");
@ -106,17 +106,6 @@ public class SubAlarmDAOImplTest {
assertEquals(subAlarms.size(), 0);
}
public void shouldNotFindDisabledAlarm() {
handle.execute("update alarm set enabled=0 where id='123' or id='456'");
final String badTenantId = TENANT_ID;
List<SubAlarm> subAlarms = dao.find(new MetricDefinitionAndTenantId(AlarmSubExpression.of("avg(cpu{instance_id=555,az=1}) > 10").getMetricDefinition(), badTenantId));
assertEquals(subAlarms.size(), 0);
subAlarms = dao.find(new MetricDefinitionAndTenantId(new MetricDefinition("cpu", null), TENANT_ID));
assertEquals(subAlarms.size(), 0);
}
public void shouldFindWithSubject() {
List<SubAlarm> expected = Arrays.asList(new SubAlarm(
"333",

2
src/test/resources/com/hpcloud/mon/infrastructure/persistence/alarm.sql
View File

@ -5,7 +5,7 @@ CREATE TABLE `alarm` (
`description` varchar(250) NOT NULL,
`expression` mediumtext NOT NULL,
`state` varchar(20) NOT NULL check state in ('UNDETERMINED','OK','ALARM'),
`enabled` tinyint(1) NOT NULL DEFAULT '1',
`actions_enabled` tinyint(1) NOT NULL DEFAULT '1',
`created_at` datetime NOT NULL,
`updated_at` datetime NOT NULL,
`deleted_at` datetime,