From cfddd6879283860bb4d2cf2972ea086f585a37ec Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Tue, 31 May 2016 16:28:06 +0900 Subject: [PATCH] Refactor IP connectivity event logging This patch removes static methods for logging IP connectivity events defined in android.net.metrics and replaces them with a single log() instance method defined on IpConnectivityLog. Event constructors are now public also. Every classes logging such events now create an instance of IpConnectivityLog for logging event objects directly instantiated with new. Removing static dependencies allow straightforward testing of logging. This patch also removes the base IpConnectivityEvent class which is not needed any more. Bug: 29035129 Change-Id: I3de700f93f46deaa48a759f938f7d00e1d8bff98 --- .../net/metrics/DefaultNetworkEvent.java | 8 +-- .../android/net/metrics/DhcpClientEvent.java | 8 +-- .../android/net/metrics/DhcpErrorEvent.java | 7 ++- core/java/android/net/metrics/DnsEvent.java | 6 +-- .../net/metrics/IpConnectivityEvent.java | 28 ----------- .../net/metrics/IpConnectivityLog.java | 9 +++- .../android/net/metrics/IpManagerEvent.java | 8 +-- .../net/metrics/IpReachabilityEvent.java | 10 ++-- .../android/net/metrics/NetworkEvent.java | 15 +++--- .../net/metrics/ValidationProbeEvent.java | 8 +-- .../android/server/ConnectivityService.java | 15 ++++-- .../connectivity/DnsEventListenerService.java | 6 ++- .../server/connectivity/NetworkMonitor.java | 50 ++++++++++++------- .../net/java/android/net/dhcp/DhcpClient.java | 12 +++-- .../net/java/android/net/ip/IpManager.java | 6 ++- .../android/net/ip/IpReachabilityMonitor.java | 13 +++-- .../net/metrics/IpConnectivityLogTest.java | 15 +++--- 17 files changed, 117 insertions(+), 107 deletions(-) delete mode 100644 core/java/android/net/metrics/IpConnectivityEvent.java diff --git a/core/java/android/net/metrics/DefaultNetworkEvent.java b/core/java/android/net/metrics/DefaultNetworkEvent.java index f8b59925cb910..b881fbb1dacc1 100644 --- a/core/java/android/net/metrics/DefaultNetworkEvent.java +++ b/core/java/android/net/metrics/DefaultNetworkEvent.java @@ -25,7 +25,7 @@ import android.os.Parcelable; * {@hide} */ @SystemApi -public final class DefaultNetworkEvent extends IpConnectivityEvent implements Parcelable { +public final class DefaultNetworkEvent implements Parcelable { // The ID of the network that has become the new default or NETID_UNSET if none. public final int netId; // The list of transport types of the new default network, for example TRANSPORT_WIFI, as @@ -37,7 +37,8 @@ public final class DefaultNetworkEvent extends IpConnectivityEvent implements Pa public final boolean prevIPv4; public final boolean prevIPv6; - private DefaultNetworkEvent(int netId, int[] transportTypes, + /** {@hide} */ + public DefaultNetworkEvent(int netId, int[] transportTypes, int prevNetId, boolean prevIPv4, boolean prevIPv6) { this.netId = netId; this.transportTypes = transportTypes; @@ -105,6 +106,5 @@ public final class DefaultNetworkEvent extends IpConnectivityEvent implements Pa public static void logEvent( int netId, int[] transports, int prevNetId, boolean hadIPv4, boolean hadIPv6) { - logEvent(new DefaultNetworkEvent(netId, transports, prevNetId, hadIPv4, hadIPv6)); } -}; +} diff --git a/core/java/android/net/metrics/DhcpClientEvent.java b/core/java/android/net/metrics/DhcpClientEvent.java index ec560bf64ccc3..3fe68b40e9f41 100644 --- a/core/java/android/net/metrics/DhcpClientEvent.java +++ b/core/java/android/net/metrics/DhcpClientEvent.java @@ -24,11 +24,12 @@ import android.os.Parcelable; * {@hide} */ @SystemApi -public final class DhcpClientEvent extends IpConnectivityEvent implements Parcelable { +public final class DhcpClientEvent implements Parcelable { public final String ifName; public final String msg; - private DhcpClientEvent(String ifName, String msg) { + /** {@hide} */ + public DhcpClientEvent(String ifName, String msg) { this.ifName = ifName; this.msg = msg; } @@ -64,6 +65,5 @@ public final class DhcpClientEvent extends IpConnectivityEvent implements Parcel }; public static void logStateEvent(String ifName, String state) { - logEvent(new DhcpClientEvent(ifName, state)); } -}; +} diff --git a/core/java/android/net/metrics/DhcpErrorEvent.java b/core/java/android/net/metrics/DhcpErrorEvent.java index 84795b89bb6b0..4206886c6bd05 100644 --- a/core/java/android/net/metrics/DhcpErrorEvent.java +++ b/core/java/android/net/metrics/DhcpErrorEvent.java @@ -27,7 +27,7 @@ import com.android.internal.util.MessageUtils; * {@hide} Event class used to record error events when parsing DHCP response packets. */ @SystemApi -public final class DhcpErrorEvent extends IpConnectivityEvent implements Parcelable { +public final class DhcpErrorEvent implements Parcelable { public static final int L2_ERROR = 1; public static final int L3_ERROR = 2; public static final int L4_ERROR = 3; @@ -61,7 +61,8 @@ public final class DhcpErrorEvent extends IpConnectivityEvent implements Parcela // byte 3: optional code public final int errorCode; - private DhcpErrorEvent(String ifName, int errorCode) { + /** {@hide} */ + public DhcpErrorEvent(String ifName, int errorCode) { this.ifName = ifName; this.errorCode = errorCode; } @@ -92,11 +93,9 @@ public final class DhcpErrorEvent extends IpConnectivityEvent implements Parcela }; public static void logParseError(String ifName, int errorCode) { - logEvent(new DhcpErrorEvent(ifName, errorCode)); } public static void logReceiveError(String ifName) { - logEvent(new DhcpErrorEvent(ifName, RECEIVE_ERROR)); } public static int errorCodeWithOption(int errorCode, int option) { diff --git a/core/java/android/net/metrics/DnsEvent.java b/core/java/android/net/metrics/DnsEvent.java index b94dda079cd8e..9eb8bdb579d0f 100644 --- a/core/java/android/net/metrics/DnsEvent.java +++ b/core/java/android/net/metrics/DnsEvent.java @@ -24,7 +24,7 @@ import android.os.Parcelable; * {@hide} */ @SystemApi -final public class DnsEvent extends IpConnectivityEvent implements Parcelable { +final public class DnsEvent implements Parcelable { public final int netId; // The event type is currently only 1 or 2, so we store it as a byte. @@ -37,7 +37,8 @@ final public class DnsEvent extends IpConnectivityEvent implements Parcelable { // queries. public final int[] latenciesMs; - private DnsEvent(int netId, byte[] eventTypes, byte[] returnCodes, int[] latenciesMs) { + /** {@hide} */ + public DnsEvent(int netId, byte[] eventTypes, byte[] returnCodes, int[] latenciesMs) { this.netId = netId; this.eventTypes = eventTypes; this.returnCodes = returnCodes; @@ -82,6 +83,5 @@ final public class DnsEvent extends IpConnectivityEvent implements Parcelable { public static void logEvent( int netId, byte[] eventTypes, byte[] returnCodes, int[] latenciesMs) { - logEvent(new DnsEvent(netId, eventTypes, returnCodes, latenciesMs)); } } diff --git a/core/java/android/net/metrics/IpConnectivityEvent.java b/core/java/android/net/metrics/IpConnectivityEvent.java deleted file mode 100644 index d3771dcb60af8..0000000000000 --- a/core/java/android/net/metrics/IpConnectivityEvent.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright (C) 2016 The Android Open Source Project - * - * 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 android.net.metrics; - -import android.os.Parcelable; - -/** {@hide} */ -public abstract class IpConnectivityEvent { - private static final IpConnectivityLog sMetricsLog = new IpConnectivityLog(); - - static void logEvent(T event) { - sMetricsLog.log(System.currentTimeMillis(), event); - } -} diff --git a/core/java/android/net/metrics/IpConnectivityLog.java b/core/java/android/net/metrics/IpConnectivityLog.java index 0126a8793111f..a7c1d40e90deb 100644 --- a/core/java/android/net/metrics/IpConnectivityLog.java +++ b/core/java/android/net/metrics/IpConnectivityLog.java @@ -34,6 +34,7 @@ public class IpConnectivityLog extends ConnectivityMetricsLogger { private static final boolean DBG = false; public IpConnectivityLog() { + // mService initialized in super constructor. } @VisibleForTesting @@ -46,11 +47,11 @@ public class IpConnectivityLog extends ConnectivityMetricsLogger { * keep track of skipped events and is thread-safe for callers. * * @param timestamp is the epoch timestamp of the event in ms. - * @param data is a Parcelable IpConnectivityEvent instance representing the event. + * @param data is a Parcelable instance representing the event. * * @return true if the event was successfully logged. */ - public boolean log(long timestamp, T data) { + public boolean log(long timestamp, Parcelable data) { if (mService == null) { if (DBG) { Log.d(TAG, CONNECTIVITY_METRICS_LOGGER_SERVICE + " service not ready"); @@ -78,4 +79,8 @@ public class IpConnectivityLog extends ConnectivityMetricsLogger { return false; } } + + public void log(Parcelable event) { + log(System.currentTimeMillis(), event); + } } diff --git a/core/java/android/net/metrics/IpManagerEvent.java b/core/java/android/net/metrics/IpManagerEvent.java index 0940bd06c3d34..a39061748ac3d 100644 --- a/core/java/android/net/metrics/IpManagerEvent.java +++ b/core/java/android/net/metrics/IpManagerEvent.java @@ -27,7 +27,7 @@ import com.android.internal.util.MessageUtils; * {@hide} */ @SystemApi -public final class IpManagerEvent extends IpConnectivityEvent implements Parcelable { +public final class IpManagerEvent implements Parcelable { public static final int PROVISIONING_OK = 1; public static final int PROVISIONING_FAIL = 2; @@ -37,7 +37,8 @@ public final class IpManagerEvent extends IpConnectivityEvent implements Parcela public final int eventType; public final long durationMs; - private IpManagerEvent(String ifName, int eventType, long duration) { + /** {@hide} */ + public IpManagerEvent(String ifName, int eventType, long duration) { this.ifName = ifName; this.eventType = eventType; this.durationMs = duration; @@ -71,7 +72,6 @@ public final class IpManagerEvent extends IpConnectivityEvent implements Parcela }; public static void logEvent(int eventType, String ifName, long durationMs) { - logEvent(new IpManagerEvent(ifName, eventType, durationMs)); } @Override @@ -84,4 +84,4 @@ public final class IpManagerEvent extends IpConnectivityEvent implements Parcela static final SparseArray constants = MessageUtils.findMessageNames( new Class[]{IpManagerEvent.class}, new String[]{"PROVISIONING_", "COMPLETE_"}); } -}; +} diff --git a/core/java/android/net/metrics/IpReachabilityEvent.java b/core/java/android/net/metrics/IpReachabilityEvent.java index d40389c039867..7d0229191563b 100644 --- a/core/java/android/net/metrics/IpReachabilityEvent.java +++ b/core/java/android/net/metrics/IpReachabilityEvent.java @@ -27,7 +27,7 @@ import com.android.internal.util.MessageUtils; * {@hide} */ @SystemApi -public final class IpReachabilityEvent extends IpConnectivityEvent implements Parcelable { +public final class IpReachabilityEvent implements Parcelable { public static final int PROBE = 1 << 8; public static final int NUD_FAILED = 2 << 8; @@ -41,7 +41,8 @@ public final class IpReachabilityEvent extends IpConnectivityEvent implements Pa // byte 3: kernel errno from RTNetlink or IpReachabilityMonitor public final int eventType; - private IpReachabilityEvent(String ifName, int eventType) { + /** {@hide} */ + public IpReachabilityEvent(String ifName, int eventType) { this.ifName = ifName; this.eventType = eventType; } @@ -72,15 +73,12 @@ public final class IpReachabilityEvent extends IpConnectivityEvent implements Pa }; public static void logProbeEvent(String ifName, int nlErrorCode) { - logEvent(new IpReachabilityEvent(ifName, PROBE | (nlErrorCode & 0xFF))); } public static void logNudFailed(String ifName) { - logEvent(new IpReachabilityEvent(ifName, NUD_FAILED)); } public static void logProvisioningLost(String ifName) { - logEvent(new IpReachabilityEvent(ifName, PROVISIONING_LOST)); } @Override @@ -94,4 +92,4 @@ public final class IpReachabilityEvent extends IpConnectivityEvent implements Pa MessageUtils.findMessageNames(new Class[]{IpReachabilityEvent.class}, new String[]{"PROBE", "PROVISIONING_", "NUD_"}); } -}; +} diff --git a/core/java/android/net/metrics/NetworkEvent.java b/core/java/android/net/metrics/NetworkEvent.java index 08c9c758bbc86..cdfe386d430ce 100644 --- a/core/java/android/net/metrics/NetworkEvent.java +++ b/core/java/android/net/metrics/NetworkEvent.java @@ -27,7 +27,7 @@ import com.android.internal.util.MessageUtils; * {@hide} */ @SystemApi -public final class NetworkEvent extends IpConnectivityEvent implements Parcelable { +public final class NetworkEvent implements Parcelable { public static final int NETWORK_CONNECTED = 1; public static final int NETWORK_VALIDATED = 2; @@ -41,12 +41,18 @@ public final class NetworkEvent extends IpConnectivityEvent implements Parcelabl public final int eventType; public final long durationMs; - private NetworkEvent(int netId, int eventType, long durationMs) { + /** {@hide} */ + public NetworkEvent(int netId, int eventType, long durationMs) { this.netId = netId; this.eventType = eventType; this.durationMs = durationMs; } + /** {@hide} */ + public NetworkEvent(int netId, int eventType) { + this(netId, eventType, 0); + } + private NetworkEvent(Parcel in) { netId = in.readInt(); eventType = in.readInt(); @@ -75,15 +81,12 @@ public final class NetworkEvent extends IpConnectivityEvent implements Parcelabl }; public static void logEvent(int netId, int eventType) { - logEvent(new NetworkEvent(netId, eventType, 0)); } public static void logValidated(int netId, long durationMs) { - logEvent(new NetworkEvent(netId, NETWORK_VALIDATED, durationMs)); } public static void logCaptivePortalFound(int netId, long durationMs) { - logEvent(new NetworkEvent(netId, NETWORK_CAPTIVE_PORTAL_FOUND, durationMs)); } @Override @@ -96,4 +99,4 @@ public final class NetworkEvent extends IpConnectivityEvent implements Parcelabl static final SparseArray constants = MessageUtils.findMessageNames( new Class[]{NetworkEvent.class}, new String[]{"NETWORK_"}); } -}; +} diff --git a/core/java/android/net/metrics/ValidationProbeEvent.java b/core/java/android/net/metrics/ValidationProbeEvent.java index 751c35f8a1443..d5ad0f6c25a92 100644 --- a/core/java/android/net/metrics/ValidationProbeEvent.java +++ b/core/java/android/net/metrics/ValidationProbeEvent.java @@ -27,7 +27,7 @@ import com.android.internal.util.MessageUtils; * {@hide} */ @SystemApi -public final class ValidationProbeEvent extends IpConnectivityEvent implements Parcelable { +public final class ValidationProbeEvent implements Parcelable { public static final int PROBE_DNS = 0; public static final int PROBE_HTTP = 1; @@ -42,7 +42,8 @@ public final class ValidationProbeEvent extends IpConnectivityEvent implements P public final int probeType; public final int returnCode; - private ValidationProbeEvent(int netId, long durationMs, int probeType, int returnCode) { + /** @hide */ + public ValidationProbeEvent(int netId, long durationMs, int probeType, int returnCode) { this.netId = netId; this.durationMs = durationMs; this.probeType = probeType; @@ -84,7 +85,6 @@ public final class ValidationProbeEvent extends IpConnectivityEvent implements P } public static void logEvent(int netId, long durationMs, int probeType, int returnCode) { - logEvent(new ValidationProbeEvent(netId, durationMs, probeType, returnCode)); } @Override @@ -97,4 +97,4 @@ public final class ValidationProbeEvent extends IpConnectivityEvent implements P static final SparseArray constants = MessageUtils.findMessageNames( new Class[]{ValidationProbeEvent.class}, new String[]{"PROBE_"}); } -}; +} diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 44b8d3d83df88..1a7a2bf88a883 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -76,6 +76,7 @@ import android.net.RouteInfo; import android.net.UidRange; import android.net.Uri; import android.net.metrics.DefaultNetworkEvent; +import android.net.metrics.IpConnectivityLog; import android.net.metrics.NetworkEvent; import android.os.Binder; import android.os.Build; @@ -454,6 +455,8 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + private final IpConnectivityLog mMetricsLog = new IpConnectivityLog(); + /** * Implements support for the legacy "one network per network type" model. * @@ -2205,7 +2208,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private void linger(NetworkAgentInfo nai) { nai.lingering = true; - NetworkEvent.logEvent(nai.network.netId, NetworkEvent.NETWORK_LINGER); + logNetworkEvent(nai, NetworkEvent.NETWORK_LINGER); nai.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_LINGER); notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_LOSING); } @@ -2219,7 +2222,7 @@ public class ConnectivityService extends IConnectivityManager.Stub nai.networkLingered.clear(); if (!nai.lingering) return; nai.lingering = false; - NetworkEvent.logEvent(nai.network.netId, NetworkEvent.NETWORK_UNLINGER); + logNetworkEvent(nai, NetworkEvent.NETWORK_UNLINGER); if (VDBG) log("Canceling linger of " + nai.name()); nai.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_CONNECTED); } @@ -5242,7 +5245,7 @@ public class ConnectivityService extends IConnectivityManager.Stub return new NetworkMonitor(context, handler, nai, defaultRequest); } - private static void logDefaultNetworkEvent(NetworkAgentInfo newNai, NetworkAgentInfo prevNai) { + private void logDefaultNetworkEvent(NetworkAgentInfo newNai, NetworkAgentInfo prevNai) { int newNetid = NETID_UNSET; int prevNetid = NETID_UNSET; int[] transports = new int[0]; @@ -5260,6 +5263,10 @@ public class ConnectivityService extends IConnectivityManager.Stub hadIPv6 = lp.hasGlobalIPv6Address() && lp.hasIPv6DefaultRoute(); } - DefaultNetworkEvent.logEvent(newNetid, transports, prevNetid, hadIPv4, hadIPv6); + mMetricsLog.log(new DefaultNetworkEvent(newNetid, transports, prevNetid, hadIPv4, hadIPv6)); + } + + private void logNetworkEvent(NetworkAgentInfo nai, int evtype) { + mMetricsLog.log(new NetworkEvent(nai.network.netId, evtype)); } } diff --git a/services/core/java/com/android/server/connectivity/DnsEventListenerService.java b/services/core/java/com/android/server/connectivity/DnsEventListenerService.java index 18ab73100b810..ae13db0ba95cd 100644 --- a/services/core/java/com/android/server/connectivity/DnsEventListenerService.java +++ b/services/core/java/com/android/server/connectivity/DnsEventListenerService.java @@ -18,6 +18,7 @@ package com.android.server.connectivity; import android.content.Context; import android.net.metrics.DnsEvent; +import android.net.metrics.IpConnectivityLog; import android.net.ConnectivityManager; import android.net.ConnectivityManager.NetworkCallback; import android.net.Network; @@ -50,7 +51,7 @@ public class DnsEventListenerService extends IDnsEventListener.Stub { // Stores the results of a number of consecutive DNS lookups on the same network. // This class is not thread-safe and it is the responsibility of the service to call its methods // on one thread at a time. - private static class DnsEventBatch { + private class DnsEventBatch { private final int mNetId; private final byte[] mEventTypes = new byte[MAX_LOOKUPS_PER_DNS_EVENT]; @@ -82,7 +83,7 @@ public class DnsEventListenerService extends IDnsEventListener.Stub { byte[] eventTypes = Arrays.copyOf(mEventTypes, mEventCount); byte[] returnCodes = Arrays.copyOf(mReturnCodes, mEventCount); int[] latenciesMs = Arrays.copyOf(mLatenciesMs, mEventCount); - DnsEvent.logEvent(mNetId, eventTypes, returnCodes, latenciesMs); + mMetricsLog.log(new DnsEvent(mNetId, eventTypes, returnCodes, latenciesMs)); maybeLog(String.format("Logging %d results for netId %d", mEventCount, mNetId)); mEventCount = 0; } @@ -103,6 +104,7 @@ public class DnsEventListenerService extends IDnsEventListener.Stub { // up to MAX_LOOKUPS_PER_DNS_EVENT lookup stats on each network when the system is shutting // down. We believe this to be sufficient for now. private final ConnectivityManager mCm; + private final IpConnectivityLog mMetricsLog = new IpConnectivityLog(); private final NetworkCallback mNetworkCallback = new NetworkCallback() { @Override public void onLost(Network network) { diff --git a/services/core/java/com/android/server/connectivity/NetworkMonitor.java b/services/core/java/com/android/server/connectivity/NetworkMonitor.java index ddaebfa0e7475..eeddff53e8ced 100644 --- a/services/core/java/com/android/server/connectivity/NetworkMonitor.java +++ b/services/core/java/com/android/server/connectivity/NetworkMonitor.java @@ -34,8 +34,9 @@ import android.net.NetworkRequest; import android.net.ProxyInfo; import android.net.TrafficStats; import android.net.Uri; -import android.net.metrics.ValidationProbeEvent; +import android.net.metrics.IpConnectivityLog; import android.net.metrics.NetworkEvent; +import android.net.metrics.ValidationProbeEvent; import android.net.wifi.WifiInfo; import android.net.wifi.WifiManager; import android.net.util.Stopwatch; @@ -230,6 +231,7 @@ public class NetworkMonitor extends StateMachine { private final WifiManager mWifiManager; private final AlarmManager mAlarmManager; private final NetworkRequest mDefaultRequest; + private final IpConnectivityLog mMetricsLog = new IpConnectivityLog(); private boolean mIsCaptivePortalCheckEnabled; private boolean mUseHttps; @@ -311,11 +313,11 @@ public class NetworkMonitor extends StateMachine { transitionTo(mLingeringState); return HANDLED; case CMD_NETWORK_CONNECTED: - NetworkEvent.logEvent(mNetId, NetworkEvent.NETWORK_CONNECTED); + logNetworkEvent(NetworkEvent.NETWORK_CONNECTED); transitionTo(mEvaluatingState); return HANDLED; case CMD_NETWORK_DISCONNECTED: - NetworkEvent.logEvent(mNetId, NetworkEvent.NETWORK_DISCONNECTED); + logNetworkEvent(NetworkEvent.NETWORK_DISCONNECTED); if (mLaunchCaptivePortalAppBroadcastReceiver != null) { mContext.unregisterReceiver(mLaunchCaptivePortalAppBroadcastReceiver); mLaunchCaptivePortalAppBroadcastReceiver = null; @@ -380,10 +382,7 @@ public class NetworkMonitor extends StateMachine { private class ValidatedState extends State { @Override public void enter() { - if (mEvaluationTimer.isRunning()) { - NetworkEvent.logValidated(mNetId, mEvaluationTimer.stop()); - mEvaluationTimer.reset(); - } + maybeLogEvaluationResult(NetworkEvent.NETWORK_VALIDATED); mConnectivityServiceHandler.sendMessage(obtainMessage(EVENT_NETWORK_TESTED, NETWORK_TEST_RESULT_VALID, mNetworkAgentInfo.network.netId, null)); } @@ -530,7 +529,7 @@ public class NetworkMonitor extends StateMachine { } else { final Message msg = obtainMessage(CMD_REEVALUATE, ++mReevaluateToken, 0); sendMessageDelayed(msg, mReevaluateDelayMs); - NetworkEvent.logEvent(mNetId, NetworkEvent.NETWORK_VALIDATION_FAILED); + logNetworkEvent(NetworkEvent.NETWORK_VALIDATION_FAILED); mConnectivityServiceHandler.sendMessage(obtainMessage( EVENT_NETWORK_TESTED, NETWORK_TEST_RESULT_INVALID, mNetId, probeResult.mRedirectUrl)); @@ -590,10 +589,7 @@ public class NetworkMonitor extends StateMachine { @Override public void enter() { - if (mEvaluationTimer.isRunning()) { - NetworkEvent.logCaptivePortalFound(mNetId, mEvaluationTimer.stop()); - mEvaluationTimer.reset(); - } + maybeLogEvaluationResult(NetworkEvent.NETWORK_CAPTIVE_PORTAL_FOUND); // Don't annoy user with sign-in notifications. if (mDontDisplaySigninNotification) return; // Create a CustomIntentReceiver that sends us a @@ -758,11 +754,12 @@ public class NetworkMonitor extends StateMachine { if (!TextUtils.isEmpty(hostToResolve)) { String probeName = ValidationProbeEvent.getProbeName(ValidationProbeEvent.PROBE_DNS); final Stopwatch dnsTimer = new Stopwatch().start(); + int dnsResult; + long dnsLatency; try { InetAddress[] addresses = mNetworkAgentInfo.network.getAllByName(hostToResolve); - long dnsLatency = dnsTimer.stop(); - ValidationProbeEvent.logEvent(mNetId, dnsLatency, - ValidationProbeEvent.PROBE_DNS, ValidationProbeEvent.DNS_SUCCESS); + dnsResult = ValidationProbeEvent.DNS_SUCCESS; + dnsLatency = dnsTimer.stop(); final StringBuffer connectInfo = new StringBuffer(", " + hostToResolve + "="); for (InetAddress address : addresses) { connectInfo.append(address.getHostAddress()); @@ -770,11 +767,11 @@ public class NetworkMonitor extends StateMachine { } validationLog(probeName + " OK " + dnsLatency + "ms" + connectInfo); } catch (UnknownHostException e) { - long dnsLatency = dnsTimer.stop(); - ValidationProbeEvent.logEvent(mNetId, dnsLatency, - ValidationProbeEvent.PROBE_DNS, ValidationProbeEvent.DNS_FAILURE); + dnsResult = ValidationProbeEvent.DNS_FAILURE; + dnsLatency = dnsTimer.stop(); validationLog(probeName + " FAIL " + dnsLatency + "ms, " + hostToResolve); } + logValidationProbe(dnsLatency, ValidationProbeEvent.PROBE_DNS, dnsResult); } CaptivePortalProbeResult result; @@ -855,7 +852,7 @@ public class NetworkMonitor extends StateMachine { urlConnection.disconnect(); } } - ValidationProbeEvent.logEvent(mNetId, probeTimer.stop(), probeType, httpResponseCode); + logValidationProbe(probeTimer.stop(), probeType, httpResponseCode); return new CaptivePortalProbeResult(httpResponseCode, redirectUrl); } @@ -1012,4 +1009,19 @@ public class NetworkMonitor extends StateMachine { protected WakeupMessage makeWakeupMessage(Context c, Handler h, String s, int i) { return new WakeupMessage(c, h, s, i); } + + private void logNetworkEvent(int evtype) { + mMetricsLog.log(new NetworkEvent(mNetId, evtype)); + } + + private void maybeLogEvaluationResult(int evtype) { + if (mEvaluationTimer.isRunning()) { + mMetricsLog.log(new NetworkEvent(mNetId, evtype, mEvaluationTimer.stop())); + mEvaluationTimer.reset(); + } + } + + private void logValidationProbe(long durationMs, int probeType, int probeResult) { + mMetricsLog.log(new ValidationProbeEvent(mNetId, durationMs, probeType, probeResult)); + } } diff --git a/services/net/java/android/net/dhcp/DhcpClient.java b/services/net/java/android/net/dhcp/DhcpClient.java index 96c852bf55f5a..5852626db83b8 100644 --- a/services/net/java/android/net/dhcp/DhcpClient.java +++ b/services/net/java/android/net/dhcp/DhcpClient.java @@ -30,6 +30,7 @@ import android.net.DhcpResults; import android.net.InterfaceConfiguration; import android.net.LinkAddress; import android.net.NetworkUtils; +import android.net.metrics.IpConnectivityLog; import android.net.metrics.DhcpClientEvent; import android.net.metrics.DhcpErrorEvent; import android.os.Message; @@ -163,6 +164,7 @@ public class DhcpClient extends StateMachine { // System services / libraries we use. private final Context mContext; private final Random mRandom; + private final IpConnectivityLog mMetricsLog = new IpConnectivityLog(); // Sockets. // - We use a packet socket to receive, because servers send us packets bound for IP addresses @@ -356,14 +358,14 @@ public class DhcpClient extends StateMachine { } catch (IOException|ErrnoException e) { if (!mStopped) { Log.e(TAG, "Read error", e); - DhcpErrorEvent.logReceiveError(mIfaceName); + logError(DhcpErrorEvent.RECEIVE_ERROR); } } catch (DhcpPacket.ParseException e) { Log.e(TAG, "Can't parse packet: " + e.getMessage()); if (PACKET_DBG) { Log.d(TAG, HexDump.dumpHexString(mPacket, 0, length)); } - DhcpErrorEvent.logParseError(mIfaceName, e.errorCode); + logError(e.errorCode); } } if (DBG) Log.d(TAG, "Receive thread stopped"); @@ -493,7 +495,7 @@ public class DhcpClient extends StateMachine { @Override public void enter() { if (STATE_DBG) Log.d(TAG, "Entering state " + getName()); - DhcpClientEvent.logStateEvent(mIfaceName, getName()); + mMetricsLog.log(new DhcpClientEvent(mIfaceName, getName())); } private String messageName(int what) { @@ -977,4 +979,8 @@ public class DhcpClient extends StateMachine { class DhcpRebootingState extends LoggingState { } + + private void logError(int errorCode) { + mMetricsLog.log(new DhcpErrorEvent(mIfaceName, errorCode)); + } } diff --git a/services/net/java/android/net/ip/IpManager.java b/services/net/java/android/net/ip/IpManager.java index cece6c8c0cac6..86e15182165d9 100644 --- a/services/net/java/android/net/ip/IpManager.java +++ b/services/net/java/android/net/ip/IpManager.java @@ -31,6 +31,7 @@ import android.net.ProxyInfo; import android.net.RouteInfo; import android.net.StaticIpConfiguration; import android.net.dhcp.DhcpClient; +import android.net.metrics.IpConnectivityLog; import android.net.metrics.IpManagerEvent; import android.os.INetworkManagementService; import android.os.Message; @@ -393,6 +394,7 @@ public class IpManager extends StateMachine { private final WakeupMessage mProvisioningTimeoutAlarm; private final WakeupMessage mDhcpActionTimeoutAlarm; private final LocalLog mLocalLog; + private final IpConnectivityLog mMetricsLog = new IpConnectivityLog(); private NetworkInterface mNetworkInterface; @@ -634,8 +636,8 @@ public class IpManager extends StateMachine { private void recordMetric(final int type) { if (mStartTimeMillis <= 0) { Log.wtf(mTag, "Start time undefined!"); } - IpManagerEvent.logEvent(type, mInterfaceName, - SystemClock.elapsedRealtime() - mStartTimeMillis); + final long duration = SystemClock.elapsedRealtime() - mStartTimeMillis; + mMetricsLog.log(new IpManagerEvent(mInterfaceName, type, duration)); } // For now: use WifiStateMachine's historical notion of provisioned. diff --git a/services/net/java/android/net/ip/IpReachabilityMonitor.java b/services/net/java/android/net/ip/IpReachabilityMonitor.java index 27600a768896b..afb644f3546bf 100644 --- a/services/net/java/android/net/ip/IpReachabilityMonitor.java +++ b/services/net/java/android/net/ip/IpReachabilityMonitor.java @@ -24,6 +24,7 @@ import android.net.LinkProperties; import android.net.LinkProperties.ProvisioningChange; import android.net.ProxyInfo; import android.net.RouteInfo; +import android.net.metrics.IpConnectivityLog; import android.net.metrics.IpReachabilityEvent; import android.net.netlink.NetlinkConstants; import android.net.netlink.NetlinkErrorMessage; @@ -151,6 +152,7 @@ public class IpReachabilityMonitor { private final Callback mCallback; private final NetlinkSocketObserver mNetlinkSocketObserver; private final Thread mObserverThread; + private final IpConnectivityLog mMetricsLog = new IpConnectivityLog(); @GuardedBy("mLock") private LinkProperties mLinkProperties = new LinkProperties(); // TODO: consider a map to a private NeighborState class holding more @@ -359,7 +361,6 @@ public class IpReachabilityMonitor { } if (delta == ProvisioningChange.LOST_PROVISIONING) { - IpReachabilityEvent.logProvisioningLost(mInterfaceName); final String logMsg = "FAILURE: LOST_PROVISIONING, " + msg; Log.w(TAG, logMsg); if (mCallback != null) { @@ -367,8 +368,9 @@ public class IpReachabilityMonitor { // an InetAddress argument. mCallback.notifyLost(ip, logMsg); } + logEvent(IpReachabilityEvent.PROVISIONING_LOST, 0); } else { - IpReachabilityEvent.logNudFailed(mInterfaceName); + logEvent(IpReachabilityEvent.NUD_FAILED, 0); } } @@ -393,7 +395,7 @@ public class IpReachabilityMonitor { break; } final int returnValue = probeNeighbor(mInterfaceIndex, target); - IpReachabilityEvent.logProbeEvent(mInterfaceName, returnValue); + logEvent(IpReachabilityEvent.PROBE, returnValue); } } @@ -413,6 +415,11 @@ public class IpReachabilityMonitor { return (numUnicastProbes * retransTimeMs) + gracePeriodMs; } + private void logEvent(int probeType, int errorCode) { + int eventType = probeType | (errorCode & 0xff ); + mMetricsLog.log(new IpReachabilityEvent(mInterfaceName, eventType)); + } + // TODO: simplify the number of objects by making this extend Thread. private final class NetlinkSocketObserver implements Runnable { private NetlinkSocket mSocket; diff --git a/services/tests/servicestests/src/android/net/metrics/IpConnectivityLogTest.java b/services/tests/servicestests/src/android/net/metrics/IpConnectivityLogTest.java index 17ac9fe3e2825..1433f959898ce 100644 --- a/services/tests/servicestests/src/android/net/metrics/IpConnectivityLogTest.java +++ b/services/tests/servicestests/src/android/net/metrics/IpConnectivityLogTest.java @@ -16,8 +16,8 @@ package android.net.metrics; +import android.os.Bundle; import android.os.Parcel; -import android.os.Parcelable; import android.net.ConnectivityMetricsEvent; import android.net.IConnectivityMetricsLogger; @@ -39,11 +39,8 @@ import java.util.List; public class IpConnectivityLogTest extends TestCase { - static class FakeEvent extends IpConnectivityEvent implements Parcelable { - public int describeContents() { return 0; } - public void writeToParcel(Parcel p, int flag) { } - } - static final FakeEvent FAKE_EV = new FakeEvent(); + // use same Parcel object everywhere for pointer equality + static final Bundle FAKE_EV = new Bundle(); @Mock IConnectivityMetricsLogger mService; ArgumentCaptor evCaptor; @@ -98,7 +95,7 @@ public class IpConnectivityLogTest extends TestCase { assertFalse(mLog.log(1, FAKE_EV)); new Thread() { public void run() { - busySpinLog(FAKE_EV); + busySpinLog(); } }.start(); @@ -116,7 +113,7 @@ public class IpConnectivityLogTest extends TestCase { for (int i = 0; i < nCallers; i++) { new Thread() { public void run() { - busySpinLog(FAKE_EV); + busySpinLog(); } }.start(); } @@ -128,7 +125,7 @@ public class IpConnectivityLogTest extends TestCase { } } - void busySpinLog(Parcelable ev) { + void busySpinLog() { final long timeout = 200; final long stop = System.currentTimeMillis() + timeout; try {