Fix stats to metrics implementation. (#1368)

* Fix stats to metrics implementation.

* Fix imports and logic to handle gauges.

* Fix checker framework

* fix style.

4 files changed, 76 insertions, 228 deletions

diff --git a/impl_core/src/main/java/io/opencensus/implcore/stats/MeasureToViewMap.java b/impl_core/src/main/java/io/opencensus/implcore/stats/MeasureToViewMap.java
index 4836960c..c5b2e1ae 100644
--- a/impl_core/src/main/java/io/opencensus/implcore/stats/MeasureToViewMap.java
+++ b/impl_core/src/main/java/io/opencensus/implcore/stats/MeasureToViewMap.java
@@ -23,13 +23,13 @@ import com.google.common.collect.Sets;
 import io.opencensus.common.Clock;
 import io.opencensus.common.Timestamp;
 import io.opencensus.metrics.Metric;
-import io.opencensus.metrics.MetricDescriptor;
 import io.opencensus.stats.Measure;
 import io.opencensus.stats.Measurement;
 import io.opencensus.stats.StatsCollectionState;
 import io.opencensus.stats.View;
 import io.opencensus.stats.ViewData;
 import io.opencensus.tags.TagContext;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -63,9 +63,6 @@ final class MeasureToViewMap {
   @GuardedBy("this")
   private final Map<String, Measure> registeredMeasures = Maps.newHashMap();
 
-  @GuardedBy("this")
-  private final MetricMap metricMap = new MetricMap();
-
   // Cached set of exported views. It must be set to null whenever a view is registered or
   // unregistered.
   @javax.annotation.Nullable private volatile Set<View> exportedViews;
@@ -122,11 +119,7 @@ final class MeasureToViewMap {
       registeredMeasures.put(measure.getName(), measure);
     }
     Timestamp now = clock.now();
-    mutableMap.put(view.getMeasure().getName(), MutableViewData.create(view, now, metricMap));
-    MetricDescriptor metricDescriptor = MetricUtils.viewToMetricDescriptor(view);
-    if (metricDescriptor != null) {
-      metricMap.registerMetricDescriptor(metricDescriptor, now);
-    }
+    mutableMap.put(view.getMeasure().getName(), MutableViewData.create(view, now));
   }
 
   @javax.annotation.Nullable
@@ -161,16 +154,24 @@ final class MeasureToViewMap {
         // unregistered measures will be ignored.
         continue;
       }
-      Collection<MutableViewData> views = mutableMap.get(measure.getName());
-      for (MutableViewData view : views) {
-        view.record(
+      Collection<MutableViewData> viewDataCollection = mutableMap.get(measure.getName());
+      for (MutableViewData viewData : viewDataCollection) {
+        viewData.record(
             tags, RecordUtils.getDoubleValueFromMeasurement(measurement), timestamp, attachments);
       }
     }
   }
 
-  synchronized List<Metric> getMetrics() {
-    return metricMap.toMetrics();
+  synchronized List<Metric> getMetrics(Clock clock, StatsCollectionState state) {
+    List<Metric> metrics = new ArrayList<Metric>();
+    Timestamp now = clock.now();
+    for (Entry<String, MutableViewData> entry : mutableMap.entries()) {
+      Metric metric = entry.getValue().toMetric(now, state);
+      if (metric != null) {
+        metrics.add(metric);
+      }
+    }
+    return metrics;
   }
 
   // Clear stats for all the current MutableViewData
@@ -180,7 +181,6 @@ final class MeasureToViewMap {
         mutableViewData.clearStats();
       }
     }
-    metricMap.clearStats();
   }
 
   // Resume stats collection for all MutableViewData.
@@ -190,6 +190,5 @@ final class MeasureToViewMap {
         mutableViewData.resumeStatsCollection(now);
       }
     }
-    metricMap.resumeStatsCollection(now);
   }
 }
