Move non-monotonic reporting to interface.

Report non-monotonic NetworkStats through an observer interface
instead of throwing, since those events are still recoverable.

Change-Id: Ic0749f4634b0ac05dbe90e95ca490957ec8b2f23

core/java/android/net/NetworkStats.java

@@ -16,8 +16,6 @@
 
 package android.net;
 
-import static com.android.internal.util.Preconditions.checkNotNull;
-
 import android.os.Parcel;
 import android.os.Parcelable;
 import android.os.SystemClock;
@@ -40,8 +38,6 @@ import java.util.HashSet;
  * @hide
  */
 public class NetworkStats implements Parcelable {
-    private static final String TAG = "NetworkStats";
-
     /** {@link #iface} value when interface details unavailable. */
     public static final String IFACE_ALL = null;
     /** {@link #uid} value when UID details unavailable. */
@@ -463,62 +459,64 @@ public class NetworkStats implements Parcelable {
      * between two snapshots in time. Assumes that statistics rows collect over
      * time, and that none of them have disappeared.
      */
-    public NetworkStats subtract(NetworkStats value) throws NonMonotonicException {
+    public NetworkStats subtract(NetworkStats right) {
-        return subtract(value, false);
+        return subtract(this, right, null);
     }
 
     /**
-     * Subtract the given {@link NetworkStats}, effectively leaving the delta
+     * Subtract the two given {@link NetworkStats} objects, returning the delta
      * between two snapshots in time. Assumes that statistics rows collect over
      * time, and that none of them have disappeared.
-     *
+     * <p>
-     * @param clampNonMonotonic When non-monotonic stats are found, just clamp
+     * If counters have rolled backwards, they are clamped to {@code 0} and
-     *            to 0 instead of throwing {@link NonMonotonicException}.
+     * reported to the given {@link NonMonotonicObserver}.
      */
-    public NetworkStats subtract(NetworkStats value, boolean clampNonMonotonic)
+    public static NetworkStats subtract(
-            throws NonMonotonicException {
+            NetworkStats left, NetworkStats right, NonMonotonicObserver observer) {
-        final long deltaRealtime = this.elapsedRealtime - value.elapsedRealtime;
+        long deltaRealtime = left.elapsedRealtime - right.elapsedRealtime;
         if (deltaRealtime < 0) {
-            throw new NonMonotonicException(this, value);
+            if (observer != null) {
+                observer.foundNonMonotonic(left, -1, right, -1);
+            }
+            deltaRealtime = 0;
         }
 
         // result will have our rows, and elapsed time between snapshots
         final Entry entry = new Entry();
-        final NetworkStats result = new NetworkStats(deltaRealtime, size);
+        final NetworkStats result = new NetworkStats(deltaRealtime, left.size);
-        for (int i = 0; i < size; i++) {
+        for (int i = 0; i < left.size; i++) {
-            entry.iface = iface[i];
+            entry.iface = left.iface[i];
-            entry.uid = uid[i];
+            entry.uid = left.uid[i];
-            entry.set = set[i];
+            entry.set = left.set[i];
-            entry.tag = tag[i];
+            entry.tag = left.tag[i];
 
             // find remote row that matches, and subtract
-            final int j = value.findIndexHinted(entry.iface, entry.uid, entry.set, entry.tag, i);
+            final int j = right.findIndexHinted(entry.iface, entry.uid, entry.set, entry.tag, i);
             if (j == -1) {
                 // newly appearing row, return entire value
-                entry.rxBytes = rxBytes[i];
+                entry.rxBytes = left.rxBytes[i];
-                entry.rxPackets = rxPackets[i];
+                entry.rxPackets = left.rxPackets[i];
-                entry.txBytes = txBytes[i];
+                entry.txBytes = left.txBytes[i];
-                entry.txPackets = txPackets[i];
+                entry.txPackets = left.txPackets[i];
-                entry.operations = operations[i];
+                entry.operations = left.operations[i];
             } else {
                 // existing row, subtract remote value
-                entry.rxBytes = rxBytes[i] - value.rxBytes[j];
+                entry.rxBytes = left.rxBytes[i] - right.rxBytes[j];
-                entry.rxPackets = rxPackets[i] - value.rxPackets[j];
+                entry.rxPackets = left.rxPackets[i] - right.rxPackets[j];
-                entry.txBytes = txBytes[i] - value.txBytes[j];
+                entry.txBytes = left.txBytes[i] - right.txBytes[j];
-                entry.txPackets = txPackets[i] - value.txPackets[j];
+                entry.txPackets = left.txPackets[i] - right.txPackets[j];
-                entry.operations = operations[i] - value.operations[j];
+                entry.operations = left.operations[i] - right.operations[j];
 
                 if (entry.rxBytes < 0 || entry.rxPackets < 0 || entry.txBytes < 0
                         || entry.txPackets < 0 || entry.operations < 0) {
-                    if (clampNonMonotonic) {
+                    if (observer != null) {
-                        entry.rxBytes = Math.max(entry.rxBytes, 0);
+                        observer.foundNonMonotonic(left, i, right, j);
-                        entry.rxPackets = Math.max(entry.rxPackets, 0);
-                        entry.txBytes = Math.max(entry.txBytes, 0);
-                        entry.txPackets = Math.max(entry.txPackets, 0);
-                        entry.operations = Math.max(entry.operations, 0);
-                    } else {
-                        throw new NonMonotonicException(this, i, value, j);
                     }
+                    entry.rxBytes = Math.max(entry.rxBytes, 0);
+                    entry.rxPackets = Math.max(entry.rxPackets, 0);
+                    entry.txBytes = Math.max(entry.txBytes, 0);
+                    entry.txPackets = Math.max(entry.txPackets, 0);
+                    entry.operations = Math.max(entry.operations, 0);
                 }
             }
 
@@ -665,22 +663,8 @@ public class NetworkStats implements Parcelable {
         }
     };
 
-    public static class NonMonotonicException extends Exception {
+    public interface NonMonotonicObserver {
-        public final NetworkStats left;
+        public void foundNonMonotonic(
-        public final NetworkStats right;
+                NetworkStats left, int leftIndex, NetworkStats right, int rightIndex);
-        public final int leftIndex;
-        public final int rightIndex;
-
-        public NonMonotonicException(NetworkStats left, NetworkStats right) {
-            this(left, -1, right, -1);
-        }
-
-        public NonMonotonicException(
-                NetworkStats left, int leftIndex, NetworkStats right, int rightIndex) {
-            this.left = checkNotNull(left, "missing left");
-            this.right = checkNotNull(right, "missing right");
-            this.leftIndex = leftIndex;
-            this.rightIndex = rightIndex;
-        }
     }
 }

core/java/android/net/TrafficStats.java

@@ -20,7 +20,6 @@ import android.app.DownloadManager;
 import android.app.backup.BackupManager;
 import android.content.Context;
 import android.media.MediaPlayer;
-import android.net.NetworkStats.NonMonotonicException;
 import android.os.RemoteException;
 import android.os.ServiceManager;
 
@@ -193,15 +192,12 @@ public class TrafficStats {
                 throw new IllegalStateException("not profiling data");
             }
 
-            try {
+            // subtract starting values and return delta
-                // subtract starting values and return delta
+            final NetworkStats profilingStop = getDataLayerSnapshotForUid(context);
-                final NetworkStats profilingStop = getDataLayerSnapshotForUid(context);
+            final NetworkStats profilingDelta = NetworkStats.subtract(
-                final NetworkStats profilingDelta = profilingStop.subtract(sActiveProfilingStart);
+                    profilingStop, sActiveProfilingStart, null);
-                sActiveProfilingStart = null;
+            sActiveProfilingStart = null;
-                return profilingDelta;
+            return profilingDelta;
-            } catch (NonMonotonicException e) {
-                throw new RuntimeException(e);
-            }
         }
     }
 

services/java/com/android/server/net/NetworkStatsService.java

@@ -71,7 +71,7 @@ import android.net.NetworkIdentity;
 import android.net.NetworkInfo;
 import android.net.NetworkState;
 import android.net.NetworkStats;
-import android.net.NetworkStats.NonMonotonicException;
+import android.net.NetworkStats.NonMonotonicObserver;
 import android.net.NetworkStatsHistory;
 import android.net.NetworkTemplate;
 import android.os.Binder;
@@ -1551,6 +1551,30 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
         }
     }
 
+    private StatsObserver mStatsObserver = new StatsObserver();
+
+    private class StatsObserver implements NonMonotonicObserver {
+        private String mCurrentType;
+
+        public void setCurrentType(String type) {
+            mCurrentType = type;
+        }
+
+        /** {@inheritDoc} */
+        public void foundNonMonotonic(
+                NetworkStats left, int leftIndex, NetworkStats right, int rightIndex) {
+            Log.w(TAG, "found non-monotonic values; saving to dropbox");
+
+            // record error for debugging
+            final StringBuilder builder = new StringBuilder();
+            builder.append("found non-monotonic " + mCurrentType + " values at left[" + leftIndex
+                    + "] - right[" + rightIndex + "]\n");
+            builder.append("left=").append(left).append('\n');
+            builder.append("right=").append(right).append('\n');
+            mDropBox.addText(TAG_NETSTATS_ERROR, builder.toString());
+        }
+    }
+
     /**
      * Return the delta between two {@link NetworkStats} snapshots, where {@code
      * before} can be {@code null}.
@@ -1558,27 +1582,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
     private NetworkStats computeStatsDelta(
             NetworkStats before, NetworkStats current, boolean collectStale, String type) {
         if (before != null) {
-            try {
+            mStatsObserver.setCurrentType(type);
-                return current.subtract(before, false);
+            return NetworkStats.subtract(current, before, mStatsObserver);
-            } catch (NonMonotonicException e) {
-                Log.w(TAG, "found non-monotonic values; saving to dropbox");
-
-                // record error for debugging
-                final StringBuilder builder = new StringBuilder();
-                builder.append("found non-monotonic " + type + " values at left[" + e.leftIndex
-                        + "] - right[" + e.rightIndex + "]\n");
-                builder.append("left=").append(e.left).append('\n');
-                builder.append("right=").append(e.right).append('\n');
-                mDropBox.addText(TAG_NETSTATS_ERROR, builder.toString());
-
-                try {
-                    // return clamped delta to help recover
-                    return current.subtract(before, true);
-                } catch (NonMonotonicException e1) {
-                    Log.wtf(TAG, "found non-monotonic values; returning empty delta", e1);
-                    return new NetworkStats(0L, 10);
-                }
-            }
         } else if (collectStale) {
             // caller is okay collecting stale stats for first call.
             return current;