Skip to content

Commit

Permalink
[Backport 2.8] [PLAT-2128] Fix alert message field to print the whole…
Browse files Browse the repository at this point in the history
… message + alert channel error message fix

Summary:
Alert message field was limited in size and long messages got truncated. Fixed that by making it fix to content.
Also, fixed message for missing SMTP config.
One more bug found during testing was that platform type alert configs were not correctly updating underlying definitions.
Also fixed tests in Idea - kamon was using old version of okhttp and added dependency tree plugin to the project

Test Plan:
Configure wrong email alert channel and assing to alert config.
Raise this alert and see the correct error message.
Modify alert channel alert config and make sure alert rule file is updated.
Check kamon metrics are available.

Screenshot of long alert message:
{F20324}

Reviewers: spotachev

Reviewed By: spotachev

Subscribers: jenkins-bot, yugaware

Differential Revision: https://phabricator.dev.yugabyte.com/D13736
  • Loading branch information
anmalysh-yb committed Nov 1, 2021
1 parent 689866c commit 64ec460
Show file tree
Hide file tree
Showing 13 changed files with 55 additions and 33 deletions.
1 change: 1 addition & 0 deletions managed/build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ libraryDependencies ++= Seq(
"commons-codec" % "commons-codec" % "1.15",
"com.google.cloud" % "google-cloud-storage" % "1.115.0",
"org.projectlombok" % "lombok" % "1.18.20",
"com.squareup.okhttp3" % "okhttp" % "4.9.1",
"com.squareup.okhttp3" % "mockwebserver" % "4.9.1" % Test,
"io.kamon" %% "kamon-bundle" % "2.2.2",
"io.kamon" %% "kamon-prometheus" % "2.2.2",
Expand Down
2 changes: 2 additions & 0 deletions managed/project/plugins.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,5 @@ addSbtPlugin("com.swoval" % "sbt-jvm-format" % "0.3.1")
//addSbtPlugin("com.hootsuite" %% "sbt-swagger" % "1.0.0")

addSbtPlugin("io.kamon" % "sbt-kanela-runner-play-2.6" % "2.0.9")

addSbtPlugin("net.virtual-void" % "sbt-dependency-graph" % "0.10.0-RC1")
28 changes: 16 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 @@ -284,14 +284,11 @@ private SendNotificationResult sendNotification(
try {
alertChannelService.validate(channel);
} catch (PlatformServiceException e) {

if (report.failuresByChannel(channel.getUuid()) == 0) {
log.warn("Channel {} skipped: {}", channel.getUuid(), e.getMessage(), e);
log.warn(String.format("Channel %s skipped: %s", channel.getUuid(), e.getMessage()), e);
}
report.failChannel(channel.getUuid());
setChannelStatusMetric(
PlatformMetrics.ALERT_MANAGER_STATUS,
channel,
"Misconfigured alert channel: " + e.getMessage());
handleChannelSendError(channel, report, "Misconfigured alert channel: " + e.getMessage());
continue;
}

Expand All @@ -301,15 +298,16 @@ private SendNotificationResult sendNotification(
handler.sendNotification(customer, tempAlert, channel);
atLeastOneSucceeded = true;
setOkChannelStatusMetric(PlatformMetrics.ALERT_MANAGER_CHANNEL_STATUS, channel);
} catch (PlatformServiceException e) {
if (report.failuresByChannel(channel.getUuid()) == 0) {
log.error(e.getMessage(), e);
}
handleChannelSendError(channel, report, e.getMessage());
} catch (Exception e) {
if (report.failuresByChannel(channel.getUuid()) == 0) {
log.error(e.getMessage());
log.error(e.getMessage(), e);
}
report.failChannel(channel.getUuid());
setChannelStatusMetric(
PlatformMetrics.ALERT_MANAGER_CHANNEL_STATUS,
channel,
"Error sending notification: " + e.getMessage());
handleChannelSendError(channel, report, "Error sending notification: " + e.getMessage());
}
}

Expand All @@ -318,6 +316,12 @@ private SendNotificationResult sendNotification(
: SendNotificationResult.FAILED_TO_RESCHEDULE;
}

private void handleChannelSendError(
AlertChannel channel, AlertNotificationReport report, String alertMessage) {
report.failChannel(channel.getUuid());
setChannelStatusMetric(PlatformMetrics.ALERT_MANAGER_CHANNEL_STATUS, channel, alertMessage);
}

@VisibleForTesting
void setOkChannelStatusMetric(PlatformMetrics metric, AlertChannel channel) {
setChannelStatusMetric(metric, channel, StringUtils.EMPTY);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,7 @@ private void manageDefinitions(
// If it exists - we need to update existing one just in case group is updated.
definition = currentDefinitions.get(0);
}
definition.setConfigWritten(false);
definition.setQuery(configuration.getTemplate().buildTemplate(customer));
if (!configuration.getTemplate().isSkipTargetLabels()) {
definition.setLabels(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,16 @@ public void sendNotification(Customer customer, Alert alert, AlertChannel channe

if (CollectionUtils.isEmpty(recipients)) {
throw new PlatformNotificationException(
String.format("Error sending email for alert %s: No recipients found.", alert.getName()));
String.format(
"Error sending email for alert %s: No recipients found for channel %s",
alert.getName(), channel.getName()));
}

if (smtpData == null) {
throw new PlatformNotificationException(
String.format(
"Error sending email for alert %s: Invalid SMTP settings found.", alert.getName()));
"Error sending email for alert %s: SMTP settings not found for channel %s.",
alert.getName(), channel.getName()));
}

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public void delete(MetricFilter filter) {
private void deleteInternal(Collection<Metric> toDelete) {
MetricFilter deleteFilter =
MetricFilter.builder()
.uuids(toDelete.stream().map(Metric::getUuid).collect(Collectors.toSet()))
.keys(toDelete.stream().map(MetricKey::from).collect(Collectors.toSet()))
.build();
int deleted = createQueryByFilter(deleteFilter).delete();
log.trace("{} metrics deleted", deleted);
Expand Down
7 changes: 6 additions & 1 deletion managed/src/main/java/com/yugabyte/yw/models/Alert.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import io.ebean.ExpressionList;
import io.ebean.Finder;
import io.ebean.Model;
import io.ebean.PersistenceContextScope;
import io.ebean.annotation.Formula;
import io.swagger.annotations.ApiModel;
import io.swagger.annotations.ApiModelProperty;
Expand Down Expand Up @@ -248,7 +249,11 @@ public List<AlertLabel> getLabels() {
}

public static ExpressionList<Alert> createQueryByFilter(AlertFilter filter) {
ExpressionList<Alert> query = find.query().fetch("labels").where();
ExpressionList<Alert> query =
find.query()
.setPersistenceContextScope(PersistenceContextScope.QUERY)
.fetch("labels")
.where();
appendInClause(query, "uuid", filter.getUuids());
appendNotInClause(query, "uuid", filter.getExcludeUuids());
if (filter.getCustomerUuid() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import io.ebean.ExpressionList;
import io.ebean.Finder;
import io.ebean.Model;
import io.ebean.PersistenceContextScope;
import io.ebean.annotation.DbJson;
import io.swagger.annotations.ApiModel;
import io.swagger.annotations.ApiModelProperty;
Expand Down Expand Up @@ -178,7 +179,8 @@ public int getPriority() {

public static ExpressionList<AlertConfiguration> createQueryByFilter(
AlertConfigurationFilter filter) {
ExpressionList<AlertConfiguration> query = find.query().where();
ExpressionList<AlertConfiguration> query =
find.query().setPersistenceContextScope(PersistenceContextScope.QUERY).where();
appendInClause(query, "uuid", filter.getUuids());
if (filter.getCustomerUuid() != null) {
query.eq("customerUUID", filter.getCustomerUuid());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import io.ebean.ExpressionList;
import io.ebean.Finder;
import io.ebean.Model;
import io.ebean.PersistenceContextScope;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
Expand Down Expand Up @@ -78,7 +79,11 @@ public class AlertDefinition extends Model {
new Finder<UUID, AlertDefinition>(AlertDefinition.class) {};

public static ExpressionList<AlertDefinition> createQueryByFilter(AlertDefinitionFilter filter) {
ExpressionList<AlertDefinition> query = find.query().fetch("labels").where();
ExpressionList<AlertDefinition> query =
find.query()
.setPersistenceContextScope(PersistenceContextScope.QUERY)
.fetch("labels")
.where();
appendInClause(query, "uuid", filter.getUuids());
if (filter.getCustomerUuid() != null) {
query.eq("customerUUID", filter.getCustomerUuid());
Expand Down
8 changes: 6 additions & 2 deletions managed/src/main/java/com/yugabyte/yw/models/Metric.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import io.ebean.Finder;
import io.ebean.Junction;
import io.ebean.Model;
import io.ebean.PersistenceContextScope;
import io.prometheus.client.Collector;
import java.util.Comparator;
import java.util.Date;
Expand Down Expand Up @@ -161,8 +162,11 @@ public List<MetricLabel> getLabels() {
}

public static ExpressionList<Metric> createQueryByFilter(MetricFilter filter) {
ExpressionList<Metric> query = find.query().fetch("labels").where();
appendInClause(query, "uuid", filter.getUuids());
ExpressionList<Metric> query =
find.query()
.setPersistenceContextScope(PersistenceContextScope.QUERY)
.fetch("labels")
.where();
if (filter.getCustomerUuid() != null) {
query.eq("customerUUID", filter.getCustomerUuid());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
@Value
@Builder
public class MetricFilter {
Set<UUID> uuids;
UUID customerUuid;
UUID sourceUuid;
List<PlatformMetrics> metrics;
Expand All @@ -34,21 +33,10 @@ public class MetricFilter {
Boolean expired;

public static class MetricFilterBuilder {
Set<UUID> uuids = new HashSet<>();
List<PlatformMetrics> metrics = new ArrayList<>();
Set<MetricSourceKey> sourceKeys = new HashSet<>();
Set<MetricKey> keys = new HashSet<>();

public MetricFilterBuilder uuids(@NonNull Collection<UUID> uuids) {
this.uuids.addAll(uuids);
return this;
}

public MetricFilterBuilder uuid(@NonNull UUID uuid) {
this.uuids.add(uuid);
return this;
}

public MetricFilterBuilder customerUuid(@NonNull UUID customerUuid) {
this.customerUuid = customerUuid;
return this;
Expand Down
2 changes: 1 addition & 1 deletion managed/ui/src/components/alerts/AlertList/AlertDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export default class AlertDetails extends Component {
className="alert-label noLeftPadding noMarginBottom"
>
<h6 className="alert-label-header">DESCRIPTION</h6>
<div className="alert-label-value">{alertDetails.message}</div>
<div className="alert-label-message">{alertDetails.message}</div>
</Col>
<Col lg={12} className="noLeftPadding">
{getSeverityLabel(alertDetails.severity)}
Expand Down
7 changes: 7 additions & 0 deletions managed/ui/src/components/alerts/AlertList/AlertDetails.scss
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@
overflow-y: auto;
margin-bottom: 5px;
}

.alert-label-message {
display: inline-block;
font-size: 12px;
max-height: 200px;
margin-bottom: 5px;
}
}
.marginTop {
margin-top: 24px;
Expand Down

0 comments on commit 64ec460

Please sign in to comment.