Move toMillis(Duration) to Utils so that it can be reused. (#1114)

* Add toMillis() method to TimeUtils.

* Reuse toMillis() method in impl and exporters.

* Add a note about overflow and precision loss.

* Move toMillis() to Duration.

8 files changed, 33 insertions, 41 deletions

diff --git a/api/src/main/java/io/opencensus/common/Duration.java b/api/src/main/java/io/opencensus/common/Duration.java
index 1ee97191..57ca87fe 100644
--- a/api/src/main/java/io/opencensus/common/Duration.java
+++ b/api/src/main/java/io/opencensus/common/Duration.java
@@ -23,6 +23,7 @@ import static io.opencensus.common.TimeUtils.NANOS_PER_MILLI;
 
 import com.google.auto.value.AutoValue;
 import io.opencensus.internal.Utils;
+import java.util.concurrent.TimeUnit;
 import javax.annotation.concurrent.Immutable;
 
 /**
@@ -79,6 +80,20 @@ public abstract class Duration implements Comparable<Duration> {
   }
 
   /**
+   * Converts a {@link Duration} to milliseconds.
+   *
+   * <p>Note that there could be overflow or loss of precision by making this conversion. See {@link
+   * TimeUnit#convert(long, TimeUnit)} for details.
+   *
+   * @param duration a {@code Duration}.
+   * @return the milliseconds representation of this {@code Duration}.
+   */
+  public static long toMillis(Duration duration) {
+    return TimeUnit.SECONDS.toMillis(duration.getSeconds())
+        + TimeUnit.NANOSECONDS.toMillis(duration.getNanos());
+  }
+
+  /**
    * Returns the number of seconds in the {@code Duration}.
    *
    * @return the number of seconds in the {@code Duration}.
diff --git a/api/src/test/java/io/opencensus/common/DurationTest.java b/api/src/test/java/io/opencensus/common/DurationTest.java
index 40682fc4..d46f2f7a 100644
--- a/api/src/test/java/io/opencensus/common/DurationTest.java
+++ b/api/src/test/java/io/opencensus/common/DurationTest.java
@@ -88,4 +88,15 @@ public class DurationTest {
     assertThat(Duration.create(-25, -42)).isNotEqualTo(Duration.create(-24, -42));
     assertThat(Duration.create(-24, -43)).isNotEqualTo(Duration.create(-24, -42));
   }
+
+  @Test
+  public void toMillis() {
+    assertThat(Duration.toMillis(Duration.create(10, 0))).isEqualTo(10000L);
+    assertThat(Duration.toMillis(Duration.create(10, 1000))).isEqualTo(10000L);
+    assertThat(Duration.toMillis(Duration.create(0, (int) 1e6))).isEqualTo(1L);
+    assertThat(Duration.toMillis(Duration.create(0, 0))).isEqualTo(0L);
+    assertThat(Duration.toMillis(Duration.create(-10, 0))).isEqualTo(-10000L);
+    assertThat(Duration.toMillis(Duration.create(-10, -1000))).isEqualTo(-10000L);
+    assertThat(Duration.toMillis(Duration.create(0, -(int) 1e6))).isEqualTo(-1L);
+  }
 }
diff --git a/exporters/stats/signalfx/src/main/java/io/opencensus/exporter/stats/signalfx/SignalFxStatsExporterWorkerThread.java b/exporters/stats/signalfx/src/main/java/io/opencensus/exporter/stats/signalfx/SignalFxStatsExporterWorkerThread.java
index 91abfc88..e8bfe426 100644
--- a/exporters/stats/signalfx/src/main/java/io/opencensus/exporter/stats/signalfx/SignalFxStatsExporterWorkerThread.java
+++ b/exporters/stats/signalfx/src/main/java/io/opencensus/exporter/stats/signalfx/SignalFxStatsExporterWorkerThread.java
@@ -28,7 +28,6 @@ import io.opencensus.stats.ViewData;
 import io.opencensus.stats.ViewManager;
 import java.io.IOException;
 import java.net.URI;
-import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -60,9 +59,7 @@ final class SignalFxStatsExporterWorkerThread extends Thread {
       String token,
       Duration interval,
       ViewManager views) {
-    this.intervalMs =
-        TimeUnit.SECONDS.toMillis(interval.getSeconds())
-            + TimeUnit.NANOSECONDS.toMillis(interval.getNanos());
+    this.intervalMs = Duration.toMillis(interval);
     this.views = views;
     this.sender = factory.create(endpoint, token, ERROR_HANDLER);
 
diff --git a/exporters/stats/stackdriver/src/main/java/io/opencensus/exporter/stats/stackdriver/StackdriverExporterWorker.java b/exporters/stats/stackdriver/src/main/java/io/opencensus/exporter/stats/stackdriver/StackdriverExporterWorker.java
index 9d9b21fb..68ca85ff 100644
--- a/exporters/stats/stackdriver/src/main/java/io/opencensus/exporter/stats/stackdriver/StackdriverExporterWorker.java
+++ b/exporters/stats/stackdriver/src/main/java/io/opencensus/exporter/stats/stackdriver/StackdriverExporterWorker.java
@@ -79,7 +79,7 @@ final class StackdriverExporterWorker implements Runnable {
       Duration exportInterval,
       ViewManager viewManager,
       MonitoredResource monitoredResource) {
-    this.scheduleDelayMillis = toMillis(exportInterval);
+    this.scheduleDelayMillis = Duration.toMillis(exportInterval);
     this.projectId = projectId;
     projectName = ProjectName.newBuilder().setProject(projectId).build();
     this.metricServiceClient = metricServiceClient;
@@ -224,13 +224,6 @@ final class StackdriverExporterWorker implements Runnable {
     }
   }
 
-  private static final long MILLIS_PER_SECOND = 1000L;
-  private static final long NANOS_PER_MILLI = 1000 * 1000;
-
-  private static long toMillis(Duration duration) {
-    return duration.getSeconds() * MILLIS_PER_SECOND + duration.getNanos() / NANOS_PER_MILLI;
-  }
-
   private static String exceptionMessage(Throwable e) {
     return e.getMessage() != null ? e.getMessage() : e.getClass().getName();
   }
diff --git a/impl_core/src/main/java/io/opencensus/implcore/stats/IntervalBucket.java b/impl_core/src/main/java/io/opencensus/implcore/stats/IntervalBucket.java
index a35948a7..f114bea6 100644
--- a/impl_core/src/main/java/io/opencensus/implcore/stats/IntervalBucket.java
+++ b/impl_core/src/main/java/io/opencensus/implcore/stats/IntervalBucket.java
@@ -18,7 +18,6 @@ package io.opencensus.implcore.stats;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
-import static io.opencensus.implcore.stats.MutableViewData.toMillis;
 
 import com.google.common.collect.Maps;
 import io.opencensus.common.Duration;
@@ -82,7 +81,7 @@ final class IntervalBucket {
     checkArgument(
         elapsedTime.compareTo(ZERO) >= 0 && elapsedTime.compareTo(duration) < 0,
         "This bucket must be current.");
-    return ((double) toMillis(elapsedTime)) / toMillis(duration);
+    return ((double) Duration.toMillis(elapsedTime)) / Duration.toMillis(duration);
   }
 
   void clearStats() {
diff --git a/impl_core/src/main/java/io/opencensus/implcore/stats/MutableViewData.java b/impl_core/src/main/java/io/opencensus/implcore/stats/MutableViewData.java
index 588b4897..f0938c06 100644
--- a/impl_core/src/main/java/io/opencensus/implcore/stats/MutableViewData.java
+++ b/impl_core/src/main/java/io/opencensus/implcore/stats/MutableViewData.java
@@ -66,9 +66,6 @@ import org.checkerframework.checker.nullness.qual.Nullable;
 @SuppressWarnings("deprecation")
 abstract class MutableViewData {
 
-  private static final long MILLIS_PER_SECOND = 1000L;
-  private static final long NANOS_PER_MILLI = 1000 * 1000;
-
   @javax.annotation.Nullable @VisibleForTesting static final TagValue UNKNOWN_TAG_VALUE = null;
 
   @VisibleForTesting static final Timestamp ZERO_TIMESTAMP = Timestamp.create(0, 0);
@@ -149,11 +146,6 @@ abstract class MutableViewData {
     return tagValues;
   }
 
-  // Returns the milliseconds representation of a Duration.
-  static long toMillis(Duration duration) {
-    return duration.getSeconds() * MILLIS_PER_SECOND + duration.getNanos() / NANOS_PER_MILLI;
-  }
-
   /**
    * Create an empty {@link MutableAggregation} based on the given {@link Aggregation}.
    *
@@ -295,7 +287,7 @@ abstract class MutableViewData {
       super(view);
       Duration totalDuration = ((View.AggregationWindow.Interval) view.getWindow()).getDuration();
       this.totalDuration = totalDuration;
-      this.bucketDuration = Duration.fromMillis(toMillis(totalDuration) / N);
+      this.bucketDuration = Duration.fromMillis(Duration.toMillis(totalDuration) / N);
 
       // When initializing. add N empty buckets prior to the start timestamp of this
       // IntervalMutableViewData, so that the last bucket will be the current one in effect.
@@ -354,8 +346,8 @@ abstract class MutableViewData {
       checkArgument(
           now.compareTo(startOfLastBucket) >= 0,
           "Current time must be within or after the last bucket.");
-      long elapsedTimeMillis = toMillis(now.subtractTimestamp(startOfLastBucket));
-      long numOfPadBuckets = elapsedTimeMillis / toMillis(bucketDuration);
+      long elapsedTimeMillis = Duration.toMillis(now.subtractTimestamp(startOfLastBucket));
+      long numOfPadBuckets = elapsedTimeMillis / Duration.toMillis(bucketDuration);
 
       shiftBucketList(numOfPadBuckets, now);
     }
diff --git a/impl_core/src/main/java/io/opencensus/implcore/trace/export/SpanExporterImpl.java b/impl_core/src/main/java/io/opencensus/implcore/trace/export/SpanExporterImpl.java
index e5973c0b..c8ed237d 100644
--- a/impl_core/src/main/java/io/opencensus/implcore/trace/export/SpanExporterImpl.java
+++ b/impl_core/src/main/java/io/opencensus/implcore/trace/export/SpanExporterImpl.java
@@ -28,7 +28,6 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.concurrent.GuardedBy;
@@ -146,9 +145,7 @@ public final class SpanExporterImpl extends SpanExporter {
     private Worker(int bufferSize, Duration scheduleDelay) {
       spans = new ArrayList<SpanImpl>(bufferSize);
       this.bufferSize = bufferSize;
-      this.scheduleDelayMillis =
-          TimeUnit.SECONDS.toMillis(scheduleDelay.getSeconds())
-              + TimeUnit.NANOSECONDS.toMillis(scheduleDelay.getNanos());
+      this.scheduleDelayMillis = Duration.toMillis(scheduleDelay);
     }
 
     // Returns an unmodifiable list of all buffered spans data to ensure that any registered
diff --git a/impl_core/src/test/java/io/opencensus/implcore/stats/MutableViewDataTest.java b/impl_core/src/test/java/io/opencensus/implcore/stats/MutableViewDataTest.java
index b4b4c85b..bde75232 100644
--- a/impl_core/src/test/java/io/opencensus/implcore/stats/MutableViewDataTest.java
+++ b/impl_core/src/test/java/io/opencensus/implcore/stats/MutableViewDataTest.java
@@ -17,10 +17,8 @@
 package io.opencensus.implcore.stats;
 
 import static com.google.common.truth.Truth.assertThat;
-import static io.opencensus.implcore.stats.MutableViewData.toMillis;
 
 import com.google.common.collect.ImmutableMap;
-import io.opencensus.common.Duration;
 import io.opencensus.common.Timestamp;
 import io.opencensus.implcore.stats.MutableAggregation.MutableCount;
 import io.opencensus.implcore.stats.MutableAggregation.MutableDistribution;
@@ -134,14 +132,4 @@ public class MutableViewDataTest {
                 Arrays.asList(new Long[] {0L, 0L, 0L, 0L})))
         .inOrder();
   }
-
-  @Test
-  public void testDurationToMillis() {
-    assertThat(toMillis(Duration.create(0, 0))).isEqualTo(0);
-    assertThat(toMillis(Duration.create(0, 987000000))).isEqualTo(987);
-    assertThat(toMillis(Duration.create(3, 456000000))).isEqualTo(3456);
-    assertThat(toMillis(Duration.create(0, -1000000))).isEqualTo(-1);
-    assertThat(toMillis(Duration.create(-1, 0))).isEqualTo(-1000);
-    assertThat(toMillis(Duration.create(-3, -456000000))).isEqualTo(-3456);
-  }
 }