From 2a5cfb9738867da634a3bf64ccef881aaefbfce5 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Wed, 22 Mar 2017 22:21:44 +0900 Subject: [PATCH] Connectivity metrics: log DnsEvents in-band This patch changes how DnsEvents are logged in IpConnectivityMetrics. The following changes are made: - DnsEventBatch are not logged after 100 queries on the same network - this allows to merge DnsEvent and DnsEventBatch into one class - DnsEventBatch are not logged after a network disconnect - this allows to remove the NetworkCallback - DnsEvent are now logged similarly to ConnectStats when statistics are flushed, in a direct call from IpConnectivityMetrics into NetdEventListenerService, in a direct call from IpConnectivityMetrics into NetdEventListenerService. - this allows to remove the Parcelable implementation of DnsEvent - transports information is added to DnsEvent. Test: - simplified NetdEventListenerServiceTest covering dns logging - updated IpConnectivityEventBuilderTest - updated IpConnectivityMetricsTest - $ runtest frameworks-net passes - manually verified $ adb shell dumpsys connmetrics list proto Bug: 34901696 Change-Id: I4fcd0ad7a7b85d587647f471a90c1e53a18fc95a Merged-In: Ia4b33fd4212741152662a2adbb0533bd1b4902ee (cherry picked from commit 0699cf98042a64e41ee076c464eb115a6579be08) --- core/java/android/net/metrics/DnsEvent.java | 85 +++-- .../IpConnectivityEventBuilder.java | 30 +- .../connectivity/IpConnectivityMetrics.java | 3 +- .../NetdEventListenerService.java | 145 +++----- .../IpConnectivityEventBuilderTest.java | 129 ++----- .../IpConnectivityMetricsTest.java | 5 +- .../NetdEventListenerServiceTest.java | 329 +++++++++--------- 7 files changed, 304 insertions(+), 422 deletions(-) diff --git a/core/java/android/net/metrics/DnsEvent.java b/core/java/android/net/metrics/DnsEvent.java index 89ae1c258ee61..dfa1ac13ea31d 100644 --- a/core/java/android/net/metrics/DnsEvent.java +++ b/core/java/android/net/metrics/DnsEvent.java @@ -16,67 +16,64 @@ package android.net.metrics; -import android.os.Parcel; -import android.os.Parcelable; +import java.util.Arrays; /** * A DNS event recorded by NetdEventListenerService. * {@hide} */ -final public class DnsEvent implements Parcelable { - public final int netId; +final public class DnsEvent { - // The event type is currently only 1 or 2, so we store it as a byte. - public final byte[] eventTypes; + private static final int SIZE_LIMIT = 20000; + + // Network id of the network associated with the event, or 0 if unspecified. + public final int netId; + // Transports of the network associated with the event, as defined in NetworkCapabilities. + // It is the caller responsability to ensure the value of transports does not change between + // calls to addResult. + public final long transports; + // The number of DNS queries recorded. Queries are stored in the structure-of-array style where + // the eventTypes, returnCodes, and latenciesMs arrays have the same length and the i-th event + // is spread across the three array at position i. + public int eventCount; + // The types of DNS queries as defined in INetdEventListener. + public byte[] eventTypes; // Current getaddrinfo codes go from 1 to EAI_MAX = 15. gethostbyname returns errno, but there // are fewer than 255 errno values. So we store the result code in a byte as well. - public final byte[] returnCodes; - // The latency is an integer because a) short arrays aren't parcelable and b) a short can only - // store a maximum latency of 32757 or 65535 ms, which is too short for pathologically slow - // queries. - public final int[] latenciesMs; + public byte[] returnCodes; + // Latencies in milliseconds of queries, stored as ints. + public int[] latenciesMs; - public DnsEvent(int netId, byte[] eventTypes, byte[] returnCodes, int[] latenciesMs) { + public DnsEvent(int netId, long transports, int initialCapacity) { this.netId = netId; - this.eventTypes = eventTypes; - this.returnCodes = returnCodes; - this.latenciesMs = latenciesMs; + this.transports = transports; + eventTypes = new byte[initialCapacity]; + returnCodes = new byte[initialCapacity]; + latenciesMs = new int[initialCapacity]; } - private DnsEvent(Parcel in) { - this.netId = in.readInt(); - this.eventTypes = in.createByteArray(); - this.returnCodes = in.createByteArray(); - this.latenciesMs = in.createIntArray(); + public void addResult(byte eventType, byte returnCode, int latencyMs) { + if (eventCount >= SIZE_LIMIT) { + // TODO: implement better rate limiting that does not biases metrics. + return; + } + if (eventCount == eventTypes.length) { + resize((int) (1.4 * eventCount)); + } + eventTypes[eventCount] = eventType; + returnCodes[eventCount] = returnCode; + latenciesMs[eventCount] = latencyMs; + eventCount++; } - @Override - public void writeToParcel(Parcel out, int flags) { - out.writeInt(netId); - out.writeByteArray(eventTypes); - out.writeByteArray(returnCodes); - out.writeIntArray(latenciesMs); - } - - @Override - public int describeContents() { - return 0; + public void resize(int newLength) { + eventTypes = Arrays.copyOf(eventTypes, newLength); + returnCodes = Arrays.copyOf(returnCodes, newLength); + latenciesMs = Arrays.copyOf(latenciesMs, newLength); } @Override public String toString() { - return String.format("DnsEvent(%d, %d events)", netId, eventTypes.length); + return String.format("DnsEvent(%d events)", eventCount); } - - public static final Parcelable.Creator CREATOR = new Parcelable.Creator() { - @Override - public DnsEvent createFromParcel(Parcel in) { - return new DnsEvent(in); - } - - @Override - public DnsEvent[] newArray(int size) { - return new DnsEvent[size]; - } - }; } diff --git a/services/core/java/com/android/server/connectivity/IpConnectivityEventBuilder.java b/services/core/java/com/android/server/connectivity/IpConnectivityEventBuilder.java index ad66faa8cf04a..f05acefadb3db 100644 --- a/services/core/java/com/android/server/connectivity/IpConnectivityEventBuilder.java +++ b/services/core/java/com/android/server/connectivity/IpConnectivityEventBuilder.java @@ -91,6 +91,21 @@ final public class IpConnectivityEventBuilder { return out; } + public static IpConnectivityEvent toProto(DnsEvent in) { + final IpConnectivityEvent out = new IpConnectivityEvent(); + IpConnectivityLogClass.DNSLookupBatch dnsLookupBatch = + new IpConnectivityLogClass.DNSLookupBatch(); + in.resize(in.eventCount); + dnsLookupBatch.eventTypes = bytesToInts(in.eventTypes); + dnsLookupBatch.returnCodes = bytesToInts(in.returnCodes); + dnsLookupBatch.latenciesMs = in.latenciesMs; + out.setDnsLookupBatch(dnsLookupBatch); + out.networkId = in.netId; + out.transports = in.transports; + inferLinkLayer(out); + return out; + } + private static boolean setEvent(IpConnectivityEvent out, Parcelable in) { if (in instanceof DhcpErrorEvent) { setDhcpErrorEvent(out, (DhcpErrorEvent) in); @@ -102,11 +117,6 @@ final public class IpConnectivityEventBuilder { return true; } - if (in instanceof DnsEvent) { - setDnsEvent(out, (DnsEvent) in); - return true; - } - if (in instanceof IpManagerEvent) { setIpManagerEvent(out, (IpManagerEvent) in); return true; @@ -163,16 +173,6 @@ final public class IpConnectivityEventBuilder { out.setDhcpEvent(dhcpEvent); } - private static void setDnsEvent(IpConnectivityEvent out, DnsEvent in) { - IpConnectivityLogClass.DNSLookupBatch dnsLookupBatch = - new IpConnectivityLogClass.DNSLookupBatch(); - dnsLookupBatch.networkId = netIdOf(in.netId); - dnsLookupBatch.eventTypes = bytesToInts(in.eventTypes); - dnsLookupBatch.returnCodes = bytesToInts(in.returnCodes); - dnsLookupBatch.latenciesMs = in.latenciesMs; - out.setDnsLookupBatch(dnsLookupBatch); - } - private static void setIpManagerEvent(IpConnectivityEvent out, IpManagerEvent in) { IpConnectivityLogClass.IpProvisioningEvent ipProvisioningEvent = new IpConnectivityLogClass.IpProvisioningEvent(); diff --git a/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java b/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java index da56a07d2396e..475d786aedd10 100644 --- a/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java +++ b/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java @@ -76,7 +76,8 @@ final public class IpConnectivityMetrics extends SystemService { @VisibleForTesting public final Impl impl = new Impl(); - private NetdEventListenerService mNetdListener; + @VisibleForTesting + NetdEventListenerService mNetdListener; @GuardedBy("mLock") private ArrayList mBuffer; diff --git a/services/core/java/com/android/server/connectivity/NetdEventListenerService.java b/services/core/java/com/android/server/connectivity/NetdEventListenerService.java index 7b9c60ca6ef31..39ad584d4abf9 100644 --- a/services/core/java/com/android/server/connectivity/NetdEventListenerService.java +++ b/services/core/java/com/android/server/connectivity/NetdEventListenerService.java @@ -18,10 +18,9 @@ package com.android.server.connectivity; import android.content.Context; import android.net.ConnectivityManager; -import android.net.ConnectivityManager.NetworkCallback; import android.net.INetdEventCallback; import android.net.Network; -import android.net.NetworkRequest; +import android.net.NetworkCapabilities; import android.net.metrics.ConnectStats; import android.net.metrics.DnsEvent; import android.net.metrics.INetdEventListener; @@ -29,17 +28,16 @@ import android.net.metrics.IpConnectivityLog; import android.os.RemoteException; import android.text.format.DateUtils; import android.util.Log; +import android.util.SparseArray; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; +import com.android.internal.util.BitUtils; import com.android.internal.util.IndentingPrintWriter; import com.android.internal.util.TokenBucket; import com.android.server.connectivity.metrics.nano.IpConnectivityLogClass.ConnectStatistics; import com.android.server.connectivity.metrics.nano.IpConnectivityLogClass.IpConnectivityEvent; import java.io.PrintWriter; -import java.util.Arrays; import java.util.List; -import java.util.SortedMap; -import java.util.TreeMap; /** * Implementation of the INetdEventListener interface. @@ -52,8 +50,7 @@ public class NetdEventListenerService extends INetdEventListener.Stub { private static final boolean DBG = false; private static final boolean VDBG = false; - // TODO: read this constant from system property - private static final int MAX_LOOKUPS_PER_DNS_EVENT = 100; + private static final int INITIAL_DNS_BATCH_SIZE = 100; // Rate limit connect latency logging to 1 measurement per 15 seconds (5760 / day) with maximum // bursts of 5000 measurements. @@ -61,74 +58,11 @@ public class NetdEventListenerService extends INetdEventListener.Stub { private static final int CONNECT_LATENCY_FILL_RATE = 15 * (int) DateUtils.SECOND_IN_MILLIS; private static final int CONNECT_LATENCY_MAXIMUM_RECORDS = 20000; - // 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 class DnsEventBatch { - private final int mNetId; - - private final byte[] mEventTypes = new byte[MAX_LOOKUPS_PER_DNS_EVENT]; - private final byte[] mReturnCodes = new byte[MAX_LOOKUPS_PER_DNS_EVENT]; - private final int[] mLatenciesMs = new int[MAX_LOOKUPS_PER_DNS_EVENT]; - private int mEventCount; - - public DnsEventBatch(int netId) { - mNetId = netId; - } - - public void addResult(byte eventType, byte returnCode, int latencyMs) { - mEventTypes[mEventCount] = eventType; - mReturnCodes[mEventCount] = returnCode; - mLatenciesMs[mEventCount] = latencyMs; - mEventCount++; - if (mEventCount == MAX_LOOKUPS_PER_DNS_EVENT) { - logAndClear(); - } - } - - public void logAndClear() { - // Did we lose a race with addResult? - if (mEventCount == 0) { - return; - } - - // Only log as many events as we actually have. - byte[] eventTypes = Arrays.copyOf(mEventTypes, mEventCount); - byte[] returnCodes = Arrays.copyOf(mReturnCodes, mEventCount); - int[] latenciesMs = Arrays.copyOf(mLatenciesMs, mEventCount); - mMetricsLog.log(new DnsEvent(mNetId, eventTypes, returnCodes, latenciesMs)); - maybeLog("Logging %d results for netId %d", mEventCount, mNetId); - mEventCount = 0; - } - - // For debugging and unit tests only. - public String toString() { - return String.format("%s %d %d", getClass().getSimpleName(), mNetId, mEventCount); - } - } - - // Only sorted for ease of debugging. Because we only typically have a handful of networks up - // at any given time, performance is not a concern. + // Sparse array of DNS events, grouped by net id. @GuardedBy("this") - private final SortedMap mEventBatches = new TreeMap<>(); + private final SparseArray mDnsEvents = new SparseArray<>(); - // We register a NetworkCallback to ensure that when a network disconnects, we flush the DNS - // queries we've logged on that network. Because we do not do this periodically, we might lose - // 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; - private final NetworkCallback mNetworkCallback = new NetworkCallback() { - @Override - public void onLost(Network network) { - synchronized (NetdEventListenerService.this) { - DnsEventBatch batch = mEventBatches.remove(network.netId); - if (batch != null) { - batch.logAndClear(); - } - } - } - }; @GuardedBy("this") private final TokenBucket mConnectTb = @@ -152,16 +86,13 @@ public class NetdEventListenerService extends INetdEventListener.Stub { } public NetdEventListenerService(Context context) { - this(context.getSystemService(ConnectivityManager.class), new IpConnectivityLog()); + this(context.getSystemService(ConnectivityManager.class)); } @VisibleForTesting - public NetdEventListenerService(ConnectivityManager cm, IpConnectivityLog log) { + public NetdEventListenerService(ConnectivityManager cm) { // We are started when boot is complete, so ConnectivityService should already be running. mCm = cm; - mMetricsLog = log; - final NetworkRequest request = new NetworkRequest.Builder().clearCapabilities().build(); - mCm.registerNetworkCallback(request, mNetworkCallback); } @Override @@ -172,16 +103,16 @@ public class NetdEventListenerService extends INetdEventListener.Stub { throws RemoteException { maybeVerboseLog("onDnsEvent(%d, %d, %d, %dms)", netId, eventType, returnCode, latencyMs); - DnsEventBatch batch = mEventBatches.get(netId); - if (batch == null) { - batch = new DnsEventBatch(netId); - mEventBatches.put(netId, batch); + DnsEvent dnsEvent = mDnsEvents.get(netId); + if (dnsEvent == null) { + dnsEvent = makeDnsEvent(netId); + mDnsEvents.put(netId, dnsEvent); } - batch.addResult((byte) eventType, (byte) returnCode, latencyMs); + dnsEvent.addResult((byte) eventType, (byte) returnCode, latencyMs); if (mNetdEventCallback != null) { - mNetdEventCallback.onDnsEvent(hostname, ipAddresses, ipAddressesCount, - System.currentTimeMillis(), uid); + long timestamp = System.currentTimeMillis(); + mNetdEventCallback.onDnsEvent(hostname, ipAddresses, ipAddressesCount, timestamp, uid); } } @@ -200,21 +131,29 @@ public class NetdEventListenerService extends INetdEventListener.Stub { } public synchronized void flushStatistics(List events) { - events.add(flushConnectStats()); - // TODO: migrate DnsEventBatch to IpConnectivityLogClass.DNSLatencies + flushConnectStats(events); + flushDnsStats(events); } - private IpConnectivityEvent connectStatsProto() { + private static IpConnectivityEvent connectStatsProto(ConnectStats connectStats) { // TODO: add transport information IpConnectivityEvent ev = new IpConnectivityEvent(); - ev.setConnectStatistics(mConnectStats.toProto()); + ev.setConnectStatistics(connectStats.toProto()); return ev; } - private IpConnectivityEvent flushConnectStats() { - IpConnectivityEvent ev = connectStatsProto(); + private void flushConnectStats(List events) { + events.add(connectStatsProto(mConnectStats)); mConnectStats = makeConnectStats(); - return ev; + } + + private void flushDnsStats(List events) { + // TODO: migrate DnsEventBatch to IpConnectivityLogClass.DNSLatencies + for (int i = 0; i < mDnsEvents.size(); i++) { + IpConnectivityEvent ev = IpConnectivityEventBuilder.toProto(mDnsEvents.valueAt(i)); + events.add(ev); + } + mDnsEvents.clear(); } public synchronized void dump(PrintWriter writer) { @@ -226,20 +165,38 @@ public class NetdEventListenerService extends INetdEventListener.Stub { } public synchronized void list(PrintWriter pw) { - for (DnsEventBatch batch : mEventBatches.values()) { - pw.println(batch.toString()); + for (int i = 0; i < mDnsEvents.size(); i++) { + pw.println(mDnsEvents.valueAt(i).toString()); } pw.println(mConnectStats.toString()); } public synchronized void listAsProtos(PrintWriter pw) { - pw.println(connectStatsProto().toString()); + for (int i = 0; i < mDnsEvents.size(); i++) { + IpConnectivityEvent ev = IpConnectivityEventBuilder.toProto(mDnsEvents.valueAt(i)); + pw.println(ev.toString()); + } + pw.println(connectStatsProto(mConnectStats).toString()); } private ConnectStats makeConnectStats() { return new ConnectStats(mConnectTb, CONNECT_LATENCY_MAXIMUM_RECORDS); } + private DnsEvent makeDnsEvent(int netId) { + long transports = getTransports(netId); + return new DnsEvent(netId, transports, INITIAL_DNS_BATCH_SIZE); + } + + private long getTransports(int netId) { + // TODO: directly query ConnectivityService instead of going through Binder interface. + NetworkCapabilities nc = mCm.getNetworkCapabilities(new Network(netId)); + if (nc == null) { + return 0; + } + return BitUtils.packBits(nc.getTransportTypes()); + } + private static void maybeLog(String s, Object... args) { if (DBG) Log.d(TAG, String.format(s, args)); } diff --git a/tests/net/java/com/android/server/connectivity/IpConnectivityEventBuilderTest.java b/tests/net/java/com/android/server/connectivity/IpConnectivityEventBuilderTest.java index d819b9642e0ce..66ca4b6b7eb55 100644 --- a/tests/net/java/com/android/server/connectivity/IpConnectivityEventBuilderTest.java +++ b/tests/net/java/com/android/server/connectivity/IpConnectivityEventBuilderTest.java @@ -57,7 +57,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { aType(IpReachabilityEvent.class), anInt(IpReachabilityEvent.NUD_FAILED)); - String want = joinLines( + String want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"\"", @@ -70,13 +70,13 @@ public class IpConnectivityEventBuilderTest extends TestCase { " if_name: \"\"", " >", ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, ev); ev.netId = 123; ev.transports = 3; // transports have priority for inferrence of link layer ev.ifname = "wlan0"; - want = joinLines( + want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"\"", @@ -89,12 +89,12 @@ public class IpConnectivityEventBuilderTest extends TestCase { " if_name: \"\"", " >", ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, ev); ev.transports = 1; ev.ifname = null; - want = joinLines( + want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"\"", @@ -107,12 +107,12 @@ public class IpConnectivityEventBuilderTest extends TestCase { " if_name: \"\"", " >", ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, ev); ev.transports = 0; ev.ifname = "not_inferred"; - want = joinLines( + want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"not_inferred\"", @@ -125,11 +125,11 @@ public class IpConnectivityEventBuilderTest extends TestCase { " if_name: \"\"", " >", ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, ev); ev.ifname = "bt-pan"; - want = joinLines( + want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"\"", @@ -142,11 +142,11 @@ public class IpConnectivityEventBuilderTest extends TestCase { " if_name: \"\"", " >", ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, ev); ev.ifname = "rmnet_ipa0"; - want = joinLines( + want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"\"", @@ -159,11 +159,11 @@ public class IpConnectivityEventBuilderTest extends TestCase { " if_name: \"\"", " >", ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, ev); ev.ifname = "wlan0"; - want = joinLines( + want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"\"", @@ -176,7 +176,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { " if_name: \"\"", " >", ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, ev); } @@ -190,7 +190,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { aBool(true), aBool(false)); - String want = joinLines( + String want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"\"", @@ -211,7 +211,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { " transport_types: 3", " >", ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, ev); } @@ -223,7 +223,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { aString("SomeState"), anInt(192)); - String want = joinLines( + String want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"\"", @@ -237,7 +237,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { " state_transition: \"SomeState\"", " >", ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, ev); } @@ -248,7 +248,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { aType(DhcpErrorEvent.class), anInt(DhcpErrorEvent.L4_NOT_UDP)); - String want = joinLines( + String want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"\"", @@ -262,59 +262,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { " error_code: 50397184", " >", ">", - "version: 2"); - - verifySerialization(want, ev); - } - - @SmallTest - public void testDnsEventSerialization() { - ConnectivityMetricsEvent ev = describeIpEvent( - aType(DnsEvent.class), - anInt(101), - aByteArray(b(1), b(1), b(2), b(1), b(1), b(1), b(2), b(2)), - aByteArray(b(0), b(0), b(22), b(3), b(1), b(0), b(200), b(178)), - anIntArray(3456, 267, 1230, 45, 2111, 450, 638, 1300)); - - String want = joinLines( - "dropped_events: 0", - "events <", - " if_name: \"\"", - " link_layer: 0", - " network_id: 0", - " time_ms: 1", - " transports: 0", - " dns_lookup_batch <", - " event_types: 1", - " event_types: 1", - " event_types: 2", - " event_types: 1", - " event_types: 1", - " event_types: 1", - " event_types: 2", - " event_types: 2", - " latencies_ms: 3456", - " latencies_ms: 267", - " latencies_ms: 1230", - " latencies_ms: 45", - " latencies_ms: 2111", - " latencies_ms: 450", - " latencies_ms: 638", - " latencies_ms: 1300", - " network_id <", - " network_id: 101", - " >", - " return_codes: 0", - " return_codes: 0", - " return_codes: 22", - " return_codes: 3", - " return_codes: 1", - " return_codes: 0", - " return_codes: 200", - " return_codes: 178", - " >", - ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, ev); } @@ -326,7 +274,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { anInt(IpManagerEvent.PROVISIONING_OK), aLong(5678)); - String want = joinLines( + String want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"\"", @@ -340,7 +288,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { " latency_ms: 5678", " >", ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, ev); } @@ -351,7 +299,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { aType(IpReachabilityEvent.class), anInt(IpReachabilityEvent.NUD_FAILED)); - String want = joinLines( + String want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"\"", @@ -364,7 +312,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { " if_name: \"\"", " >", ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, ev); } @@ -377,7 +325,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { anInt(5), aLong(20410)); - String want = joinLines( + String want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"\"", @@ -393,7 +341,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { " >", " >", ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, ev); } @@ -406,7 +354,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { anInt(ValidationProbeEvent.PROBE_HTTP), anInt(204)); - String want = joinLines( + String want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"\"", @@ -420,7 +368,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { " probe_type: 1", " >", ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, ev); } @@ -436,7 +384,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { anInt(2048), anInt(3)); - String want = joinLines( + String want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"\"", @@ -454,7 +402,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { " program_length: 2048", " >", ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, ev); } @@ -474,7 +422,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { anInt(3), anInt(2048)); - String want = joinLines( + String want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"\"", @@ -495,7 +443,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { " zero_lifetime_ras: 1", " >", ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, ev); } @@ -511,7 +459,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { aLong(1000), aLong(-1)); - String want = joinLines( + String want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"\"", @@ -528,7 +476,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { " router_lifetime: 2000", " >", ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, ev); } @@ -543,13 +491,4 @@ public class IpConnectivityEventBuilderTest extends TestCase { fail(e.toString()); } } - - static String joinLines(String ... elems) { - StringBuilder b = new StringBuilder(); - for (String s : elems) { - b.append(s); - b.append("\n"); - } - return b.toString(); - } } diff --git a/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java b/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java index 68786d0b4212a..fd81c90a050c7 100644 --- a/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java +++ b/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java @@ -138,6 +138,7 @@ public class IpConnectivityMetricsTest extends TestCase { } @SmallTest + // TODO: add NetdEventListenerService coverage too public void testEndToEndLogging() { // TODO: instead of comparing textpb to textpb, parse textpb and compare proto to proto. IpConnectivityLog logger = new IpConnectivityLog(mService.impl); @@ -177,7 +178,7 @@ public class IpConnectivityMetricsTest extends TestCase { logger.log(ev); } - String want = joinLines( + String want = String.join("\n", "dropped_events: 0", "events <", " if_name: \"\"", @@ -279,7 +280,7 @@ public class IpConnectivityMetricsTest extends TestCase { " router_lifetime: 2000", " >", ">", - "version: 2"); + "version: 2\n"); verifySerialization(want, getdump("flush")); } diff --git a/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java b/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java index 0ab440641d53a..05c976732505d 100644 --- a/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java +++ b/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java @@ -16,161 +16,161 @@ package com.android.server.connectivity; -import android.net.ConnectivityManager; -import android.net.ConnectivityManager.NetworkCallback; -import android.net.Network; -import android.net.metrics.ConnectStats; -import android.net.metrics.DnsEvent; -import android.net.metrics.INetdEventListener; -import android.net.metrics.IpConnectivityLog; -import android.os.Parcelable; -import android.os.RemoteException; -import android.system.OsConstants; -import android.test.suitebuilder.annotation.SmallTest; -import com.android.server.connectivity.metrics.nano.IpConnectivityLogClass.IpConnectivityEvent; -import java.io.FileOutputStream; -import java.io.PrintWriter; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Comparator; -import java.util.List; -import java.util.OptionalInt; -import java.util.stream.IntStream; -import junit.framework.TestCase; -import org.junit.Before; -import org.junit.Test; -import org.mockito.ArgumentCaptor; -import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.MockitoAnnotations; - -import static org.junit.Assert.assertArrayEquals; -import static org.junit.Assert.assertTrue; +import static android.net.metrics.INetdEventListener.EVENT_GETADDRINFO; +import static android.net.metrics.INetdEventListener.EVENT_GETHOSTBYNAME; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; import static org.mockito.Mockito.any; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.anyInt; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; -public class NetdEventListenerServiceTest extends TestCase { - - // TODO: read from NetdEventListenerService after this constant is read from system property - static final int BATCH_SIZE = 100; - static final int EVENT_TYPE = INetdEventListener.EVENT_GETADDRINFO; - // TODO: read from INetdEventListener - static final int RETURN_CODE = 1; - - static final byte[] EVENT_TYPES = new byte[BATCH_SIZE]; - static final byte[] RETURN_CODES = new byte[BATCH_SIZE]; - static final int[] LATENCIES = new int[BATCH_SIZE]; - static { - for (int i = 0; i < BATCH_SIZE; i++) { - EVENT_TYPES[i] = EVENT_TYPE; - RETURN_CODES[i] = RETURN_CODE; - LATENCIES[i] = i; - } - } +import android.content.Context; +import android.net.ConnectivityManager; +import android.net.Network; +import android.net.NetworkCapabilities; +import android.support.test.runner.AndroidJUnit4; +import android.system.OsConstants; +import android.test.suitebuilder.annotation.SmallTest; +import android.util.Base64; +import com.android.server.connectivity.metrics.nano.IpConnectivityLogClass.DNSLookupBatch; +import com.android.server.connectivity.metrics.nano.IpConnectivityLogClass.IpConnectivityEvent; +import com.android.server.connectivity.metrics.nano.IpConnectivityLogClass.IpConnectivityLog; +import java.io.FileOutputStream; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Comparator; +import java.util.List; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +@RunWith(AndroidJUnit4.class) +@SmallTest +public class NetdEventListenerServiceTest { private static final String EXAMPLE_IPV4 = "192.0.2.1"; private static final String EXAMPLE_IPV6 = "2001:db8:1200::2:1"; NetdEventListenerService mNetdEventListenerService; + ConnectivityManager mCm; - @Mock ConnectivityManager mCm; - @Mock IpConnectivityLog mLog; - ArgumentCaptor mCallbackCaptor; - ArgumentCaptor mDnsEvCaptor; - + @Before public void setUp() { - MockitoAnnotations.initMocks(this); - mCallbackCaptor = ArgumentCaptor.forClass(NetworkCallback.class); - mDnsEvCaptor = ArgumentCaptor.forClass(DnsEvent.class); - mNetdEventListenerService = new NetdEventListenerService(mCm, mLog); - - verify(mCm, times(1)).registerNetworkCallback(any(), mCallbackCaptor.capture()); + mCm = mock(ConnectivityManager.class); + mNetdEventListenerService = new NetdEventListenerService(mCm); } - @SmallTest - public void testOneDnsBatch() throws Exception { - log(105, LATENCIES); - log(106, Arrays.copyOf(LATENCIES, BATCH_SIZE - 1)); // one lookup short of a batch event + @Test + public void testDnsLogging() throws Exception { + NetworkCapabilities ncWifi = new NetworkCapabilities(); + NetworkCapabilities ncCell = new NetworkCapabilities(); + ncWifi.addTransportType(NetworkCapabilities.TRANSPORT_WIFI); + ncCell.addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR); - verifyLoggedDnsEvents(new DnsEvent(105, EVENT_TYPES, RETURN_CODES, LATENCIES)); + when(mCm.getNetworkCapabilities(new Network(100))).thenReturn(ncWifi); + when(mCm.getNetworkCapabilities(new Network(101))).thenReturn(ncCell); - log(106, Arrays.copyOfRange(LATENCIES, BATCH_SIZE - 1, BATCH_SIZE)); + dnsEvent(100, EVENT_GETADDRINFO, 0, 3456); + dnsEvent(100, EVENT_GETADDRINFO, 0, 267); + dnsEvent(100, EVENT_GETHOSTBYNAME, 22, 1230); + dnsEvent(100, EVENT_GETADDRINFO, 3, 45); + dnsEvent(100, EVENT_GETADDRINFO, 1, 2111); + dnsEvent(100, EVENT_GETADDRINFO, 0, 450); + dnsEvent(100, EVENT_GETHOSTBYNAME, 200, 638); + dnsEvent(100, EVENT_GETHOSTBYNAME, 178, 1300); + dnsEvent(101, EVENT_GETADDRINFO, 0, 56); + dnsEvent(101, EVENT_GETADDRINFO, 0, 78); + dnsEvent(101, EVENT_GETADDRINFO, 0, 14); + dnsEvent(101, EVENT_GETHOSTBYNAME, 0, 56); + dnsEvent(101, EVENT_GETADDRINFO, 0, 78); + dnsEvent(101, EVENT_GETADDRINFO, 0, 14); - mDnsEvCaptor = ArgumentCaptor.forClass(DnsEvent.class); // reset argument captor - verifyLoggedDnsEvents( - new DnsEvent(105, EVENT_TYPES, RETURN_CODES, LATENCIES), - new DnsEvent(106, EVENT_TYPES, RETURN_CODES, LATENCIES)); + String got = flushStatistics(); + String want = String.join("\n", + "dropped_events: 0", + "events <", + " if_name: \"\"", + " link_layer: 0", + " network_id: 0", + " time_ms: 0", + " transports: 0", + " connect_statistics <", + " connect_blocking_count: 0", + " connect_count: 0", + " ipv6_addr_count: 0", + " >", + ">", + "events <", + " if_name: \"\"", + " link_layer: 4", + " network_id: 100", + " time_ms: 0", + " transports: 2", + " dns_lookup_batch <", + " event_types: 1", + " event_types: 1", + " event_types: 2", + " event_types: 1", + " event_types: 1", + " event_types: 1", + " event_types: 2", + " event_types: 2", + " latencies_ms: 3456", + " latencies_ms: 267", + " latencies_ms: 1230", + " latencies_ms: 45", + " latencies_ms: 2111", + " latencies_ms: 450", + " latencies_ms: 638", + " latencies_ms: 1300", + " return_codes: 0", + " return_codes: 0", + " return_codes: 22", + " return_codes: 3", + " return_codes: 1", + " return_codes: 0", + " return_codes: 200", + " return_codes: 178", + " >", + ">", + "events <", + " if_name: \"\"", + " link_layer: 2", + " network_id: 101", + " time_ms: 0", + " transports: 1", + " dns_lookup_batch <", + " event_types: 1", + " event_types: 1", + " event_types: 1", + " event_types: 2", + " event_types: 1", + " event_types: 1", + " latencies_ms: 56", + " latencies_ms: 78", + " latencies_ms: 14", + " latencies_ms: 56", + " latencies_ms: 78", + " latencies_ms: 14", + " return_codes: 0", + " return_codes: 0", + " return_codes: 0", + " return_codes: 0", + " return_codes: 0", + " return_codes: 0", + " >", + ">", + "version: 2\n"); + assertEquals(want, got); } - @SmallTest - public void testSeveralDmsBatches() throws Exception { - log(105, LATENCIES); - log(106, LATENCIES); - log(105, LATENCIES); - log(107, LATENCIES); - - verifyLoggedDnsEvents( - new DnsEvent(105, EVENT_TYPES, RETURN_CODES, LATENCIES), - new DnsEvent(106, EVENT_TYPES, RETURN_CODES, LATENCIES), - new DnsEvent(105, EVENT_TYPES, RETURN_CODES, LATENCIES), - new DnsEvent(107, EVENT_TYPES, RETURN_CODES, LATENCIES)); - } - - @SmallTest - public void testDnsBatchAndNetworkLost() throws Exception { - byte[] eventTypes = Arrays.copyOf(EVENT_TYPES, 20); - byte[] returnCodes = Arrays.copyOf(RETURN_CODES, 20); - int[] latencies = Arrays.copyOf(LATENCIES, 20); - - log(105, LATENCIES); - log(105, latencies); - mCallbackCaptor.getValue().onLost(new Network(105)); - log(105, LATENCIES); - - verifyLoggedDnsEvents( - new DnsEvent(105, eventTypes, returnCodes, latencies), - new DnsEvent(105, EVENT_TYPES, RETURN_CODES, LATENCIES), - new DnsEvent(105, EVENT_TYPES, RETURN_CODES, LATENCIES)); - } - - @SmallTest - public void testConcurrentDnsBatchesAndDumps() throws Exception { - final long stop = System.currentTimeMillis() + 100; - final PrintWriter pw = new PrintWriter(new FileOutputStream("/dev/null")); - new Thread() { - public void run() { - while (System.currentTimeMillis() < stop) { - mNetdEventListenerService.dump(pw); - } - } - }.start(); - - logDnsAsync(105, LATENCIES); - logDnsAsync(106, LATENCIES); - logDnsAsync(107, LATENCIES); - - verifyLoggedDnsEvents(500, - new DnsEvent(105, EVENT_TYPES, RETURN_CODES, LATENCIES), - new DnsEvent(106, EVENT_TYPES, RETURN_CODES, LATENCIES), - new DnsEvent(107, EVENT_TYPES, RETURN_CODES, LATENCIES)); - } - - @SmallTest - public void testConcurrentDnsBatchesAndNetworkLoss() throws Exception { - logDnsAsync(105, LATENCIES); - Thread.sleep(10L); - // call onLost() asynchronously to logDnsAsync's onDnsEvent() calls. - mCallbackCaptor.getValue().onLost(new Network(105)); - - // do not verify batch with unpredictable length - verify(mLog, timeout(500).times(1)).log(any(Parcelable.class)); - } - - @SmallTest + @Test public void testConnectLogging() throws Exception { final int OK = 0; Thread[] logActions = { @@ -268,44 +268,18 @@ public class NetdEventListenerServiceTest extends TestCase { }); } - void log(int netId, int[] latencies) { - try { - for (int l : latencies) { - mNetdEventListenerService.onDnsEvent(netId, EVENT_TYPE, RETURN_CODE, l, null, null, - 0, 0); + void dnsEvent(int netId, int type, int result, int latency) throws Exception { + mNetdEventListenerService.onDnsEvent(netId, type, result, latency, "", null, 0, 0); + } + + Thread dumpAction(long durationMs) throws Exception { + final long stop = System.currentTimeMillis() + durationMs; + final PrintWriter pw = new PrintWriter(new FileOutputStream("/dev/null")); + return new Thread(() -> { + while (System.currentTimeMillis() < stop) { + mNetdEventListenerService.dump(pw); } - } catch (RemoteException re) { - throw re.rethrowFromSystemServer(); - } - } - - void logDnsAsync(int netId, int[] latencies) { - new Thread(() -> log(netId, latencies)).start(); - } - - void verifyLoggedDnsEvents(DnsEvent... expected) { - verifyLoggedDnsEvents(0, expected); - } - - void verifyLoggedDnsEvents(int wait, DnsEvent... expectedEvents) { - verify(mLog, timeout(wait).times(expectedEvents.length)).log(mDnsEvCaptor.capture()); - for (DnsEvent got : mDnsEvCaptor.getAllValues()) { - OptionalInt index = IntStream.range(0, expectedEvents.length) - .filter(i -> dnsEventsEqual(expectedEvents[i], got)) - .findFirst(); - // Don't match same expected event more than once. - index.ifPresent(i -> expectedEvents[i] = null); - assertTrue(index.isPresent()); - } - } - - /** equality function for DnsEvent to avoid overriding equals() and hashCode(). */ - static boolean dnsEventsEqual(DnsEvent expected, DnsEvent got) { - return (expected == got) || ((expected != null) && (got != null) - && (expected.netId == got.netId) - && Arrays.equals(expected.eventTypes, got.eventTypes) - && Arrays.equals(expected.returnCodes, got.returnCodes) - && Arrays.equals(expected.latenciesMs, got.latenciesMs)); + }); } static void verifyConnectEvent(String expected, IpConnectivityEvent got) { @@ -318,4 +292,17 @@ public class NetdEventListenerServiceTest extends TestCase { fail(e.toString()); } } + + // TODO: instead of comparing textpb to textpb, parse textpb and compare proto to proto. + String flushStatistics() throws Exception { + IpConnectivityMetrics metricsService = + new IpConnectivityMetrics(mock(Context.class), (ctx) -> 2000); + metricsService.mNetdListener = mNetdEventListenerService; + + StringWriter buffer = new StringWriter(); + PrintWriter writer = new PrintWriter(buffer); + metricsService.impl.dump(null, writer, new String[]{"flush"}); + byte[] bytes = Base64.decode(buffer.toString(), Base64.DEFAULT); + return IpConnectivityLog.parseFrom(bytes).toString(); + } }