diff --git a/impl_core/src/main/java/io/opencensus/implcore/stats/MetricMap.java b/impl_core/src/main/java/io/opencensus/implcore/stats/MetricMap.java
deleted file mode 100644
index 5ae2eed5..00000000
--- a/impl_core/src/main/java/io/opencensus/implcore/stats/MetricMap.java
+++ /dev/null
@@ -1,197 +0,0 @@
-/*
- * Copyright 2018, OpenCensus Authors
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package io.opencensus.implcore.stats;
-
-import io.opencensus.common.Timestamp;
-import io.opencensus.metrics.LabelValue;
-import io.opencensus.metrics.Metric;
-import io.opencensus.metrics.MetricDescriptor;
-import io.opencensus.metrics.MetricDescriptor.Type;
-import io.opencensus.metrics.Point;
-import io.opencensus.metrics.TimeSeriesCumulative;
-import io.opencensus.metrics.TimeSeriesGauge;
-import io.opencensus.metrics.TimeSeriesList;
-import io.opencensus.metrics.TimeSeriesList.TimeSeriesCumulativeList;
-import io.opencensus.metrics.TimeSeriesList.TimeSeriesGaugeList;
-import io.opencensus.tags.TagValue;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Map.Entry;
-import javax.annotation.concurrent.GuardedBy;
-
-/*>>>
-import org.checkerframework.checker.nullness.qual.Nullable;
-*/
-
-// A class that stores a mapping from MetricDescriptor to lists of MutableMetricRows.
-final class MetricMap {
-
-  @GuardedBy("this")
-  private final Map<MetricDescriptor, MutableMetricRows> map =
-      new HashMap<MetricDescriptor, MutableMetricRows>();
-
-  // Registers a MetricDescriptor, creates an entry in the map.
-  // This method should only be called from MeasureToMap.registerView().
-  synchronized void registerMetricDescriptor(
-      MetricDescriptor metricDescriptor, Timestamp timestamp) {
-    if (map.containsKey(metricDescriptor)) {
-      return;
-    }
-    map.put(metricDescriptor, MutableMetricRows.create(metricDescriptor.getType(), timestamp));
-  }
-
-  // Records a data point to this MetricMap.
-  // This method should only be called from CumulativeMutableViewData.record().
-  synchronized void record(
-      MetricDescriptor metricDescriptor,
-      List</*@Nullable*/ TagValue> tagValues,
-      MutableAggregation mutableAggregation,
-      Timestamp now) {
-    if (!map.containsKey(metricDescriptor)) {
-      return;
-    }
-    map.get(metricDescriptor)
-        .record(tagValues, mutableAggregation, now, metricDescriptor.getType());
-  }
-
-  synchronized void clearStats() {
-    for (Entry<MetricDescriptor, MutableMetricRows> entry : map.entrySet()) {
-      entry.getValue().map.clear();
-    }
-  }
-
-  synchronized void resumeStatsCollection(Timestamp now) {
-    for (Entry<MetricDescriptor, MutableMetricRows> entry : map.entrySet()) {
-      entry.getValue().resumeStatsCollection(now);
-    }
-  }
-
-  synchronized List<Metric> toMetrics() {
-    List<Metric> metrics = new ArrayList<Metric>();
-    for (Entry<MetricDescriptor, MutableMetricRows> entry : map.entrySet()) {
-      MutableMetricRows mutableMetricRows = entry.getValue();
-      if (mutableMetricRows.map.isEmpty()) {
-        continue; // Skip MetricDescriptor with no data.
-      }
-      metrics.add(Metric.create(entry.getKey(), mutableMetricRows.toTimeSeriesList()));
-
-      // Reset the data map once the rows are exported, so that we don't export duplicated Points.
-      mutableMetricRows.map.clear();
-    }
-    return metrics;
-  }
-
-  // A class that stores a mapping from lists of label values to lists of points.
-  // Each MutableMetricRows correspond to one MetricDescriptor.
-  // Think of this class as a set of mutable time series.
-  private abstract static class MutableMetricRows {
-
-    /*
-     * Each entry in this map is a list of rows, for example:
-     *   [v1, v2] -> [1, 5, 10]
-     *   [v1, v3] -> [-5, -8]
-     *   ...
-     */
-    private final Map<List<LabelValue>, List<Point>> map =
-        new LinkedHashMap<List<LabelValue>, List<Point>>();
-
-    // Create MutableMetricRows based on the given type.
-    private static MutableMetricRows create(Type type, Timestamp timestamp) {
-      switch (type) {
-        case GAUGE_INT64:
-        case GAUGE_DOUBLE:
-          return createGauge();
-        case CUMULATIVE_DISTRIBUTION:
-        case CUMULATIVE_DOUBLE:
-        case CUMULATIVE_INT64:
-          return createCumulative(timestamp);
-      }
-      throw new AssertionError();
-    }
-
-    private static MutableMetricRows createCumulative(Timestamp timestamp) {
-      return new MutableMetricRowsCumulative(timestamp);
-    }
-
-    private static MutableMetricRows createGauge() {
-      return new MutableMetricRowsGauge();
-    }
-
-    private void record(
-        List</*@Nullable*/ TagValue> tagValues,
-        MutableAggregation mutableAggregation,
-        Timestamp timestamp,
-        Type type) {
-      List<LabelValue> labelValues = MetricUtils.tagValuesToLabelValues(tagValues);
-      Point point = MetricUtils.mutableAggregationToPoint(mutableAggregation, timestamp, type);
-      if (!map.containsKey(labelValues)) {
-        map.put(labelValues, new ArrayList<Point>());
-      }
-      map.get(labelValues).add(point);
-    }
-
-    abstract TimeSeriesList toTimeSeriesList();
-
-    abstract void resumeStatsCollection(Timestamp now);
-
-    private static final class MutableMetricRowsCumulative extends MutableMetricRows {
-
-      // Only cumulative time series has a start timestamp.
-      private Timestamp startTime;
-
-      private MutableMetricRowsCumulative(Timestamp startTime) {
-        this.startTime = startTime;
-      }
-
-      @Override
-      TimeSeriesList toTimeSeriesList() {
-        List<TimeSeriesCumulative> timeSeriesCumulatives = new ArrayList<TimeSeriesCumulative>();
-        for (Entry<List<LabelValue>, List<Point>> entry : super.map.entrySet()) {
-          timeSeriesCumulatives.add(
-              TimeSeriesCumulative.create(entry.getKey(), entry.getValue(), startTime));
-        }
-        return TimeSeriesCumulativeList.create(timeSeriesCumulatives);
-      }
-
-      @Override
-      void resumeStatsCollection(Timestamp now) {
-        // Reset start time to current time.
-        this.startTime = now;
-      }
-    }
-
-    private static final class MutableMetricRowsGauge extends MutableMetricRows {
-
-      @Override
-      TimeSeriesList toTimeSeriesList() {
-        List<TimeSeriesGauge> timeSeriesGauges = new ArrayList<TimeSeriesGauge>();
-        for (Entry<List<LabelValue>, List<Point>> entry : super.map.entrySet()) {
-          timeSeriesGauges.add(TimeSeriesGauge.create(entry.getKey(), entry.getValue()));
-        }
-        return TimeSeriesGaugeList.create(timeSeriesGauges);
-      }
-
-      @Override
-      void resumeStatsCollection(Timestamp now) {
-        // Do nothing for Gauge stats.
-      }
-    }
-  }
-}
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 0664681c..71c92a82 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
@@ -31,7 +31,15 @@ import io.opencensus.common.Function;
 import io.opencensus.common.Functions;
 import io.opencensus.common.Timestamp;
 import io.opencensus.implcore.internal.CheckerFrameworkUtils;
