Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Alert condition titles #2282

Merged
merged 18 commits into from May 31, 2016
Merged

Alert condition titles #2282

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
93ca197
Adding title field for Alert Conditions.
dennisoelkers May 18, 2016
78e1341
Consolidating repeated content negotiation annotations.
dennisoelkers May 18, 2016
af24194
Reformaating resources.
dennisoelkers May 18, 2016
9195358
Adapting tests to added title attribute.
dennisoelkers May 19, 2016
e8a5abc
Removing unneeded public declaration.
dennisoelkers May 19, 2016
672d5d1
Using correct way to retrieve alert conditions from stream.
dennisoelkers May 19, 2016
b37db7f
Showing/updating alert condition title in overview.
dennisoelkers May 23, 2016
b1d5a74
Using proper types for alert conditions in REST resource.
dennisoelkers May 25, 2016
c3fed47
Adding model response classes.
dennisoelkers May 25, 2016
87ece4c
Annotating potential nulls.
dennisoelkers May 25, 2016
c3c9b25
Documenting API change in upgrade notes.
dennisoelkers May 30, 2016
59fef0f
Style alert condition title helper text
May 30, 2016
78ebcdb
Fixing linter hints of touched files.
dennisoelkers May 31, 2016
0018225
Serializing type enum as lowercase.
dennisoelkers May 31, 2016
644e6f0
Unifying response format of alert conditions.
dennisoelkers May 31, 2016
49de2cc
Move help text styling into a stylesheet file
May 31, 2016
b4bf0eb
Unifying alert condition titles.
dennisoelkers May 31, 2016
357f945
Remove default parameters so defaults are used.
dennisoelkers May 31, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion UPGRADING.rst
View file
Open in desktop
Expand Up @@ -11,4 +11,11 @@ for the complete upgrade notes.
From 2.0 to 2.1
===============

tba
For Plugin Authors
------------------

Between 2.0 and 2.1 we also made changes to the Plugin API. These include:

* Removing ``org.graylog2.plugin.streams.Stream#getAlertCondition``, as it was faulty and not easily replaceable with a working version without breaking our separation of models and persistence services.

If you are maintaining a plugin that was originally written for 1.x or 2.0, you need to make sure that your plugin is still compiling and working under 2.1 or adapt it if necessary.
19 changes: 16 additions & 3 deletions graylog2-server/src/main/java/org/graylog2/alerts/AbstractAlertCondition.java
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@
package org.graylog2.alerts;

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.google.common.annotations.VisibleForTesting;
import com.fasterxml.jackson.annotation.JsonValue;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import org.graylog2.plugin.MessageSummary;
Expand All @@ -40,7 +40,12 @@ public enum Type {
MESSAGE_COUNT,
FIELD_VALUE,
FIELD_CONTENT_VALUE,
DUMMY
DUMMY;

@JsonValue
public String toString() {
return super.toString().toLowerCase(Locale.ENGLISH);
}
}

