Skip to content

Commit

Permalink
Suppress noisy notification in cloud (#20968)
Browse files Browse the repository at this point in the history 
* suppress noisy notification in cloud

* CL

* adjust test
  • Loading branch information
@patrickmann @janheise
patrickmann authored and janheise committed Nov 27, 2024
1 parent c543d5f commit a0837e0
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 19 deletions.
5 changes: 5 additions & 0 deletions changelog/unreleased/pr-20968.toml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type = "c"
message = "Suppress noisy aggregation search error when it is likely due to cloud maintenance."

issues = ["Graylog2/graylog-plugin-enterprise#8919"]
pulls = ["20968"]
48 changes: 35 additions & 13 deletions ...server/src/main/java/org/graylog/events/processor/aggregation/PivotAggregationSearch.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.common.util.concurrent.Uninterruptibles;
import com.google.inject.assistedinject.Assisted;
import jakarta.inject.Inject;
import jakarta.inject.Named;
import org.graylog.events.configuration.EventsConfigurationProvider;
import org.graylog.events.processor.EventDefinition;
import org.graylog.events.processor.EventProcessorException;
Expand Down Expand Up @@ -102,6 +103,7 @@ public class PivotAggregationSearch implements AggregationSearch {
private final PermittedStreams permittedStreams;
private final NotificationService notificationService;
private final QueryStringDecorators queryStringDecorators;
private final boolean isCloud;

@Inject
public PivotAggregationSearch(@Assisted AggregationEventProcessorConfig config,
Expand All @@ -115,7 +117,8 @@ public PivotAggregationSearch(@Assisted AggregationEventProcessorConfig config,
MoreSearch moreSearch,
PermittedStreams permittedStreams,
NotificationService notificationService,
QueryStringDecorators queryStringDecorators) {
QueryStringDecorators queryStringDecorators,
@Named("is_cloud") boolean isCloud) {
this.config = config;
this.parameters = parameters;
this.searchOwner = searchOwner;
Expand All @@ -128,6 +131,7 @@ public PivotAggregationSearch(@Assisted AggregationEventProcessorConfig config,
this.permittedStreams = permittedStreams;
this.notificationService = notificationService;
this.queryStringDecorators = queryStringDecorators;
this.isCloud = isCloud;
}

private String metricName(SeriesSpec series) {
Expand Down Expand Up @@ -170,17 +174,19 @@ public AggregationResult doSearch() throws EventProcessorException {
return AggregationResult.empty();
}

final String description = f("Event definition %s (%s) failed: %s",
eventDefinition.title(), eventDefinition.id(),
errors.stream().map(SearchError::description).collect(Collectors.joining("\n")));
Notification systemNotification = notificationService.buildNow()
.addType(Notification.Type.SEARCH_ERROR)
.addSeverity(Notification.Severity.NORMAL)
.addTimestamp(DateTime.now(DateTimeZone.UTC))
.addKey(eventDefinition.id())
.addDetail("title", "Aggregation search failed")
.addDetail("description", description);
notificationService.publishIfFirst(systemNotification);
if (!suppressInCloud(errors)) {
final String description = f("Event definition %s (%s) failed: %s",
eventDefinition.title(), eventDefinition.id(),
errors.stream().map(SearchError::description).collect(Collectors.joining("\n")));
Notification systemNotification = notificationService.buildNow()
.addType(Notification.Type.SEARCH_ERROR)
.addSeverity(Notification.Severity.NORMAL)
.addTimestamp(DateTime.now(DateTimeZone.UTC))
.addKey(eventDefinition.id())
.addDetail("title", "Aggregation search failed")
.addDetail("description", description);
notificationService.publishIfFirst(systemNotification);
}

if (errors.size() > 1) {
throw new EventProcessorException("Pivot search failed with multiple errors.", false, eventDefinition);
Expand All @@ -201,6 +207,22 @@ public AggregationResult doSearch() throws EventProcessorException {
.build();
}

// Suppress notification when error likely due to Cloud maintenance work
private boolean suppressInCloud(Set<SearchError> errors) {
if (!isCloud) {
return false;
}
if (errors.stream()
.map(SearchError::description)
.filter(s -> s.contains("node_not_connected"))
.findFirst()
.isEmpty()) {
return false;
}
LOG.debug("Suppressed node_not_connected notification in Cloud");
return true;
}

private ImmutableSet<String> extractSourceStreams(PivotResult pivotResult) {
return pivotResult.rows().stream()
// "non-leaf" values can show up when the "rollup" feature is enabled in the pivot search type
Expand Down Expand Up @@ -349,7 +371,7 @@ ImmutableList<AggregationKeyResult> extractValues(PivotResult pivotResult) throw
values.add(seriesValue);
} else {
// Should not happen
throw new IllegalStateException("Got unexpected non-number value for " + series.toString() + " " + row.toString() + " " + value.toString());
throw new IllegalStateException("Got unexpected non-number value for " + series + " " + row + " " + value);
}
}
}
Expand Down
18 changes: 12 additions & 6 deletions ...er/src/test/java/org/graylog/events/processor/aggregation/PivotAggregationSearchTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ public void testExtractValuesWithGroupBy() throws Exception {
moreSearch,
permittedStreams,
notificationService,
new QueryStringDecorators(Optional.empty())
new QueryStringDecorators(Optional.empty()),
false
);

final String toString = timerange.getTo().toString();
Expand Down Expand Up @@ -208,7 +209,8 @@ public void testExtractValuesWithoutGroupBy() throws Exception {
moreSearch,
permittedStreams,
notificationService,
new QueryStringDecorators(Optional.empty())
new QueryStringDecorators(Optional.empty()),
false
);

final PivotResult pivotResult = PivotResult.builder()
Expand Down Expand Up @@ -287,7 +289,8 @@ public void testExtractAdditionalSearchTypes() throws Exception {
moreSearch,
permittedStreams,
notificationService,
new QueryStringDecorators(Optional.empty())
new QueryStringDecorators(Optional.empty()),
false
);

final PivotResult pivotResult = PivotResult.builder()
Expand Down Expand Up @@ -353,7 +356,8 @@ public void testExtractValuesWithNullValues() throws Exception {
moreSearch,
permittedStreams,
notificationService,
new QueryStringDecorators(Optional.empty())
new QueryStringDecorators(Optional.empty()),
false
);

final PivotResult pivotResult = PivotResult.builder()
Expand Down Expand Up @@ -497,7 +501,8 @@ public void testQueryParameterSubstitution() {
} else {
throw new IllegalArgumentException("Unexpected query decoration request!");
}
}))
})),
false
);
final Query query = pivotAggregationSearch.getAggregationQuery(parameters, WINDOW_LENGTH, WINDOW_LENGTH);
Assertions.assertThat(query.query().queryString()).isEqualTo("source:example.org");
Expand Down Expand Up @@ -542,7 +547,8 @@ public void testAdditionalSearchTypes() {
moreSearch,
new PermittedStreams(() -> Stream.of("00001"), (categories) -> Stream.of()),
notificationService,
new QueryStringDecorators(Optional.empty())
new QueryStringDecorators(Optional.empty()),
false
);
final Query query = pivotAggregationSearch.getAggregationQuery(parameters, WINDOW_LENGTH, WINDOW_LENGTH);
Assertions.assertThatCollection(query.searchTypes()).contains(
Expand Down

0 comments on commit a0837e0

Please sign in to comment.