+import io.opencensus.metrics.LabelValue;
+import io.opencensus.metrics.Metric;
 import io.opencensus.metrics.MetricDescriptor;
+import io.opencensus.metrics.MetricDescriptor.Type;
+import io.opencensus.metrics.Point;
+import io.opencensus.metrics.TimeSeriesCumulative;
+import io.opencensus.metrics.TimeSeriesGauge;
+import io.opencensus.metrics.TimeSeriesList.TimeSeriesCumulativeList;
+import io.opencensus.metrics.TimeSeriesList.TimeSeriesGaugeList;
 import io.opencensus.stats.Aggregation;
 import io.opencensus.stats.AggregationData;
 import io.opencensus.stats.StatsCollectionState;
@@ -39,6 +47,7 @@ import io.opencensus.stats.View;
 import io.opencensus.stats.ViewData;
 import io.opencensus.tags.TagContext;
 import io.opencensus.tags.TagValue;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
@@ -66,13 +75,12 @@ abstract class MutableViewData {
    *
    * @param view the {@code View} linked with this {@code MutableViewData}.
    * @param start the start {@code Timestamp}.
-   * @param metricMap a reference to {@code MetricMap}.
    * @return a {@code MutableViewData}.
    */
-  static MutableViewData create(final View view, final Timestamp start, final MetricMap metricMap) {
+  static MutableViewData create(final View view, final Timestamp start) {
     return view.getWindow()
         .match(
-            new CreateCumulative(view, start, metricMap),
+            new CreateCumulative(view, start),
             new CreateInterval(view, start),
             Functions.<MutableViewData>throwAssertionError());
   }
@@ -82,6 +90,9 @@ abstract class MutableViewData {
     return view;
   }
 
+  @javax.annotation.Nullable
+  abstract Metric toMetric(Timestamp now, StatsCollectionState state);
+
   /** Record stats with the given tags. */
   abstract void record(
       TagContext context, double value, Timestamp timestamp, Map<String, String> attachments);
@@ -101,15 +112,12 @@ abstract class MutableViewData {
     private Timestamp start;
     private final Map<List</*@Nullable*/ TagValue>, MutableAggregation> tagValueAggregationMap =
         Maps.newHashMap();
-    private final MetricMap metricMap;
-
     // Cache a MetricDescriptor to avoid converting View to MetricDescriptor in the future.
     private final MetricDescriptor metricDescriptor;
 
-    private CumulativeMutableViewData(View view, Timestamp start, MetricMap metricMap) {
+    private CumulativeMutableViewData(View view, Timestamp start) {
       super(view);
       this.start = start;
-      this.metricMap = metricMap;
       MetricDescriptor metricDescriptor = MetricUtils.viewToMetricDescriptor(view);
       if (metricDescriptor == null) {
         throw new AssertionError(
@@ -119,6 +127,42 @@ abstract class MutableViewData {
       }
     }
 
+    @javax.annotation.Nullable
+    @Override
+    Metric toMetric(Timestamp now, StatsCollectionState state) {
+      if (state == StatsCollectionState.DISABLED) {
+        return null;
+      }
+      // TODO(bdrutu): Refactor this after TimeSeriesGauge and TimeSeriesCumulative are combined.
+      Type type = metricDescriptor.getType();
+      if (type == Type.GAUGE_INT64 || type == Type.GAUGE_DOUBLE) {
+        List<TimeSeriesGauge> timeSeriesGauges = new ArrayList<TimeSeriesGauge>();
+        for (Entry<List</*@Nullable*/ TagValue>, MutableAggregation> entry :
+            tagValueAggregationMap.entrySet()) {
+          List<LabelValue> labelValues = MetricUtils.tagValuesToLabelValues(entry.getKey());
+          Point point = MetricUtils.mutableAggregationToPoint(entry.getValue(), now, type);
+          timeSeriesGauges.add(
+              TimeSeriesGauge.create(labelValues, Collections.singletonList(point)));
+        }
+        return Metric.create(metricDescriptor, TimeSeriesGaugeList.create(timeSeriesGauges));
+      } else {
+        List<TimeSeriesCumulative> timeSeriesCumulatives = new ArrayList<TimeSeriesCumulative>();
+        for (Entry<
+                List<
+                    /*@Nullable*/
+                    TagValue>,
+                MutableAggregation>
+            entry : tagValueAggregationMap.entrySet()) {
+          List<LabelValue> labelValues = MetricUtils.tagValuesToLabelValues(entry.getKey());
+          Point point = MetricUtils.mutableAggregationToPoint(entry.getValue(), now, type);
+          timeSeriesCumulatives.add(
+              TimeSeriesCumulative.create(labelValues, Collections.singletonList(point), start));
+        }
+        return Metric.create(
+            metricDescriptor, TimeSeriesCumulativeList.create(timeSeriesCumulatives));
+      }
+    }
+
     @Override
     void record(
         TagContext context, double value, Timestamp timestamp, Map<String, String> attachments) {
@@ -128,9 +172,7 @@ abstract class MutableViewData {
         tagValueAggregationMap.put(
             tagValues, createMutableAggregation(super.view.getAggregation()));
       }
-      MutableAggregation mutableAggregation = tagValueAggregationMap.get(tagValues);
-      mutableAggregation.add(value, attachments, timestamp);
-      metricMap.record(metricDescriptor, tagValues, mutableAggregation, timestamp);
+      tagValueAggregationMap.get(tagValues).add(value, attachments, timestamp);
     }
 
     @Override
@@ -216,6 +258,12 @@ abstract class MutableViewData {
       shiftBucketList(N + 1, start);
     }
 
+    @javax.annotation.Nullable
+    @Override
+    Metric toMetric(Timestamp now, StatsCollectionState state) {
+      return null;
+    }
+
     @Override
     void record(
         TagContext context, double value, Timestamp timestamp, Map<String, String> attachments) {
@@ -401,17 +449,15 @@ abstract class MutableViewData {
       implements Function<View.AggregationWindow.Cumulative, MutableViewData> {
     @Override
     public MutableViewData apply(View.AggregationWindow.Cumulative arg) {
-      return new CumulativeMutableViewData(view, start, metricMap);
+      return new CumulativeMutableViewData(view, start);
     }
 
     private final View view;
     private final Timestamp start;
-    private final MetricMap metricMap;
 
-    private CreateCumulative(View view, Timestamp start, MetricMap metricMap) {
+    private CreateCumulative(View view, Timestamp start) {
       this.view = view;
       this.start = start;
-      this.metricMap = metricMap;
     }
   }
 
diff --git a/impl_core/src/main/java/io/opencensus/implcore/stats/StatsManager.java b/impl_core/src/main/java/io/opencensus/implcore/stats/StatsManager.java
index 282773f2..17ba8a9e 100644
--- a/impl_core/src/main/java/io/opencensus/implcore/stats/StatsManager.java
+++ b/impl_core/src/main/java/io/opencensus/implcore/stats/StatsManager.java
@@ -71,7 +71,7 @@ final class StatsManager {
   }
 
   Collection<Metric> getMetrics() {
-    return measureToViewMap.getMetrics();
+    return measureToViewMap.getMetrics(clock, state.get());
   }
 
   void clearStats() {