From 1c8119e81a131bf5682c0e1f6cc43882c07ba884 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Thu, 7 Jan 2021 19:28:32 +0900 Subject: [PATCH] Stop using IIpConnectivityMetrics in ConnectivityService. Currently, ConnectivityService calls the IpConnectivityMetrics service class directly to log default network events. This is incompatible with ConnectivityService being in a mainline module. Replace direct access to IIpConnectivityMetrics with public methods in IpConnectivityLog, which is @SystemApi class. The new methods are not yet @SystemApi, but they can be made so if desired. Alternatively, these metrics could be deleted. Also remove the IpConectivityMetrics service from the service-connectivity JAR, and go back to starting it from SystemServer.java, which is what was happening a few hours ago before aosp/1542626 was merged. Test: builds, boots Test: atest FrameworksNetTests Test: "dumpsys connmetrics" shows events, including default network events Change-Id: I9d6147d93590363a2f8f83f39f05c03d001b4851 --- .../android/server/ConnectivityService.java | 41 +++++++++++-------- .../server/ConnectivityServiceTest.java | 3 -- .../IpConnectivityMetricsTest.java | 22 ++++++++-- 3 files changed, 44 insertions(+), 22 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 020c17a770..0986b00d68 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -87,7 +87,6 @@ import android.net.ICaptivePortal; import android.net.IConnectivityDiagnosticsCallback; import android.net.IConnectivityManager; import android.net.IDnsResolver; -import android.net.IIpConnectivityMetrics; import android.net.INetd; import android.net.INetworkManagementEventObserver; import android.net.INetworkMonitor; @@ -154,7 +153,6 @@ import android.os.PersistableBundle; import android.os.PowerManager; import android.os.Process; import android.os.RemoteException; -import android.os.ServiceManager; import android.os.ServiceSpecificException; import android.os.SystemClock; import android.os.SystemProperties; @@ -927,14 +925,6 @@ public class ConnectivityService extends IConnectivityManager.Stub "no IpConnectivityMetrics service"); } - /** - * @see IpConnectivityMetrics - */ - public IIpConnectivityMetrics getIpConnectivityMetrics() { - return IIpConnectivityMetrics.Stub.asInterface( - ServiceManager.getService(IpConnectivityLog.SERVICE_NAME)); - } - public IBatteryStats getBatteryStatsService() { return BatteryStatsService.getService(); } @@ -2983,9 +2973,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } if (valid != nai.lastValidated) { if (wasDefault) { - mDeps.getMetricsLogger() - .defaultNetworkMetrics().logDefaultNetworkValidity( - SystemClock.elapsedRealtime(), valid); + mMetricsLog.logDefaultNetworkValidity(valid); } final int oldScore = nai.getCurrentScore(); nai.lastValidated = valid; @@ -3413,7 +3401,9 @@ public class ConnectivityService extends IConnectivityManager.Stub // if there is a fallback. Taken together, the two form a X -> 0, 0 -> Y sequence // whose timestamps tell how long it takes to recover a default network. long now = SystemClock.elapsedRealtime(); - mDeps.getMetricsLogger().defaultNetworkMetrics().logDefaultNetworkEvent(now, null, nai); + mMetricsLog.logDefaultNetworkEvent(null, 0, false, + null /* lp */, null /* nc */, nai.network, nai.getCurrentScore(), + nai.linkProperties, nai.networkCapabilities); } notifyIfacesChangedForNetworkStats(); // TODO - we shouldn't send CALLBACK_LOST to requests that can be satisfied @@ -7167,9 +7157,28 @@ public class ConnectivityService extends IConnectivityManager.Stub updateDataActivityTracking(newDefaultNetwork, oldDefaultNetwork); // Notify system services of the new default. makeDefault(newDefaultNetwork); + // Log 0 -> X and Y -> X default network transitions, where X is the new default. - mDeps.getMetricsLogger().defaultNetworkMetrics().logDefaultNetworkEvent( - now, newDefaultNetwork, oldDefaultNetwork); + final Network network = (newDefaultNetwork != null) ? newDefaultNetwork.network : null; + final int score = (newDefaultNetwork != null) ? newDefaultNetwork.getCurrentScore() : 0; + final boolean validated = newDefaultNetwork != null && newDefaultNetwork.lastValidated; + final LinkProperties lp = (newDefaultNetwork != null) + ? newDefaultNetwork.linkProperties : null; + final NetworkCapabilities nc = (newDefaultNetwork != null) + ? newDefaultNetwork.networkCapabilities : null; + + final Network prevNetwork = (oldDefaultNetwork != null) + ? oldDefaultNetwork.network : null; + final int prevScore = (oldDefaultNetwork != null) + ? oldDefaultNetwork.getCurrentScore() : 0; + final LinkProperties prevLp = (oldDefaultNetwork != null) + ? oldDefaultNetwork.linkProperties : null; + final NetworkCapabilities prevNc = (oldDefaultNetwork != null) + ? oldDefaultNetwork.networkCapabilities : null; + + mMetricsLog.logDefaultNetworkEvent(network, score, validated, lp, nc, + prevNetwork, prevScore, prevLp, prevNc); + // Have a new default network, release the transition wakelock in scheduleReleaseNetworkTransitionWakelock(); } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 24b1343d8e..7b93438192 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -161,7 +161,6 @@ import android.net.DataStallReportParcelable; import android.net.EthernetManager; import android.net.IConnectivityDiagnosticsCallback; import android.net.IDnsResolver; -import android.net.IIpConnectivityMetrics; import android.net.INetd; import android.net.INetworkMonitor; import android.net.INetworkMonitorCallbacks; @@ -359,7 +358,6 @@ public class ConnectivityServiceTest { private HandlerThread mAlarmManagerThread; private TestNetIdManager mNetIdManager; - @Mock IIpConnectivityMetrics mIpConnectivityMetrics; @Mock IpConnectivityMetrics.Logger mMetricsService; @Mock DefaultNetworkMetrics mDefaultNetworkMetrics; @Mock DeviceIdleInternal mDeviceIdleInternal; @@ -1373,7 +1371,6 @@ public class ConnectivityServiceTest { doReturn(mock(ProxyTracker.class)).when(deps).makeProxyTracker(any(), any()); doReturn(mMetricsService).when(deps).getMetricsLogger(); doReturn(true).when(deps).queryUserAccess(anyInt(), anyInt()); - doReturn(mIpConnectivityMetrics).when(deps).getIpConnectivityMetrics(); doReturn(mBatteryStatsService).when(deps).getBatteryStatsService(); doAnswer(inv -> { mPolicyTracker = new WrappedMultinetworkPolicyTracker( diff --git a/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java b/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java index 3a071667a5..8c5d1d6d05 100644 --- a/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java +++ b/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java @@ -124,6 +124,22 @@ public class IpConnectivityMetricsTest { assertEquals("", output2); } + private void logDefaultNetworkEvent(long timeMs, NetworkAgentInfo nai, + NetworkAgentInfo oldNai) { + final Network network = (nai != null) ? nai.network() : null; + final int score = (nai != null) ? nai.getCurrentScore() : 0; + final boolean validated = (nai != null) ? nai.lastValidated : false; + final LinkProperties lp = (nai != null) ? nai.linkProperties : null; + final NetworkCapabilities nc = (nai != null) ? nai.networkCapabilities : null; + + final Network prevNetwork = (oldNai != null) ? oldNai.network() : null; + final int prevScore = (oldNai != null) ? oldNai.getCurrentScore() : 0; + final LinkProperties prevLp = (oldNai != null) ? oldNai.linkProperties : null; + final NetworkCapabilities prevNc = (oldNai != null) ? oldNai.networkCapabilities : null; + + mService.mDefaultNetworkMetrics.logDefaultNetworkEvent(timeMs, network, score, validated, + lp, nc, prevNetwork, prevScore, prevLp, prevNc); + } @Test public void testDefaultNetworkEvents() throws Exception { final long cell = BitUtils.packBits(new int[]{NetworkCapabilities.TRANSPORT_CELLULAR}); @@ -147,7 +163,7 @@ public class IpConnectivityMetricsTest { for (NetworkAgentInfo[] pair : defaultNetworks) { timeMs += durationMs; durationMs += durationMs; - mService.mDefaultNetworkMetrics.logDefaultNetworkEvent(timeMs, pair[1], pair[0]); + logDefaultNetworkEvent(timeMs, pair[1], pair[0]); } String want = String.join("\n", @@ -331,8 +347,8 @@ public class IpConnectivityMetricsTest { final long wifi = BitUtils.packBits(new int[]{NetworkCapabilities.TRANSPORT_WIFI}); NetworkAgentInfo cellNai = makeNai(100, 50, false, true, cell); NetworkAgentInfo wifiNai = makeNai(101, 60, true, false, wifi); - mService.mDefaultNetworkMetrics.logDefaultNetworkEvent(timeMs + 200, cellNai, null); - mService.mDefaultNetworkMetrics.logDefaultNetworkEvent(timeMs + 300, wifiNai, cellNai); + logDefaultNetworkEvent(timeMs + 200L, cellNai, null); + logDefaultNetworkEvent(timeMs + 300L, wifiNai, cellNai); String want = String.join("\n", "dropped_events: 0",