Address pr comments

Author: varunnvs92

core/metrics-spi/pom.xml

@@ -26,10 +26,6 @@
             <artifactId>utils</artifactId>
             <version>${awsjavasdk.version}</version>
         </dependency>
-        <dependency>
-            <groupId>org.slf4j</groupId>
-            <artifactId>slf4j-api</artifactId>
-        </dependency>
 
         <dependency>
             <groupId>software.amazon.awssdk</groupId>

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

@@ -20,7 +20,7 @@
 
 /**
  * A enum class representing the different types of metric categories in the SDK.
- *
+ * <p>
  * A metric can be tagged with multiple categories. Clients can enable/disable metric collection
  * at a {@link MetricCategory} level.
  *
@@ -48,7 +48,7 @@ public enum MetricCategory {
     /**
      * This is an umbrella category (provided for convenience) that records metrics belonging to every category
      * defined in this enum. Clients who wish to collect lot of SDK metrics data should use this.
-     *
+     * <p>
      * 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.
      */

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

@@ -19,7 +19,7 @@
 import software.amazon.awssdk.metrics.MetricCategory;
 import software.amazon.awssdk.utils.SystemSetting;
 
- /**
+/**
  * System properties to configure metrics options in the SDK.
  */
 @SdkInternalApi

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

@@ -16,7 +16,6 @@
 package software.amazon.awssdk.metrics.internal;
 
 import static software.amazon.awssdk.metrics.MetricCategory.DEFAULT;
-import static software.amazon.awssdk.metrics.MetricCategory.HTTP_CLIENT;
 
 import java.util.Arrays;
 import java.util.Collections;
@@ -25,6 +24,7 @@
 import software.amazon.awssdk.annotations.SdkInternalApi;
 import software.amazon.awssdk.metrics.MetricCategory;
 import software.amazon.awssdk.metrics.meter.Metric;
+import software.amazon.awssdk.utils.ToString;
 
 /**
  * A enum representing the metrics supported by the SDK. All metrics collected by the SDK will be declared in this
@@ -95,11 +95,6 @@ public enum SdkMetric implements Metric {
      */
     ExtendedRequestId("ExtendedRequestId", DEFAULT),
 
-    /**
-     * Maximum number of streams allowed on a http2 connection
-     */
-    MaxStreamCount("MaxStreamCount", DEFAULT, HTTP_CLIENT)
-
     ;
 
     private final String value;
@@ -122,9 +117,8 @@ public Set<MetricCategory> categories() {
 
     @Override
     public String toString() {
-        return "{" +
-               "value='" + value + '\'' +
-               ", categories=" + categories +
-               '}';
+        return ToString.builder(value)
+                       .add("categories", categories)
+                       .build();
     }
 }

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