protected final String id;
Expand All @@ -49,10 +54,12 @@ public enum Type {
protected final DateTime createdAt;
protected final String creatorUserId;
protected final int grace;
protected final String title;

private final Map<String, Object> parameters;

protected AbstractAlertCondition(Stream stream, String id, Type type, DateTime createdAt, String creatorUserId, Map<String, Object> parameters) {
protected AbstractAlertCondition(Stream stream, String id, Type type, DateTime createdAt, String creatorUserId, Map<String, Object> parameters, String title) {
this.title = title;
if (id == null) {
this.id = UUID.randomUUID().toString();
} else {
Expand Down Expand Up @@ -84,6 +91,11 @@ public String getTypeString() {
return type.toString();
}

@Override
public String getTitle() {
return title;
}

@Override
public DateTime getCreatedAt() {
return createdAt;
Expand Down Expand Up @@ -123,6 +135,7 @@ public Map<String, Object> getPersistedFields() {
.put("creator_user_id", creatorUserId)
.put("created_at", Tools.getISO8601String(createdAt))
.put("parameters", parameters)
.put("title", title)
.build();
}

Expand Down
81 changes: 47 additions & 34 deletions graylog2-server/src/main/java/org/graylog2/alerts/AlertServiceImpl.java
View file
Open in desktop
Expand Up @@ -89,10 +89,10 @@ public List<Alert> loadRecentOfStream(String streamId, DateTime since) {
BasicDBObject sort = new BasicDBObject("triggered_at", -1);

final List<DBObject> alertObjects = query(AlertImpl.class,
qb.get(),
sort,
AlertImpl.MAX_LIST_COUNT,
0
qb.get(),
sort,
AlertImpl.MAX_LIST_COUNT,
0
);

List<Alert> alerts = Lists.newArrayList();
Expand All @@ -107,7 +107,7 @@ public List<Alert> loadRecentOfStream(String streamId, DateTime since) {
@Override
public int triggeredSecondsAgo(String streamId, String conditionId) {
DBObject query = QueryBuilder.start("stream_id").is(streamId)
.and("condition_id").is(conditionId).get();
.and("condition_id").is(conditionId).get();
BasicDBObject sort = new BasicDBObject("triggered_at", -1);

DBObject alert = findOne(AlertImpl.class, query, sort);
Expand Down Expand Up @@ -142,21 +142,28 @@ public AlertCondition fromPersisted(Map<String, Object> fields, Stream stream) t
}

return createAlertCondition(type,
stream,
(String) fields.get("id"),
DateTime.parse((String) fields.get("created_at")),
(String) fields.get("creator_user_id"),
(Map<String, Object>) fields.get("parameters"));
stream,
(String) fields.get("id"),
DateTime.parse((String) fields.get("created_at")),
(String) fields.get("creator_user_id"),
(Map<String, Object>) fields.get("parameters"),
(String) fields.get("title"));
}

private AbstractAlertCondition createAlertCondition(AbstractAlertCondition.Type type, Stream stream, String id, DateTime createdAt, String creatorId, Map<String, Object> parameters) throws AbstractAlertCondition.NoSuchAlertConditionTypeException {
private AbstractAlertCondition createAlertCondition(AbstractAlertCondition.Type type,
Stream stream,
String id,
DateTime createdAt,
String creatorId,
Map<String, Object> parameters,
String title) throws AbstractAlertCondition.NoSuchAlertConditionTypeException {
switch (type) {
case MESSAGE_COUNT:
return messageCountAlertFactory.createAlertCondition(stream, id, createdAt, creatorId, parameters);
return messageCountAlertFactory.createAlertCondition(stream, id, createdAt, creatorId, parameters, title);
case FIELD_VALUE:
return fieldValueAlertFactory.createAlertCondition(stream, id, createdAt, creatorId, parameters);
return fieldValueAlertFactory.createAlertCondition(stream, id, createdAt, creatorId, parameters, title);
case FIELD_CONTENT_VALUE:
return fieldContentValueAlertFactory.createAlertCondition(stream, id, createdAt, creatorId, parameters);
return fieldContentValueAlertFactory.createAlertCondition(stream, id, createdAt, creatorId, parameters, title);
}

throw new AbstractAlertCondition.NoSuchAlertConditionTypeException("Unhandled alert condition type: " + type);
Expand All @@ -176,26 +183,27 @@ public AbstractAlertCondition fromRequest(CreateConditionRequest ccr, Stream str
throw new AbstractAlertCondition.NoSuchAlertConditionTypeException("No such alert condition type: [" + type + "]");
}

return createAlertCondition(alertConditionType, stream, null, Tools.nowUTC(), userId, ccr.parameters());
return createAlertCondition(alertConditionType, stream, null, Tools.nowUTC(), userId, ccr.parameters(), ccr.title());
}

@Override
public AbstractAlertCondition updateFromRequest(AlertCondition alertCondition, CreateConditionRequest ccr) throws AbstractAlertCondition.NoSuchAlertConditionTypeException {
AbstractAlertCondition.Type type = ((AbstractAlertCondition) alertCondition).getType();
final AbstractAlertCondition.Type type = ((AbstractAlertCondition) alertCondition).getType();

Map<String, Object> parameters = ccr.parameters();
final Map<String, Object> parameters = ccr.parameters();
for (Map.Entry<String, Object> stringObjectEntry : alertCondition.getParameters().entrySet()) {
if (!parameters.containsKey(stringObjectEntry.getKey())) {
parameters.put(stringObjectEntry.getKey(), stringObjectEntry.getValue());
}
}

return createAlertCondition(type,
alertCondition.getStream(),
alertCondition.getId(),
alertCondition.getCreatedAt(),
alertCondition.getCreatorUserId(),
parameters
alertCondition.getStream(),
alertCondition.getId(),
alertCondition.getCreatedAt(),
alertCondition.getCreatorUserId(),
parameters,
ccr.title()
);
}

Expand Down Expand Up @@ -230,14 +238,19 @@ public AlertCondition.CheckResult triggered(AlertCondition alertCondition) {

@Override
public Map<String, Object> asMap(final AlertCondition alertCondition) {
return ImmutableMap.<String, Object>builder()
.put("id", alertCondition.getId())
.put("type", alertCondition.getTypeString().toLowerCase(Locale.ENGLISH))
.put("creator_user_id", alertCondition.getCreatorUserId())
.put("created_at", Tools.getISO8601String(alertCondition.getCreatedAt()))
.put("parameters", alertCondition.getParameters())
.put("in_grace", inGracePeriod(alertCondition))
.build();
ImmutableMap.Builder<String, Object> builder = ImmutableMap.<String, Object>builder()
.put("id", alertCondition.getId())
.put("type", alertCondition.getTypeString().toLowerCase(Locale.ENGLISH))
.put("creator_user_id", alertCondition.getCreatorUserId())
.put("created_at", Tools.getISO8601String(alertCondition.getCreatedAt()))
.put("parameters", alertCondition.getParameters())
.put("in_grace", inGracePeriod(alertCondition));

if (alertCondition.getTitle() != null) {
builder = builder.put("title", alertCondition.getTitle());
}

return builder.build();
}

@Override
Expand All @@ -247,10 +260,10 @@ public List<Alert> listForStreamId(String streamId, int skip, int limit) {
BasicDBObject sort = new BasicDBObject("triggered_at", -1);

final List<DBObject> alertObjects = query(AlertImpl.class,
qb.get(),
sort,
limit,
skip
qb.get(),
sort,
limit,
skip
);

final List<Alert> alerts = Lists.newArrayListWithCapacity(alertObjects.size());
Expand Down
2 changes: 2 additions & 0 deletions graylog2-server/src/main/java/org/graylog2/alerts/FormattedEmailAlertSender.java
View file
Open in desktop
Expand Up @@ -42,6 +42,7 @@ public class FormattedEmailAlertSender extends StaticEmailAlertSender implements
"Stream ID: ${stream.id}\n" +
"Stream title: ${stream.title}\n" +
"Stream description: ${stream.description}\n" +
"Alert Condition Title: ${alertCondition.title}\n" +
"${if stream_url}Stream URL: ${stream_url}${end}\n" +
"\n" +
"Triggered condition: ${check_result.triggeredCondition}\n" +
Expand Down Expand Up @@ -107,6 +108,7 @@ private Map<String, Object> getModel(Stream stream, AlertCondition.CheckResult c
model.put("stream", stream);
model.put("check_result", checkResult);
model.put("stream_url", buildStreamDetailsURL(configuration.getWebInterfaceUri(), checkResult, stream));
model.put("alertCondition", checkResult.getTriggeredCondition());

final List<Message> messages = firstNonNull(backlog, Collections.<Message>emptyList());
model.put("backlog", messages);
Expand Down
4 changes: 2 additions & 2 deletions graylog2-server/src/main/java/org/graylog2/alerts/types/DummyAlertCondition.java
View file
Open in desktop
Expand Up @@ -26,8 +26,8 @@
public class DummyAlertCondition extends AbstractAlertCondition {
final String description = "Dummy alert to test notifications";

public DummyAlertCondition(Stream stream, String id, DateTime createdAt, String creatorUserId, Map<String, Object> parameters) {
super(stream, id, Type.DUMMY, createdAt, creatorUserId, parameters);
public DummyAlertCondition(Stream stream, String id, DateTime createdAt, String creatorUserId, Map<String, Object> parameters, String title) {
super(stream, id, Type.DUMMY, createdAt, creatorUserId, parameters, title);
}

@Override
Expand Down
18 changes: 15 additions & 3 deletions graylog2-server/src/main/java/org/graylog2/alerts/types/FieldContentValueAlertCondition.java
View file
Open in desktop
Expand Up @@ -54,12 +54,24 @@ public class FieldContentValueAlertCondition extends AbstractAlertCondition {
private final String value;

public interface Factory {
FieldContentValueAlertCondition createAlertCondition(Stream stream, String id, DateTime createdAt, @Assisted("userid") String creatorUserId, Map<String, Object> parameters);
FieldContentValueAlertCondition createAlertCondition(Stream stream,
@Assisted("id") String id,
DateTime createdAt,
@Assisted("userid") String creatorUserId,
Map<String, Object> parameters,
@Assisted("title") @Nullable String title);
}

@AssistedInject
public FieldContentValueAlertCondition(Searches searches, Configuration configuration, @Assisted Stream stream, @Nullable @Assisted String id, @Assisted DateTime createdAt, @Assisted("userid") String creatorUserId, @Assisted Map<String, Object> parameters) {
super(stream, id, Type.FIELD_CONTENT_VALUE, createdAt, creatorUserId, parameters);
public FieldContentValueAlertCondition(Searches searches,
Configuration configuration,
@Assisted Stream stream,
@Nullable @Assisted("id") String id,
@Assisted DateTime createdAt,
@Assisted("userid") String creatorUserId,
@Assisted Map<String, Object> parameters,
@Assisted("title") @Nullable String title) {
super(stream, id, Type.FIELD_CONTENT_VALUE, createdAt, creatorUserId, parameters, title);
this.searches = searches;
this.configuration = configuration;
this.field = (String) parameters.get("field");
Expand Down
17 changes: 14 additions & 3 deletions graylog2-server/src/main/java/org/graylog2/alerts/types/FieldValueAlertCondition.java
View file
Open in desktop
Expand Up @@ -57,7 +57,12 @@ public enum ThresholdType {
}

public interface Factory {
FieldValueAlertCondition createAlertCondition(Stream stream, String id, DateTime createdAt, @Assisted("userid") String creatorUserId, Map<String, Object> parameters);
FieldValueAlertCondition createAlertCondition(Stream stream,
@Assisted("id") String id,
DateTime createdAt,
@Assisted("userid") String creatorUserId,
Map<String, Object> parameters,
@Assisted("title") @Nullable String title);
}

private final int time;
Expand All @@ -69,8 +74,14 @@ public interface Factory {
private final Searches searches;

@AssistedInject
public FieldValueAlertCondition(Searches searches, @Assisted Stream stream, @Nullable @Assisted String id, @Assisted DateTime createdAt, @Assisted("userid") String creatorUserId, @Assisted Map<String, Object> parameters) {
super(stream, id, Type.FIELD_VALUE, createdAt, creatorUserId, parameters);
public FieldValueAlertCondition(Searches searches,
@Assisted Stream stream,
@Nullable @Assisted("id") String id,
@Assisted DateTime createdAt,
@Assisted("userid") String creatorUserId,
@Assisted Map<String, Object> parameters,
@Assisted("title") @Nullable String title) {
super(stream, id, Type.FIELD_VALUE, createdAt, creatorUserId, parameters, title);
this.searches = searches;

this.decimalFormat = new DecimalFormat("#.###", DecimalFormatSymbols.getInstance(Locale.ENGLISH));
Expand Down
17 changes: 14 additions & 3 deletions graylog2-server/src/main/java/org/graylog2/alerts/types/MessageCountAlertCondition.java
View file
Open in desktop
Expand Up @@ -49,7 +49,12 @@ public enum ThresholdType {
}

public interface Factory {
MessageCountAlertCondition createAlertCondition(Stream stream, String id, DateTime createdAt, @Assisted("userid") String creatorUserId, Map<String, Object> parameters);
MessageCountAlertCondition createAlertCondition(Stream stream,
@Assisted("id") String id,
DateTime createdAt,
@Assisted("userid") String creatorUserId,
Map<String, Object> parameters,
@Assisted("title") @Nullable String title);
}

private final int time;
Expand All @@ -58,8 +63,14 @@ public interface Factory {
private final Searches searches;

@AssistedInject
public MessageCountAlertCondition(Searches searches, @Assisted Stream stream, @Nullable @Assisted String id, @Assisted DateTime createdAt, @Assisted("userid") String creatorUserId, @Assisted Map<String, Object> parameters) {
super(stream, id, Type.MESSAGE_COUNT, createdAt, creatorUserId, parameters);
public MessageCountAlertCondition(Searches searches,
@Assisted Stream stream,
@Nullable @Assisted("id") String id,
@Assisted DateTime createdAt,
@Assisted("userid") String creatorUserId,
@Assisted Map<String, Object> parameters,
@Nullable @Assisted("title") String title) {
super(stream, id, Type.MESSAGE_COUNT, createdAt, creatorUserId, parameters, title);

this.searches = searches;
this.time = getNumber(parameters.get("time")).orElse(0).intValue();
Expand Down
7 changes: 7 additions & 0 deletions graylog2-server/src/main/java/org/graylog2/plugin/alarms/AlertCondition.java
View file
Open in desktop
Expand Up @@ -16,6 +16,7 @@
*/
package org.graylog2.plugin.alarms;

import com.fasterxml.jackson.annotation.JsonIgnore;
import org.graylog2.plugin.MessageSummary;
import org.graylog2.plugin.streams.Stream;
import org.joda.time.DateTime;
Expand All @@ -24,6 +25,7 @@
import java.util.Map;

public interface AlertCondition {
@JsonIgnore
String getDescription();

String getId();
Expand All @@ -36,12 +38,17 @@ public interface AlertCondition {

Map<String, Object> getParameters();

@JsonIgnore
Integer getBacklog();

@JsonIgnore
int getGrace();

@JsonIgnore
String getTypeString();

String getTitle();

interface CheckResult {
boolean isTriggered();
String getResultDescription();
Expand Down
2 changes: 1 addition & 1 deletion graylog2-server/src/main/java/org/graylog2/plugin/database/EmbeddedPersistable.java
View file
Open in desktop
Expand Up @@ -23,6 +23,6 @@
*/
public interface EmbeddedPersistable {

public Map<String, Object> getPersistedFields();
Map<String, Object> getPersistedFields();

}
2 changes: 0 additions & 2 deletions graylog2-server/src/main/java/org/graylog2/plugin/streams/Stream.java
View file
Open in desktop
Expand Up @@ -50,8 +50,6 @@ public static MatchingType valueOfOrDefault(String name) {

String getContentPack();

Collection<AlertCondition> getAlertConditions();

void setTitle(String title);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would break existing plugins that are working with Graylog 2.0.x and which are using this method. Any reason to remove it and not simply mark it as deprecated (to remove it in Graylog 3.x)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method, as it was before, was plain broken. It worked only because of type erasure, as it was actually returning a Collection<BasicDBObject>. It was only used implicitly by Jackson when serializing a Stream instance as JSON, so it was also using a different representation that any other place that was serializing an AlertCondition instance and it was only identical by accident.

It was also a duplicate of StreamService#getAlertConditions which goes the hard path of instantiating an actual AlertCondition. Unfortunately we cannot just move that logic to the Stream implementation class (or pass the list of correct alert conditions to its constructor) as the AlertCondition instance itself references the Stream which holds it.

It's not the cleanroom technique to just remove it for a minor version, but it's actually balancing between api breakage and offering a method that is just plain broken.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Works for me. At least it'll be obvious for plugin authors and users when stuff breaks.

But please add this to the UPGRADING.rst file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check!


void setDescription(String description);
Expand Down