diff --git a/titus-api/src/main/java/com/netflix/titus/api/appscale/model/AlarmConfiguration.java b/titus-api/src/main/java/com/netflix/titus/api/appscale/model/AlarmConfiguration.java index 6f825dc8fb..3e5702786c 100644 --- a/titus-api/src/main/java/com/netflix/titus/api/appscale/model/AlarmConfiguration.java +++ b/titus-api/src/main/java/com/netflix/titus/api/appscale/model/AlarmConfiguration.java @@ -16,6 +16,8 @@ package com.netflix.titus.api.appscale.model; +import java.util.Collections; +import java.util.List; import java.util.Optional; import com.fasterxml.jackson.annotation.JsonProperty; @@ -32,10 +34,11 @@ public class AlarmConfiguration { private final String metricNamespace; private final String metricName; private final Statistic statistic; + private final List dimensions; public AlarmConfiguration(String name, String region, Optional actionsEnabled, ComparisonOperator comparisonOperator, int evaluationPeriods, int periodSec, double threshold, String metricNamespace, String metricName, - Statistic statistic) { + Statistic statistic, List dimensions) { this.name = name; this.region = region; this.actionsEnabled = actionsEnabled; @@ -47,6 +50,7 @@ public AlarmConfiguration(String name, String region, Optional actionsE this.metricNamespace = metricNamespace; this.metricName = metricName; this.statistic = statistic; + this.dimensions = dimensions; } @JsonProperty @@ -99,6 +103,11 @@ public Statistic getStatistic() { return statistic; } + @JsonProperty + public List getDimensions() { + return dimensions; + } + @Override public String toString() { return "AlarmConfiguration{" + @@ -111,7 +120,8 @@ public String toString() { ", threshold=" + threshold + ", metricNamespace='" + metricNamespace + '\'' + ", metricName='" + metricName + '\'' + - ", statistic=" + statistic + + ", statistic=" + statistic + '\n' + + ", dimensions=" + dimensions + '}'; } @@ -131,6 +141,7 @@ public static class Builder { private String metricNamespace; private String metricName; private Statistic statistic; + private List dimensions = Collections.emptyList(); private Builder() { @@ -191,9 +202,14 @@ public Builder withStatistic(Statistic staticstic) { return this; } + public Builder withDimensions(List dimensions) { + this.dimensions = dimensions; + return this; + } + public AlarmConfiguration build() { return new AlarmConfiguration(name, region, actionsEnabled, comparisonOperator, - evaluationPeriods, periodSec, threshold, metricNamespace, metricName, statistic); + evaluationPeriods, periodSec, threshold, metricNamespace, metricName, statistic, dimensions); } } diff --git a/titus-api/src/main/java/com/netflix/titus/api/appscale/store/mixin/AlarmConfigurationMixIn.java b/titus-api/src/main/java/com/netflix/titus/api/appscale/store/mixin/AlarmConfigurationMixIn.java index 9a5352f678..4336d3efd5 100644 --- a/titus-api/src/main/java/com/netflix/titus/api/appscale/store/mixin/AlarmConfigurationMixIn.java +++ b/titus-api/src/main/java/com/netflix/titus/api/appscale/store/mixin/AlarmConfigurationMixIn.java @@ -17,12 +17,14 @@ package com.netflix.titus.api.appscale.store.mixin; +import java.util.List; import java.util.Optional; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; import com.netflix.titus.api.appscale.model.ComparisonOperator; +import com.netflix.titus.api.appscale.model.MetricDimension; import com.netflix.titus.api.appscale.model.Statistic; @JsonIgnoreProperties(ignoreUnknown = true) @@ -38,7 +40,8 @@ public abstract class AlarmConfigurationMixIn { @JsonProperty("threshold") double threshold, @JsonProperty("metricNamespace") String metricNamespace, @JsonProperty("metricName") String metricName, - @JsonProperty("statistic") Statistic statistic) { + @JsonProperty("statistic") Statistic statistic, + @JsonProperty("dimensions") List dimensions ) { } } diff --git a/titus-api/src/test/java/com/netflix/titus/api/appscale/model/PolicyConfigurationTest.java b/titus-api/src/test/java/com/netflix/titus/api/appscale/model/PolicyConfigurationTest.java new file mode 100644 index 0000000000..3eb970180a --- /dev/null +++ b/titus-api/src/test/java/com/netflix/titus/api/appscale/model/PolicyConfigurationTest.java @@ -0,0 +1,95 @@ +package com.netflix.titus.api.appscale.model; + +import com.netflix.titus.api.json.ObjectMappers; +import org.junit.Test; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +public class PolicyConfigurationTest { + @Test + public void deserializePolicyConfiguration() { + String policyConfigStrNoMetricDimensions = "{\n" + + " \"name\": null,\n" + + " \"policyType\": \"StepScaling\",\n" + + " \"stepScalingPolicyConfiguration\": {\n" + + " \"metricAggregationType\": \"Maximum\",\n" + + " \"adjustmentType\": \"ChangeInCapacity\",\n" + + " \"minAdjustmentMagnitude\": null,\n" + + " \"steps\": [\n" + + " {\n" + + " \"scalingAdjustment\": 1,\n" + + " \"metricIntervalLowerBound\": 0.0,\n" + + " \"metricIntervalUpperBound\": null\n" + + " }\n" + + " ],\n" + + " \"coolDownSec\": 60\n" + + " },\n" + + " \"alarmConfiguration\": {\n" + + " \"name\": null,\n" + + " \"region\": null,\n" + + " \"actionsEnabled\": true,\n" + + " \"comparisonOperator\": \"GreaterThanThreshold\",\n" + + " \"evaluationPeriods\": 1,\n" + + " \"threshold\": 2.0,\n" + + " \"metricNamespace\": \"titus/integrationTest\",\n" + + " \"metricName\": \"RequestCount\",\n" + + " \"statistic\": \"Sum\",\n" + + " \"periodSec\": 60\n" + + " },\n" + + " \"targetTrackingPolicy\": null\n" + + "}"; + + PolicyConfiguration policyConfigNoMetricDimension = ObjectMappers.readValue(ObjectMappers.appScalePolicyMapper(), + policyConfigStrNoMetricDimensions, PolicyConfiguration.class); + assertThat(policyConfigNoMetricDimension).isNotNull(); + assertThat(policyConfigNoMetricDimension.getAlarmConfiguration().getDimensions()).isNull(); + + String policyConfigStrWithMetricDimensions = "{\n" + + " \"name\": null,\n" + + " \"policyType\": \"StepScaling\",\n" + + " \"stepScalingPolicyConfiguration\": {\n" + + " \"metricAggregationType\": \"Maximum\",\n" + + " \"adjustmentType\": \"ChangeInCapacity\",\n" + + " \"minAdjustmentMagnitude\": null,\n" + + " \"steps\": [\n" + + " {\n" + + " \"scalingAdjustment\": 1,\n" + + " \"metricIntervalLowerBound\": 0.0,\n" + + " \"metricIntervalUpperBound\": null\n" + + " }\n" + + " ],\n" + + " \"coolDownSec\": 60\n" + + " },\n" + + " \"alarmConfiguration\": {\n" + + " \"name\": null,\n" + + " \"region\": null,\n" + + " \"actionsEnabled\": true,\n" + + " \"comparisonOperator\": \"GreaterThanThreshold\",\n" + + " \"evaluationPeriods\": 1,\n" + + " \"threshold\": 2.0,\n" + + " \"metricNamespace\": \"titus/integrationTest\",\n" + + " \"metricName\": \"RequestCount\",\n" + + " \"statistic\": \"Sum\",\n" + + " \"periodSec\": 60,\n" + + " \"unknownField\": 100,\n" + + " \"dimensions\": [\n" + + " {\n" + + " \"Name\": \"foo\",\n" + + " \"Value\": \"bar\"\n" + + " },\n" + + " {\n" + + " \"Name\": \"tier\",\n" + + " \"Value\": \"1\"\n" + + " }\n" + + " ]\n" + + " },\n" + + " \"targetTrackingPolicy\": null\n" + + "}"; + + PolicyConfiguration policyConfigWithMetricDimensions = ObjectMappers.readValue(ObjectMappers.appScalePolicyMapper(), + policyConfigStrWithMetricDimensions, PolicyConfiguration.class); + assertThat(policyConfigWithMetricDimensions).isNotNull(); + assertThat(policyConfigWithMetricDimensions.getAlarmConfiguration().getDimensions()).isNotNull(); + assertThat(policyConfigWithMetricDimensions.getAlarmConfiguration().getDimensions().size()).isEqualTo(2); + } +} diff --git a/titus-ext/aws/src/main/java/com/netflix/titus/ext/aws/cloudwatch/CloudWatchClient.java b/titus-ext/aws/src/main/java/com/netflix/titus/ext/aws/cloudwatch/CloudWatchClient.java index d2be8dfa1b..8f7f12cfd3 100644 --- a/titus-ext/aws/src/main/java/com/netflix/titus/ext/aws/cloudwatch/CloudWatchClient.java +++ b/titus-ext/aws/src/main/java/com/netflix/titus/ext/aws/cloudwatch/CloudWatchClient.java @@ -16,6 +16,7 @@ package com.netflix.titus.ext.aws.cloudwatch; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; import javax.inject.Inject; @@ -31,9 +32,11 @@ import com.amazonaws.services.cloudwatch.model.PutMetricAlarmRequest; import com.amazonaws.services.cloudwatch.model.PutMetricAlarmResult; import com.amazonaws.services.cloudwatch.model.ResourceNotFoundException; +import com.google.common.annotations.VisibleForTesting; import com.netflix.spectator.api.Counter; import com.netflix.spectator.api.Registry; import com.netflix.titus.api.appscale.model.AlarmConfiguration; +import com.netflix.titus.api.appscale.model.MetricDimension; import com.netflix.titus.api.appscale.service.AutoScalePolicyException; import com.netflix.titus.api.connector.cloud.CloudAlarmClient; import com.netflix.titus.ext.aws.AwsConfiguration; @@ -84,10 +87,8 @@ public Observable createOrUpdateAlarm(String policyRefId, AlarmConfiguration alarmConfiguration, String autoScalingGroup, List actions) { - Dimension dimension = new Dimension(); - dimension.setName(AUTO_SCALING_GROUP_NAME); - dimension.setValue(autoScalingGroup); + List metricDimensions = buildMetricDimensions(alarmConfiguration, autoScalingGroup); String cloudWatchName = buildCloudWatchName(policyRefId, jobId); PutMetricAlarmRequest putMetricAlarmRequest = new PutMetricAlarmRequest(); @@ -96,7 +97,7 @@ public Observable createOrUpdateAlarm(String policyRefId, } putMetricAlarmRequest.setAlarmActions(actions); putMetricAlarmRequest.setAlarmName(cloudWatchName); - putMetricAlarmRequest.setDimensions(Arrays.asList(dimension)); + putMetricAlarmRequest.setDimensions(metricDimensions); putMetricAlarmRequest.setNamespace(alarmConfiguration.getMetricNamespace()); putMetricAlarmRequest.setComparisonOperator(alarmConfiguration.getComparisonOperator().name()); putMetricAlarmRequest.setStatistic(alarmConfiguration.getStatistic().name()); @@ -159,5 +160,22 @@ private String buildCloudWatchName(String policyRefId, String jobId) { return String.format("%s/%s", jobId, policyRefId); } - + @VisibleForTesting + static List buildMetricDimensions(AlarmConfiguration alarmConfiguration, String autoScalingGroup) { + List metricDimensions = new ArrayList<>(1); + if (alarmConfiguration.getDimensions() != null && ! alarmConfiguration.getDimensions().isEmpty()) { + for (MetricDimension customMetricDimension : alarmConfiguration.getDimensions()) { + Dimension dimension = new Dimension(); + dimension.setName(customMetricDimension.getName()); + dimension.setValue(customMetricDimension.getValue()); + metricDimensions.add(dimension); + } + } else { + Dimension dimension = new Dimension(); + dimension.setName(AUTO_SCALING_GROUP_NAME); + dimension.setValue(autoScalingGroup); + metricDimensions.add(dimension); + } + return metricDimensions; + } } diff --git a/titus-ext/aws/src/test/java/com/netflix/titus/ext/aws/cloudwatch/CloudWatchClientTest.java b/titus-ext/aws/src/test/java/com/netflix/titus/ext/aws/cloudwatch/CloudWatchClientTest.java new file mode 100644 index 0000000000..eaa8991e1e --- /dev/null +++ b/titus-ext/aws/src/test/java/com/netflix/titus/ext/aws/cloudwatch/CloudWatchClientTest.java @@ -0,0 +1,55 @@ +package com.netflix.titus.ext.aws.cloudwatch; + +import java.util.Arrays; +import java.util.List; + +import com.amazonaws.services.cloudwatch.model.Dimension; +import com.netflix.titus.api.appscale.model.AlarmConfiguration; +import com.netflix.titus.api.appscale.model.ComparisonOperator; +import com.netflix.titus.api.appscale.model.MetricDimension; +import com.netflix.titus.api.appscale.model.Statistic; +import org.junit.Test; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +public class CloudWatchClientTest { + + private AlarmConfiguration.Builder getAlarmConfigBuilder() { + return AlarmConfiguration.newBuilder() + .withName("alarm-config-1") + .withMetricName("metric-1") + .withMetricNamespace("standard") + .withStatistic(Statistic.Average) + .withActionsEnabled(true) + .withPeriodSec(60) + .withThreshold(5) + .withEvaluationPeriods(2) + .withComparisonOperator(ComparisonOperator.GreaterThanOrEqualToThreshold); + } + + @Test + public void buildDefaultMetricDimensions() { + AlarmConfiguration alarmConfiguration = getAlarmConfigBuilder().build(); + List dimensions = CloudWatchClient.buildMetricDimensions(alarmConfiguration, "foo-bar"); + assertThat(dimensions).isNotNull(); + assertThat(dimensions.size()).isEqualTo(1); + assertThat(dimensions.get(0).getName()).isEqualTo("AutoScalingGroupName"); + assertThat(dimensions.get(0).getValue()).isEqualTo("foo-bar"); + } + + @Test + public void buildCustomMetricDimensions() { + MetricDimension md1 = MetricDimension.newBuilder().withName("foo").withValue("bar").build(); + MetricDimension md2 = MetricDimension.newBuilder().withName("service-tier").withValue("1").build(); + List customMetricDimensions = Arrays.asList(md1, md2); + AlarmConfiguration alarmConfiguration = getAlarmConfigBuilder().withDimensions(customMetricDimensions).build(); + List dimensions = CloudWatchClient.buildMetricDimensions(alarmConfiguration, "foo-bar"); + assertThat(dimensions).isNotNull(); + assertThat(dimensions.size()).isEqualTo(2); + assertThat(dimensions.get(0).getName()).isEqualTo("foo"); + assertThat(dimensions.get(0).getValue()).isEqualTo("bar"); + assertThat(dimensions.get(1).getName()).isEqualTo("service-tier"); + assertThat(dimensions.get(1).getValue()).isEqualTo("1"); + } + +} \ No newline at end of file diff --git a/titus-server-master/src/main/java/com/netflix/titus/master/appscale/endpoint/v3/grpc/DefaultAutoScalingServiceGrpc.java b/titus-server-master/src/main/java/com/netflix/titus/master/appscale/endpoint/v3/grpc/DefaultAutoScalingServiceGrpc.java index 64df8260a4..d4a089b0d9 100644 --- a/titus-server-master/src/main/java/com/netflix/titus/master/appscale/endpoint/v3/grpc/DefaultAutoScalingServiceGrpc.java +++ b/titus-server-master/src/main/java/com/netflix/titus/master/appscale/endpoint/v3/grpc/DefaultAutoScalingServiceGrpc.java @@ -29,6 +29,7 @@ import com.netflix.titus.api.service.TitusServiceException; import com.netflix.titus.common.model.sanitizer.EntitySanitizer; import com.netflix.titus.common.util.rx.ReactorExt; +import com.netflix.titus.grpc.protogen.AlarmConfiguration; import com.netflix.titus.grpc.protogen.AutoScalingServiceGrpc; import com.netflix.titus.grpc.protogen.GetPolicyResult; import com.netflix.titus.grpc.protogen.PutPolicyRequest; diff --git a/titus-server-master/src/main/java/com/netflix/titus/master/appscale/endpoint/v3/grpc/InternalModelConverters.java b/titus-server-master/src/main/java/com/netflix/titus/master/appscale/endpoint/v3/grpc/InternalModelConverters.java index ce0448ba69..724d48929a 100644 --- a/titus-server-master/src/main/java/com/netflix/titus/master/appscale/endpoint/v3/grpc/InternalModelConverters.java +++ b/titus-server-master/src/main/java/com/netflix/titus/master/appscale/endpoint/v3/grpc/InternalModelConverters.java @@ -201,6 +201,7 @@ private static AlarmConfiguration toAlarmConfiguration(com.netflix.titus.grpc.pr if (alarmConfigGrpc.getStatisticOneofCase() != com.netflix.titus.grpc.protogen.AlarmConfiguration.StatisticOneofCase.STATISTICONEOF_NOT_SET) { alarmConfigBuilder.withStatistic(toStatistic(alarmConfigGrpc.getStatistic())); } + alarmConfigBuilder.withDimensions(toMetricDimensionList(alarmConfigGrpc.getMetricDimensionsList())); // TODO(Andrew L): Do we want to just always set empty string, if unset? alarmConfigBuilder.withMetricNamespace(alarmConfigGrpc.getMetricNamespace());