From 24913d0e8052ef39e2da739ee66922855deb6ee4 Mon Sep 17 00:00:00 2001 From: Paul Hu Date: Tue, 18 Apr 2023 05:50:14 +0000 Subject: [PATCH] Revert "Revert "Record DiscoveryManager history logs for better ..." Revert submission 2542434-revert-2535219-TAMNEZLAHT Reason for revert: Prepare a fix for the original topic Reverted changes: /q/submissionid:2542434-revert-2535219-TAMNEZLAHT Change-Id: I718418e4499784255b177622e3c08a82d0b24640 --- .../mdns/MdnsDiscoveryManager.java | 23 +++++++------ .../mdns/MdnsServiceTypeClient.java | 32 +++++++++++++------ .../mdns/MdnsServiceTypeClientTests.java | 15 +++++---- 3 files changed, 45 insertions(+), 25 deletions(-) diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java b/service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java index fb8af8d121..491698d30c 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java @@ -23,16 +23,16 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.RequiresPermission; import android.net.Network; -import android.text.TextUtils; import android.util.ArrayMap; import android.util.Log; import android.util.Pair; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; -import com.android.server.connectivity.mdns.util.MdnsLogger; +import com.android.net.module.util.SharedLog; import java.io.IOException; +import java.io.PrintWriter; import java.util.ArrayList; import java.util.List; @@ -43,7 +43,7 @@ import java.util.List; public class MdnsDiscoveryManager implements MdnsSocketClientBase.Callback { private static final String TAG = MdnsDiscoveryManager.class.getSimpleName(); public static final boolean DBG = Log.isLoggable(TAG, Log.DEBUG); - private static final MdnsLogger LOGGER = new MdnsLogger("MdnsDiscoveryManager"); + private static final SharedLog LOGGER = new SharedLog(TAG); private final ExecutorProvider executorProvider; private final MdnsSocketClientBase socketClient; @@ -120,9 +120,7 @@ public class MdnsDiscoveryManager implements MdnsSocketClientBase.Callback { @NonNull String serviceType, @NonNull MdnsServiceBrowserListener listener, @NonNull MdnsSearchOptions searchOptions) { - LOGGER.log( - "Registering listener for subtypes: %s", - TextUtils.join(",", searchOptions.getSubtypes())); + LOGGER.i("Registering listener for serviceType: " + serviceType); if (perNetworkServiceTypeClients.isEmpty()) { // First listener. Starts the socket client. try { @@ -157,8 +155,7 @@ public class MdnsDiscoveryManager implements MdnsSocketClientBase.Callback { @RequiresPermission(permission.CHANGE_WIFI_MULTICAST_STATE) public synchronized void unregisterListener( @NonNull String serviceType, @NonNull MdnsServiceBrowserListener listener) { - LOGGER.log("Unregistering listener for service type: %s", serviceType); - if (DBG) Log.d(TAG, "Unregistering listener for serviceType:" + serviceType); + LOGGER.i("Unregistering listener for serviceType:" + serviceType); final List serviceTypeClients = perNetworkServiceTypeClients.getByServiceType(serviceType); if (serviceTypeClients.isEmpty()) { @@ -198,11 +195,19 @@ public class MdnsDiscoveryManager implements MdnsSocketClientBase.Callback { } } + /** Dump info to dumpsys */ + public void dump(PrintWriter pw) { + LOGGER.reverseDump(pw); + } + @VisibleForTesting MdnsServiceTypeClient createServiceTypeClient(@NonNull String serviceType, @Nullable Network network) { + LOGGER.log("createServiceTypeClient for serviceType:" + serviceType + + " network:" + network); return new MdnsServiceTypeClient( serviceType, socketClient, - executorProvider.newServiceTypeClientSchedulerExecutor(), network); + executorProvider.newServiceTypeClientSchedulerExecutor(), network, + LOGGER.forSubComponent(serviceType + "-" + network)); } } \ No newline at end of file diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java b/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java index 5298aef63c..72b931d0ac 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java @@ -28,7 +28,7 @@ import android.util.Pair; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; -import com.android.server.connectivity.mdns.util.MdnsLogger; +import com.android.net.module.util.SharedLog; import java.net.Inet4Address; import java.net.Inet6Address; @@ -49,8 +49,6 @@ import java.util.concurrent.ScheduledExecutorService; public class MdnsServiceTypeClient { private static final int DEFAULT_MTU = 1500; - private static final MdnsLogger LOGGER = new MdnsLogger("MdnsServiceTypeClient"); - private final String serviceType; private final String[] serviceTypeLabels; @@ -58,6 +56,7 @@ public class MdnsServiceTypeClient { private final MdnsResponseDecoder responseDecoder; private final ScheduledExecutorService executor; @Nullable private final Network network; + @NonNull private final SharedLog sharedLog; private final Object lock = new Object(); private final ArrayMap listeners = new ArrayMap<>(); @@ -90,8 +89,10 @@ public class MdnsServiceTypeClient { @NonNull String serviceType, @NonNull MdnsSocketClientBase socketClient, @NonNull ScheduledExecutorService executor, - @Nullable Network network) { - this(serviceType, socketClient, executor, new MdnsResponseDecoder.Clock(), network); + @Nullable Network network, + @NonNull SharedLog sharedLog) { + this(serviceType, socketClient, executor, new MdnsResponseDecoder.Clock(), network, + sharedLog); } @VisibleForTesting @@ -100,7 +101,8 @@ public class MdnsServiceTypeClient { @NonNull MdnsSocketClientBase socketClient, @NonNull ScheduledExecutorService executor, @NonNull MdnsResponseDecoder.Clock clock, - @Nullable Network network) { + @Nullable Network network, + @NonNull SharedLog sharedLog) { this.serviceType = serviceType; this.socketClient = socketClient; this.executor = executor; @@ -108,6 +110,7 @@ public class MdnsServiceTypeClient { this.responseDecoder = new MdnsResponseDecoder(clock, serviceTypeLabels); this.clock = clock; this.network = network; + this.sharedLog = sharedLog; } private static MdnsServiceInfo buildMdnsServiceInfoFromResponse( @@ -261,20 +264,20 @@ public class MdnsServiceTypeClient { } private void onResponseModified(@NonNull MdnsResponse response) { + final String serviceInstanceName = response.getServiceInstanceName(); final MdnsResponse currentResponse = - instanceNameToResponse.get(response.getServiceInstanceName()); + instanceNameToResponse.get(serviceInstanceName); boolean newServiceFound = false; boolean serviceBecomesComplete = false; if (currentResponse == null) { newServiceFound = true; - String serviceInstanceName = response.getServiceInstanceName(); if (serviceInstanceName != null) { instanceNameToResponse.put(serviceInstanceName, response); } } else { boolean before = currentResponse.isComplete(); - instanceNameToResponse.put(response.getServiceInstanceName(), response); + instanceNameToResponse.put(serviceInstanceName, response); boolean after = response.isComplete(); serviceBecomesComplete = !before && after; } @@ -285,13 +288,16 @@ public class MdnsServiceTypeClient { if (!responseMatchesOptions(response, listeners.valueAt(i))) continue; final MdnsServiceBrowserListener listener = listeners.keyAt(i); if (newServiceFound) { + sharedLog.log("onServiceNameDiscovered: " + serviceInstanceName); listener.onServiceNameDiscovered(serviceInfo); } if (response.isComplete()) { if (newServiceFound || serviceBecomesComplete) { + sharedLog.log("onServiceFound: " + serviceInstanceName); listener.onServiceFound(serviceInfo); } else { + sharedLog.log("onServiceUpdated: " + serviceInstanceName); listener.onServiceUpdated(serviceInfo); } } @@ -309,8 +315,10 @@ public class MdnsServiceTypeClient { final MdnsServiceInfo serviceInfo = buildMdnsServiceInfoFromResponse(response, serviceTypeLabels); if (response.isComplete()) { + sharedLog.log("onServiceRemoved: " + serviceInstanceName); listener.onServiceRemoved(serviceInfo); } + sharedLog.log("onServiceNameRemoved: " + serviceInstanceName); listener.onServiceNameRemoved(serviceInfo); } } @@ -485,7 +493,7 @@ public class MdnsServiceTypeClient { servicesToResolve) .call(); } catch (RuntimeException e) { - LOGGER.e(String.format("Failed to run EnqueueMdnsQueryCallable for subtype: %s", + sharedLog.e(String.format("Failed to run EnqueueMdnsQueryCallable for subtype: %s", TextUtils.join(",", config.subtypes)), e); result = null; } @@ -534,8 +542,12 @@ public class MdnsServiceTypeClient { buildMdnsServiceInfoFromResponse( existingResponse, serviceTypeLabels); if (existingResponse.isComplete()) { + sharedLog.log("TTL expired. onServiceRemoved: " + + serviceInstanceName); listener.onServiceRemoved(serviceInfo); } + sharedLog.log("TTL expired. onServiceNameRemoved: " + + serviceInstanceName); listener.onServiceNameRemoved(serviceInfo); } } diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java b/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java index 746994f388..34b44fc1a9 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java +++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java @@ -41,6 +41,7 @@ import android.net.InetAddresses; import android.net.Network; import android.text.TextUtils; +import com.android.net.module.util.SharedLog; import com.android.server.connectivity.mdns.MdnsServiceInfo.TextEntry; import com.android.server.connectivity.mdns.MdnsServiceTypeClient.QueryTaskConfig; import com.android.testutils.DevSdkIgnoreRule; @@ -99,6 +100,8 @@ public class MdnsServiceTypeClientTests { private Network mockNetwork; @Mock private MdnsResponseDecoder.Clock mockDecoderClock; + @Mock + private SharedLog mockSharedLog; @Captor private ArgumentCaptor serviceInfoCaptor; @@ -166,7 +169,7 @@ public class MdnsServiceTypeClientTests { client = new MdnsServiceTypeClient(SERVICE_TYPE, mockSocketClient, currentThreadExecutor, - mockDecoderClock, mockNetwork) { + mockDecoderClock, mockNetwork, mockSharedLog) { @Override MdnsPacketWriter createMdnsPacketWriter() { return mockPacketWriter; @@ -701,7 +704,7 @@ public class MdnsServiceTypeClientTests { final String serviceInstanceName = "service-instance-1"; client = new MdnsServiceTypeClient(SERVICE_TYPE, mockSocketClient, currentThreadExecutor, - mockDecoderClock, mockNetwork) { + mockDecoderClock, mockNetwork, mockSharedLog) { @Override MdnsPacketWriter createMdnsPacketWriter() { return mockPacketWriter; @@ -740,7 +743,7 @@ public class MdnsServiceTypeClientTests { final String serviceInstanceName = "service-instance-1"; client = new MdnsServiceTypeClient(SERVICE_TYPE, mockSocketClient, currentThreadExecutor, - mockDecoderClock, mockNetwork) { + mockDecoderClock, mockNetwork, mockSharedLog) { @Override MdnsPacketWriter createMdnsPacketWriter() { return mockPacketWriter; @@ -773,7 +776,7 @@ public class MdnsServiceTypeClientTests { final String serviceInstanceName = "service-instance-1"; client = new MdnsServiceTypeClient(SERVICE_TYPE, mockSocketClient, currentThreadExecutor, - mockDecoderClock, mockNetwork) { + mockDecoderClock, mockNetwork, mockSharedLog) { @Override MdnsPacketWriter createMdnsPacketWriter() { return mockPacketWriter; @@ -898,7 +901,7 @@ public class MdnsServiceTypeClientTests { @Test public void testProcessResponse_Resolve() throws Exception { client = new MdnsServiceTypeClient( - SERVICE_TYPE, mockSocketClient, currentThreadExecutor, mockNetwork); + SERVICE_TYPE, mockSocketClient, currentThreadExecutor, mockNetwork, mockSharedLog); final String instanceName = "service-instance"; final String[] hostname = new String[] { "testhost "}; @@ -995,7 +998,7 @@ public class MdnsServiceTypeClientTests { @Test public void testProcessResponse_ResolveExcludesOtherServices() { client = new MdnsServiceTypeClient( - SERVICE_TYPE, mockSocketClient, currentThreadExecutor, mockNetwork); + SERVICE_TYPE, mockSocketClient, currentThreadExecutor, mockNetwork, mockSharedLog); final String requestedInstance = "instance1"; final String otherInstance = "instance2";