From 6bf67f9e0aeb47a6e57b060f81e29a01ca4b91e7 Mon Sep 17 00:00:00 2001 From: Guilherme Trein Date: Fri, 6 Dec 2024 14:59:54 -0500 Subject: [PATCH] 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 | 14 +- README.md | 33 +-- .../java/com/uber/m3/tally/CapableOf.java | 8 +- .../java/com/uber/m3/tally/CounterImpl.java | 34 +-- .../uber/m3/tally/CounterSnapshotImpl.java | 6 +- .../java/com/uber/m3/tally/GaugeImpl.java | 14 +- .../com/uber/m3/tally/GaugeSnapshotImpl.java | 6 +- .../java/com/uber/m3/tally/HistogramImpl.java | 112 +++------ .../uber/m3/tally/HistogramSnapshotImpl.java | 36 +-- .../java/com/uber/m3/tally/MetricBase.java | 1 - .../com/uber/m3/tally/MonotonicClock.java | 124 ++++++++++ .../java/com/uber/m3/tally/Reportable.java | 1 - .../java/com/uber/m3/tally/ScopeBuilder.java | 27 ++- .../java/com/uber/m3/tally/ScopeImpl.java | 132 +++-------- .../m3/tally/SnapshotBasedStatsReporter.java | 134 +++++++++++ .../java/com/uber/m3/tally/SnapshotImpl.java | 8 +- .../java/com/uber/m3/tally/Stopwatch.java | 4 +- .../java/com/uber/m3/tally/TestScope.java | 38 ++- .../java/com/uber/m3/tally/TimerImpl.java | 111 +-------- .../com/uber/m3/tally/TimerSnapshotImpl.java | 20 +- .../java/com/uber/m3/tally/ValueBuckets.java | 6 +- .../com/uber/m3/tally/BucketPairImplTest.java | 13 +- .../java/com/uber/m3/tally/CapableOfTest.java | 24 +- .../com/uber/m3/tally/CounterImplTest.java | 44 ++-- .../java/com/uber/m3/tally/GaugeImplTest.java | 45 ++-- .../com/uber/m3/tally/HistogramImplTest.java | 156 +++++-------- .../m3/tally/MonotonicClockFakeClockTest.java | 75 ++++++ .../java/com/uber/m3/tally/NoopScopeTest.java | 119 +++++----- .../java/com/uber/m3/tally/ScopeImplTest.java | 74 +++--- .../java/com/uber/m3/tally/TestScopeTest.java | 220 +++++++++++++++++- .../com/uber/m3/tally/TestStatsReporter.java | 14 +- .../java/com/uber/m3/tally/TimerImplTest.java | 102 ++++---- .../com/uber/m3/tally/ValueBucketsTest.java | 15 +- .../java/com/uber/m3/util/DurationTest.java | 17 +- .../com/uber/m3/util/ImmutableListTest.java | 16 +- .../com/uber/m3/util/ImmutableMapTest.java | 25 +- .../com/uber/m3/util/ImmutableSetTest.java | 16 +- 37 files changed, 1089 insertions(+), 755 deletions(-) create mode 100644 core/src/main/java/com/uber/m3/tally/MonotonicClock.java create mode 100644 core/src/main/java/com/uber/m3/tally/SnapshotBasedStatsReporter.java create mode 100644 core/src/test/java/com/uber/m3/tally/MonotonicClockFakeClockTest.java diff --git a/DEVELOPER.md b/DEVELOPER.md index 182d194..a48b8c0 100644 --- a/DEVELOPER.md +++ b/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 diff --git a/README.md b/README.md index c087560..b9b5128 100644 --- a/README.md +++ b/README.md @@ -4,9 +4,12 @@ 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 @@ -14,7 +17,7 @@ By default it buffers counters, gauges and histograms at a specified interval bu - **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 @@ -22,12 +25,12 @@ StatsReporter reporter = new MyStatsReporter(); Map 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 diff --git a/core/src/main/java/com/uber/m3/tally/CapableOf.java b/core/src/main/java/com/uber/m3/tally/CapableOf.java index 9997bce..19bd96d 100644 --- a/core/src/main/java/com/uber/m3/tally/CapableOf.java +++ b/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; } diff --git a/core/src/main/java/com/uber/m3/tally/CounterImpl.java b/core/src/main/java/com/uber/m3/tally/CounterImpl.java index 314207a..74d5f02 100644 --- a/core/src/main/java/com/uber/m3/tally/CounterImpl.java +++ b/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 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 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(); } diff --git a/core/src/main/java/com/uber/m3/tally/CounterSnapshotImpl.java b/core/src/main/java/com/uber/m3/tally/CounterSnapshotImpl.java index a9de959..d369c74 100644 --- a/core/src/main/java/com/uber/m3/tally/CounterSnapshotImpl.java +++ b/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 tags; - private long value; + private final String name; + private final ImmutableMap tags; + private final long value; CounterSnapshotImpl(String name, ImmutableMap tags, long value) { this.name = name; diff --git a/core/src/main/java/com/uber/m3/tally/GaugeImpl.java b/core/src/main/java/com/uber/m3/tally/GaugeImpl.java index c65a54c..71d311d 100644 --- a/core/src/main/java/com/uber/m3/tally/GaugeImpl.java +++ b/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,10 +44,6 @@ public void update(double value) { updated.set(true); } - double value() { - return Double.longBitsToDouble(curr.get()); - } - @Override public void report(ImmutableMap tags, StatsReporter reporter) { if (updated.getAndSet(false)) { @@ -55,7 +51,7 @@ public void report(ImmutableMap tags, StatsReporter reporter) { } } - double snapshot() { - return value(); + double value() { + return Double.longBitsToDouble(curr.get()); } } diff --git a/core/src/main/java/com/uber/m3/tally/GaugeSnapshotImpl.java b/core/src/main/java/com/uber/m3/tally/GaugeSnapshotImpl.java index 61fbe3c..b2c5e10 100644 --- a/core/src/main/java/com/uber/m3/tally/GaugeSnapshotImpl.java +++ b/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 tags; - private double value; + private final String name; + private final ImmutableMap tags; + private final double value; GaugeSnapshotImpl(String name, ImmutableMap tags, double value) { this.name = name; diff --git a/core/src/main/java/com/uber/m3/tally/HistogramImpl.java b/core/src/main/java/com/uber/m3/tally/HistogramImpl.java index f9c3369..c041503 100644 --- a/core/src/main/java/com/uber/m3/tally/HistogramImpl.java +++ b/core/src/main/java/com/uber/m3/tally/HistogramImpl.java @@ -24,34 +24,31 @@ import com.uber.m3.util.ImmutableMap; import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; /** * Default implementation of a {@link Histogram}. */ class HistogramImpl extends MetricBase implements Histogram, StopwatchRecorder { + private final MonotonicClock clock; + private final ScopeImpl scope; private final Type type; - private final ImmutableMap tags; - private final ImmutableBuckets specification; // NOTE: Bucket counters are lazily initialized. Since ref updates are atomic in JMM, // no dedicated synchronization is used on the read path, only on the write path private final CounterImpl[] bucketCounters; - private final ScopeImpl scope; - HistogramImpl( + MonotonicClock clock, ScopeImpl scope, String fqn, ImmutableMap tags, Buckets buckets ) { super(fqn); - + this.clock = clock; this.scope = scope; this.type = buckets instanceof DurationBuckets ? Type.DURATION : Type.VALUE; this.tags = tags; @@ -80,9 +77,9 @@ private CounterImpl getOrCreateCounter(int index) { } List bucketsBounds = - this.type == Type.VALUE - ? specification.getValueUpperBounds() - : specification.getDurationUpperBounds(); + this.type == Type.VALUE + ? specification.getValueUpperBounds() + : specification.getDurationUpperBounds(); // To maintain lock granularity we synchronize only on a // particular bucket leveraging bucket's boundary as a sync target @@ -122,81 +119,34 @@ static int toBucketIndex(int binarySearchResult) { @Override public Stopwatch start() { - return new Stopwatch(System.nanoTime(), this); - } - - ImmutableMap getTags() { - return tags; - } - - private Duration getUpperBoundDurationForBucket(int bucketIndex) { - return bucketIndex < specification.getDurationUpperBounds().size() ? specification.getDurationUpperBounds().get(bucketIndex) : Duration.MAX_VALUE; - } - - private Duration getLowerBoundDurationForBucket(int bucketIndex) { - return bucketIndex == 0 ? Duration.MIN_VALUE : specification.getDurationUpperBounds().get(bucketIndex - 1); - } - - private double getUpperBoundValueForBucket(int bucketIndex) { - return bucketIndex < specification.getValueUpperBounds().size() ? specification.getValueUpperBounds().get(bucketIndex) : Double.MAX_VALUE; - } - - private double getLowerBoundValueForBucket(int bucketIndex) { - return bucketIndex == 0 ? Double.MIN_VALUE : specification.getValueUpperBounds().get(bucketIndex - 1); - } - - private long snapshotCounterValue(int index) { - return bucketCounters[index] != null ? bucketCounters[index].snapshot() : 0; - } - - // NOTE: Only used in testing - Map snapshotValues() { - if (type == Type.DURATION) { - return null; - } - - Map values = new HashMap<>(bucketCounters.length, 1); - - for (int i = 0; i < bucketCounters.length; ++i) { - values.put(getUpperBoundValueForBucket(i), snapshotCounterValue(i)); - } - - return values; - } - - Map snapshotDurations() { - if (type == Type.VALUE) { - return null; - } - - Map durations = new HashMap<>(bucketCounters.length, 1); - - for (int i = 0; i < bucketCounters.length; ++i) { - durations.put(getUpperBoundDurationForBucket(i), snapshotCounterValue(i)); - } - - return durations; + return new Stopwatch(clock.nowNanos(), this); } @Override public void recordStopwatch(long stopwatchStart) { - recordDuration(Duration.between(stopwatchStart, System.nanoTime())); + recordDuration(Duration.between(stopwatchStart, clock.nowNanos())); + } + + /** + * Returns the tags associated with this histogram. + */ + ImmutableMap getTags() { + return tags; } - enum Type { + private enum Type { VALUE, DURATION } /** - * Extension of the {@link CounterImpl} adjusting it's reporting procedure - * to adhere to histogram format + * Extension of the {@link CounterImpl} adjusting its reporting procedure to adhere to histogram format. */ - class HistogramBucketCounterImpl extends CounterImpl { + private class HistogramBucketCounterImpl extends CounterImpl { private final int bucketIndex; - protected HistogramBucketCounterImpl(ScopeImpl scope, String fqn, int bucketIndex) { + private HistogramBucketCounterImpl(ScopeImpl scope, String fqn, int bucketIndex) { super(scope, fqn); this.bucketIndex = bucketIndex; @@ -204,20 +154,26 @@ protected HistogramBucketCounterImpl(ScopeImpl scope, String fqn, int bucketInde @Override public void report(ImmutableMap tags, StatsReporter reporter) { - long inc = value(); - if (inc == 0) { - // Nothing to report - return; + long inc = snapshot(); + if (reporter instanceof SnapshotBasedStatsReporter) { + // Always report snapshots. + reportBucket(tags, reporter, inc); + } else if (inc != 0) { + // Only report when there is a change in the counter. NOTE: we call value() here to + // update the previous value. + reportBucket(tags, reporter, value()); } + } + private void reportBucket(ImmutableMap tags, StatsReporter reporter, long inc) { switch (type) { case VALUE: reporter.reportHistogramValueSamples( getQualifiedName(), tags, (Buckets) specification, - getLowerBoundValueForBucket(bucketIndex), - getUpperBoundValueForBucket(bucketIndex), + specification.getValueLowerBoundFor(bucketIndex), + specification.getValueUpperBoundFor(bucketIndex), inc ); break; @@ -226,8 +182,8 @@ public void report(ImmutableMap tags, StatsReporter reporter) { getQualifiedName(), tags, (Buckets) specification, - getLowerBoundDurationForBucket(bucketIndex), - getUpperBoundDurationForBucket(bucketIndex), + specification.getDurationLowerBoundFor(bucketIndex), + specification.getDurationUpperBoundFor(bucketIndex), inc ); break; diff --git a/core/src/main/java/com/uber/m3/tally/HistogramSnapshotImpl.java b/core/src/main/java/com/uber/m3/tally/HistogramSnapshotImpl.java index 7d3be11..8a7c691 100644 --- a/core/src/main/java/com/uber/m3/tally/HistogramSnapshotImpl.java +++ b/core/src/main/java/com/uber/m3/tally/HistogramSnapshotImpl.java @@ -24,26 +24,20 @@ import com.uber.m3.util.ImmutableMap; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; /** * Default implementation of a {@link HistogramSnapshot}. */ class HistogramSnapshotImpl implements HistogramSnapshot { - private String name; - private ImmutableMap tags; - private Map values; - private Map durations; + private final String name; + private final ImmutableMap tags; + private final Map values = new ConcurrentHashMap<>(); + private final Map durations = new ConcurrentHashMap<>(); - HistogramSnapshotImpl( - String name, - ImmutableMap tags, - Map values, - Map durations - ) { + HistogramSnapshotImpl(String name, ImmutableMap tags) { this.name = name; this.tags = tags; - this.values = values; - this.durations = durations; } @Override @@ -58,11 +52,25 @@ public Map tags() { @Override public Map values() { - return values; + return new ImmutableMap<>(values); } @Override public Map durations() { - return durations; + return new ImmutableMap<>(durations); + } + + /** + * Appends a new duration to the current snapshot. We kept the access modifier as default to avoid any external access. + */ + void addDuration(Duration upperBound, long samples) { + durations.put(upperBound, samples); + } + + /** + * Appends a new value to the current snapshot. We kept the access modifier as default to avoid any external access. + */ + void addValue(double upperBound, long samples) { + values.put(upperBound, samples); } } diff --git a/core/src/main/java/com/uber/m3/tally/MetricBase.java b/core/src/main/java/com/uber/m3/tally/MetricBase.java index edf58a9..ab5a559 100644 --- a/core/src/main/java/com/uber/m3/tally/MetricBase.java +++ b/core/src/main/java/com/uber/m3/tally/MetricBase.java @@ -34,5 +34,4 @@ protected MetricBase(String fqn) { final String getQualifiedName() { return fullyQualifiedName; } - } diff --git a/core/src/main/java/com/uber/m3/tally/MonotonicClock.java b/core/src/main/java/com/uber/m3/tally/MonotonicClock.java new file mode 100644 index 0000000..75836a8 --- /dev/null +++ b/core/src/main/java/com/uber/m3/tally/MonotonicClock.java @@ -0,0 +1,124 @@ +// Copyright (c) 2021 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package com.uber.m3.tally; + +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; + +import com.uber.m3.util.Duration; + +/** + * An interface for a monotonic clock. This is specially useful when manipulating time in tests. + * + *

This interface is used in time dependent sensors rather than {@link java.time.Clock} because + * we are only interested in measuring elapsed time and don't want to incur additional complexity + * due to the nature of wall-clocks time (e.g., zones, offsets, etc.). + */ +public interface MonotonicClock { + /** + * Returns the current value of a monotonically increasing clock measured in nanoseconds This + * method can only be used to measure elapsed time and is not related to any other notion of + * system or wall-clock time. The value returned represents nanoseconds since some fixed but + * arbitrary origin time (perhaps in the future, so values may be negative). The same origin is + * used by all invocations of this method in an instance of a Java virtual machine; other virtual + * machine instances are likely to use a different origin. + */ + long nowNanos(); + + /** + * Returns a fake monotonic clock that can be manipulated for testing purposes. + */ + static MonotonicClock.FakeClock fake() { + return FakeClock.create(); + } + + /** + * Returns a monotonic clock based on {@link System#nanoTime()}. + */ + static MonotonicClock system() { + return new SystemClock(); + } + + /** + * A fake monotonic clock that can be manipulated for testing purposes. + */ + class FakeClock implements MonotonicClock { + private final AtomicLong nowNanos = new AtomicLong(0); + private volatile long autoAdvanceNanos = 0; + + // Private constructor to prevent instantiation. Use factory methods instead. + private FakeClock() { + } + + /** + * Creates a new instance of {@link FakeClock}. + */ + static FakeClock create() { + return new FakeClock(); + } + + /** + * Adds nanoseconds to the current measure of time. + */ + public void addNanos(long nanos) { + this.nowNanos.addAndGet(nanos); + } + + /** + * Adds the specified duration to the current measure of time. + */ + public void addDuration(Duration duration) { + addNanos(TimeUnit.MILLISECONDS.toNanos(duration.toMillis())); + } + + /** + * Advances the clock by the given nanoseconds every time {@link #nowNanos()} is called. + */ + public void autoAdvanceNanos(long nanos) { + this.autoAdvanceNanos = nanos; + } + + /** + * Advances the clock by the given duration every time {@link #nowNanos()} is called. + */ + public void autoAdvanceDuration(Duration duration) { + autoAdvanceNanos(TimeUnit.MILLISECONDS.toNanos(duration.toMillis())); + } + + @Override + public long nowNanos() { + return nowNanos.updateAndGet(operand -> operand + autoAdvanceNanos); + } + } + + /** + * A monotonic clock implementation that uses {@link System#nanoTime()} + */ + class SystemClock implements MonotonicClock { + private SystemClock() { + } + + @Override + public long nowNanos() { + return System.nanoTime(); + } + } +} diff --git a/core/src/main/java/com/uber/m3/tally/Reportable.java b/core/src/main/java/com/uber/m3/tally/Reportable.java index 0073ece..34751ad 100644 --- a/core/src/main/java/com/uber/m3/tally/Reportable.java +++ b/core/src/main/java/com/uber/m3/tally/Reportable.java @@ -28,5 +28,4 @@ interface Reportable { void report(ImmutableMap tags, StatsReporter reporter); - } diff --git a/core/src/main/java/com/uber/m3/tally/ScopeBuilder.java b/core/src/main/java/com/uber/m3/tally/ScopeBuilder.java index 08cdd3f..2973ed3 100644 --- a/core/src/main/java/com/uber/m3/tally/ScopeBuilder.java +++ b/core/src/main/java/com/uber/m3/tally/ScopeBuilder.java @@ -55,6 +55,7 @@ public class ScopeBuilder { protected String separator = DEFAULT_SEPARATOR; protected ImmutableMap tags; protected Buckets defaultBuckets = DEFAULT_SCOPE_BUCKETS; + protected MonotonicClock clock = MonotonicClock.system(); private ScheduledExecutorService scheduler; private ScopeImpl.Registry registry; @@ -67,7 +68,7 @@ protected ScopeBuilder(ScheduledExecutorService scheduler, ScopeImpl.Registry re } /** - * Update the reporter + * Updates the reporter. * @param reporter value to update to * @return Builder with new param updated */ @@ -77,7 +78,7 @@ public ScopeBuilder reporter(StatsReporter reporter) { } /** - * Update the prefix + * Updates the prefix. * @param prefix value to update to * @return Builder with new param updated */ @@ -87,7 +88,7 @@ public ScopeBuilder prefix(String prefix) { } /** - * Update the separator + * Updates the separator. * @param separator value to update to * @return Builder with new param updated */ @@ -97,7 +98,7 @@ public ScopeBuilder separator(String separator) { } /** - * Update the tags, cloning the tags map to an ImmutableMap + * Updates the tags, cloning the tags map to an ImmutableMap. * @param tags value to update to * @return Builder with new param updated */ @@ -108,7 +109,7 @@ public ScopeBuilder tags(Map tags) { } /** - * Update the tags. Since this function takes an ImmutableMap, we don't need to clone it + * Updates the tags. Since this function takes an ImmutableMap, we don't need to clone it. * @param tags value to update to * @return Builder with new param updated */ @@ -119,7 +120,7 @@ public ScopeBuilder tags(ImmutableMap tags) { } /** - * Update the defaultBuckets + * Updates the defaultBuckets. * @param defaultBuckets value to update to * @return Builder with new param updated */ @@ -128,6 +129,16 @@ public ScopeBuilder defaultBuckets(Buckets defaultBuckets) { return this; } + /** + * Updates the monotonic clock that should be used when measuring elapsed time. + * @param clock value to update to + * @return Builder with new param updated + */ + public ScopeBuilder clock(MonotonicClock clock) { + this.clock = clock; + return this; + } + // Private build method - clients should rely on `reportEvery` to create root scopes, and // a root scope's `tagged` and `subScope` functions to create subscopes. ScopeImpl build() { @@ -135,7 +146,7 @@ ScopeImpl build() { } /** - * Creates a root scope and starts reporting with the specified interval + * Creates a root scope and starts reporting with the specified interval. * @param interval duration between each report * @return the root scope created */ @@ -144,7 +155,7 @@ public Scope reportEvery(Duration interval) { } /** - * Creates a root scope and starts reporting with the specified interval + * Creates a root scope and starts reporting with the specified interval. * @param interval duration between each report * @param uncaughtExceptionHandler an {@link java.lang.Thread.UncaughtExceptionHandler} that's * called when there's an uncaught exception in the report loop diff --git a/core/src/main/java/com/uber/m3/tally/ScopeImpl.java b/core/src/main/java/com/uber/m3/tally/ScopeImpl.java index 6133957..d40904d 100644 --- a/core/src/main/java/com/uber/m3/tally/ScopeImpl.java +++ b/core/src/main/java/com/uber/m3/tally/ScopeImpl.java @@ -23,7 +23,6 @@ import com.uber.m3.util.ImmutableMap; import javax.annotation.Nullable; -import java.util.ArrayList; import java.util.Collection; import java.util.Map; import java.util.Optional; @@ -35,14 +34,15 @@ * Default {@link Scope} implementation. */ class ScopeImpl implements Scope, TestScope { - private StatsReporter reporter; - private String prefix; - private String separator; - private ImmutableMap tags; - private Buckets defaultBuckets; + private final StatsReporter reporter; + private final String prefix; + private final String separator; + private final ImmutableMap tags; + private final Buckets defaultBuckets; + private final MonotonicClock clock; - private ScheduledExecutorService scheduler; - private Registry registry; + private final ScheduledExecutorService scheduler; + private final Registry registry; private final ConcurrentHashMap counters = new ConcurrentHashMap<>(); private final ConcurrentHashMap gauges = new ConcurrentHashMap<>(); @@ -62,41 +62,42 @@ class ScopeImpl implements Scope, TestScope { this.separator = builder.separator; this.tags = builder.tags; this.defaultBuckets = builder.defaultBuckets; + this.clock = builder.clock; } @Override public Counter counter(String name) { return counters.computeIfAbsent(name, ignored -> - // NOTE: This will called at most once - new CounterImpl(this, fullyQualifiedName(name)) + // NOTE: This will be called at most once + new CounterImpl(this, fullyQualifiedName(name)) ); } @Override public Gauge gauge(String name) { return gauges.computeIfAbsent(name, ignored -> - // NOTE: This will called at most once - new GaugeImpl(this, fullyQualifiedName(name))); + // NOTE: This will be called at most once + new GaugeImpl(this, fullyQualifiedName(name))); } @Override public Timer timer(String name) { // Timers report directly to the {@code StatsReporter}, and therefore not added to reporting queue // i.e. they are not buffered - return timers.computeIfAbsent(name, ignored -> new TimerImpl(fullyQualifiedName(name), tags, reporter)); + return timers.computeIfAbsent(name, ignored -> new TimerImpl(clock, fullyQualifiedName(name), tags, reporter)); } @Override public Histogram histogram(String name, @Nullable Buckets buckets) { return histograms.computeIfAbsent(name, ignored -> - // NOTE: This will be called at most once - new HistogramImpl( - this, - fullyQualifiedName(name), - tags, - Optional.ofNullable(buckets) - .orElse(defaultBuckets) - ) + // NOTE: This will be called at most once + new HistogramImpl( + clock, + this, + fullyQualifiedName(name), + tags, + buckets != null ? buckets : defaultBuckets + ) ); } @@ -141,6 +142,7 @@ void addToReportingQueue(T metric) { /** * Reports using the specified reporter. + * * @param reporter the reporter to report */ void report(StatsReporter reporter) { @@ -156,7 +158,7 @@ static ScopeKey keyForPrefixedStringMap(String prefix, ImmutableMap scopes = new ArrayList<>(); - scopes.add(this); - scopes.addAll(registry.subscopes.values()); - - for (ScopeImpl subscope : scopes) { - ImmutableMap tags = new ImmutableMap.Builder() - .putAll(this.tags) - .putAll(subscope.tags) - .build(); - - for (Map.Entry counter : subscope.counters.entrySet()) { - String name = subscope.fullyQualifiedName(counter.getKey()); - - ScopeKey scopeKey = keyForPrefixedStringMap(name, tags); - - snap.counters().put( - scopeKey, - new CounterSnapshotImpl( - name, - tags, - counter.getValue().snapshot() - ) - ); - } - - for (Map.Entry gauge : subscope.gauges.entrySet()) { - String name = subscope.fullyQualifiedName(gauge.getKey()); - - ScopeKey scopeKey = keyForPrefixedStringMap(name, tags); - - snap.gauges().put( - scopeKey, - new GaugeSnapshotImpl( - name, - tags, - gauge.getValue().snapshot() - ) - ); - } - - for (Map.Entry timer : subscope.timers.entrySet()) { - String name = subscope.fullyQualifiedName(timer.getKey()); - - ScopeKey scopeKey = keyForPrefixedStringMap(name, tags); - - snap.timers().put( - scopeKey, - new TimerSnapshotImpl( - name, - tags, - timer.getValue().snapshot() - ) - ); - } - - for (Map.Entry histogram : subscope.histograms.entrySet()) { - String name = subscope.fullyQualifiedName(histogram.getKey()); - - ScopeKey scopeKey = keyForPrefixedStringMap(name, tags); - - snap.histograms().put( - scopeKey, - new HistogramSnapshotImpl( - name, - tags, - histogram.getValue().snapshotValues(), - histogram.getValue().snapshotDurations() - ) - ); - } + if (!(reporter instanceof SnapshotBasedStatsReporter)) { + throw new IllegalStateException("Snapshots can only be computed when using a SnapshotBasedStatsReporter"); } - - return snap; + // We will create snapshots leveraging the reporting mechanism. + // NOTE: Timers report directly to the reporter, so they are not handled by the report methods here. + report(reporter); + reportLoopIteration(); + return ((SnapshotBasedStatsReporter) reporter).getFlushedSnapshot(); } // Helper function used to create subscopes @@ -277,6 +212,7 @@ protected Scope computeSubscopeIfAbsent(String prefix, ScopeKey key, ImmutableMa return registry.subscopes.computeIfAbsent( key, (k) -> new ScopeBuilder(scheduler, registry) + .clock(clock) .reporter(reporter) .prefix(prefix) .separator(separator) @@ -326,7 +262,7 @@ private void reportUncaughtException(Exception uncaughtException) { } static class Registry { - Map subscopes = new ConcurrentHashMap<>(); + final Map subscopes = new ConcurrentHashMap<>(); } } diff --git a/core/src/main/java/com/uber/m3/tally/SnapshotBasedStatsReporter.java b/core/src/main/java/com/uber/m3/tally/SnapshotBasedStatsReporter.java new file mode 100644 index 0000000..f2eed84 --- /dev/null +++ b/core/src/main/java/com/uber/m3/tally/SnapshotBasedStatsReporter.java @@ -0,0 +1,134 @@ +// Copyright (c) 2023 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package com.uber.m3.tally; + +import static com.uber.m3.tally.ScopeImpl.keyForPrefixedStringMap; + +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; + +import com.uber.m3.util.Duration; +import com.uber.m3.util.ImmutableMap; + +/** + * A stats reporter implementation enabling snapshot recording. + */ +public class SnapshotBasedStatsReporter implements StatsReporter { + private final AtomicReference currentSnapshot = new AtomicReference<>(new SnapshotImpl()); + + // Not synchronized as it's a read-only snapshot. + private volatile Snapshot flushedSnapshot = new SnapshotImpl(); + + /** + * Returns the last flushed snapshot. + */ + Snapshot getFlushedSnapshot() { + return flushedSnapshot; + } + + @Override + public Capabilities capabilities() { + return CapableOf.NONE; + } + + @Override + public void flush() { + currentSnapshot.updateAndGet(snapshot -> { + flushedSnapshot = snapshot; + return new SnapshotImpl(); + }); + } + + @Override + public void close() { + flush(); + } + + @Override + public void reportCounter(String name, Map tags, long value) { + ImmutableMap tagsKey = toImmutable(tags); + ScopeKey scopeKey = keyForPrefixedStringMap(name, tagsKey); + currentSnapshot.get().counters().compute(scopeKey, (k, v) -> new CounterSnapshotImpl(name, tagsKey, value)); + } + + @Override + public void reportGauge(String name, Map tags, double value) { + ImmutableMap tagsKey = toImmutable(tags); + ScopeKey scopeKey = keyForPrefixedStringMap(name, tagsKey); + currentSnapshot.get().gauges().compute(scopeKey, (k, v) -> new GaugeSnapshotImpl(name, tagsKey, value)); + } + + @Override + public void reportTimer(String name, Map tags, Duration interval) { + ImmutableMap tagsKey = toImmutable(tags); + ScopeKey scopeKey = keyForPrefixedStringMap(name, tagsKey); + currentSnapshot.get().timers().computeIfAbsent(scopeKey, k -> new TimerSnapshotImpl(name, tagsKey)); + currentSnapshot.get().timers().computeIfPresent(scopeKey, (k, snapshot) -> { + ((TimerSnapshotImpl) snapshot).addDuration(interval); + return snapshot; + }); + } + + @Override + public void reportHistogramValueSamples(String name, Map tags, Buckets buckets, double bucketLowerBound, double bucketUpperBound, long samples) { + ImmutableMap tagsKey = toImmutable(tags); + ScopeKey scopeKey = keyForPrefixedStringMap(name, tagsKey); + ensureHistogramBucketsFor(name, tagsKey, buckets); + currentSnapshot.get().histograms().computeIfPresent(scopeKey, (k, snapshot) -> { + ((HistogramSnapshotImpl) snapshot).addValue(bucketUpperBound, samples); + return snapshot; + }); + } + + @Override + public void reportHistogramDurationSamples(String name, Map tags, Buckets buckets, Duration bucketLowerBound, Duration bucketUpperBound, long samples) { + ImmutableMap tagsKey = toImmutable(tags); + ScopeKey scopeKey = keyForPrefixedStringMap(name, toImmutable(tags)); + ensureHistogramBucketsFor(name, tagsKey, buckets); + currentSnapshot.get().histograms().computeIfPresent(scopeKey, (k, snapshot) -> { + ((HistogramSnapshotImpl) snapshot).addDuration(bucketUpperBound, samples); + return snapshot; + }); + } + + private void ensureHistogramBucketsFor(String name, ImmutableMap tags, Buckets buckets) { + ScopeKey scopeKey = keyForPrefixedStringMap(name, tags); + currentSnapshot.get().histograms().computeIfAbsent(scopeKey, k -> { + HistogramSnapshotImpl snapshot = new HistogramSnapshotImpl(name, tags); + for (int i = 0; i <= buckets.asValues().length; ++i) { + if (buckets instanceof DurationBuckets) { + snapshot.addDuration(buckets.getDurationUpperBoundFor(i), 0); + } + if (buckets instanceof ValueBuckets) { + snapshot.addValue(buckets.getValueUpperBoundFor(i), 0); + } + } + return snapshot; + }); + } + + private static ImmutableMap toImmutable(Map tags) { + if (tags == null) { + return ImmutableMap.EMPTY; + } + return new ImmutableMap<>(tags); + } +} diff --git a/core/src/main/java/com/uber/m3/tally/SnapshotImpl.java b/core/src/main/java/com/uber/m3/tally/SnapshotImpl.java index d4e98ce..15565f4 100644 --- a/core/src/main/java/com/uber/m3/tally/SnapshotImpl.java +++ b/core/src/main/java/com/uber/m3/tally/SnapshotImpl.java @@ -27,10 +27,10 @@ * Default implementation of a {@link Snapshot}. */ class SnapshotImpl implements Snapshot { - Map counters = new ConcurrentHashMap<>(); - Map gauges = new ConcurrentHashMap<>(); - Map timers = new ConcurrentHashMap<>(); - Map histograms = new ConcurrentHashMap<>(); + private final Map counters = new ConcurrentHashMap<>(); + private final Map gauges = new ConcurrentHashMap<>(); + private final Map timers = new ConcurrentHashMap<>(); + private final Map histograms = new ConcurrentHashMap<>(); @Override public Map counters() { diff --git a/core/src/main/java/com/uber/m3/tally/Stopwatch.java b/core/src/main/java/com/uber/m3/tally/Stopwatch.java index f4c8f51..369bfbf 100644 --- a/core/src/main/java/com/uber/m3/tally/Stopwatch.java +++ b/core/src/main/java/com/uber/m3/tally/Stopwatch.java @@ -27,8 +27,8 @@ * of the stopwatch are comparable with one another. */ public class Stopwatch { - private long startNanos; - private StopwatchRecorder recorder; + private final long startNanos; + private final StopwatchRecorder recorder; /** * Creates a stopwatch. diff --git a/core/src/main/java/com/uber/m3/tally/TestScope.java b/core/src/main/java/com/uber/m3/tally/TestScope.java index af6c5b0..d00b59c 100644 --- a/core/src/main/java/com/uber/m3/tally/TestScope.java +++ b/core/src/main/java/com/uber/m3/tally/TestScope.java @@ -34,8 +34,19 @@ public interface TestScope extends Scope { */ static TestScope create() { return new RootScopeBuilder() - .reporter(new NullStatsReporter()) - .build(); + .reporter(new SnapshotBasedStatsReporter()) + .build(); + } + + /** + * Creates a new TestScope that adds the ability to manipulate time and take snapshots of + * metrics emitted to it. + */ + static TestScope create(MonotonicClock clock) { + return new RootScopeBuilder() + .clock(clock) + .reporter(new SnapshotBasedStatsReporter()) + .build(); } /** @@ -44,14 +55,27 @@ static TestScope create() { */ static TestScope create(String prefix, Map tags) { return new RootScopeBuilder() - .prefix(prefix) - .tags(tags) - .reporter(new NullStatsReporter()) - .build(); + .prefix(prefix) + .tags(tags) + .reporter(new SnapshotBasedStatsReporter()) + .build(); + } + + /** + * Creates a new TestScope with given prefix/tags that adds the ability to manipulate time and + * take snapshots of metrics emitted to it. + */ + static TestScope create(String prefix, Map tags, MonotonicClock clock) { + return new RootScopeBuilder() + .clock(clock) + .prefix(prefix) + .tags(tags) + .reporter(new SnapshotBasedStatsReporter()) + .build(); } /** - * Snapshot returns a copy of all values since the last report execution + * Snapshot returns a copy of all values since the last report execution. * This is an expensive operation and should only be used for testing purposes. */ Snapshot snapshot(); diff --git a/core/src/main/java/com/uber/m3/tally/TimerImpl.java b/core/src/main/java/com/uber/m3/tally/TimerImpl.java index 29c2613..a275b1e 100644 --- a/core/src/main/java/com/uber/m3/tally/TimerImpl.java +++ b/core/src/main/java/com/uber/m3/tally/TimerImpl.java @@ -23,31 +23,20 @@ import com.uber.m3.util.Duration; import com.uber.m3.util.ImmutableMap; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReadWriteLock; -import java.util.concurrent.locks.ReentrantReadWriteLock; - /** * Default implementation of a {@link Timer}. */ class TimerImpl implements Timer, StopwatchRecorder { - private String name; - private ImmutableMap tags; - private StatsReporter reporter; - private Values unreported = new Values(); + private final MonotonicClock clock; + private final String name; + private final ImmutableMap tags; + private final StatsReporter reporter; - TimerImpl(String name, ImmutableMap tags, StatsReporter reporter) { + TimerImpl(MonotonicClock clock, String name, ImmutableMap tags, StatsReporter reporter) { + this.clock = clock; this.name = name; this.tags = tags; - - if (reporter == null) { - this.reporter = new NoReporterSink(); - } else { - this.reporter = reporter; - } + this.reporter = reporter; } @Override @@ -57,7 +46,7 @@ public void record(Duration interval) { @Override public Stopwatch start() { - return new Stopwatch(System.nanoTime(), this); + return new Stopwatch(clock.nowNanos(), this); } /** @@ -66,88 +55,6 @@ public Stopwatch start() { */ @Override public void recordStopwatch(long stopwatchStart) { - record(Duration.between(stopwatchStart, System.nanoTime())); - } - - Duration[] snapshot() { - unreported.readLock().lock(); - - Duration[] snap = new Duration[unreported.getValues().size()]; - - for (int i = 0; i < unreported.getValues().size(); i++) { - snap[i] = unreported.getValues().get(i); - } - - unreported.readLock().unlock(); - - return snap; - } - - static class Values { - // Using a ReadWriteLock here to protect against multithreaded reads - // and writes separately. In other places, synchronized blocks are used - // instead as this separation is not needed e.g. we only lock a - // ConcurrentHashMap when doing writes and not for reads. - private final ReadWriteLock rwlock = new ReentrantReadWriteLock(); - private List values = new ArrayList<>(); - - Lock readLock() { - return rwlock.readLock(); - } - - Lock writeLock() { - return rwlock.writeLock(); - } - - public List getValues() { - return values; - } + record(Duration.between(stopwatchStart, clock.nowNanos())); } - - class NoReporterSink implements StatsReporter { - @Override - public Capabilities capabilities() { - return CapableOf.REPORTING_TAGGING; - } - - @Override - public void flush() { - // No-op - } - - @Override - public void close() { - // No-op - } - - @Override - public void reportCounter(String name, Map tags, long value) { - throw new UnsupportedOperationException(); - } - - @Override - public void reportGauge(String name, Map tags, double value) { - throw new UnsupportedOperationException(); - } - - @Override - public void reportTimer(String name, Map tags, Duration interval) { - unreported.writeLock().lock(); - - unreported.getValues().add(interval); - - unreported.writeLock().unlock(); - } - - @Override - public void reportHistogramValueSamples(String name, Map tags, Buckets buckets, double bucketLowerBound, double bucketUpperBound, long samples) { - throw new UnsupportedOperationException(); - } - - @Override - public void reportHistogramDurationSamples(String name, Map tags, Buckets buckets, Duration bucketLowerBound, Duration bucketUpperBound, long samples) { - throw new UnsupportedOperationException(); - } - } - } diff --git a/core/src/main/java/com/uber/m3/tally/TimerSnapshotImpl.java b/core/src/main/java/com/uber/m3/tally/TimerSnapshotImpl.java index 20720cf..a18f8d3 100644 --- a/core/src/main/java/com/uber/m3/tally/TimerSnapshotImpl.java +++ b/core/src/main/java/com/uber/m3/tally/TimerSnapshotImpl.java @@ -23,20 +23,21 @@ import com.uber.m3.util.Duration; import com.uber.m3.util.ImmutableMap; +import java.util.List; import java.util.Map; +import java.util.concurrent.CopyOnWriteArrayList; /** * Default implementation of a {@link TimerSnapshot}. */ class TimerSnapshotImpl implements TimerSnapshot { - private String name; - private ImmutableMap tags; - private Duration[] values; + private final String name; + private final ImmutableMap tags; + private final List values = new CopyOnWriteArrayList<>(); - TimerSnapshotImpl(String name, ImmutableMap tags, Duration[] values) { + TimerSnapshotImpl(String name, ImmutableMap tags) { this.name = name; this.tags = tags; - this.values = values; } @Override @@ -51,6 +52,13 @@ public Map tags() { @Override public Duration[] values() { - return values; + return values.toArray(new Duration[0]); + } + + /** + * Appends a new duration to the current snapshot. We kept the access modifier as default to avoid any external access. + */ + void addDuration(Duration duration) { + values.add(duration); } } diff --git a/core/src/main/java/com/uber/m3/tally/ValueBuckets.java b/core/src/main/java/com/uber/m3/tally/ValueBuckets.java index dc6051d..9d1cc3b 100644 --- a/core/src/main/java/com/uber/m3/tally/ValueBuckets.java +++ b/core/src/main/java/com/uber/m3/tally/ValueBuckets.java @@ -124,10 +124,10 @@ public static ValueBuckets linear(double start, double width, int numBuckets) { /** * Helper function to create {@link ValueBuckets} of exponential spacing. - * @param start the starting bucket's value} + * @param start the starting bucket's value * @param factor the factor between each bucket * @param numBuckets the number of buckets to create - * @return {@link ValueBuckets} of the specified paramters + * @return {@link ValueBuckets} of the specified parameters */ public static ValueBuckets exponential(double start, double factor, int numBuckets) { if (numBuckets <= 0) { @@ -155,7 +155,7 @@ public static ValueBuckets exponential(double start, double factor, int numBucke * Helper function to create {@link ValueBuckets} with custom buckets. * * @param sortedBucketUpperValues sorted (ascending order) values of bucket's upper bound - * @return {@link ValueBuckets} of the specified paramters + * @return {@link ValueBuckets} of the specified parameters */ public static ValueBuckets custom(double... sortedBucketUpperValues) { if (sortedBucketUpperValues == null || sortedBucketUpperValues.length == 0) { diff --git a/core/src/test/java/com/uber/m3/tally/BucketPairImplTest.java b/core/src/test/java/com/uber/m3/tally/BucketPairImplTest.java index 3224729..05a0355 100644 --- a/core/src/test/java/com/uber/m3/tally/BucketPairImplTest.java +++ b/core/src/test/java/com/uber/m3/tally/BucketPairImplTest.java @@ -23,10 +23,7 @@ import com.uber.m3.util.Duration; import org.junit.Test; -import static org.junit.Assert.assertArrayEquals; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; public class BucketPairImplTest { @Test @@ -97,11 +94,11 @@ public void equalsHashCode() { BucketPair bucketPair = BucketPairImpl.bucketPairs(null)[0]; BucketPair sameBucketPair = BucketPairImpl.bucketPairs(null)[0]; - assertTrue(bucketPair.equals(sameBucketPair)); - assertTrue(bucketPair.equals(bucketPair)); + assertEquals(bucketPair, sameBucketPair); + assertEquals(bucketPair, bucketPair); assertEquals(bucketPair.hashCode(), sameBucketPair.hashCode()); - assertFalse(bucketPair.equals(null)); - assertFalse(bucketPair.equals(9)); + assertNotEquals(null, bucketPair); + assertNotEquals(9, bucketPair); } } diff --git a/core/src/test/java/com/uber/m3/tally/CapableOfTest.java b/core/src/test/java/com/uber/m3/tally/CapableOfTest.java index 9b522aa..326b8cf 100644 --- a/core/src/test/java/com/uber/m3/tally/CapableOfTest.java +++ b/core/src/test/java/com/uber/m3/tally/CapableOfTest.java @@ -20,15 +20,13 @@ package com.uber.m3.tally; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - import org.junit.Test; +import static org.junit.Assert.*; + public class CapableOfTest { @Test - public void capabilities() throws Exception { + public void capabilities() { Capabilities capabilities = new CapableOf(false, true); assertFalse(capabilities.reporting()); @@ -45,16 +43,16 @@ public void capabilities() throws Exception { } @Test - public void equalsHashCode() throws Exception { - assertFalse(CapableOf.NONE.equals(null)); - assertFalse(CapableOf.NONE.equals(9)); + public void equalsHashCode() { + assertNotEquals(null, CapableOf.NONE); + assertNotEquals(9, CapableOf.NONE); - assertFalse(CapableOf.NONE.equals(CapableOf.REPORTING)); - assertFalse(new CapableOf(true, false).equals(new CapableOf(false, true))); + assertNotEquals(CapableOf.NONE, CapableOf.REPORTING); + assertNotEquals(new CapableOf(true, false), new CapableOf(false, true)); - assertTrue(CapableOf.REPORTING.equals(CapableOf.REPORTING)); - assertTrue(CapableOf.REPORTING.equals(new CapableOf(true, false))); + assertEquals(CapableOf.REPORTING, CapableOf.REPORTING); + assertEquals(CapableOf.REPORTING, new CapableOf(true, false)); assertEquals(CapableOf.REPORTING.hashCode(), new CapableOf(true, false).hashCode()); - assertTrue(new CapableOf(false, false).equals(new CapableOf(false, false))); + assertEquals(new CapableOf(false, false), new CapableOf(false, false)); } } diff --git a/core/src/test/java/com/uber/m3/tally/CounterImplTest.java b/core/src/test/java/com/uber/m3/tally/CounterImplTest.java index 5719e72..594218c 100644 --- a/core/src/test/java/com/uber/m3/tally/CounterImplTest.java +++ b/core/src/test/java/com/uber/m3/tally/CounterImplTest.java @@ -20,29 +20,20 @@ package com.uber.m3.tally; -import org.junit.Before; +import com.uber.m3.util.ImmutableMap; import org.junit.Test; +import static com.uber.m3.tally.ScopeImpl.keyForPrefixedStringMap; import static org.junit.Assert.assertEquals; public class CounterImplTest { - private TestStatsReporter reporter; - private CounterImpl counter; - private ScopeImpl scope; - - @Before - public void setUp() { - reporter = new TestStatsReporter(); - scope = - new ScopeBuilder(null, new ScopeImpl.Registry()) - .reporter(reporter) - .build(); - - counter = new CounterImpl(scope, "counter"); - } + private final TestStatsReporter reporter = new TestStatsReporter(); + private final ScopeImpl scope = new ScopeBuilder(null, new ScopeImpl.Registry()).reporter(reporter).build(); @Test public void inc() { + CounterImpl counter = new CounterImpl(scope, "counter"); + counter.inc(1); counter.report(null, reporter); assertEquals(1, reporter.nextCounterVal()); @@ -68,6 +59,8 @@ public void inc() { @Test public void value() { + CounterImpl counter = new CounterImpl(scope, "counter"); + assertEquals(0, counter.value()); counter.inc(10); @@ -82,15 +75,24 @@ public void value() { @Test public void snapshot() { - assertEquals(0, counter.snapshot()); + ScopeImpl scope = new ScopeBuilder(null, new ScopeImpl.Registry()).reporter(new SnapshotBasedStatsReporter()).build(); + Counter counter = new CounterImpl(scope, "counter"); - counter.inc(1); - assertEquals(1, counter.snapshot()); - assertEquals(1, counter.snapshot()); + Snapshot snapshot1 = scope.snapshot(); + assertEquals(0, getSnapshot(snapshot1, "counter").value()); counter.inc(1); + Snapshot snapshot2 = scope.snapshot(); + assertEquals(1, getSnapshot(snapshot2, "counter").value()); + counter.inc(1); - assertEquals(3, counter.snapshot()); - assertEquals(3, counter.snapshot()); + counter.inc(2); + Snapshot snapshot3 = scope.snapshot(); + assertEquals(4, getSnapshot(snapshot3, "counter").value()); + } + + private static CounterSnapshot getSnapshot(Snapshot snapshot, String name) { + ScopeKey key = keyForPrefixedStringMap(name, ImmutableMap.EMPTY); + return snapshot.counters().get(key); } } diff --git a/core/src/test/java/com/uber/m3/tally/GaugeImplTest.java b/core/src/test/java/com/uber/m3/tally/GaugeImplTest.java index 54524c8..40f6e35 100644 --- a/core/src/test/java/com/uber/m3/tally/GaugeImplTest.java +++ b/core/src/test/java/com/uber/m3/tally/GaugeImplTest.java @@ -20,32 +20,21 @@ package com.uber.m3.tally; -import org.junit.Before; +import com.uber.m3.util.ImmutableMap; import org.junit.Test; +import static com.uber.m3.tally.ScopeImpl.keyForPrefixedStringMap; import static org.junit.Assert.assertEquals; public class GaugeImplTest { - private final double EPSILON = 1e-10; - - private TestStatsReporter reporter; - private GaugeImpl gauge; - - private ScopeImpl scope; - - @Before - public void setUp() { - reporter = new TestStatsReporter(); - scope = - new ScopeBuilder(null, new ScopeImpl.Registry()) - .reporter(reporter) - .build(); - - gauge = new GaugeImpl(scope, "gauge"); - } + private static final double EPSILON = 1e-10; + private final TestStatsReporter reporter = new TestStatsReporter(); + private final ScopeImpl scope = new ScopeBuilder(null, new ScopeImpl.Registry()).reporter(reporter).build(); @Test public void update() { + GaugeImpl gauge = new GaugeImpl(scope, "gauge"); + gauge.update(42); gauge.report(null, reporter); assertEquals(42, reporter.nextGaugeVal(), EPSILON); @@ -67,6 +56,8 @@ public void update() { @Test public void value() { + GaugeImpl gauge = new GaugeImpl(scope, "gauge"); + assertEquals(0, gauge.value(), EPSILON); gauge.update(55); @@ -79,11 +70,25 @@ public void value() { @Test public void snapshot() { + ScopeImpl scope = + new ScopeBuilder(null, new ScopeImpl.Registry()) + .reporter(new SnapshotBasedStatsReporter()) + .build(); + Gauge gauge = new GaugeImpl(scope, "gauge"); + gauge.update(70); - assertEquals(70, gauge.snapshot(), EPSILON); + Snapshot snapshot = scope.snapshot(); + assertEquals(70, getSnapshot(snapshot, "gauge").value(), EPSILON); gauge.update(71); gauge.update(72); - assertEquals(72, gauge.snapshot(), EPSILON); + Snapshot snapshot1 = scope.snapshot(); + + assertEquals(72, getSnapshot(snapshot1, "gauge").value(), EPSILON); + } + + private static GaugeSnapshot getSnapshot(Snapshot snapshot, String name) { + ScopeKey key = keyForPrefixedStringMap(name, ImmutableMap.EMPTY); + return snapshot.gauges().get(key); } } diff --git a/core/src/test/java/com/uber/m3/tally/HistogramImplTest.java b/core/src/test/java/com/uber/m3/tally/HistogramImplTest.java index c0ad225..9a72a84 100644 --- a/core/src/test/java/com/uber/m3/tally/HistogramImplTest.java +++ b/core/src/test/java/com/uber/m3/tally/HistogramImplTest.java @@ -22,52 +22,33 @@ import com.uber.m3.util.Duration; import com.uber.m3.util.ImmutableMap; -import org.junit.Before; import org.junit.Test; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.stream.Collectors; import java.util.stream.IntStream; +import static com.uber.m3.tally.ScopeImpl.keyForPrefixedStringMap; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; public class HistogramImplTest { - private static final double[] BUCKETS = new double[] { + private static final double[] BUCKETS = new double[]{ 1.0, 5.0, 10.0, 15.0, 20.0, 25.0, 30.0, 40.0, 50.0, 60.0, 70.0, 80.0, 90.0, 100.0, 125.0, 150.0, 175.0, 200.0, 225.0, 250.0, 300.0, 350.0, 400.0, 450.0, 500.0, 550.0, 600.0, 650.0, 700.0, 750.0, 800.0, 850.0, 900.0, 950.0, 1000.0 }; - private TestStatsReporter reporter; - private ScopeImpl scope; - - private HistogramImpl histogram; - - @Before - public void setUp() { - reporter = new TestStatsReporter(); - scope = - new ScopeBuilder(null, new ScopeImpl.Registry()) - .reporter(reporter) - .build(); - } + private final MonotonicClock.FakeClock clock = MonotonicClock.fake(); + private final TestStatsReporter reporter = new TestStatsReporter(); + private final ScopeImpl scope = new ScopeBuilder(null, new ScopeImpl.Registry()).clock(clock).reporter(reporter).build(); @Test public void test() { - histogram = new HistogramImpl(scope, "histogram", ImmutableMap.EMPTY, ValueBuckets.custom(BUCKETS)); + HistogramImpl histogram = new HistogramImpl(clock, scope, "histogram", ImmutableMap.EMPTY, ValueBuckets.custom(BUCKETS)); double maxUpperBound = BUCKETS[BUCKETS.length - 1]; - List values = - IntStream.range(0, 10000) - .mapToDouble(ignored -> Math.random() * (maxUpperBound + 1)) - .mapToObj(a -> a) - .collect(Collectors.toList()); + List values = IntStream.range(0, 10000).mapToDouble(ignored -> Math.random() * (maxUpperBound + 1)).boxed().collect(Collectors.toList()); Map expected = new HashMap<>(); @@ -100,14 +81,7 @@ public void test() { @Test public void recordValue() { Buckets buckets = ValueBuckets.linear(0, 10, 10); - - histogram = - new HistogramImpl( - scope, - "", - null, - buckets - ); + HistogramImpl histogram = new HistogramImpl(clock, scope, "", null, buckets); List log = new ArrayList<>(); @@ -126,22 +100,15 @@ public void recordValue() { // Report will actually be fulfilled through scope scope.report(reporter); - assertEquals(new Long(3L), reporter.getValueSamples().get(10d)); - assertEquals(new Long(5L), reporter.getValueSamples().get(60d)); + assertEquals(Long.valueOf(3L), reporter.getValueSamples().get(10d)); + assertEquals(Long.valueOf(5L), reporter.getValueSamples().get(60d)); assertEquals(buckets, reporter.getBuckets()); } @Test public void recordDuration() { Buckets buckets = DurationBuckets.linear(Duration.ZERO, Duration.ofMillis(10), 10); - - histogram = - new HistogramImpl( - scope, - "", - null, - buckets - ); + HistogramImpl histogram = new HistogramImpl(clock, scope, "", null, buckets); for (int i = 0; i < 3; i++) { histogram.recordDuration(Duration.ofMillis(Math.random() * 10)); @@ -154,44 +121,34 @@ public void recordDuration() { // Report will actually be fulfilled through scope scope.report(reporter); - assertEquals(new Long(3L), reporter.getDurationSamples().get(Duration.ofMillis(10))); - assertEquals(new Long(5L), reporter.getDurationSamples().get(Duration.ofMillis(60))); + assertEquals(Long.valueOf(3L), reporter.getDurationSamples().get(Duration.ofMillis(10))); + assertEquals(Long.valueOf(5L), reporter.getDurationSamples().get(Duration.ofMillis(60))); assertEquals(buckets, reporter.getBuckets()); } @Test public void recordStopwatch() { Buckets buckets = DurationBuckets.linear(Duration.ZERO, Duration.ofMillis(10), 10); - - histogram = - new HistogramImpl( - scope, - "", - null, - buckets - ); + HistogramImpl histogram = new HistogramImpl(clock, scope, "", null, buckets); Stopwatch stopwatch = histogram.start(); - assertNotNull(stopwatch); + clock.addDuration(Duration.ofMillis(5)); stopwatch.stop(); // Report will actually be fulfilled through scope scope.report(reporter); - assertEquals(new Long(1L), reporter.getDurationSamples().get(Duration.ofMillis(10))); + assertEquals(Long.valueOf(1L), reporter.getDurationSamples().get(Duration.ofMillis(10))); } @Test public void snapshotValues() { + ScopeImpl scope = + new ScopeBuilder(null, new ScopeImpl.Registry()) + .reporter(new SnapshotBasedStatsReporter()) + .build(); Buckets buckets = ValueBuckets.linear(0, 10, 10); - - histogram = - new HistogramImpl( - scope, - "", - null, - buckets - ); + HistogramImpl histogram = new HistogramImpl(clock, scope, "histogram", null, buckets); for (int i = 0; i < 3; i++) { histogram.recordValue(Math.random() * 10); @@ -214,42 +171,37 @@ public void snapshotValues() { expectedMap.put(90d, 0L); expectedMap.put(Double.MAX_VALUE, 0L); - assertEquals(expectedMap, histogram.snapshotValues()); + Snapshot snapshot = scope.snapshot(); + assertEquals(expectedMap, getSnapshot(snapshot, "histogram").values()); } @Test public void snapshotValuesIsIdempotent() { + ScopeImpl scope = + new ScopeBuilder(null, new ScopeImpl.Registry()) + .reporter(new SnapshotBasedStatsReporter()) + .build(); ValueBuckets buckets = ValueBuckets.custom(0, 10); - - histogram = - new HistogramImpl( - scope, - "", - null, - buckets - ); + HistogramImpl histogram = new HistogramImpl(clock, scope, "histogram", null, buckets); histogram.recordValue(5); + Snapshot snapshot1 = scope.snapshot(); - Map snapshot = histogram.snapshotValues(); - assertEquals(1, snapshot.get(10D).longValue()); + assertEquals(1, getSnapshot(snapshot1, "histogram").values().get(10D).longValue()); - // snapshot again to ensure the first snapshot didn't mutate internal state - snapshot = histogram.snapshotValues(); - assertEquals(1, snapshot.get(10D).longValue()); + // Snapshot again to ensure the first snapshot didn't mutate internal state. + Snapshot snapshot2 = scope.snapshot(); + assertEquals(1, getSnapshot(snapshot2, "histogram").values().get(10D).longValue()); } @Test public void snapshotDurations() { + ScopeImpl scope = + new ScopeBuilder(null, new ScopeImpl.Registry()) + .reporter(new SnapshotBasedStatsReporter()) + .build(); Buckets buckets = DurationBuckets.linear(Duration.ZERO, Duration.ofMillis(10), 5); - - histogram = - new HistogramImpl( - scope, - "", - null, - buckets - ); + HistogramImpl histogram = new HistogramImpl(clock, scope, "histogram", null, buckets); for (int i = 0; i < 3; i++) { histogram.recordDuration(Duration.ofMillis(Math.random() * 10)); @@ -267,28 +219,30 @@ public void snapshotDurations() { expectedMap.put(Duration.ofMillis(40d), 0L); expectedMap.put(Duration.MAX_VALUE, 5L); - assertEquals(expectedMap, histogram.snapshotDurations()); + Snapshot snapshot = scope.snapshot(); + assertEquals(expectedMap, getSnapshot(snapshot, "histogram").durations()); } @Test public void snapshotDurationsIsIdempotent() { + ScopeImpl scope = + new ScopeBuilder(null, new ScopeImpl.Registry()) + .reporter(new SnapshotBasedStatsReporter()) + .build(); DurationBuckets buckets = DurationBuckets.custom(Duration.ofMillis(0), Duration.ofMillis(10)); - - histogram = - new HistogramImpl( - scope, - "", - null, - buckets - ); + HistogramImpl histogram = new HistogramImpl(clock, scope, "histogram", null, buckets); histogram.recordDuration(Duration.ofMillis(5)); + Snapshot snapshot1 = scope.snapshot(); + assertEquals(1, getSnapshot(snapshot1, "histogram").durations().get(Duration.ofMillis(10)).longValue()); - Map snapshot = histogram.snapshotDurations(); - assertEquals(1, snapshot.get(Duration.ofMillis(10)).longValue()); + // Snapshot again to ensure the first snapshot didn't mutate internal state. + Snapshot snapshot2 = scope.snapshot(); + assertEquals(1, getSnapshot(snapshot2, "histogram").durations().get(Duration.ofMillis(10)).longValue()); + } - // snapshot again to ensure the first snapshot didn't mutate internal state - snapshot = histogram.snapshotDurations(); - assertEquals(1, snapshot.get(Duration.ofMillis(10)).longValue()); + private static HistogramSnapshot getSnapshot(Snapshot snapshot, String name) { + ScopeKey key = keyForPrefixedStringMap(name, ImmutableMap.EMPTY); + return snapshot.histograms().get(key); } } diff --git a/core/src/test/java/com/uber/m3/tally/MonotonicClockFakeClockTest.java b/core/src/test/java/com/uber/m3/tally/MonotonicClockFakeClockTest.java new file mode 100644 index 0000000..103ba2f --- /dev/null +++ b/core/src/test/java/com/uber/m3/tally/MonotonicClockFakeClockTest.java @@ -0,0 +1,75 @@ +// Copyright (c) 2021 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package com.uber.m3.tally; + +import com.uber.m3.util.Duration; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +public class MonotonicClockFakeClockTest { + + @Test + public void testAddNanos() { + MonotonicClock.FakeClock clock = MonotonicClock.fake(); + + assertEquals(0, clock.nowNanos()); + clock.addNanos(100); + assertEquals(100, clock.nowNanos()); + } + + @Test + public void testAddDuration() { + MonotonicClock.FakeClock clock = MonotonicClock.fake(); + + assertEquals(0, clock.nowNanos()); + clock.addDuration(Duration.ofMillis(10)); + assertEquals(10_000_000, clock.nowNanos()); + } + + @Test + public void testAutoAdvanceNanos() { + MonotonicClock.FakeClock clock = MonotonicClock.fake(); + + clock.autoAdvanceNanos(10); + assertEquals(10, clock.nowNanos()); + assertEquals(20, clock.nowNanos()); + assertEquals(30, clock.nowNanos()); + + clock.autoAdvanceNanos(20); + assertEquals(50, clock.nowNanos()); + assertEquals(70, clock.nowNanos()); + } + + @Test + public void testAutoAdvanceDuration() { + MonotonicClock.FakeClock clock = MonotonicClock.fake(); + + clock.autoAdvanceDuration(Duration.ofMillis(10)); + assertEquals(10_000_000, clock.nowNanos()); + assertEquals(20_000_000, clock.nowNanos()); + assertEquals(30_000_000, clock.nowNanos()); + + clock.autoAdvanceDuration(Duration.ofMillis(20)); + assertEquals(50_000_000, clock.nowNanos()); + assertEquals(70_000_000, clock.nowNanos()); + } +} diff --git a/core/src/test/java/com/uber/m3/tally/NoopScopeTest.java b/core/src/test/java/com/uber/m3/tally/NoopScopeTest.java index 06c9367..0162b7e 100644 --- a/core/src/test/java/com/uber/m3/tally/NoopScopeTest.java +++ b/core/src/test/java/com/uber/m3/tally/NoopScopeTest.java @@ -20,12 +20,13 @@ package com.uber.m3.tally; +import org.junit.Test; + import java.util.HashMap; import java.util.Map; -import org.hamcrest.CoreMatchers; -import org.junit.Assert; -import org.junit.Test; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.junit.Assert.assertThat; public class NoopScopeTest { @@ -34,15 +35,15 @@ public void testCounter() { NoopScope noopScope = new NoopScope(); Counter noopCounter1 = noopScope.counter("test1"); Counter noopCounter2 = noopScope.counter("test2"); - Assert.assertThat( - "same noop counter returned", - noopCounter1 == noopCounter2, - CoreMatchers.equalTo(true) + assertThat( + "same noop counter returned", + noopCounter1 == noopCounter2, + equalTo(true) ); - Assert.assertThat( - "noop counter", - noopCounter1 == NoopScope.NOOP_COUNTER, - CoreMatchers.equalTo(true) + assertThat( + "noop counter", + noopCounter1 == NoopScope.NOOP_COUNTER, + equalTo(true) ); } @@ -51,15 +52,15 @@ public void testGauge() { NoopScope noopScope = new NoopScope(); Gauge noopGauge1 = noopScope.gauge("test1"); Gauge noopGauge2 = noopScope.gauge("test2"); - Assert.assertThat( - "same noop gauge returned", - noopGauge1 == noopGauge2, - CoreMatchers.equalTo(true) + assertThat( + "same noop gauge returned", + noopGauge1 == noopGauge2, + equalTo(true) ); - Assert.assertThat( - "noop gauge", - noopGauge1 == NoopScope.NOOP_GAUGE, - CoreMatchers.equalTo(true) + assertThat( + "noop gauge", + noopGauge1 == NoopScope.NOOP_GAUGE, + equalTo(true) ); } @@ -68,20 +69,20 @@ public void testTimer() { NoopScope noopScope = new NoopScope(); Timer noopTimer1 = noopScope.timer("test1"); Timer noopTimer2 = noopScope.timer("test2"); - Assert.assertThat( - "same noop timer returned", - noopTimer1 == noopTimer2, - CoreMatchers.equalTo(true) + assertThat( + "same noop timer returned", + noopTimer1 == noopTimer2, + equalTo(true) ); - Assert.assertThat( - "noop timer", - noopTimer1 == NoopScope.NOOP_TIMER, - CoreMatchers.equalTo(true) + assertThat( + "noop timer", + noopTimer1 == NoopScope.NOOP_TIMER, + equalTo(true) ); - Assert.assertThat( - "noop stopwatch", - noopTimer1.start() == NoopScope.NOOP_STOPWATCH, - CoreMatchers.equalTo(true) + assertThat( + "noop stopwatch", + noopTimer1.start() == NoopScope.NOOP_STOPWATCH, + equalTo(true) ); } @@ -91,15 +92,15 @@ public void testHistogram() { Histogram noopHistogram1 = noopScope.histogram("test1", ValueBuckets.custom(1, 2, 3)); Histogram noopHistogram2 = noopScope.histogram("test2", ValueBuckets.custom(4, 5, 6)); - Assert.assertThat( - "same noop histogram returned", - noopHistogram1 == noopHistogram2, - CoreMatchers.equalTo(true) + assertThat( + "same noop histogram returned", + noopHistogram1 == noopHistogram2, + equalTo(true) ); - Assert.assertThat( - "noop histogram", - noopHistogram1 == NoopScope.NOOP_HISTOGRAM, - CoreMatchers.equalTo(true) + assertThat( + "noop histogram", + noopHistogram1 == NoopScope.NOOP_HISTOGRAM, + equalTo(true) ); } @@ -108,15 +109,15 @@ public void testCapabilities() { NoopScope noopScope = new NoopScope(); Capabilities noopCapabilities1 = noopScope.capabilities(); Capabilities noopCapabilities2 = noopScope.capabilities(); - Assert.assertThat( - "same noop capabilities returned", - noopCapabilities1 == noopCapabilities2, - CoreMatchers.equalTo(true) + assertThat( + "same noop capabilities returned", + noopCapabilities1 == noopCapabilities2, + equalTo(true) ); - Assert.assertThat( - "noop capabilities", - noopCapabilities1 == NoopScope.NOOP_CAPABILITIES, - CoreMatchers.equalTo(true) + assertThat( + "noop capabilities", + noopCapabilities1 == NoopScope.NOOP_CAPABILITIES, + equalTo(true) ); } @@ -125,13 +126,13 @@ public void testSubscope() { NoopScope noopScope = new NoopScope(); Scope noopScope1 = noopScope.subScope("test1"); Scope noopScope2 = noopScope.subScope("test2"); - Assert.assertThat("same noop scope returned", - noopScope1 == noopScope2, - CoreMatchers.equalTo(true) + assertThat("same noop scope returned", + noopScope1 == noopScope2, + equalTo(true) ); - Assert.assertThat("noop scope returned", - noopScope1 == noopScope, - CoreMatchers.equalTo(true) + assertThat("noop scope returned", + noopScope1 == noopScope, + equalTo(true) ); } @@ -148,13 +149,13 @@ public void testTagged() { Scope noopScope1 = noopScope.tagged(tagMap1); Scope noopScope2 = noopScope.tagged(tagMap2); - Assert.assertThat("same noop scope returned", - noopScope1 == noopScope2, - CoreMatchers.equalTo(true) + assertThat("same noop scope returned", + noopScope1 == noopScope2, + equalTo(true) ); - Assert.assertThat("noop scope returned", - noopScope1 == noopScope, - CoreMatchers.equalTo(true) + assertThat("noop scope returned", + noopScope1 == noopScope, + equalTo(true) ); } } diff --git a/core/src/test/java/com/uber/m3/tally/ScopeImplTest.java b/core/src/test/java/com/uber/m3/tally/ScopeImplTest.java index 39a5e3a..8065d9b 100644 --- a/core/src/test/java/com/uber/m3/tally/ScopeImplTest.java +++ b/core/src/test/java/com/uber/m3/tally/ScopeImplTest.java @@ -26,12 +26,11 @@ import com.uber.m3.util.Duration; import com.uber.m3.util.ImmutableMap; + import java.lang.Thread.UncaughtExceptionHandler; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.concurrent.atomic.AtomicInteger; + import org.junit.Test; public class ScopeImplTest { @@ -77,8 +76,8 @@ public void metricCreation() { assertNotNull(histogram); Histogram histogramDefaultBuckets = scope.histogram( - "new-histogram-default-buckets", - null + "new-histogram-default-buckets", + null ); assertNotNull(histogramDefaultBuckets); @@ -158,9 +157,9 @@ public void subscopes() { ImmutableMap additionalTags = new ImmutableMap.Builder(2) - .put("new_key", "new_val") - .put("baz", "quz") - .build(); + .put("new_key", "new_val") + .put("baz", "quz") + .build(); Scope taggedSubscope = rootScope.tagged(additionalTags); Timer taggedTimer = taggedSubscope.timer("tagged_timer"); taggedTimer.record(Duration.ofSeconds(6)); @@ -188,42 +187,33 @@ public void subscopes() { assertEquals("tagged_timer", timer.getName()); ImmutableMap expectedTags = new ImmutableMap.Builder(4) - .putAll(tags) - .putAll(additionalTags) - .build(); + .putAll(tags) + .putAll(additionalTags) + .build(); assertEquals(expectedTags, timer.getTags()); } @Test public void snapshot() { - final double EPSILON = 1e-9; - final int REPORT_INTERVAL_MILLIS = 10; - final int SLEEP_MILLIS = 20; - - TestStatsReporter reporter = new TestStatsReporter(); - + MonotonicClock.FakeClock clock = MonotonicClock.fake(); Scope rootScope = new RootScopeBuilder() - .reporter(reporter) - .reportEvery(Duration.ofMillis(REPORT_INTERVAL_MILLIS)); + .clock(clock) + .reporter(new SnapshotBasedStatsReporter()) + .build(); Counter counter = rootScope.counter("snapshot-counter"); counter.inc(110); - Gauge gauge = rootScope.gauge("snapshot-gauge"); - gauge.update(120); - Gauge gauge2 = rootScope.gauge("snapshot-gauge2"); - gauge2.update(220); - Gauge gauge3 = rootScope.gauge("snapshot-gauge3"); - gauge3.update(320); + rootScope.gauge("snapshot-gauge").update(120); + rootScope.gauge("snapshot-gauge2").update(220); + rootScope.gauge("snapshot-gauge3").update(320); Timer timer = rootScope.timer("snapshot-timer"); timer.record(Duration.ofMillis(130)); - try { - Thread.sleep(SLEEP_MILLIS); - } catch (InterruptedException e) { - System.err.println("Interrupted while sleeping! Let's continue anyway..."); - } + Stopwatch stopwatch = timer.start(); + clock.addNanos(50); + stopwatch.stop(); Snapshot snapshot = ((ScopeImpl) rootScope).snapshot(); @@ -255,6 +245,7 @@ public void snapshot() { TimerSnapshot timerSnapshotActual = timers.get(ScopeImpl.keyForPrefixedStringMap("snapshot-timer", null)); assertEquals("snapshot-timer", timerSnapshotActual.name()); assertEquals(ImmutableMap.EMPTY, timerSnapshotActual.tags()); + assertEquals(Arrays.asList(Duration.ofMillis(130), Duration.ofNanos(50)), Arrays.asList(timerSnapshotActual.values())); } @Test(expected = IllegalArgumentException.class) @@ -264,35 +255,26 @@ public void nonPositiveReportInterval() { @Test public void exceptionInReportLoop() throws ScopeCloseException, InterruptedException { - final AtomicInteger uncaghtExceptionReported = new AtomicInteger(); + AtomicInteger uncaughtExceptionReported = new AtomicInteger(); ThrowingStatsReporter reporter = new ThrowingStatsReporter(); - final UncaughtExceptionHandler uncaughtExceptionHandler = new UncaughtExceptionHandler() { - @Override - public void uncaughtException(Thread t, Throwable e) { - uncaghtExceptionReported.incrementAndGet(); - } - }; + UncaughtExceptionHandler uncaughtExceptionHandler = (t, e) -> uncaughtExceptionReported.incrementAndGet(); - Scope scope = new RootScopeBuilder() + try (Scope scope = new RootScopeBuilder() .reporter(reporter) .reportEvery(Duration.ofMillis(REPORT_INTERVAL_MILLIS), - uncaughtExceptionHandler); - - try { + uncaughtExceptionHandler)) { scope.counter("hi").inc(1); Thread.sleep(SLEEP_MILLIS); - assertEquals(1, uncaghtExceptionReported.get()); + assertEquals(1, uncaughtExceptionReported.get()); assertEquals(1, reporter.getNumberOfReportedMetrics()); // Run again to verify it reports again. scope.counter("hi").inc(1); Thread.sleep(SLEEP_MILLIS); - assertEquals(2, uncaghtExceptionReported.get()); + assertEquals(2, uncaughtExceptionReported.get()); assertEquals(2, reporter.getNumberOfReportedMetrics()); - } finally { - scope.close(); } } diff --git a/core/src/test/java/com/uber/m3/tally/TestScopeTest.java b/core/src/test/java/com/uber/m3/tally/TestScopeTest.java index 5e84dab..12df65d 100644 --- a/core/src/test/java/com/uber/m3/tally/TestScopeTest.java +++ b/core/src/test/java/com/uber/m3/tally/TestScopeTest.java @@ -20,16 +20,14 @@ package com.uber.m3.tally; +import com.uber.m3.util.Duration; import com.uber.m3.util.ImmutableMap; import org.junit.Test; import java.util.Map; import static org.hamcrest.CoreMatchers.instanceOf; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.*; public class TestScopeTest { @@ -114,5 +112,219 @@ public void testCreateWithTagsAndSubscope() { assertEquals(subScopeTags, counterSnapshot.tags()); assertEquals(1, counterSnapshot.value()); } + + @Test + public void testCreateWithClock() { + Buckets buckets = DurationBuckets.linear(Duration.ZERO, Duration.ofMillis(10), 10); + MonotonicClock.FakeClock clock = MonotonicClock.fake(); + TestScope testScope = TestScope.create(clock); + + Stopwatch stopwatch = testScope.histogram("histogram", buckets).start(); + clock.addDuration(Duration.ofMillis(15)); + stopwatch.stop(); + + Snapshot snapshot = testScope.snapshot(); + assertNotNull(snapshot); + + Map histograms = snapshot.histograms(); + assertNotNull(histograms); + assertEquals(1, histograms.size()); + + HistogramSnapshot histogramSnapshot = histograms.get(new ScopeKey("histogram", ImmutableMap.EMPTY)); + + assertNotNull(histogramSnapshot); + assertEquals("histogram", histogramSnapshot.name()); + assertEquals(1, histogramSnapshot.durations().get(Duration.ofMillis(20)).longValue()); + } + + @Test + public void testCreateWithTagsAndClock() { + Buckets buckets = DurationBuckets.linear(Duration.ZERO, Duration.ofMillis(10), 10); + ImmutableMap tags = ImmutableMap.of("key", "value"); + MonotonicClock.FakeClock clock = MonotonicClock.fake(); + TestScope testScope = TestScope.create("prefix", tags, clock); + + Stopwatch stopwatch = testScope.histogram("histogram", buckets).start(); + clock.addDuration(Duration.ofMillis(25)); + stopwatch.stop(); + + Snapshot snapshot = testScope.snapshot(); + assertNotNull(snapshot); + + Map histograms = snapshot.histograms(); + assertNotNull(histograms); + assertEquals(1, histograms.size()); + + HistogramSnapshot histogramSnapshot = histograms.get(new ScopeKey("prefix.histogram", tags)); + + assertNotNull(histogramSnapshot); + assertEquals("prefix.histogram", histogramSnapshot.name()); + assertEquals(1, histogramSnapshot.durations().get(Duration.ofMillis(30)).longValue()); + } + + @Test + public void testCreateWithClockAndTimer() { + MonotonicClock.FakeClock clock = MonotonicClock.fake(); + TestScope testScope = TestScope.create(clock); + + Stopwatch stopwatch = testScope.timer("timer").start(); + clock.addNanos(10); + stopwatch.stop(); + + Snapshot snapshot = testScope.snapshot(); + assertNotNull(snapshot); + + Map timers = snapshot.timers(); + assertNotNull(timers); + assertEquals(1, timers.size()); + + TimerSnapshot timerSnapshot = timers.get(new ScopeKey("timer", ImmutableMap.EMPTY)); + + assertNotNull(timerSnapshot); + assertEquals("timer", timerSnapshot.name()); + assertEquals(1, timerSnapshot.values().length); + assertEquals(Duration.ofNanos(10), timerSnapshot.values()[0]); + } + + @Test + public void testCreateWithTagsAndClockAndTimer() { + ImmutableMap tags = ImmutableMap.of("key", "value"); + MonotonicClock.FakeClock clock = MonotonicClock.fake(); + TestScope testScope = TestScope.create("prefix", tags, clock); + Scope tagged = testScope.tagged(ImmutableMap.of("other_key", "other_value")); + + Stopwatch stopwatch = tagged.timer("timer").start(); + clock.addNanos(10); + stopwatch.stop(); + + Snapshot snapshot = testScope.snapshot(); + assertNotNull(snapshot); + + Map timers = snapshot.timers(); + assertNotNull(timers); + assertEquals(1, timers.size()); + + ImmutableMap totalTags = ImmutableMap.of("key", "value", "other_key", "other_value"); + TimerSnapshot timerSnapshot = timers.get(new ScopeKey("prefix.timer", totalTags)); + + assertNotNull(timerSnapshot); + assertEquals("prefix.timer", timerSnapshot.name()); + assertEquals(totalTags, timerSnapshot.tags()); + assertEquals(1, timerSnapshot.values().length); + assertEquals(Duration.ofNanos(10), timerSnapshot.values()[0]); + } + + @Test + public void testCounterSnapshot() { + TestScope testScope = TestScope.create("prefix", ImmutableMap.EMPTY); + testScope.tagged(ImmutableMap.of("key", "value")).counter("counter").inc(1); + + Snapshot snapshot = testScope.snapshot(); + assertNotNull(snapshot); + + Map counters = snapshot.counters(); + assertNotNull(counters); + assertEquals(1, counters.size()); + + ImmutableMap totalTags = ImmutableMap.of("key", "value"); + CounterSnapshot counterSnapshot = counters.get(new ScopeKey("prefix.counter", totalTags)); + + assertNotNull(counterSnapshot); + assertEquals("prefix.counter", counterSnapshot.name()); + assertEquals(totalTags, counterSnapshot.tags()); + assertEquals(1, counterSnapshot.value()); + } + + @Test + public void testTimerSnapshot() { + MonotonicClock.FakeClock clock = MonotonicClock.fake(); + TestScope testScope = TestScope.create("prefix", ImmutableMap.EMPTY, clock); + Scope subscope = testScope.tagged(ImmutableMap.of("key", "value")); + + Timer timer = subscope.timer("timer"); + + // Test record directly. + timer.record(Duration.ofNanos(100)); + + // Test record via stopwatch. + Stopwatch stopwatch = timer.start(); + clock.addNanos(10); + stopwatch.stop(); + + Snapshot snapshot = testScope.snapshot(); + assertNotNull(snapshot); + + Map timers = snapshot.timers(); + assertNotNull(timers); + assertEquals(1, timers.size()); + + ImmutableMap totalTags = ImmutableMap.of("key", "value"); + TimerSnapshot timerSnapshot = timers.get(new ScopeKey("prefix.timer", totalTags)); + + assertNotNull(timerSnapshot); + assertEquals("prefix.timer", timerSnapshot.name()); + assertEquals(totalTags, timerSnapshot.tags()); + assertArrayEquals(new Duration[]{Duration.ofNanos(100), Duration.ofNanos(10)}, timerSnapshot.values()); + } + + @Test + public void testHistogramSnapshot() { + MonotonicClock.FakeClock clock = MonotonicClock.fake(); + Buckets buckets = DurationBuckets.linear(Duration.ZERO, Duration.ofMillis(10), 5); + TestScope testScope = TestScope.create("prefix", ImmutableMap.EMPTY); + Scope subscope = testScope.tagged(ImmutableMap.of("key", "value")); + + Histogram histogram = subscope.histogram("histogram", buckets); + // Test record directly. + histogram.recordDuration(Duration.ofMillis(15)); + + // Test record via stopwatch. + Stopwatch stopwatch = histogram.start(); + clock.addDuration(Duration.ofMillis(25)); + stopwatch.stop(); + + Snapshot snapshot = testScope.snapshot(); + assertNotNull(snapshot); + + Map histogramSnapshots = snapshot.histograms(); + assertNotNull(histogramSnapshots); + assertEquals(1, histogramSnapshots.size()); + + ImmutableMap totalTags = ImmutableMap.of("key", "value"); + HistogramSnapshot histogramSnapshot = histogramSnapshots.get(new ScopeKey("prefix.histogram", totalTags)); + + assertNotNull(histogramSnapshot); + assertEquals("prefix.histogram", histogramSnapshot.name()); + assertEquals(totalTags, histogramSnapshot.tags()); + + // Total of 2 samples. + assertEquals(2, histogramSnapshot.durations().values().stream().mapToLong(it -> it).sum()); + assertEquals(1, histogramSnapshot.durations().get(Duration.ofMillis(20)).longValue()); + assertEquals(1, histogramSnapshot.durations().get(Duration.ofMillis(10)).longValue()); + } + + @Test + public void testGaugeSnapshot() { + TestScope testScope = TestScope.create("prefix", ImmutableMap.EMPTY); + Scope subscope = testScope.tagged(ImmutableMap.of("key", "value")); + + subscope.gauge("gauge").update(20.0); + + Snapshot snapshot = testScope.snapshot(); + assertNotNull(snapshot); + + Map gauge = snapshot.gauges(); + assertNotNull(gauge); + assertEquals(1, gauge.size()); + + ImmutableMap totalTags = ImmutableMap.of("key", "value"); + GaugeSnapshot gaugeSnapshot = gauge.get(new ScopeKey("prefix.gauge", totalTags)); + + assertNotNull(gaugeSnapshot); + assertEquals("prefix.gauge", gaugeSnapshot.name()); + assertEquals(totalTags, gaugeSnapshot.tags()); + + assertEquals(20.0, gaugeSnapshot.value(), 0.01); + } } diff --git a/core/src/test/java/com/uber/m3/tally/TestStatsReporter.java b/core/src/test/java/com/uber/m3/tally/TestStatsReporter.java index b8a1fed..035fb23 100644 --- a/core/src/test/java/com/uber/m3/tally/TestStatsReporter.java +++ b/core/src/test/java/com/uber/m3/tally/TestStatsReporter.java @@ -29,14 +29,14 @@ import java.util.concurrent.ConcurrentLinkedQueue; public class TestStatsReporter implements StatsReporter { - private Queue> counters = new ConcurrentLinkedQueue<>(); - private Queue> gauges = new ConcurrentLinkedQueue<>(); - private Queue> timers = new ConcurrentLinkedQueue<>(); - private Buckets buckets; - private Map valueSamples = new HashMap<>(); - private Map durationSamples = new HashMap<>(); + private final Queue> counters = new ConcurrentLinkedQueue<>(); + private final Queue> gauges = new ConcurrentLinkedQueue<>(); + private final Queue> timers = new ConcurrentLinkedQueue<>(); + private final Map valueSamples = new HashMap<>(); + private final Map durationSamples = new HashMap<>(); + private final Map cumulativeValueSamples = new HashMap<>(); - private Map cumulativeValueSamples = new HashMap<>(); + private Buckets buckets; @Override public void reportCounter(String name, Map tags, long value) { diff --git a/core/src/test/java/com/uber/m3/tally/TimerImplTest.java b/core/src/test/java/com/uber/m3/tally/TimerImplTest.java index d7152be..e5babb7 100644 --- a/core/src/test/java/com/uber/m3/tally/TimerImplTest.java +++ b/core/src/test/java/com/uber/m3/tally/TimerImplTest.java @@ -21,26 +21,21 @@ package com.uber.m3.tally; import com.uber.m3.util.Duration; -import org.junit.Before; +import com.uber.m3.util.ImmutableMap; import org.junit.Test; +import static com.uber.m3.tally.ScopeImpl.keyForPrefixedStringMap; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; public class TimerImplTest { - private TestStatsReporter reporter; - private TimerImpl timer; - - @Before - public void setUp() { - reporter = new TestStatsReporter(); - timer = new TimerImpl("", null, reporter); - } + private final MonotonicClock.FakeClock clock = MonotonicClock.fake(); + private final TestStatsReporter reporter = new TestStatsReporter(); @Test public void record() { + TimerImpl timer = new TimerImpl(clock, "", null, reporter); + timer.record(Duration.ofMillis(42)); assertEquals(Duration.ofMillis(42), reporter.nextTimerVal()); @@ -48,50 +43,71 @@ public void record() { assertEquals(Duration.ofMinutes(2), reporter.nextTimerVal()); Stopwatch stopwatch = timer.start(); + clock.addDuration(Duration.ofMillis(200)); stopwatch.stop(); - Duration duration = reporter.nextTimerVal(); - assertNotNull(duration); - assertTrue(duration.compareTo(Duration.ZERO) > 0); + assertEquals(Duration.ofMillis(200), reporter.nextTimerVal()); } @Test - public void noReporterSinkSnapshot() { - timer = new TimerImpl("no-reporter-timer", null, null); - - StatsReporter sink = timer.new NoReporterSink(); - - // No-ops - sink.flush(); - sink.close(); + public void snapshotDuration() { + SnapshotBasedStatsReporter snapshotReporter = new SnapshotBasedStatsReporter(); + ScopeImpl scope = + new ScopeBuilder(null, new ScopeImpl.Registry()) + .reporter(snapshotReporter) + .clock(clock) + .build(); + Timer timer = scope.timer("timer"); - assertEquals(CapableOf.REPORTING_TAGGING, sink.capabilities()); - sink.reportTimer("new-timer", null, Duration.ofMillis(888)); + timer.record(Duration.ofMillis(42)); + Snapshot snapshot1 = scope.snapshot(); - assertArrayEquals(new Duration[]{Duration.ofMillis(888)}, timer.snapshot()); - } + assertArrayEquals( + new Duration[]{Duration.ofMillis(42)}, + getSnapshot(snapshot1, "timer").values()); - @Test(expected = UnsupportedOperationException.class) - public void unsupportedCounter() { - StatsReporter sink = new TimerImpl("", null, null).new NoReporterSink(); - sink.reportCounter(null, null, 0); - } + timer.record(Duration.ofMillis(3)); + timer.record(Duration.ofMillis(2)); + timer.record(Duration.ofMillis(1)); + Snapshot snapshot2 = scope.snapshot(); - @Test(expected = UnsupportedOperationException.class) - public void unsupportedGauge() { - StatsReporter sink = new TimerImpl("", null, null).new NoReporterSink(); - sink.reportGauge(null, null, 0); + assertArrayEquals( + new Duration[]{Duration.ofMillis(3), Duration.ofMillis(2), Duration.ofMillis(1)}, + getSnapshot(snapshot2, "timer").values()); } - @Test(expected = UnsupportedOperationException.class) - public void unsupportedHistogramValue() { - StatsReporter sink = new TimerImpl("", null, null).new NoReporterSink(); - sink.reportHistogramValueSamples(null, null, null, 0, 0, 0); + @Test + public void snapshotStopwatch() { + SnapshotBasedStatsReporter snapshotReporter = new SnapshotBasedStatsReporter(); + ScopeImpl scope = + new ScopeBuilder(null, new ScopeImpl.Registry()) + .reporter(snapshotReporter) + .clock(clock) + .build(); + Timer timer = scope.timer("timer"); + + Stopwatch stopwatch1 = timer.start(); + clock.addNanos(100); + stopwatch1.stop(); + Snapshot snapshot1 = scope.snapshot(); + assertArrayEquals( + new Duration[]{Duration.ofNanos(100)}, + getSnapshot(snapshot1, "timer").values()); + + Stopwatch stopwatch2 = timer.start(); + clock.addNanos(200); + stopwatch2.stop(); + Stopwatch stopwatch3 = timer.start(); + clock.addNanos(150); + stopwatch3.stop(); + Snapshot snapshot2 = scope.snapshot(); + assertArrayEquals( + new Duration[]{Duration.ofNanos(200), Duration.ofNanos(150)}, + getSnapshot(snapshot2, "timer").values()); } - @Test(expected = UnsupportedOperationException.class) - public void unsupportedHistogramDuration() { - StatsReporter sink = new TimerImpl("", null, null).new NoReporterSink(); - sink.reportHistogramDurationSamples(null, null, null, null, null, 0); + private static TimerSnapshot getSnapshot(Snapshot snapshot, String name) { + ScopeKey key = keyForPrefixedStringMap(name, ImmutableMap.EMPTY); + return snapshot.timers().get(key); } } diff --git a/core/src/test/java/com/uber/m3/tally/ValueBucketsTest.java b/core/src/test/java/com/uber/m3/tally/ValueBucketsTest.java index 652e31d..649642c 100644 --- a/core/src/test/java/com/uber/m3/tally/ValueBucketsTest.java +++ b/core/src/test/java/com/uber/m3/tally/ValueBucketsTest.java @@ -20,16 +20,11 @@ package com.uber.m3.tally; +import com.uber.m3.util.Duration; import org.hamcrest.CoreMatchers; import org.junit.Test; -import com.uber.m3.util.Duration; - -import static org.junit.Assert.assertArrayEquals; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.*; public class ValueBucketsTest { @Test @@ -151,10 +146,10 @@ public void equalsHashcode() { ValueBuckets buckets = ValueBuckets.linear(0, 10, 3); ValueBuckets sameBuckets = ValueBuckets.linear(0, 10, 3); - assertTrue(buckets.equals(sameBuckets)); + assertEquals(buckets, sameBuckets); assertEquals(buckets.hashCode(), sameBuckets.hashCode()); - assertFalse(buckets.equals(null)); - assertFalse(buckets.equals(9)); + assertNotEquals(null, buckets); + assertNotEquals(9, buckets); } } diff --git a/core/src/test/java/com/uber/m3/util/DurationTest.java b/core/src/test/java/com/uber/m3/util/DurationTest.java index d04b9ed..7a94d52 100644 --- a/core/src/test/java/com/uber/m3/util/DurationTest.java +++ b/core/src/test/java/com/uber/m3/util/DurationTest.java @@ -23,8 +23,7 @@ import org.junit.Test; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertNotEquals; public class DurationTest { private static final double EPSILON = 1e-10; @@ -102,15 +101,15 @@ public void compareTo() { @Test public void equals() { - assertTrue(Duration.ZERO.equals(Duration.ZERO)); - assertTrue(Duration.ZERO.equals(Duration.ofNanos(0))); - assertTrue(Duration.ofMillis(6000).equals(Duration.ofSeconds(6))); - assertTrue(Duration.ofHours(1).equals(Duration.ofNanos(3_600_000_000_000L))); + assertEquals(Duration.ZERO, Duration.ZERO); + assertEquals(Duration.ZERO, Duration.ofNanos(0)); + assertEquals(Duration.ofMillis(6000), Duration.ofSeconds(6)); + assertEquals(Duration.ofHours(1), Duration.ofNanos(3_600_000_000_000L)); assertEquals(Duration.ofHours(1).hashCode(), Duration.ofNanos(3_600_000_000_000L).hashCode()); - assertFalse(Duration.ofMillis(6001).equals(Duration.ofSeconds(6))); - assertFalse(Duration.ZERO.equals(null)); - assertFalse(Duration.ZERO.equals(1)); + assertNotEquals(Duration.ofMillis(6001), Duration.ofSeconds(6)); + assertNotEquals(null, Duration.ZERO); + assertNotEquals(1, Duration.ZERO); } @Test diff --git a/core/src/test/java/com/uber/m3/util/ImmutableListTest.java b/core/src/test/java/com/uber/m3/util/ImmutableListTest.java index f82dc69..e5a0434 100644 --- a/core/src/test/java/com/uber/m3/util/ImmutableListTest.java +++ b/core/src/test/java/com/uber/m3/util/ImmutableListTest.java @@ -25,11 +25,7 @@ import java.util.ArrayList; -import static org.junit.Assert.assertArrayEquals; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; public class ImmutableListTest { private ArrayList helperList; @@ -202,13 +198,13 @@ public void equals() { helperList.add("zz"); ImmutableList differentList = new ImmutableList<>(helperList); - assertTrue(list.equals(sameList)); + assertEquals(list, sameList); assertEquals(list.hashCode(), sameList.hashCode()); - assertFalse(list.equals(differentList)); + assertNotEquals(list, differentList); assertNotEquals(list.hashCode(), differentList.hashCode()); - assertFalse(list.equals(null)); - assertTrue(list.equals(list)); - assertFalse(list.equals(1)); + assertNotEquals(null, list); + assertEquals(list, list); + assertNotEquals(1, list); } } diff --git a/core/src/test/java/com/uber/m3/util/ImmutableMapTest.java b/core/src/test/java/com/uber/m3/util/ImmutableMapTest.java index 02005a5..b20f631 100644 --- a/core/src/test/java/com/uber/m3/util/ImmutableMapTest.java +++ b/core/src/test/java/com/uber/m3/util/ImmutableMapTest.java @@ -25,10 +25,7 @@ import java.util.HashMap; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; public class ImmutableMapTest { private HashMap helperMap; @@ -73,11 +70,11 @@ public void size() { @Test public void isEmpty() { - assertEquals(false, map.isEmpty()); + assertFalse(map.isEmpty()); map = new ImmutableMap<>(new HashMap(0, 1)); - assertEquals(true, map.isEmpty()); + assertTrue(map.isEmpty()); } @Test @@ -102,7 +99,7 @@ public void containsValue() { public void get() { assertEquals("val1", map.get("key1")); - assertEquals(null, map.get("key9")); + assertNull(map.get("key9")); } @Test(expected = UnsupportedOperationException.class) @@ -145,27 +142,27 @@ public void builderPutAll() { ImmutableMap sameMap = new ImmutableMap.Builder() .putAll(helperMap).build(); - assertTrue(map.equals(sameMap)); + assertEquals(map, sameMap); } @Test public void equals() { - assertFalse(map.equals(null)); - assertFalse(map.equals(1)); - assertTrue(map.equals(map)); + assertNotEquals(null, map); + assertNotEquals(1, map); + assertEquals(map, map); ImmutableMap sameMap = new ImmutableMap<>(helperMap); - assertTrue(map.equals(sameMap)); + assertEquals(map, sameMap); assertEquals(map.hashCode(), sameMap.hashCode()); helperMap.put("key7", "val7"); ImmutableMap differentMap = new ImmutableMap<>(helperMap); - assertFalse(map.equals(differentMap)); + assertNotEquals(map, differentMap); assertNotEquals(map.hashCode(), differentMap.hashCode()); - assertTrue(ImmutableMap.EMPTY.equals(new ImmutableMap(new HashMap()))); + assertEquals(ImmutableMap.EMPTY, new ImmutableMap(new HashMap())); } @Test diff --git a/core/src/test/java/com/uber/m3/util/ImmutableSetTest.java b/core/src/test/java/com/uber/m3/util/ImmutableSetTest.java index bc49789..459aa46 100644 --- a/core/src/test/java/com/uber/m3/util/ImmutableSetTest.java +++ b/core/src/test/java/com/uber/m3/util/ImmutableSetTest.java @@ -25,11 +25,7 @@ import java.util.HashSet; -import static org.junit.Assert.assertArrayEquals; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; public class ImmutableSetTest { private HashSet helperSet; @@ -141,18 +137,18 @@ public void clear() { public void equals() { ImmutableSet equalSet = new ImmutableSet<>(helperSet); - assertTrue(set.equals(equalSet)); + assertEquals(set, equalSet); assertEquals(set.hashCode(), equalSet.hashCode()); helperSet.add("zz"); ImmutableSet differentSet = new ImmutableSet<>(helperSet); - assertFalse(set.equals(differentSet)); + assertNotEquals(set, differentSet); assertNotEquals(set.hashCode(), differentSet.hashCode()); - assertFalse(set.equals(null)); - assertTrue(set.equals(set)); - assertFalse(set.equals(2)); + assertNotEquals(null, set); + assertEquals(set, set); + assertNotEquals(2, set); } @Test