Improve components that are used to unit test instrumentation logic (#…

…129)

* Add support for monotomic clocks to manipulate time

 The `MonotonicClock` interface offers an abstraction for us to
 manipulate time in test. It aims to improve the implementation of
 `TestScope` so that develpers can verify metric recording of timers
 and histograms.

* Fix Snapshot computation in TestScope

The following changes have been made as part of this commit:

1. Refactored how snapshots are created to leverage the same reporting
   mechanism used in regular metrics recording.

2. Fixed snapshots for Timers. The previous `TimerImpl` implementation
   relied on `NoReporterSink` to properly create `TimerSnapshots`.
   However given `TestScope` instances are built with `NullStatsReporter`,
   `TimerSnapshots` were not being created at all when using `TestScope`.

3. Leveraged the `ImmutableBuckets` implementation to compute bucket bounds
   in `HistogramImpl`, allowing removal of a substantial amount of
   duplicated code.

4. Added unit tests to cover the fixes for `TimerSnapshots` as well as
   the changes to the snapshot creation logic.

* Remove code warnings

No functionality change. Just removal of warnings in the code.

* Remove warnings and fix typos in *.md files

DEVELOPER.md

@@ -25,15 +25,14 @@ To run the JMH benchmark tests:
 ./gradlew runJmhTests
 ```
 
-By default, the benchmark test results are writen to `benchmark-tests.txt`.
-Use `output` input parameter to configure the output path.
+By default, the benchmark test results are written to `benchmark-tests.txt`.
+Use the `output` input parameter to configure the output path.
 E.g. the following command will run the benchmark tests for `tally-prometheus` sub-project and store 
 the results to `custom/path/result.txt`.  
 ```bash
 ./gradlew :tally-prometheus:runJmhTests -Poutput="custom/path/result.txt"
 ``` 
 
-
 By default, the build does *not* compile Thrift files to generate sources. If you make changes to Thrift files and need
 regenerate sources, make sure you have thrift 0.9.x installed and build with the `genThrift` property set, e.g.
 ```bash
