Review comments

Author: dagnir

core/metrics-spi/src/main/java/software/amazon/awssdk/metrics/MetricCategory.java

@@ -30,17 +30,17 @@ public enum MetricCategory {
      * All metrics defined by the SDK are classified under this category at a minimum. If the metrics feature is enabled
      * but the category to collect is not, only metrics that are classified under this category are collected by the SDK
      */
-    DEFAULT("default"),
+    DEFAULT("Default"),
 
     /**
      * Metrics collected at the http client level are classified under this category.
      */
-    HTTP_CLIENT("httpclient"),
+    HTTP_CLIENT("HttpClient"),
 
     /**
      * Metrics specific to streaming, eventStream APIs are classified under this category.
      */
-    STREAMING("streaming"),
+    STREAMING("Streaming"),
 
     /**
      * This is an umbrella category (provided for convenience) that records metrics belonging to every category
@@ -49,7 +49,7 @@ public enum MetricCategory {
      * Note: Enabling this option is verbose and can be expensive based on the platform the metrics are uploaded to.
      * Please make sure you need all this data before using this category.
      */
-    ALL("all")
+    ALL("All")
 
     ;
 

core/metrics-spi/src/main/java/software/amazon/awssdk/metrics/MetricCollector.java

@@ -20,7 +20,7 @@
 import software.amazon.awssdk.metrics.internal.DefaultMetricCollector;
 
 /**
- * Used to collect metrics collected by the SDK.
+ * Used to collect metrics reported by the SDK.
  */
 @NotThreadSafe
 @SdkPublicApi
@@ -36,6 +36,7 @@ public interface MetricCollector {
     <T> void reportMetric(SdkMetric<T> metric, T data);
 
     /**
+     * Create a child of this metric collector.
      *
      * @param name The name of the child collector.
      * @return The child collector.

core/metrics-spi/src/main/java/software/amazon/awssdk/metrics/SdkMetric.java

@@ -15,21 +15,6 @@
 
 package software.amazon.awssdk.metrics;
 
-/*
- * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
- *
- * Licensed under the Apache License, Version 2.0 (the "License").
- * You may not use this file except in compliance with the License.
- * A copy of the License is located at
- *
- *  http://aws.amazon.com/apache2.0
- *
- * or in the "license" file accompanying this file. This file is distributed
- * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
- * express or implied. See the License for the specific language governing
- * permissions and limitations under the License.
- */
-
 import java.util.Set;
 import software.amazon.awssdk.annotations.SdkPublicApi;
 import software.amazon.awssdk.metrics.internal.DefaultSdkMetric;
@@ -79,8 +64,8 @@ public interface SdkMetric<T> {
      *
      * @throws IllegalArgumentException If a metric of the same name has already been created.
      */
-    static <T> SdkMetric<T> of(String name, Class<T> clzz, MetricCategory c1, MetricCategory... cn) {
-        return DefaultSdkMetric.of(name, clzz, c1, cn);
+    static <T> SdkMetric<T> create(String name, Class<T> clzz, MetricCategory c1, MetricCategory... cn) {
+        return DefaultSdkMetric.create(name, clzz, c1, cn);
     }
 
     /**
@@ -94,7 +79,7 @@ static <T> SdkMetric<T> of(String name, Class<T> clzz, MetricCategory c1, Metric
      *
      * @throws IllegalArgumentException If a metric of the same name has already been created.
      */
-    static <T> SdkMetric<T> of(String name, Class<T> clzz, Set<MetricCategory> categories) {
-        return DefaultSdkMetric.of(name, clzz, categories);
+    static <T> SdkMetric<T> create(String name, Class<T> clzz, Set<MetricCategory> categories) {
+        return DefaultSdkMetric.create(name, clzz, categories);
     }
 }

core/metrics-spi/src/main/java/software/amazon/awssdk/metrics/internal/DefaultSdkMetric.java

@@ -97,13 +97,13 @@ public int hashCode() {
      *
      * @throws IllegalArgumentException If a metric of the same name has already been created.
      */
-    public static <T> SdkMetric<T> of(String name, Class<T> clzz, MetricCategory c1, MetricCategory... cn) {
+    public static <T> SdkMetric<T> create(String name, Class<T> clzz, MetricCategory c1, MetricCategory... cn) {
         Stream<MetricCategory> categoryStream = Stream.of(c1);
         if (cn != null) {
             categoryStream = Stream.concat(categoryStream, Stream.of(cn));
         }
         Set<MetricCategory> categories = categoryStream.collect(Collectors.toSet());
-        return of(name, clzz, categories);
+        return create(name, clzz, categories);
     }
 
     /**
@@ -117,7 +117,7 @@ public static <T> SdkMetric<T> of(String name, Class<T> clzz, MetricCategory c1,
      *
      * @throws IllegalArgumentException If a metric of the same name has already been created.
      */
-    public static <T> SdkMetric<T> of(String name, Class<T> clzz, Set<MetricCategory> categories) {
+    public static <T> SdkMetric<T> create(String name, Class<T> clzz, Set<MetricCategory> categories) {
         Validate.noNullElements(categories, "categories must not contain null elements");
         SdkMetric<T> event = new DefaultSdkMetric<>(name, clzz, categories);
         if (SDK_METRICS.putIfAbsent(event, Boolean.TRUE) != null) {

core/metrics-spi/src/test/java/software/amazon/awssdk/metrics/internal/DefaultMetricCollectionTest.java

@@ -31,7 +31,7 @@
 import software.amazon.awssdk.metrics.SdkMetric;
 
 public class DefaultMetricCollectionTest {
-    private static final SdkMetric<Integer> M1 = SdkMetric.of("m1", Integer.class, MetricCategory.DEFAULT);
+    private static final SdkMetric<Integer> M1 = SdkMetric.create("m1", Integer.class, MetricCategory.DEFAULT);
 
     @AfterClass
     public static void teardown() {

core/metrics-spi/src/test/java/software/amazon/awssdk/metrics/internal/DefaultMetricCollectorTest.java

@@ -27,7 +27,7 @@
 import software.amazon.awssdk.metrics.SdkMetric;
 
 public class DefaultMetricCollectorTest {
-    private static final SdkMetric<Integer> M1 = SdkMetric.of("m1", Integer.class, MetricCategory.DEFAULT);
+    private static final SdkMetric<Integer> M1 = SdkMetric.create("m1", Integer.class, MetricCategory.DEFAULT);
 
     @Rule
     public ExpectedException thrown = ExpectedException.none();
@@ -72,6 +72,7 @@ public void testCollect_returnedCollectionContainsAllChildren() {
     @Test
     public void testReportMetric_collected_throws() {
         thrown.expect(IllegalStateException.class);
+        thrown.expectMessage("This collector has already been closed");
 
         MetricCollector collector = MetricCollector.create("collector");
         collector.collect();

core/metrics-spi/src/test/java/software/amazon/awssdk/metrics/internal/DefaultSdkMetricRecordTest.java

@@ -27,7 +27,7 @@
 public class DefaultSdkMetricRecordTest {
     @Test
     public void testGetters() {
-        SdkMetric<Integer> event = SdkMetric.of("foo", Integer.class, MetricCategory.DEFAULT);
+        SdkMetric<Integer> event = SdkMetric.create("foo", Integer.class, MetricCategory.DEFAULT);
 
         MetricRecord<Integer> record = new DefaultMetricRecord<>(event, 2);
 

core/metrics-spi/src/test/java/software/amazon/awssdk/metrics/internal/DefaultSdkMetricTest.java

@@ -37,7 +37,7 @@ public void methodSetup() {
 
     @Test
     public void testOf_variadicOverload_createdProperly() {
-        SdkMetric<Integer> event = SdkMetric.of("event", Integer.class, MetricCategory.DEFAULT);
+        SdkMetric<Integer> event = SdkMetric.create("event", Integer.class, MetricCategory.DEFAULT);
 
         assertThat(event.categories()).containsExactly(MetricCategory.DEFAULT);
         assertThat(event.name()).isEqualTo("event");
@@ -46,7 +46,7 @@ public void testOf_variadicOverload_createdProperly() {
 
     @Test
     public void testOf_setOverload_createdProperly() {
-        SdkMetric<Integer> event = SdkMetric.of("event", Integer.class, Stream.of(MetricCategory.DEFAULT)
+        SdkMetric<Integer> event = SdkMetric.create("event", Integer.class, Stream.of(MetricCategory.DEFAULT)
                 .collect(Collectors.toSet()));
 
         assertThat(event.categories()).containsExactly(MetricCategory.DEFAULT);
@@ -58,33 +58,33 @@ public void testOf_setOverload_createdProperly() {
     public void testOf_variadicOverload_c1Null_throws() {
         thrown.expect(IllegalArgumentException.class);
         thrown.expectMessage("must not contain null elements");
-        SdkMetric.of("event", Integer.class, (MetricCategory) null);
+        SdkMetric.create("event", Integer.class, (MetricCategory) null);
     }
 
     @Test
     public void testOf_variadicOverload_c1NotNull_cnNull_doesNotThrow() {
-        SdkMetric.of("event", Integer.class, MetricCategory.DEFAULT, null);
+        SdkMetric.create("event", Integer.class, MetricCategory.DEFAULT, null);
     }
 
     @Test
     public void testOf_variadicOverload_cnContainsNull_throws() {
         thrown.expect(IllegalArgumentException.class);
         thrown.expectMessage("must not contain null elements");
-        SdkMetric.of("event", Integer.class, MetricCategory.DEFAULT, new MetricCategory[]{ null });
+        SdkMetric.create("event", Integer.class, MetricCategory.DEFAULT, new MetricCategory[]{ null });
     }
 
     @Test
     public void testOf_setOverload_null_throws() {
         thrown.expect(NullPointerException.class);
         thrown.expectMessage("object is null");
-        SdkMetric.of("event", Integer.class, (Set<MetricCategory>) null);
+        SdkMetric.create("event", Integer.class, (Set<MetricCategory>) null);
     }
 
     @Test
     public void testOf_setOverload_nullElement_throws() {
         thrown.expect(IllegalArgumentException.class);
         thrown.expectMessage("categories must not contain null elements");
-        SdkMetric.of("event", Integer.class, Stream.of((MetricCategory) null).collect(Collectors.toSet()));
+        SdkMetric.create("event", Integer.class, Stream.of((MetricCategory) null).collect(Collectors.toSet()));
     }
 
     @Test
@@ -94,8 +94,8 @@ public void testOf_namePreviouslyUsed_throws() {
         thrown.expect(IllegalArgumentException.class);
         thrown.expectMessage(fooName + " has already been created");
 
-        SdkMetric.of(fooName, Integer.class, MetricCategory.DEFAULT);
-        SdkMetric.of(fooName, Integer.class, MetricCategory.DEFAULT);
+        SdkMetric.create(fooName, Integer.class, MetricCategory.DEFAULT);
+        SdkMetric.create(fooName, Integer.class, MetricCategory.DEFAULT);
     }
 
     @Test
@@ -105,8 +105,8 @@ public void testOf_namePreviouslyUsed_differentArgs_throws() {
         thrown.expect(IllegalArgumentException.class);
         thrown.expectMessage(fooName + " has already been created");
 
-        SdkMetric.of(fooName, Integer.class, MetricCategory.DEFAULT);
-        SdkMetric.of(fooName, Long.class, MetricCategory.STREAMING);
+        SdkMetric.create(fooName, Integer.class, MetricCategory.DEFAULT);
+        SdkMetric.create(fooName, Long.class, MetricCategory.STREAMING);
     }
 
     @Test
@@ -116,9 +116,9 @@ public void testOf_namePreviouslyUsed_doesNotReplaceExisting() {
         thrown.expect(IllegalArgumentException.class);
         thrown.expectMessage(fooName + " has already been created");
 
-        SdkMetric.of(fooName, Integer.class, MetricCategory.DEFAULT);
+        SdkMetric.create(fooName, Integer.class, MetricCategory.DEFAULT);
         try {
-            SdkMetric.of(fooName, Long.class, MetricCategory.STREAMING);
+            SdkMetric.create(fooName, Long.class, MetricCategory.STREAMING);
         } finally {
             SdkMetric<?> fooMetric = DefaultSdkMetric.declaredEvents()
                 .stream()