Skip to content

Commit

Permalink
[PLAT-1613] Platform: Alerts: Logs filled with NPE related to "Error …
Browse files Browse the repository at this point in the history
…while sending notification for alert "

Summary:
  - Fixed possible NPE exceptions in logs related to notifications.

Test Plan:
    To check that for a new customer (without alerting configuration)
there is no NullPointerExceptions in the platform logs with the next
stack:

2021-08-30 22:44:55.515 [error] AlertManager.java:135
[application-akka.actor.default-dispatcher-7]
 Error while sending notification for alert
064b7bce-a92a-4465-8d4a-fa157b7e2f71
 java.lang.NullPointerException: null

 at com.yugabyte.yw.common.EmailHelper.getDestinations(EmailHelper.java:197)
 at com.yugabyte.yw.common.AlertManager.sendNotification(AlertManager.java:206)
 at com.yugabyte.yw.common.AlertManager.sendNotificationForState(AlertManager.java:108)
 ...

Reviewers: sb-yb, amalyshev

Reviewed By: amalyshev

Subscribers: yugaware, jenkins-bot

Differential Revision: https://phabricator.dev.yugabyte.com/D12815
  • Loading branch information
SergeyPotachev committed Aug 31, 2021
1 parent e130615 commit 71a8de2
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 27 deletions.
41 changes: 29 additions & 12 deletions managed/src/main/java/com/yugabyte/yw/common/AlertManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,22 @@
import static com.yugabyte.yw.models.helpers.CommonUtils.nowPlusWithoutMillis;