@@ -66,6 +66,9 @@ public static final class Builder<BuilderT> {
         private BuilderT value;
         private final Set<MetricCategory> categories = new HashSet<>();
 
+        private Builder() {
+        }
+
         /**
          * @param value the value to store in the gauge
          * @return This object for method chaining

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

@@ -73,6 +73,9 @@ public static final class Builder<BuilderT> {
         private BuilderT value;
         private final Set<MetricCategory> categories = new HashSet<>();
 
+        private Builder() {
+        }
+
         /**
          * @param value the initial value to store in the gauge
          * @return This object for method chaining

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

@@ -15,12 +15,15 @@
 
 package software.amazon.awssdk.metrics.meter;
 
+import software.amazon.awssdk.annotations.SdkPublicApi;
+
 /**
  * A gauge metric is an instantaneous value recorded at a point in time.
  *
  * @param <T> the type of the value recorded in the Gauge
  */
 @FunctionalInterface
+@SdkPublicApi
 public interface Gauge<T> extends Metric {
 
     /**

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

@@ -21,6 +21,7 @@
 import java.util.concurrent.atomic.LongAdder;
 import software.amazon.awssdk.annotations.SdkPublicApi;
 import software.amazon.awssdk.metrics.MetricCategory;
+import software.amazon.awssdk.utils.Validate;
 
 /**
  * A {@link Counter} implementation that stores {@link Long} values.
@@ -43,6 +44,7 @@ public void increment() {
 
     @Override
     public void increment(Long value) {
+        Validate.isNotNegative(value, "Value should not be negative. Use decrement(...) method to reduce count.");
         count.add(value);
     }
 
@@ -53,6 +55,7 @@ public void decrement() {
 
     @Override
     public void decrement(Long value) {
+        Validate.isNotNegative(value, "Value should be positive. Use increment(...) method to increase count.");
         count.add(-value);
     }
 
@@ -78,9 +81,11 @@ public static LongCounter create() {
      * Builder class to create instances of {@link LongCounter}
      */
     public static final class Builder {
-
         private final Set<MetricCategory> categories = new HashSet<>();
 
+        private Builder() {
+        }
+
         /**
          * Register the given categories in this metric
          * @param categories the set of {@link MetricCategory} this metric belongs to

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

@@ -1,5 +1,3 @@
-package software.amazon.awssdk.metrics.meter;
-
 /*
  * Copyright 2010-2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
  *
@@ -15,15 +13,19 @@
  * permissions and limitations under the License.
  */
 
+package software.amazon.awssdk.metrics.meter;
+
 import software.amazon.awssdk.annotations.SdkPublicApi;
 
 /**
  * A NoOp implementation of the {@link Counter} metric
  */
 @SdkPublicApi
-public enum NoOpCounter implements Counter<Number> {
-    /** Singleton instance **/
-    INSTANCE;
+public final class NoOpCounter implements Counter<Number> {
+    private static final NoOpCounter INSTANCE = new NoOpCounter();
+
+    private NoOpCounter() {
+    }
 
     @Override
     public void increment() {
@@ -49,4 +51,8 @@ public void decrement(Number value) {
     public Number count() {
         return 0;
     }
+
+    public static NoOpCounter instance() {
+        return INSTANCE;
+    }
 }

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

@@ -21,14 +21,19 @@
  * A NoOp implementation of the {@link Gauge} metric
  */
 @SdkPublicApi
-public enum NoOpGauge implements Gauge {
-    /** Singleton instance **/
-    INSTANCE;
-
+public final class NoOpGauge implements Gauge {
+    private static final NoOpGauge INSTANCE = new NoOpGauge();
     private static final Object EMPTY_OBJECT = new Object();
 
+    private NoOpGauge() {
+    }
+
     @Override
     public Object value() {
         return EMPTY_OBJECT;
     }
+
+    public static NoOpGauge instance() {
+        return INSTANCE;
+    }
 }

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

@@ -23,13 +23,15 @@
 
 /**
  * A NoOp implementation of the {@link Timer} metric.
- *
+ * <p>
  * Always returns {@link Duration#ZERO} as the total duration.
  */
 @SdkPublicApi
-public enum NoOpTimer implements Timer {
-    /** Singleton instance **/
-    INSTANCE;
+public final class NoOpTimer implements Timer {
+    private static final NoOpTimer INSTANCE = new NoOpTimer();
+
+    private NoOpTimer() {
+    }
 
     @Override
     public void record(long duration, TimeUnit timeUnit) {
@@ -71,4 +73,8 @@ public Instant endTime() {
     public Duration totalTime() {
         return Duration.ZERO;
     }
+
+    public static NoOpTimer instance() {
+        return INSTANCE;
+    }
 }

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

@@ -21,6 +21,7 @@
 import java.util.Set;
 import software.amazon.awssdk.annotations.SdkPublicApi;
 import software.amazon.awssdk.metrics.MetricCategory;
+import software.amazon.awssdk.utils.CollectionUtils;
 
 /**
  * The default implementation of {@link MetricConfigurationProvider} interface.
@@ -54,7 +55,7 @@ public static Builder builder() {
     }
 
     private Set<MetricCategory> resolveCategories(Set<MetricCategory> categories) {
-        if (categories == null || categories.isEmpty()) {
+        if (CollectionUtils.isNullOrEmpty(categories)) {
             return new HashSet<>(Arrays.asList(MetricCategory.DEFAULT));
         }
 
@@ -67,6 +68,9 @@ public static final class Builder {
 
         private final Set<MetricCategory> categories = new HashSet<>();
 
+        private Builder() {
+        }
+
         /**
          * @param enabled set true to enable metrics. Set to false by default
          * @return

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

@@ -21,7 +21,7 @@
 
 /**
  * Interface to configure the options in metrics feature.
- *
+ * <p>
  * This interface acts as a feature flag for metrics. The methods in the interface are called for each request.
  * This gives flexibility for metrics feature to be enabled/disabled at runtime and configuration changes
  * can be picked up at runtime without need for deploying the application (depending on the implementation).

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

@@ -15,18 +15,15 @@
 
 package software.amazon.awssdk.metrics.provider;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
 import java.util.Set;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import software.amazon.awssdk.annotations.SdkProtectedApi;
 import software.amazon.awssdk.metrics.MetricCategory;
+import software.amazon.awssdk.utils.Logger;
+import software.amazon.awssdk.utils.Validate;
 
 /**
  * Composite {@link MetricConfigurationProvider} that lets you specify a list of providers to get metric configuration.
- *
+ * <p>
  * This chain decides the {@link MetricConfigurationProvider} to use at object construction time and cannot be changed
  * later. The resolution policy is as follows:
  *
@@ -36,44 +33,35 @@
  */
 @SdkProtectedApi
 public class MetricConfigurationProviderChain implements MetricConfigurationProvider {
-
-    private static final Logger log = LoggerFactory.getLogger(MetricConfigurationProviderChain.class);
-
-    private final List<MetricConfigurationProvider> providers;
+    private static final Logger log = Logger.loggerFor(MetricConfigurationProviderChain.class);
 
     private final MetricConfigurationProvider resolvedProvider;
 
     public MetricConfigurationProviderChain(MetricConfigurationProvider... providers) {
-        this.providers = new ArrayList<>(providers.length);
-        Collections.addAll(this.providers, providers);
-
-        this.resolvedProvider = resolve();
+        Validate.isPositive(providers.length, "Atleast one provider should be set");
+        this.resolvedProvider = resolve(providers);
     }
 
     /**
      * Determine the provider to use from the chain. This is determined at object construction time and cannot
      * be changed later.
      */
-    private MetricConfigurationProvider resolve() {
+    private MetricConfigurationProvider resolve(MetricConfigurationProvider... providers) {
         for (MetricConfigurationProvider provider : providers) {
             try {
                 if (provider.enabled()) {
                     return provider;
                 }
             } catch (Exception e) {
                 // Ignore any exceptions and move onto the next provider
-                if (log.isDebugEnabled()) {
-                    log.debug("Metrics are not enabled in {}:{}", provider.toString(), e.getMessage());
-                }
+                log.debug(() -> "Metrics are not enabled in " + provider.toString(), e);
             }
         }
 
-        if (log.isDebugEnabled()) {
-            log.debug("Metrics are not enabled in any of the providers in the chain. So using the first provider in"
-                      + " the chain: ", providers.get(0).toString());
-        }
+        log.debug(() -> "Metrics are not enabled in any of the providers in the chain. "
+                        + "So using the first provider in the chain: " +  providers[0]);
 
-        return providers.get(0);
+        return providers[0];
     }
 
     @Override

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

@@ -23,7 +23,7 @@
 
 /**
  * A {@link MetricConfigurationProvider} implementation that uses system properties to configure the metrics feature.
- *
+ * <p>
  * As Java system properties can only be set during application start time or using application code,
  * it is not possible to change the behavior at runtime without code changes.
  */
@@ -50,8 +50,8 @@ public Set<MetricCategory> metricCategories() {
 
     private boolean resolveEnabled() {
         return MetricSystemSetting.AWS_JAVA_SDK_METRICS_ENABLED.getStringValue()
-                                                               .filter(s -> Boolean.TRUE.toString().equals(s.toLowerCase()))
-                                                               .map(s -> Boolean.TRUE.booleanValue())
+                                                               .filter(s -> Boolean.TRUE.toString().equalsIgnoreCase(s))
+                                                               .map(s -> true)
                                                                .orElse(false);
     }