@@ -48,17 +47,16 @@ track [issues](https://help.github.com/articles/about-issues/) and create
 If you have not contributed to the project before, please add your details to the `developers`
 section in the top-level [build file](build.gradle).
 
-### Encypting for Travis
+### Encrypting for Travis
 In order to pass secrets to Travis securely for authentication and signing, we need to encrypt them
 first before checking in. The full documentation [here](https://docs.travis-ci.com/user/encryption-keys/)
 for encrypting keys, and [here](https://docs.travis-ci.com/user/encrypting-files/) for encrypting files.
 
 These are the secrets that need to be passed:
 1. [OSSRH](http://central.sonatype.org/pages/ossrh-guide.html) **username** and **password**. These are
-the credentials used to upload artifacts to the Sonatype Nexus Repository, which is used to sync to
-Maven Central
-1. Signing **key ID**, **password**, and **secret key ring file**. These three are used to sign
-artifacts that get created, which is a requirement in order to upload to Maven Central.
+   the credentials used to upload artifacts to the Sonatype Nexus Repository, which is used to sync to Maven Central.
+2. Signing **key ID**, **password**, and **secret key ring file**. These three are used to sign artifacts that get
+   created, which is a requirement in order to upload to Maven Central.
 
 In order to pass these along, first login to Travis:
 ```bash

README.md

@@ -4,30 +4,33 @@ Fast, buffered, hierarchical stats collection in Java. [Go here](https://github.
 
 ## Abstract
 
-Tally provides a common interface for emitting metrics, while letting you not worry about the velocity of metrics emission.
+Tally provides a common interface for emitting metrics, while letting you not worry about the velocity of metrics
+emission.
 
-By default it buffers counters, gauges and histograms at a specified interval but does not buffer timer values.  This is primarily so timer values can have all their values sampled if desired and if not they can be sampled as summaries or histograms independently by a reporter.
+By default, it buffers counters, gauges and histograms at a specified interval but does not buffer timer values. This is
+primarily so timer values can have all their values sampled if desired and if not they can be sampled as summaries or
+histograms independently by a reporter.
 
 ## Structure
 
 - **Scope**: Keeps track of metrics, and their common metadata.
 - **Metrics**: Counters, Gauges, Timers and Histograms.
 - **Reporter**: Implemented by you. Accepts aggregated values from the scope. Forwards the aggregated values to your metrics ingestion pipeline.
 
-### Acquire a Scope
+### Create a `Scope`
 
 ```java
 // Implement as you will
 StatsReporter reporter = new MyStatsReporter();
 
 Map<String, String> tags = new HashMap<>(2, 1);
 tags.put("dc", "east-1");
-tags.put("type", "master");
+tags.put("type","leader");
 
 Scope scope = new RootScopeBuilder()
     .reporter(reporter)
     .tags(tags)
-    .reportEvery(Duration.ofSeconds(1))
+    .reportEvery(Duration.ofSeconds(1));
 ```
 
 ### Get/Create a metric; use it
@@ -43,27 +46,29 @@ queueGauge.update(42);
 
 Use one of the inbuilt reporters or implement your own using the `StatsReporter` interface.
 
-## Example Usage
+## Usage examples
 
 Run the example by running:
 ```bash
 $ ./gradlew run
 ```
+
 This runs the `PrintStatsReporterExample` class in the `tally-example` project.
 
-## Artifacts Published
+## Artifacts publishing
 
 All artifacts are published under the group `com.uber.m3`.
 
-1. `tally-m3`: The tally M3 reporter
-1. `tally-statsd`: The tally StatsD reporter
-1. `tally-core`: tally core functionality that includes interfaces and utilities to report metrics to M3
-1. `tally-example`: Example usages with different reporters
-1. `tally-prometheus`: The tally Prometheus reporter (experimental; see prometheus/README.md)
+1. `tally-m3`: The tally M3 reporter.
+1. `tally-statsd`: The tally StatsD reporter.
+1. `tally-core`: The tally core functionality that includes interfaces and utilities to report metrics to M3.
+1. `tally-example`: Usage examples with different reporters.
+1. `tally-prometheus`: The tally Prometheus reporter (experimental; see prometheus/README.md).
 
 ## Versioning
-We follow semantic versioning outlined [here](http://semver.org/spec/v2.0.0.html). In summary,
-given a version of MAJOR.MINOR.PATCH (e.g. 1.2.0):
+
+We follow semantic versioning outlined [here](http://semver.org/spec/v2.0.0.html). In summary, given a version of
+MAJOR.MINOR.PATCH (e.g. 1.2.0):
 
 - MAJOR version changes are breaking changes to the public API
 - MINOR version changes are backwards-compatible changes that include new functionality

core/src/main/java/com/uber/m3/tally/CapableOf.java

@@ -29,8 +29,8 @@ public class CapableOf implements Capabilities {
     public static final CapableOf REPORTING = new CapableOf(true, false);
     public static final CapableOf REPORTING_TAGGING = new CapableOf(true, true);
 
-    private boolean reporting;
-    private boolean tagging;
+    private final boolean reporting;
+    private final boolean tagging;
 
     public CapableOf(boolean reporting, boolean tagging) {
         this.reporting = reporting;
@@ -69,8 +69,8 @@ public boolean equals(Object other) {
     public int hashCode() {
         int code = 0;
 
-        code = 31 * code + new Boolean(reporting).hashCode();
-        code = 31 * code + new Boolean(tagging).hashCode();
+        code = 31 * code + Boolean.valueOf(reporting).hashCode();
+        code = 31 * code + Boolean.valueOf(tagging).hashCode();
 
         return code;
     }

core/src/main/java/com/uber/m3/tally/CounterImpl.java

@@ -31,7 +31,7 @@ class CounterImpl extends MetricBase implements Counter, Reportable {
     private final AtomicLong prev = new AtomicLong(0);
     private final AtomicLong curr = new AtomicLong(0);
 
-    protected CounterImpl(ScopeImpl scope, String fqn) {
+    CounterImpl(ScopeImpl scope, String fqn) {
         super(fqn);
 
         scope.addToReportingQueue(this);
@@ -42,30 +42,34 @@ public void inc(long delta) {
         curr.getAndAdd(delta);
     }
 
+    @Override
+    public void report(ImmutableMap<String, String> tags, StatsReporter reporter) {
+        long delta = snapshot();
+        if (reporter instanceof SnapshotBasedStatsReporter) {
+            // Always report snapshots.
+            reporter.reportCounter(getQualifiedName(), tags, delta);
+        } else if (delta != 0) {
+            // Only report deltas if they are non-zero. NOTE: we call value() here to update the previous value.
+            reporter.reportCounter(getQualifiedName(), tags, value());
+        }
+    }
+
+    /**
+     * Returns the delta between the current and previous values. NOTE: This method has side effects.
+     */
     long value() {
         long current = curr.get();
         long previous = prev.get();
-
         if (current == previous) {
             return 0;
         }
-
         prev.set(current);
-
         return current - previous;
     }
 
-    @Override
-    public void report(ImmutableMap<String, String> tags, StatsReporter reporter) {
-        long delta = value();
-
-        if (delta == 0) {
-            return;
-        }
-
-        reporter.reportCounter(getQualifiedName(), tags, delta);
-    }
-
+    /**
+     * Returns the difference between the current and previous values without mutating counters.
+     */
     long snapshot() {
         return curr.get() - prev.get();
     }

core/src/main/java/com/uber/m3/tally/CounterSnapshotImpl.java

@@ -28,9 +28,9 @@
  * Default implementation of a {@link CounterSnapshot}.
  */
 class CounterSnapshotImpl implements CounterSnapshot {
-    private String name;
-    private ImmutableMap<String, String> tags;
-    private long value;
+    private final String name;
+    private final ImmutableMap<String, String> tags;
+    private final long value;
 
     CounterSnapshotImpl(String name, ImmutableMap<String, String> tags, long value) {
         this.name = name;

core/src/main/java/com/uber/m3/tally/GaugeImpl.java

@@ -29,10 +29,10 @@
  * Default implementation of a {@link Gauge}.
  */
 class GaugeImpl extends MetricBase implements Gauge, Reportable {
-    private AtomicBoolean updated = new AtomicBoolean(false);
-    private AtomicLong curr = new AtomicLong(0);
+    private final AtomicBoolean updated = new AtomicBoolean(false);
+    private final AtomicLong curr = new AtomicLong(0);
 
-    protected GaugeImpl(ScopeImpl scope, String fqn) {
+    GaugeImpl(ScopeImpl scope, String fqn) {
         super(fqn);
 
         scope.addToReportingQueue(this);
@@ -44,18 +44,14 @@ public void update(double value) {
         updated.set(true);
     }
 
-    double value() {
-        return Double.longBitsToDouble(curr.get());
-    }
-
     @Override
     public void report(ImmutableMap<String, String> tags, StatsReporter reporter) {
         if (updated.getAndSet(false)) {
             reporter.reportGauge(getQualifiedName(), tags, value());
         }
     }
 
-    double snapshot() {
-        return value();
+    double value() {
+        return Double.longBitsToDouble(curr.get());
     }
 }

core/src/main/java/com/uber/m3/tally/GaugeSnapshotImpl.java

@@ -28,9 +28,9 @@
  * Default implementation of a {@link GaugeSnapshot}.
  */
 class GaugeSnapshotImpl implements GaugeSnapshot {
-    private String name;
-    private ImmutableMap<String, String> tags;
-    private double value;
+    private final String name;
+    private final ImmutableMap<String, String> tags;
+    private final double value;
 
     GaugeSnapshotImpl(String name, ImmutableMap<String, String> tags, double value) {
         this.name = name;