Explicitly aggregate counts within a context in MetricRecorder

metrics-api/src/main/java/software/amazon/swage/metrics/MetricContext.java

@@ -53,9 +53,10 @@ default void record(Metric label, Number value, Unit unit) {
      * is often used to record occurrences of an event, such as the number of times
      * a method is called, an error occurred, or the amount of data sent.
      * <p/>
-     * Changes to the count are not timestamped as only the total value of all
-     * counts for a metric have any meaning. If the individual change needs to
-     * be tracked, it should be recorded as a gauged event.
+     * Changes to the count are timestamped with the time of context closure,
+     * since only the total value of all counts for a metric at the end of a context
+     * have any meaning. If the individual change needs to be tracked, it should be
+     * recorded as a gauged event.
      *
      * @param label the metric being recorded
      * @param delta the change in the value

metrics-api/src/main/java/software/amazon/swage/metrics/record/MetricRecorder.java

@@ -15,6 +15,8 @@
 package software.amazon.swage.metrics.record;
 
 import java.time.Instant;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 import software.amazon.swage.collection.TypedMap;
 import software.amazon.swage.metrics.Metric;
@@ -121,6 +123,8 @@ public TypedMap attributes() {
     public MetricContext context(TypedMap attributes) {
         R context = newRecorderContext(attributes);
         return new MetricContext() {
+            private final Map<Metric,Long> counts = new ConcurrentHashMap<>();
+
             @Override
             public TypedMap attributes() {
                 return context.attributes();
@@ -133,11 +137,12 @@ public void record(Metric label, Number value, Unit unit, Instant time) {
 
             @Override
             public void count(Metric label, long delta) {
-                MetricRecorder.this.count(label, delta, context);
+                counts.compute(label, (x, previous) -> (previous == null) ? delta : previous + delta);
             }
 
             @Override
             public void close() {
+                counts.forEach((key, value) -> MetricRecorder.this.count(key, value, context));
                 MetricRecorder.this.close(context);
             }
         };
@@ -172,29 +177,28 @@ protected abstract void record(
             R context);
 
     /**
-     * Count the increase or decrease of a metric in the given context.
+     * Record the value of a count metric at the closure of a context.
      *
      * These are explicitly aggregated values, where only the total number of
      * occurrences in a context matter.
      * Examples include counting number of times a method is called, or keeping
      * track of the number of errors encountered, or the amount of attributes sent
      * while in the given context.
      *
+     * Changes to the count are timestamped with the time of context closure,
+     * as only the total value of all counts for a metric have any meaning.
      * If you need to distinguish between occurrences of an event, or care that
      * an event did not occur (for ratios of success for example) then you want
      * to record those individually and not use a count.
-     * Changes to the count are not timestamped as only the total value of all
-     * counts for a metric have any meaning - if the individual change needs to
-     * be tracked, you probably want to record the change as a gauged event.
      *
      * This method will fail silently, as application code is not expected
      * to depend on success or failure.
      *
      * @param label the metric to capture
-     * @param delta the increase (or decrease) in the value
+     * @param value the total value of the count at the end of the context
      * @param context the context the value belongs in.
      */
-    protected abstract void count(Metric label, long delta, R context);
+    protected abstract void count(Metric label, long value, R context);
 
 
     /**

metrics-api/src/test/java/software/amazon/swage/metrics/record/MetricRecorderTest.java

@@ -16,12 +16,16 @@
 
 import java.time.Instant;
 
+import org.junit.Before;
 import org.junit.Test;
 
 import static org.junit.Assert.assertSame;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.ArgumentMatchers.same;
 import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import software.amazon.swage.collection.TypedMap;
@@ -31,31 +35,55 @@
 
 public class MetricRecorderTest {
     private static final Metric METRIC = Metric.define("Metric");
+    private TypedMap attributes = TypedMap.empty();
+    private MetricRecorder.RecorderContext recorderContext = new MetricRecorder.RecorderContext(attributes);
+    private MetricRecorder<MetricRecorder.RecorderContext> recorder = spy(new NullRecorder());
+
+    @Before
+    public void setup() {
+        when(recorder.newRecorderContext(attributes)).thenReturn(recorderContext);
+    }
 
     @Test
     public void recorderContextReturnsSuppliedAttributes() {
-        TypedMap attributes = TypedMap.empty();
         MetricRecorder.RecorderContext context = new MetricRecorder.RecorderContext(attributes);
         assertSame(attributes, context.attributes());
     }
 
     @Test
-    public void recorderDelegatesToImplementation() throws Exception {
-        TypedMap attributes = TypedMap.empty();
-        MetricRecorder.RecorderContext recorderContext = new MetricRecorder.RecorderContext(attributes);
-
-        MetricRecorder<MetricRecorder.RecorderContext> recorder = spy(new NullRecorder());
-        when(recorder.newRecorderContext(attributes)).thenReturn(recorderContext);
-
+    public void recorderDelegatesRecordToImplementation() throws Exception {
         MetricContext context = recorder.context(attributes);
-
         Instant timestamp = Instant.now();
         context.record(METRIC, 42L, Unit.NONE, timestamp);
+        verify(recorder).record(eq(METRIC), eq(42L), eq(Unit.NONE), eq(timestamp), same(recorderContext));
+    }
+
+    @Test
+    public void recorderDoesNotDelegateCountToImplementationBeforeClose() throws Exception {
+        MetricContext context = recorder.context(attributes);
         context.count(METRIC, 4L);
-        context.close();
+        context.count(METRIC, -2L);
+        context.count(METRIC, 40L);
+        verify(recorder, times(0)).count(any(Metric.class), anyLong(), any(MetricRecorder.RecorderContext.class));
+    }
 
-        verify(recorder).record(eq(METRIC), eq(42L), eq(Unit.NONE), eq(timestamp), same(recorderContext));
+    @Test
+    public void recorderDelegatesCountToImplementationOnClose() throws Exception {
+        MetricContext context = recorder.context(attributes);
+        context.count(METRIC, 4L);
+        context.close();
         verify(recorder).count(eq(METRIC), eq(4L), same(recorderContext));
         verify(recorder).close(same(recorderContext));
     }
+
+    @Test
+    public void recorderAggregatesCountsAndDelegatesOnce() throws Exception {
+        MetricContext context = recorder.context(attributes);
+        context.count(METRIC, 4L);
+        context.count(METRIC, -2L);
+        context.count(METRIC, 40L);
+        context.close();
+        verify(recorder, times(1)).count(eq(METRIC), eq(42L), same(recorderContext));
+        verify(recorder).close(same(recorderContext));
+    }
 }

metrics-api/src/test/java/software/amazon/swage/metrics/record/MultiRecorderTest.java

@@ -7,8 +7,11 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import org.mockito.Spy;
 import org.mockito.junit.MockitoJUnitRunner;
@@ -43,9 +46,18 @@ public void recordIsSentToAllDelegates() {
     }
 
     @Test
-    public void countIsSentToAllDelegates() {
+    public void countIsNotSentToDelegatesBeforeClose() {
         MetricContext context = recorder.context(attributes);
         context.count(METRIC, 42L);
+        verify(delegate1, times(0)).count(any(Metric.class), anyLong(), any(MetricRecorder.RecorderContext.class));
+        verify(delegate2, times(0)).count(any(Metric.class), anyLong(), any(MetricRecorder.RecorderContext.class));
+    }
+
+    @Test
+    public void countIsSentToAllDelegatesOnClose() {
+        MetricContext context = recorder.context(attributes);
+        context.count(METRIC, 42L);
+        context.close();
         verify(delegate1).count(eq(METRIC), eq(42L), argThat(t -> attributes == t.attributes()));
         verify(delegate2).count(eq(METRIC), eq(42L), argThat(t -> attributes == t.attributes()));
     }

metrics-core/src/test/java/software/amazon/swage/metrics/record/cloudwatch/CloudWatchRecorderTest.java

@@ -197,10 +197,10 @@ public void aggregation() throws Exception {
                                StandardUnit.Percent));
         expected.add(makeDatum(id,
                                M_FAIL.toString(),
-                               Arrays.stream(failCnts).sum(),
-                               Arrays.stream(failCnts).min().getAsInt(),
-                               Arrays.stream(failCnts).max().getAsInt(),
-                               failCnts.length,
+                               46,
+                               46,
+                               46,
+                               1,
                                StandardUnit.Count));