Skip writing the metrics before T.

Repeated fields are only supported on T+ and the metrics in
KeepaliveStatsTracker contains repeated fields. Hence, guard the write
with a T+ check.

Bug: 289344384
Test: Manual test
Change-Id: If3be75292b5a79aa753bddb772fe2c52c9dde994

service/src/com/android/server/connectivity/KeepaliveStatsTracker.java

@@ -45,6 +45,7 @@ import com.android.metrics.DurationPerNumOfKeepalive;
 import com.android.metrics.KeepaliveLifetimeForCarrier;
 import com.android.metrics.KeepaliveLifetimePerCarrier;
 import com.android.modules.utils.BackgroundThread;
+import com.android.modules.utils.build.SdkLevel;
 import com.android.net.module.util.CollectionUtils;
 import com.android.server.ConnectivityStatsLog;
 
@@ -251,6 +252,22 @@ public class KeepaliveStatsTracker {
         public long getElapsedRealtime() {
             return SystemClock.elapsedRealtime();
         }
+
+        /**
+         * Writes a DAILY_KEEPALIVE_INFO_REPORTED to ConnectivityStatsLog.
+         *
+         * @param dailyKeepaliveInfoReported the proto to write to statsD.
+         */
+        public void writeStats(DailykeepaliveInfoReported dailyKeepaliveInfoReported) {
+            ConnectivityStatsLog.write(
+                    ConnectivityStatsLog.DAILY_KEEPALIVE_INFO_REPORTED,
+                    dailyKeepaliveInfoReported.getDurationPerNumOfKeepalive().toByteArray(),
+                    dailyKeepaliveInfoReported.getKeepaliveLifetimePerCarrier().toByteArray(),
+                    dailyKeepaliveInfoReported.getKeepaliveRequests(),
+                    dailyKeepaliveInfoReported.getAutomaticKeepaliveRequests(),
+                    dailyKeepaliveInfoReported.getDistinctUserCount(),
+                    CollectionUtils.toIntArray(dailyKeepaliveInfoReported.getUidList()));
+        }
     }
 
     public KeepaliveStatsTracker(@NonNull Context context, @NonNull Handler handler) {
@@ -637,15 +654,15 @@ public class KeepaliveStatsTracker {
     /** Writes the stored metrics to ConnectivityStatsLog and resets.  */
     public void writeAndResetMetrics() {
         ensureRunningOnHandlerThread();
+        // Keepalive stats use repeated atoms, which are only supported on T+. If written to statsd
+        // on S- they will bootloop the system, so they must not be sent on S-. See b/289471411.
+        if (!SdkLevel.isAtLeastT()) {
+            Log.d(TAG, "KeepaliveStatsTracker is disabled before T, skipping write");
+            return;
+        }
+
         final DailykeepaliveInfoReported dailyKeepaliveInfoReported = buildAndResetMetrics();
-        ConnectivityStatsLog.write(
+        mDependencies.writeStats(dailyKeepaliveInfoReported);
-                ConnectivityStatsLog.DAILY_KEEPALIVE_INFO_REPORTED,
-                dailyKeepaliveInfoReported.getDurationPerNumOfKeepalive().toByteArray(),
-                dailyKeepaliveInfoReported.getKeepaliveLifetimePerCarrier().toByteArray(),
-                dailyKeepaliveInfoReported.getKeepaliveRequests(),
-                dailyKeepaliveInfoReported.getAutomaticKeepaliveRequests(),
-                dailyKeepaliveInfoReported.getDistinctUserCount(),
-                CollectionUtils.toIntArray(dailyKeepaliveInfoReported.getUidList()));
     }
 
     private void ensureRunningOnHandlerThread() {

tests/unit/java/com/android/server/connectivity/KeepaliveStatsTrackerTest.java

@@ -19,6 +19,8 @@ package com.android.server.connectivity;
 import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR;
 import static android.net.NetworkCapabilities.TRANSPORT_WIFI;
 
+import static com.android.testutils.DevSdkIgnoreRule.IgnoreAfter;
+import static com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo;
 import static com.android.testutils.HandlerUtils.visibleOnHandlerThread;
 
 import static org.junit.Assert.assertArrayEquals;
@@ -31,6 +33,7 @@ import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doCallRealMethod;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 
 import android.content.BroadcastReceiver;
@@ -62,6 +65,7 @@ import com.android.testutils.DevSdkIgnoreRunner;
 import com.android.testutils.HandlerUtils;
 
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
@@ -103,6 +107,8 @@ public class KeepaliveStatsTrackerTest {
                 .build();
     }
 
+    @Rule public final DevSdkIgnoreRule ignoreRule = new DevSdkIgnoreRule();
+
     private HandlerThread mHandlerThread;
     private Handler mTestHandler;
 
@@ -1192,4 +1198,43 @@ public class KeepaliveStatsTrackerTest {
                     expectKeepaliveCarrierStats1, expectKeepaliveCarrierStats2
                 });
     }
+
+    @Test
+    @IgnoreAfter(Build.VERSION_CODES.S_V2)
+    public void testWriteMetrics_doNothingBeforeT() {
+        // Keepalive stats use repeated atoms, which are only supported on T+. If written to statsd
+        // on S- they will bootloop the system, so they must not be sent on S-. See b/289471411.
+        final int writeTime = 1000;
+        setElapsedRealtime(writeTime);
+        visibleOnHandlerThread(mTestHandler, () -> mKeepaliveStatsTracker.writeAndResetMetrics());
+        verify(mDependencies, never()).writeStats(any());
+    }
+
+    @Test
+    @IgnoreUpTo(Build.VERSION_CODES.S_V2)
+    public void testWriteMetrics() {
+        final int writeTime = 1000;
+
+        final ArgumentCaptor<DailykeepaliveInfoReported> dailyKeepaliveInfoReportedCaptor =
+                ArgumentCaptor.forClass(DailykeepaliveInfoReported.class);
+
+        setElapsedRealtime(writeTime);
+        visibleOnHandlerThread(mTestHandler, () -> mKeepaliveStatsTracker.writeAndResetMetrics());
+        // Ensure writeStats is called with the correct DailykeepaliveInfoReported metrics.
+        verify(mDependencies).writeStats(dailyKeepaliveInfoReportedCaptor.capture());
+        final DailykeepaliveInfoReported dailyKeepaliveInfoReported =
+                dailyKeepaliveInfoReportedCaptor.getValue();
+
+        // Same as the no keepalive case
+        final int[] expectRegisteredDurations = new int[] {writeTime};
+        final int[] expectActiveDurations = new int[] {writeTime};
+        assertDailyKeepaliveInfoReported(
+                dailyKeepaliveInfoReported,
+                /* expectRequestsCount= */ 0,
+                /* expectAutoRequestsCount= */ 0,
+                /* expectAppUids= */ new int[0],
+                expectRegisteredDurations,
+                expectActiveDurations,
+                new KeepaliveCarrierStats[0]);
+    }
 }