metrics refactor to publish in bulk (#311)

ammokhov · web-flow · commit da4a7f346cbf · 2020-09-21T15:36:00.000-07:00
diff --git a/src/main/java/software/amazon/cloudformation/LambdaWrapper.java b/src/main/java/software/amazon/cloudformation/LambdaWrapper.java
@@ -29,7 +29,6 @@
 import java.time.Instant;
 import java.util.Arrays;
 import java.util.Date;
-import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -242,11 +241,8 @@ public void handleRequest(final InputStream inputStream, final OutputStream outp
             // A response will be output on all paths, though CloudFormation will
             // not block on invoking the handlers, but rather listen for callbacks
 
-            if (handlerResponse != null) {
-                publishExceptionCodeAndCountMetric(request == null ? null : request.getAction(), handlerResponse.getErrorCode(),
-                    handlerResponse.getStatus() == OperationStatus.FAILED);
-            }
             writeResponse(outputStream, handlerResponse);
+            publishExceptionCodeAndCountMetrics(request == null ? null : request.getAction(), handlerResponse.getErrorCode());
         }
     }
 
@@ -493,12 +489,9 @@ private void publishExceptionMetric(final Action action, final Throwable ex, fin
     /*
      * null-safe exception metrics delivery
      */
-    private void
-        publishExceptionCodeAndCountMetric(final Action action, final HandlerErrorCode handlerErrorCode, final boolean thrown) {
+    private void publishExceptionCodeAndCountMetrics(final Action action, final HandlerErrorCode handlerErrorCode) {
         if (this.metricsPublisherProxy != null) {
-            EnumSet.allOf(HandlerErrorCode.class).forEach(errorCode -> this.metricsPublisherProxy
-                .publishExceptionByErrorCodeMetric(Instant.now(), action, errorCode, thrown && errorCode == handlerErrorCode));
-            this.metricsPublisherProxy.publishExceptionCountMetric(Instant.now(), action, thrown);
+            this.metricsPublisherProxy.publishExceptionByErrorCodeAndCountBulkMetrics(Instant.now(), action, handlerErrorCode);
         }
     }
 
diff --git a/src/main/java/software/amazon/cloudformation/metrics/MetricsPublisher.java b/src/main/java/software/amazon/cloudformation/metrics/MetricsPublisher.java
@@ -42,13 +42,9 @@ public void publishExceptionMetric(final Instant timestamp,
                                        final HandlerErrorCode handlerErrorCode) {
     }
 
-    public void publishExceptionByErrorCodeMetric(final Instant timestamp,
-                                                  final Action action,
-                                                  final HandlerErrorCode handlerErrorCode,
-                                                  final boolean thrown) {
-    }
-
-    public void publishExceptionCountMetric(final Instant timestamp, final Action action, final boolean thrown) {
+    public void publishExceptionByErrorCodeAndCountBulkMetrics(final Instant timestamp,
+                                                               final Action action,
+                                                               final HandlerErrorCode handlerErrorCode) {
     }
 
     public void publishInvocationMetric(final Instant timestamp, final Action action) {
diff --git a/src/main/java/software/amazon/cloudformation/metrics/MetricsPublisherImpl.java b/src/main/java/software/amazon/cloudformation/metrics/MetricsPublisherImpl.java
@@ -14,11 +14,11 @@
 */
 package software.amazon.cloudformation.metrics;
 
+import com.google.common.collect.Sets;
 import java.time.Instant;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.EnumSet;
+import java.util.HashSet;
+import java.util.Set;
 import software.amazon.awssdk.services.cloudwatch.CloudWatchClient;
 import software.amazon.awssdk.services.cloudwatch.model.Dimension;
 import software.amazon.awssdk.services.cloudwatch.model.MetricDatum;
@@ -57,86 +57,83 @@ public void publishExceptionMetric(final Instant timestamp,
                                        final Action action,
                                        final Throwable e,
                                        final HandlerErrorCode handlerErrorCode) {
-        Map<String, String> dimensions = new HashMap<>();
-        dimensions.put(Metric.DIMENSION_KEY_ACTION_TYPE, action == null ? "NO_ACTION" : action.name());
-        dimensions.put(Metric.DIMENSION_KEY_EXCEPTION_TYPE, e.getClass().toString());
-        dimensions.put(Metric.DIMENSION_KEY_RESOURCE_TYPE, this.getResourceTypeName());
-        dimensions.put(Metric.DIMENSION_KEY_HANDLER_ERROR_CODE, handlerErrorCode.name());
-
-        publishMetric(Metric.METRIC_NAME_HANDLER_EXCEPTION, dimensions, StandardUnit.COUNT, 1.0, timestamp);
+        publishBulkMetrics(MetricDatum.builder().timestamp(timestamp).metricName(Metric.METRIC_NAME_HANDLER_EXCEPTION)
+            .unit(StandardUnit.COUNT).value(1.0)
+            .dimensions(Sets.newHashSet(
+                Dimension.builder().name(Metric.DIMENSION_KEY_ACTION_TYPE).value(action == null ? "NO_ACTION" : action.name())
+                    .build(),
+                Dimension.builder().name(Metric.DIMENSION_KEY_EXCEPTION_TYPE).value(e.getClass().toString()).build(),
+                Dimension.builder().name(Metric.DIMENSION_KEY_RESOURCE_TYPE).value(this.getResourceTypeName()).build(),
+                Dimension.builder().name(Metric.DIMENSION_KEY_HANDLER_ERROR_CODE).value(handlerErrorCode.name()).build()))
+            .build());
     }
 
     @Override
-    public void publishExceptionByErrorCodeMetric(final Instant timestamp,
-                                                  final Action action,
-                                                  final HandlerErrorCode handlerErrorCode,
-                                                  final boolean thrown) {
-        Map<String, String> dimensions = new HashMap<>();
-        dimensions.put(Metric.DIMENSION_KEY_ACTION_TYPE, action == null ? "NO_ACTION" : action.name());
-        dimensions.put(Metric.DIMENSION_KEY_HANDLER_ERROR_CODE, handlerErrorCode.name());
-
-        publishMetric(Metric.METRIC_NAME_HANDLER_EXCEPTION_BY_ERROR_CODE, dimensions, StandardUnit.COUNT, thrown ? 1.0 : 0.0,
-            timestamp);
-    }
-
-    public void publishExceptionCountMetric(final Instant timestamp, final Action action, final boolean thrown) {
-        Map<String, String> dimensions = new HashMap<>();
-        dimensions.put(Metric.DIMENSION_KEY_ACTION_TYPE, action == null ? "NO_ACTION" : action.name());
-
-        publishMetric(Metric.METRIC_NAME_HANDLER_EXCEPTION_BY_EXCEPTION_COUNT, dimensions, StandardUnit.COUNT, thrown ? 1.0 : 0.0,
-            timestamp);
+    public void publishExceptionByErrorCodeAndCountBulkMetrics(final Instant timestamp,
+                                                               final Action action,
+                                                               final HandlerErrorCode handlerErrorCode) {
+        Set<MetricDatum> bulkData = new HashSet<>();
+
+        // By Error Code dimensions
+
+        EnumSet.allOf(HandlerErrorCode.class).forEach(
+            errorCode -> bulkData.add(MetricDatum.builder().metricName(Metric.METRIC_NAME_HANDLER_EXCEPTION_BY_ERROR_CODE)
+                .unit(StandardUnit.COUNT).value(errorCode == handlerErrorCode ? 1.0 : 0.0)
+                .dimensions(Sets.newHashSet(
+                    Dimension.builder().name(Metric.DIMENSION_KEY_ACTION_TYPE).value(action == null ? "NO_ACTION" : action.name())
+                        .build(),
+                    Dimension.builder().name(Metric.DIMENSION_KEY_HANDLER_ERROR_CODE).value(errorCode.name()).build()))
+                .timestamp(timestamp).build()));
+
+        // By Count dimensions
+        bulkData.add(MetricDatum.builder().metricName(Metric.METRIC_NAME_HANDLER_EXCEPTION_BY_EXCEPTION_COUNT)
+            .unit(StandardUnit.COUNT).value(handlerErrorCode == null ? 0.0 : 1.0).dimensions(Dimension.builder()
+                .name(Metric.DIMENSION_KEY_ACTION_TYPE).value(action == null ? "NO_ACTION" : action.name()).build())
+            .timestamp(timestamp).build());
+
+        publishBulkMetrics(bulkData.toArray(new MetricDatum[bulkData.size()]));
     }
 
     @Override
     public void publishProviderLogDeliveryExceptionMetric(final Instant timestamp, final Throwable e) {
-        Map<String, String> dimensions = new HashMap<>();
-        dimensions.put(Metric.DIMENSION_KEY_ACTION_TYPE, "ProviderLogDelivery");
-        dimensions.put(Metric.DIMENSION_KEY_EXCEPTION_TYPE, e.getClass().toString());
-        dimensions.put(Metric.DIMENSION_KEY_RESOURCE_TYPE, this.getResourceTypeName());
-
-        publishMetric(Metric.METRIC_NAME_HANDLER_EXCEPTION, dimensions, StandardUnit.COUNT, 1.0, timestamp);
+        publishBulkMetrics(
+            MetricDatum.builder().metricName(Metric.METRIC_NAME_HANDLER_EXCEPTION).unit(StandardUnit.COUNT).value(1.0)
+                .dimensions(Sets.newHashSet(
+                    Dimension.builder().name(Metric.DIMENSION_KEY_ACTION_TYPE).value("ProviderLogDelivery").build(),
+                    Dimension.builder().name(Metric.DIMENSION_KEY_EXCEPTION_TYPE).value(e.getClass().toString()).build(),
+                    Dimension.builder().name(Metric.DIMENSION_KEY_RESOURCE_TYPE).value(this.getResourceTypeName()).build()))
+                .timestamp(timestamp).build());
     }
 
     @Override
     public void publishInvocationMetric(final Instant timestamp, final Action action) {
-        Map<String, String> dimensions = new HashMap<>();
-        dimensions.put(Metric.DIMENSION_KEY_ACTION_TYPE, action == null ? "NO_ACTION" : action.name());
-        dimensions.put(Metric.DIMENSION_KEY_RESOURCE_TYPE, this.getResourceTypeName());
-
-        publishMetric(Metric.METRIC_NAME_HANDLER_INVOCATION_COUNT, dimensions, StandardUnit.COUNT, 1.0, timestamp);
+        publishBulkMetrics(
+            MetricDatum.builder().metricName(Metric.METRIC_NAME_HANDLER_INVOCATION_COUNT).unit(StandardUnit.COUNT).value(1.0)
+                .dimensions(Sets.newHashSet(
+                    Dimension.builder().name(Metric.DIMENSION_KEY_ACTION_TYPE).value(action == null ? "NO_ACTION" : action.name())
+                        .build(),
+                    Dimension.builder().name(Metric.DIMENSION_KEY_RESOURCE_TYPE).value(this.getResourceTypeName()).build()))
+                .timestamp(timestamp).build());
     }
 
     @Override
     public void publishDurationMetric(final Instant timestamp, final Action action, final long milliseconds) {
-        Map<String, String> dimensions = new HashMap<>();
-        dimensions.put(Metric.DIMENSION_KEY_ACTION_TYPE, action == null ? "NO_ACTION" : action.name());
-        dimensions.put(Metric.DIMENSION_KEY_RESOURCE_TYPE, this.getResourceTypeName());
-
-        publishMetric(Metric.METRIC_NAME_HANDLER_DURATION, dimensions, StandardUnit.MILLISECONDS, (double) milliseconds,
-            timestamp);
+        publishBulkMetrics(MetricDatum.builder().metricName(Metric.METRIC_NAME_HANDLER_DURATION).unit(StandardUnit.MILLISECONDS)
+            .value((double) milliseconds)
+            .dimensions(Sets.newHashSet(
+                Dimension.builder().name(Metric.DIMENSION_KEY_ACTION_TYPE).value(action == null ? "NO_ACTION" : action.name())
+                    .build(),
+                Dimension.builder().name(Metric.DIMENSION_KEY_RESOURCE_TYPE).value(this.getResourceTypeName()).build()))
+            .timestamp(timestamp).build());
     }
 
-    private void publishMetric(final String metricName,
-                               final Map<String, String> dimensionData,
-                               final StandardUnit unit,
-                               final Double value,
-                               final Instant timestamp) {
+    private void publishBulkMetrics(final MetricDatum... metricData) {
         assert cloudWatchClient != null : "CloudWatchEventsClient was not initialised. You must call refreshClient() first.";
 
-        List<Dimension> dimensions = new ArrayList<>();
-        for (Map.Entry<String, String> kvp : dimensionData.entrySet()) {
-            Dimension dimension = Dimension.builder().name(kvp.getKey()).value(kvp.getValue()).build();
-            dimensions.add(dimension);
-        }
-
-        MetricDatum metricDatum = MetricDatum.builder().metricName(metricName).unit(unit).value(value).dimensions(dimensions)
-            .timestamp(timestamp).build();
-
-        PutMetricDataRequest putMetricDataRequest = PutMetricDataRequest.builder()
-            .namespace(String.format("%s/%s", Metric.METRIC_NAMESPACE_ROOT, resourceNamespace)).metricData(metricDatum).build();
-
         try {
-            this.cloudWatchClient.putMetricData(putMetricDataRequest);
+            this.cloudWatchClient.putMetricData(
+                PutMetricDataRequest.builder().namespace(String.format("%s/%s", Metric.METRIC_NAMESPACE_ROOT, resourceNamespace))
+                    .metricData(metricData).build());
         } catch (final Exception e) {
             log(String.format("An error occurred while publishing metrics: %s", e.getMessage()));
         }
diff --git a/src/main/java/software/amazon/cloudformation/proxy/MetricsPublisherProxy.java b/src/main/java/software/amazon/cloudformation/proxy/MetricsPublisherProxy.java
@@ -35,17 +35,11 @@ public void publishExceptionMetric(final Instant timestamp,
             .forEach(metricsPublisher -> metricsPublisher.publishExceptionMetric(timestamp, action, e, handlerErrorCode));
     }
 
-    public void publishExceptionByErrorCodeMetric(final Instant timestamp,
-                                                  final Action action,
-                                                  final HandlerErrorCode handlerErrorCode,
-                                                  final boolean thrown) {
-        metricsPublishers.stream().forEach(
-            metricsPublisher -> metricsPublisher.publishExceptionByErrorCodeMetric(timestamp, action, handlerErrorCode, thrown));
-    }
-
-    public void publishExceptionCountMetric(final Instant timestamp, final Action action, final boolean thrown) {
-        metricsPublishers.stream()
-            .forEach(metricsPublisher -> metricsPublisher.publishExceptionCountMetric(timestamp, action, thrown));
+    public void publishExceptionByErrorCodeAndCountBulkMetrics(final Instant timestamp,
+                                                               final Action action,
+                                                               final HandlerErrorCode handlerErrorCode) {
+        metricsPublishers.stream().forEach(metricsPublisher -> metricsPublisher
+            .publishExceptionByErrorCodeAndCountBulkMetrics(timestamp, action, handlerErrorCode));
     }
 
     public void publishInvocationMetric(final Instant timestamp, final Action action) {
diff --git a/src/test/java/software/amazon/cloudformation/LambdaWrapperTest.java b/src/test/java/software/amazon/cloudformation/LambdaWrapperTest.java
@@ -161,9 +161,8 @@ public void invokeHandler_nullResponse_returnsFailure(final String requestDataPa
             // validation failure metric should be published for final error handling
             verify(providerMetricsPublisher).publishExceptionMetric(any(Instant.class), any(), any(TerminalException.class),
                 any(HandlerErrorCode.class));
-            verify(providerMetricsPublisher).publishExceptionByErrorCodeMetric(any(Instant.class), any(),
-                any(HandlerErrorCode.class), eq(Boolean.TRUE));
-            verify(providerMetricsPublisher).publishExceptionCountMetric(any(Instant.class), any(), any(Boolean.class));
+            verify(providerMetricsPublisher).publishExceptionByErrorCodeAndCountBulkMetrics(any(Instant.class), any(),
+                any(HandlerErrorCode.class));
 
             // all metrics should be published even on terminal failure
             verify(providerMetricsPublisher).publishInvocationMetric(any(Instant.class), eq(action));
@@ -215,8 +214,10 @@ providerEventsLogger, providerMetricsPublisher, new Validator() {
             verifyInitialiseRuntime();
 
             // verify output response
-            verifyHandlerResponse(out, ProgressEvent.<TestModel, TestContext>builder().errorCode(HandlerErrorCode.InvalidRequest)
-                .status(OperationStatus.FAILED).message("Resource properties validation failed with invalid configuration").build());
+            verifyHandlerResponse(out,
+                ProgressEvent.<TestModel, TestContext>builder().errorCode(HandlerErrorCode.InvalidRequest)
+                    .status(OperationStatus.FAILED).message("Resource properties validation failed with invalid configuration")
+                    .build());
         }
     }
 
@@ -443,9 +444,8 @@ public void invokeHandler_InProgress_returnsInProgress(final String requestDataP
 
             }
 
-            verify(providerMetricsPublisher, atLeastOnce()).publishExceptionByErrorCodeMetric(any(Instant.class), eq(action),
-                any(), any(Boolean.class));
-            verify(providerMetricsPublisher).publishExceptionCountMetric(any(Instant.class), any(), any(Boolean.class));
+            verify(providerMetricsPublisher).publishExceptionByErrorCodeAndCountBulkMetrics(any(Instant.class), eq(action),
+                any());
 
             // validation failure metric should not be published
             verifyNoMoreInteractions(providerMetricsPublisher);
@@ -488,9 +488,8 @@ public void reInvokeHandler_InProgress_returnsInProgress(final String requestDat
             // all metrics should be published, once for a single invocation
             verify(providerMetricsPublisher).publishInvocationMetric(any(Instant.class), eq(action));
             verify(providerMetricsPublisher).publishDurationMetric(any(Instant.class), eq(action), anyLong());
-            verify(providerMetricsPublisher, atLeastOnce()).publishExceptionByErrorCodeMetric(any(Instant.class), eq(action),
-                any(), eq(Boolean.FALSE));
-            verify(providerMetricsPublisher).publishExceptionCountMetric(any(Instant.class), eq(action), eq(Boolean.FALSE));
+            verify(providerMetricsPublisher).publishExceptionByErrorCodeAndCountBulkMetrics(any(Instant.class), eq(action),
+                any());
 
             // validation failure metric should not be published
             verifyNoMoreInteractions(providerMetricsPublisher);
@@ -875,11 +874,8 @@ public void invokeHandler_metricPublisherThrowable_returnsFailureResponse() thro
             // verify initialiseRuntime was called and initialised dependencies
             verifyInitialiseRuntime();
 
-            verify(providerMetricsPublisher, atLeastOnce()).publishExceptionByErrorCodeMetric(any(Instant.class),
-                any(Action.class), any(HandlerErrorCode.class), any(Boolean.class));
-
-            verify(providerMetricsPublisher).publishExceptionCountMetric(any(Instant.class), any(Action.class),
-                any(Boolean.class));
+            verify(providerMetricsPublisher).publishExceptionByErrorCodeAndCountBulkMetrics(any(Instant.class), any(Action.class),
+                any(HandlerErrorCode.class));
 
             // no further calls to metrics publisher should occur
             verifyNoMoreInteractions(providerMetricsPublisher);