import com.google.common.annotations.VisibleForTesting;
import com.yugabyte.yw.common.alerts.AlertConfigurationService;
import com.yugabyte.yw.common.metrics.MetricLabelsBuilder;
import com.yugabyte.yw.common.alerts.AlertNotificationReport;
import com.yugabyte.yw.common.alerts.AlertChannelEmailParams;
import com.yugabyte.yw.common.alerts.AlertChannelInterface;
import com.yugabyte.yw.common.alerts.AlertChannelManager;
import com.yugabyte.yw.common.alerts.AlertChannelService;
import com.yugabyte.yw.common.alerts.AlertConfigurationService;
import com.yugabyte.yw.common.alerts.AlertDestinationService;
import com.yugabyte.yw.common.alerts.AlertNotificationReport;
import com.yugabyte.yw.common.alerts.AlertService;
import com.yugabyte.yw.common.alerts.AlertUtils;
import com.yugabyte.yw.common.metrics.MetricService;
import com.yugabyte.yw.common.alerts.PlatformValidationException;
import com.yugabyte.yw.common.metrics.MetricLabelsBuilder;
import com.yugabyte.yw.common.metrics.MetricService;
import com.yugabyte.yw.models.Alert;
import com.yugabyte.yw.models.Alert.State;
import com.yugabyte.yw.models.AlertConfiguration;
import com.yugabyte.yw.models.AlertChannel;
import com.yugabyte.yw.models.AlertConfiguration;
import com.yugabyte.yw.models.AlertDestination;
import com.yugabyte.yw.models.Customer;
import com.yugabyte.yw.models.Metric;
Expand Down Expand Up @@ -62,6 +62,12 @@ public class AlertManager {
private final AlertService alertService;
private final MetricService metricService;

private enum SendNotificationResult {
FAILED_TO_RESCHEDULE,
SUCCEEDED,
FAILED_NO_RESCHEDULE
}

@Inject
public AlertManager(
EmailHelper emailHelper,
Expand Down Expand Up @@ -109,12 +115,17 @@ private Optional<AlertDestination> getDestinationByAlert(Alert alert) {

@VisibleForTesting
boolean sendNotificationForState(Alert alert, State state, AlertNotificationReport report) {
boolean result = false;
SendNotificationResult result = SendNotificationResult.FAILED_TO_RESCHEDULE;
try {
result = sendNotification(alert, state, report);
if (result == SendNotificationResult.FAILED_NO_RESCHEDULE) {
// Failed, no reschedule is required.
report.failAttempt();
return false;
}

alert.setNotificationAttemptTime(new Date());
if (!result) {
if (result == SendNotificationResult.FAILED_TO_RESCHEDULE) {
alert.setNotificationsFailed(alert.getNotificationsFailed() + 1);
// For now using fixed delay before the notification repeat. Later the behavior
// can be adjusted using an amount of failed attempts (using progressive value).
Expand All @@ -140,7 +151,7 @@ boolean sendNotificationForState(Alert alert, State state, AlertNotificationRepo
report.failAttempt();
log.error("Error while sending notification for alert {}", alert.getUuid(), e);
}
return result;
return result == SendNotificationResult.SUCCEEDED;
}

public void sendNotifications() {
Expand Down Expand Up @@ -183,7 +194,7 @@ public void sendNotifications() {
}
}

private boolean sendNotification(
private SendNotificationResult sendNotification(
Alert alert, State stateToNotify, AlertNotificationReport report) {
Customer customer = Customer.get(alert.getCustomerUUID());

Expand All @@ -204,7 +215,7 @@ private boolean sendNotification(
metricService.setStatusMetric(
MetricService.buildMetricTemplate(PlatformMetrics.ALERT_MANAGER_STATUS, customer),
"Unable to notify about alert(s), there is no default destination specified.");
return false;
return SendNotificationResult.FAILED_TO_RESCHEDULE;
}

List<AlertChannel> defaultChannels = defaultDestination.getChannelsList();
Expand All @@ -217,7 +228,7 @@ private boolean sendNotification(
MetricService.buildMetricTemplate(PlatformMetrics.ALERT_MANAGER_STATUS, customer),
"Unable to notify about alert(s) using default destination, "
+ "there are no recipients configured in the customer's profile.");
return false;
return SendNotificationResult.FAILED_TO_RESCHEDULE;
}

log.debug(
Expand All @@ -232,6 +243,10 @@ private boolean sendNotification(
// Not going to save the alert, only to use with another state for the
// notification.
Alert tempAlert = alertService.get(alert.getUuid());
if (tempAlert == null) {
// The alert was not found. Most probably it is removed during the processing.
return SendNotificationResult.FAILED_NO_RESCHEDULE;
}
tempAlert.setState(stateToNotify);

for (AlertChannel channel : channels) {
Expand Down Expand Up @@ -267,7 +282,9 @@ private boolean sendNotification(
}
}

return atLeastOneSucceeded;
return atLeastOneSucceeded
? SendNotificationResult.SUCCEEDED
: SendNotificationResult.FAILED_TO_RESCHEDULE;
}

@VisibleForTesting
Expand Down
16 changes: 9 additions & 7 deletions managed/src/main/java/com/yugabyte/yw/common/EmailHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -194,14 +194,16 @@ public List<String> getDestinations(UUID customerUUID) {
List<String> destinations = new ArrayList<>();
String ybEmail = getYbEmail(customer);
CustomerConfig config = CustomerConfig.getAlertConfig(customer.uuid);
CustomerRegisterFormData.AlertingData alertingData =
Json.fromJson(config.data, CustomerRegisterFormData.AlertingData.class);
if (alertingData.sendAlertsToYb && !StringUtils.isEmpty(ybEmail)) {
destinations.add(ybEmail);
}
if (config != null) {
CustomerRegisterFormData.AlertingData alertingData =
Json.fromJson(config.data, CustomerRegisterFormData.AlertingData.class);
if (alertingData.sendAlertsToYb && !StringUtils.isEmpty(ybEmail)) {
destinations.add(ybEmail);
}

if (!StringUtils.isEmpty(alertingData.alertingEmail)) {
destinations.add(alertingData.alertingEmail);
if (!StringUtils.isEmpty(alertingData.alertingEmail)) {
destinations.add(alertingData.alertingEmail);
}
}
return destinations;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,12 @@ public void testGetDestinations(String emailTo, boolean sendAlertsToYb, int expe
}
}

@Test
public void testGetDestinations_NoAlertConfiguration_EmptyList() {
List<String> destinations = emailHelper.getDestinations(defaultCustomer.uuid);
assertEquals(0, destinations.size());
}

@Test
public void testGetSmtpData_NoDbConfig() {
SmtpData smtpData = emailHelper.getSmtpData(defaultCustomer.uuid);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,32 +31,32 @@
import com.yugabyte.yw.common.FakeDBApplication;
import com.yugabyte.yw.common.ModelFactory;
import com.yugabyte.yw.common.ValidatingFormFactory;
import com.yugabyte.yw.common.alerts.AlertConfigurationService;
import com.yugabyte.yw.common.alerts.AlertDefinitionService;
import com.yugabyte.yw.common.metrics.MetricLabelsBuilder;
import com.yugabyte.yw.common.alerts.AlertChannelEmailParams;
import com.yugabyte.yw.common.alerts.AlertChannelParams;
import com.yugabyte.yw.common.alerts.AlertChannelService;
import com.yugabyte.yw.common.alerts.AlertChannelSlackParams;
import com.yugabyte.yw.common.alerts.AlertConfigurationService;
import com.yugabyte.yw.common.alerts.AlertDefinitionService;
import com.yugabyte.yw.common.alerts.AlertDestinationService;
import com.yugabyte.yw.common.alerts.AlertService;
import com.yugabyte.yw.common.alerts.AlertUtils;
import com.yugabyte.yw.common.metrics.MetricService;
import com.yugabyte.yw.common.alerts.SmtpData;
import com.yugabyte.yw.common.config.impl.SettableRuntimeConfigFactory;
import com.yugabyte.yw.common.metrics.MetricLabelsBuilder;
import com.yugabyte.yw.common.metrics.MetricService;
import com.yugabyte.yw.forms.filters.AlertApiFilter;
import com.yugabyte.yw.forms.filters.AlertConfigurationApiFilter;
import com.yugabyte.yw.forms.filters.AlertTemplateApiFilter;
import com.yugabyte.yw.forms.paging.AlertConfigurationPagedApiQuery;
import com.yugabyte.yw.forms.paging.AlertPagedApiQuery;
import com.yugabyte.yw.models.Alert;
import com.yugabyte.yw.models.AlertChannel;
import com.yugabyte.yw.models.AlertChannel.ChannelType;
import com.yugabyte.yw.models.AlertConfiguration;
import com.yugabyte.yw.models.AlertConfigurationThreshold;
import com.yugabyte.yw.models.AlertDefinition;
import com.yugabyte.yw.models.AlertConfiguration.SortBy;
import com.yugabyte.yw.models.AlertConfigurationTarget;
import com.yugabyte.yw.models.AlertChannel;
import com.yugabyte.yw.models.AlertChannel.ChannelType;
import com.yugabyte.yw.models.AlertConfigurationThreshold;
import com.yugabyte.yw.models.AlertDefinition;
import com.yugabyte.yw.models.AlertDestination;
import com.yugabyte.yw.models.Customer;
import com.yugabyte.yw.models.Metric;
Expand Down Expand Up @@ -814,6 +814,9 @@ public void testAcknowledgeAlert() {

JsonNode alertsJson = Json.parse(contentAsString(result));
Alert acknowledged = Json.fromJson(alertsJson, Alert.class);
if (!alertsJson.has("nextNotificationTime")) {
acknowledged.setNextNotificationTime(null);
}

initial.setState(Alert.State.ACKNOWLEDGED);
initial.setAcknowledgedTime(acknowledged.getAcknowledgedTime());
Expand Down

0 comments on commit 71a8de2

Please sign in to comment.