diff --git a/service-t/src/com/android/metrics/NetworkNsdReportedMetrics.java b/service-t/src/com/android/metrics/NetworkNsdReportedMetrics.java index 71788d2d06..c3b5086b4e 100644 --- a/service-t/src/com/android/metrics/NetworkNsdReportedMetrics.java +++ b/service-t/src/com/android/metrics/NetworkNsdReportedMetrics.java @@ -173,4 +173,37 @@ public class NetworkNsdReportedMetrics { builder.setFoundServiceCount(servicesCount); mDependencies.statsWrite(builder.build()); } + + /** + * Report service resolution success metric data. + * + * @param transactionId The transaction id of service resolution. + * @param durationMs The duration of resolving services. + * @param isServiceFromCache Whether the resolved service is from cache. + */ + public void reportServiceResolved(int transactionId, long durationMs, + boolean isServiceFromCache) { + final Builder builder = makeReportedBuilder(); + builder.setTransactionId(transactionId); + builder.setType(NsdEventType.NET_RESOLVE); + builder.setQueryResult(MdnsQueryResult.MQR_SERVICE_RESOLVED); + builder.setEventDurationMillisec(durationMs); + builder.setIsKnownService(isServiceFromCache); + mDependencies.statsWrite(builder.build()); + } + + /** + * Report service resolution failed metric data. + * + * @param transactionId The transaction id of service resolution. + * @param durationMs The duration of service resolution failed. + */ + public void reportServiceResolutionFailed(int transactionId, long durationMs) { + final Builder builder = makeReportedBuilder(); + builder.setTransactionId(transactionId); + builder.setType(NsdEventType.NET_RESOLVE); + builder.setQueryResult(MdnsQueryResult.MQR_SERVICE_RESOLUTION_FAILED); + builder.setEventDurationMillisec(durationMs); + mDependencies.statsWrite(builder.build()); + } } diff --git a/service-t/src/com/android/server/NsdService.java b/service-t/src/com/android/server/NsdService.java index 786a1810bc..b975a1befd 100644 --- a/service-t/src/com/android/server/NsdService.java +++ b/service-t/src/com/android/server/NsdService.java @@ -265,7 +265,8 @@ public class NsdService extends INsdManager.Stub { } @Override - public void onServiceFound(@NonNull MdnsServiceInfo serviceInfo) { } + public void onServiceFound(@NonNull MdnsServiceInfo serviceInfo, + boolean isServiceFromCache) { } @Override public void onServiceUpdated(@NonNull MdnsServiceInfo serviceInfo) { } @@ -274,7 +275,8 @@ public class NsdService extends INsdManager.Stub { public void onServiceRemoved(@NonNull MdnsServiceInfo serviceInfo) { } @Override - public void onServiceNameDiscovered(@NonNull MdnsServiceInfo serviceInfo) { } + public void onServiceNameDiscovered(@NonNull MdnsServiceInfo serviceInfo, + boolean isServiceFromCache) { } @Override public void onServiceNameRemoved(@NonNull MdnsServiceInfo serviceInfo) { } @@ -300,10 +302,11 @@ public class NsdService extends INsdManager.Stub { } @Override - public void onServiceNameDiscovered(@NonNull MdnsServiceInfo serviceInfo) { + public void onServiceNameDiscovered(@NonNull MdnsServiceInfo serviceInfo, + boolean isServiceFromCache) { mNsdStateMachine.sendMessage(MDNS_DISCOVERY_MANAGER_EVENT, mTransactionId, NsdManager.SERVICE_FOUND, - new MdnsEvent(mClientRequestId, serviceInfo)); + new MdnsEvent(mClientRequestId, serviceInfo, isServiceFromCache)); } @Override @@ -322,10 +325,10 @@ public class NsdService extends INsdManager.Stub { } @Override - public void onServiceFound(MdnsServiceInfo serviceInfo) { + public void onServiceFound(MdnsServiceInfo serviceInfo, boolean isServiceFromCache) { mNsdStateMachine.sendMessage(MDNS_DISCOVERY_MANAGER_EVENT, mTransactionId, NsdManager.RESOLVE_SERVICE_SUCCEEDED, - new MdnsEvent(mClientRequestId, serviceInfo)); + new MdnsEvent(mClientRequestId, serviceInfo, isServiceFromCache)); } } @@ -337,10 +340,11 @@ public class NsdService extends INsdManager.Stub { } @Override - public void onServiceFound(@NonNull MdnsServiceInfo serviceInfo) { + public void onServiceFound(@NonNull MdnsServiceInfo serviceInfo, + boolean isServiceFromCache) { mNsdStateMachine.sendMessage(MDNS_DISCOVERY_MANAGER_EVENT, mTransactionId, NsdManager.SERVICE_UPDATED, - new MdnsEvent(mClientRequestId, serviceInfo)); + new MdnsEvent(mClientRequestId, serviceInfo, isServiceFromCache)); } @Override @@ -463,10 +467,17 @@ public class NsdService extends INsdManager.Stub { final int mClientRequestId; @NonNull final MdnsServiceInfo mMdnsServiceInfo; + final boolean mIsServiceFromCache; MdnsEvent(int clientRequestId, @NonNull MdnsServiceInfo mdnsServiceInfo) { + this(clientRequestId, mdnsServiceInfo, false /* isServiceFromCache */); + } + + MdnsEvent(int clientRequestId, @NonNull MdnsServiceInfo mdnsServiceInfo, + boolean isServiceFromCache) { mClientRequestId = clientRequestId; mMdnsServiceInfo = mdnsServiceInfo; + mIsServiceFromCache = isServiceFromCache; } } @@ -615,7 +626,7 @@ public class NsdService extends INsdManager.Stub { case NsdManager.RESOLVE_SERVICE: cInfo = getClientInfoForReply(msg); if (cInfo != null) { - cInfo.onResolveServiceFailed( + cInfo.onResolveServiceFailedImmediately( clientRequestId, NsdManager.FAILURE_INTERNAL_ERROR); } break; @@ -682,9 +693,9 @@ public class NsdService extends INsdManager.Stub { } private void storeLegacyRequestMap(int clientRequestId, int transactionId, - ClientInfo clientInfo, int what) { - clientInfo.mClientRequests.put(clientRequestId, new LegacyClientRequest( - transactionId, what, mClock.elapsedRealtime())); + ClientInfo clientInfo, int what, long startTimeMs) { + clientInfo.mClientRequests.put(clientRequestId, + new LegacyClientRequest(transactionId, what, startTimeMs)); mTransactionIdToClientInfoMap.put(transactionId, clientInfo); // Remove the cleanup event because here comes a new request. cancelStop(); @@ -810,8 +821,8 @@ public class NsdService extends INsdManager.Stub { Log.d(TAG, "Discover " + msg.arg2 + " " + transactionId + info.getServiceType()); } - storeLegacyRequestMap( - clientRequestId, transactionId, clientInfo, msg.what); + storeLegacyRequestMap(clientRequestId, transactionId, clientInfo, + msg.what, mClock.elapsedRealtime()); clientInfo.onDiscoverServicesStarted( clientRequestId, info, transactionId); } else { @@ -912,8 +923,8 @@ public class NsdService extends INsdManager.Stub { Log.d(TAG, "Register " + clientRequestId + " " + transactionId); } - storeLegacyRequestMap( - clientRequestId, transactionId, clientInfo, msg.what); + storeLegacyRequestMap(clientRequestId, transactionId, clientInfo, + msg.what, mClock.elapsedRealtime()); // Return success after mDns reports success } else { unregisterService(transactionId); @@ -986,8 +997,8 @@ public class NsdService extends INsdManager.Stub { || mDeps.isMdnsDiscoveryManagerEnabled(mContext) || useDiscoveryManagerForType(serviceType)) { if (serviceType == null) { - clientInfo.onResolveServiceFailed(clientRequestId, - NsdManager.FAILURE_INTERNAL_ERROR); + clientInfo.onResolveServiceFailedImmediately( + clientRequestId, NsdManager.FAILURE_INTERNAL_ERROR); break; } final String resolveServiceType = serviceType + ".local"; @@ -1009,7 +1020,7 @@ public class NsdService extends INsdManager.Stub { + " for service type:" + resolveServiceType); } else { if (clientInfo.mResolvedService != null) { - clientInfo.onResolveServiceFailed( + clientInfo.onResolveServiceFailedImmediately( clientRequestId, NsdManager.FAILURE_ALREADY_ACTIVE); break; } @@ -1017,10 +1028,10 @@ public class NsdService extends INsdManager.Stub { maybeStartDaemon(); if (resolveService(transactionId, info)) { clientInfo.mResolvedService = new NsdServiceInfo(); - storeLegacyRequestMap( - clientRequestId, transactionId, clientInfo, msg.what); + storeLegacyRequestMap(clientRequestId, transactionId, clientInfo, + msg.what, mClock.elapsedRealtime()); } else { - clientInfo.onResolveServiceFailed( + clientInfo.onResolveServiceFailedImmediately( clientRequestId, NsdManager.FAILURE_INTERNAL_ERROR); } } @@ -1279,10 +1290,11 @@ public class NsdService extends INsdManager.Stub { final int transactionId2 = getUniqueId(); if (getAddrInfo(transactionId2, info.hostname, info.interfaceIdx)) { storeLegacyRequestMap(clientRequestId, transactionId2, clientInfo, - NsdManager.RESOLVE_SERVICE); + NsdManager.RESOLVE_SERVICE, request.mStartTimeMs); } else { - clientInfo.onResolveServiceFailed( - clientRequestId, NsdManager.FAILURE_INTERNAL_ERROR); + clientInfo.onResolveServiceFailed(clientRequestId, + NsdManager.FAILURE_INTERNAL_ERROR, transactionId, + request.calculateRequestDurationMs(mClock.elapsedRealtime())); clientInfo.mResolvedService = null; } break; @@ -1291,16 +1303,18 @@ public class NsdService extends INsdManager.Stub { /* NNN resolveId errorCode */ stopResolveService(transactionId); removeRequestMap(clientRequestId, transactionId, clientInfo); - clientInfo.onResolveServiceFailed( - clientRequestId, NsdManager.FAILURE_INTERNAL_ERROR); + clientInfo.onResolveServiceFailed(clientRequestId, + NsdManager.FAILURE_INTERNAL_ERROR, transactionId, + request.calculateRequestDurationMs(mClock.elapsedRealtime())); clientInfo.mResolvedService = null; break; case IMDnsEventListener.SERVICE_GET_ADDR_FAILED: /* NNN resolveId errorCode */ stopGetAddrInfo(transactionId); removeRequestMap(clientRequestId, transactionId, clientInfo); - clientInfo.onResolveServiceFailed( - clientRequestId, NsdManager.FAILURE_INTERNAL_ERROR); + clientInfo.onResolveServiceFailed(clientRequestId, + NsdManager.FAILURE_INTERNAL_ERROR, transactionId, + request.calculateRequestDurationMs(mClock.elapsedRealtime())); clientInfo.mResolvedService = null; break; case IMDnsEventListener.SERVICE_GET_ADDR_SUCCESS: { @@ -1323,10 +1337,11 @@ public class NsdService extends INsdManager.Stub { setServiceNetworkForCallback(clientInfo.mResolvedService, netId, info.interfaceIdx); clientInfo.onResolveServiceSucceeded( - clientRequestId, clientInfo.mResolvedService); + clientRequestId, clientInfo.mResolvedService, request); } else { - clientInfo.onResolveServiceFailed( - clientRequestId, NsdManager.FAILURE_INTERNAL_ERROR); + clientInfo.onResolveServiceFailed(clientRequestId, + NsdManager.FAILURE_INTERNAL_ERROR, transactionId, + request.calculateRequestDurationMs(mClock.elapsedRealtime())); } stopGetAddrInfo(transactionId); removeRequestMap(clientRequestId, transactionId, clientInfo); @@ -1429,11 +1444,13 @@ public class NsdService extends INsdManager.Stub { final List addresses = getInetAddresses(serviceInfo); if (addresses.size() != 0) { info.setHostAddresses(addresses); - clientInfo.onResolveServiceSucceeded(clientRequestId, info); + request.setServiceFromCache(event.mIsServiceFromCache); + clientInfo.onResolveServiceSucceeded(clientRequestId, info, request); } else { // No address. Notify resolution failure. - clientInfo.onResolveServiceFailed( - clientRequestId, NsdManager.FAILURE_INTERNAL_ERROR); + clientInfo.onResolveServiceFailed(clientRequestId, + NsdManager.FAILURE_INTERNAL_ERROR, transactionId, + request.calculateRequestDurationMs(mClock.elapsedRealtime())); } // Unregister the listener immediately like IMDnsEventListener design @@ -2200,6 +2217,7 @@ public class NsdService extends INsdManager.Stub { private int mFoundServiceCount = 0; private int mLostServiceCount = 0; private final Set mServices = new ArraySet<>(); + private boolean mIsServiceFromCache = false; private ClientRequest(int transactionId, long startTimeMs) { mTransactionId = transactionId; @@ -2232,6 +2250,14 @@ public class NsdService extends INsdManager.Stub { public int getServicesCount() { return mServices.size(); } + + public void setServiceFromCache(boolean isServiceFromCache) { + mIsServiceFromCache = isServiceFromCache; + } + + public boolean isServiceFromCache() { + return mIsServiceFromCache; + } } private static class LegacyClientRequest extends ClientRequest { @@ -2544,7 +2570,13 @@ public class NsdService extends INsdManager.Stub { } } - void onResolveServiceFailed(int listenerKey, int error) { + void onResolveServiceFailedImmediately(int listenerKey, int error) { + onResolveServiceFailed(listenerKey, error, NO_TRANSACTION, 0L /* durationMs */); + } + + void onResolveServiceFailed(int listenerKey, int error, int transactionId, + long durationMs) { + mMetrics.reportServiceResolutionFailed(transactionId, durationMs); try { mCb.onResolveServiceFailed(listenerKey, error); } catch (RemoteException e) { @@ -2552,7 +2584,12 @@ public class NsdService extends INsdManager.Stub { } } - void onResolveServiceSucceeded(int listenerKey, NsdServiceInfo info) { + void onResolveServiceSucceeded(int listenerKey, NsdServiceInfo info, + ClientRequest request) { + mMetrics.reportServiceResolved( + request.mTransactionId, + request.calculateRequestDurationMs(mClock.elapsedRealtime()), + request.isServiceFromCache()); try { mCb.onResolveServiceSucceeded(listenerKey, info); } catch (RemoteException e) { diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceBrowserListener.java b/service-t/src/com/android/server/connectivity/mdns/MdnsServiceBrowserListener.java index 7c1935975c..4c3cbc0043 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceBrowserListener.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsServiceBrowserListener.java @@ -32,8 +32,9 @@ public interface MdnsServiceBrowserListener { * service records (PTR, SRV, TXT, A or AAAA) are received . * * @param serviceInfo The found mDNS service instance. + * @param isServiceFromCache Whether the found mDNS service is from cache. */ - void onServiceFound(@NonNull MdnsServiceInfo serviceInfo); + void onServiceFound(@NonNull MdnsServiceInfo serviceInfo, boolean isServiceFromCache); /** * Called when an mDNS service instance is updated. This method would be called only if all @@ -84,8 +85,9 @@ public interface MdnsServiceBrowserListener { * record has been received. * * @param serviceInfo The discovered mDNS service instance. + * @param isServiceFromCache Whether the discovered mDNS service is from cache. */ - void onServiceNameDiscovered(@NonNull MdnsServiceInfo serviceInfo); + void onServiceNameDiscovered(@NonNull MdnsServiceInfo serviceInfo, boolean isServiceFromCache); /** * Called when a discovered mDNS service instance is no longer valid and removed. 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 53a7ab9a6a..861d8d10e8 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java @@ -297,9 +297,9 @@ public class MdnsServiceTypeClient { if (!responseMatchesOptions(existingResponse, searchOptions)) continue; final MdnsServiceInfo info = buildMdnsServiceInfoFromResponse(existingResponse, serviceTypeLabels); - listener.onServiceNameDiscovered(info); + listener.onServiceNameDiscovered(info, true /* isServiceFromCache */); if (existingResponse.isComplete()) { - listener.onServiceFound(info); + listener.onServiceFound(info, true /* isServiceFromCache */); hadReply = true; } } @@ -512,13 +512,13 @@ public class MdnsServiceTypeClient { final MdnsServiceBrowserListener listener = listeners.keyAt(i); if (newServiceFound) { sharedLog.log("onServiceNameDiscovered: " + serviceInfo); - listener.onServiceNameDiscovered(serviceInfo); + listener.onServiceNameDiscovered(serviceInfo, false /* isServiceFromCache */); } if (response.isComplete()) { if (newServiceFound || serviceBecomesComplete) { sharedLog.log("onServiceFound: " + serviceInfo); - listener.onServiceFound(serviceInfo); + listener.onServiceFound(serviceInfo, false /* isServiceFromCache */); } else { sharedLog.log("onServiceUpdated: " + serviceInfo); listener.onServiceUpdated(serviceInfo); diff --git a/tests/unit/java/com/android/metrics/NetworkNsdReportedMetricsTest.kt b/tests/unit/java/com/android/metrics/NetworkNsdReportedMetricsTest.kt index fdc12c908d..872326ebee 100644 --- a/tests/unit/java/com/android/metrics/NetworkNsdReportedMetricsTest.kt +++ b/tests/unit/java/com/android/metrics/NetworkNsdReportedMetricsTest.kt @@ -160,4 +160,45 @@ class NetworkNsdReportedMetricsTest { assertEquals(durationMs, it.eventDurationMillisec) } } + + @Test + fun testReportServiceResolved() { + val clientId = 99 + val transactionId = 100 + val durationMs = 10L + val metrics = NetworkNsdReportedMetrics(true /* isLegacy */, clientId, deps) + metrics.reportServiceResolved(transactionId, durationMs, true /* isServiceFromCache */) + + val eventCaptor = ArgumentCaptor.forClass(NetworkNsdReported::class.java) + verify(deps).statsWrite(eventCaptor.capture()) + eventCaptor.value.let { + assertTrue(it.isLegacy) + assertEquals(clientId, it.clientId) + assertEquals(transactionId, it.transactionId) + assertEquals(NsdEventType.NET_RESOLVE, it.type) + assertEquals(MdnsQueryResult.MQR_SERVICE_RESOLVED, it.queryResult) + assertTrue(it.isKnownService) + assertEquals(durationMs, it.eventDurationMillisec) + } + } + + @Test + fun testReportServiceResolutionFailed() { + val clientId = 99 + val transactionId = 100 + val durationMs = 10L + val metrics = NetworkNsdReportedMetrics(false /* isLegacy */, clientId, deps) + metrics.reportServiceResolutionFailed(transactionId, durationMs) + + val eventCaptor = ArgumentCaptor.forClass(NetworkNsdReported::class.java) + verify(deps).statsWrite(eventCaptor.capture()) + eventCaptor.value.let { + assertFalse(it.isLegacy) + assertEquals(clientId, it.clientId) + assertEquals(transactionId, it.transactionId) + assertEquals(NsdEventType.NET_RESOLVE, it.type) + assertEquals(MdnsQueryResult.MQR_SERVICE_RESOLUTION_FAILED, it.queryResult) + assertEquals(durationMs, it.eventDurationMillisec) + } + } } diff --git a/tests/unit/java/com/android/server/NsdServiceTest.java b/tests/unit/java/com/android/server/NsdServiceTest.java index 8ed065fc84..2789c9a060 100644 --- a/tests/unit/java/com/android/server/NsdServiceTest.java +++ b/tests/unit/java/com/android/server/NsdServiceTest.java @@ -457,19 +457,24 @@ public class NsdServiceTest { eq(interfaceIdx)); final String serviceAddress = "192.0.2.123"; + final int getAddrId = getAddrIdCaptor.getValue(); final GetAddressInfo addressInfo = new GetAddressInfo( - getAddrIdCaptor.getValue(), + getAddrId, IMDnsEventListener.SERVICE_GET_ADDR_SUCCESS, SERVICE_FULL_NAME, serviceAddress, interfaceIdx, INetd.LOCAL_NET_ID); + doReturn(TEST_TIME_MS + 10L).when(mClock).elapsedRealtime(); eventListener.onGettingServiceAddressStatus(addressInfo); waitForIdle(); final ArgumentCaptor resInfoCaptor = ArgumentCaptor.forClass(NsdServiceInfo.class); verify(resolveListener, timeout(TIMEOUT_MS)).onServiceResolved(resInfoCaptor.capture()); + verify(mMetrics).reportServiceResolved( + getAddrId, 10L /* durationMs */, false /* isServiceFromCache */); + final NsdServiceInfo resolvedService = resInfoCaptor.getValue(); assertEquals(SERVICE_NAME, resolvedService.getServiceName()); assertEquals("." + SERVICE_TYPE, resolvedService.getServiceType()); @@ -609,8 +614,9 @@ public class NsdServiceTest { eq("local.") /* domain */, eq(IFACE_IDX_ANY)); // Fail to resolve service. + final int resolvId = resolvIdCaptor.getValue(); final ResolutionInfo resolutionFailedInfo = new ResolutionInfo( - resolvIdCaptor.getValue(), + resolvId, IMDnsEventListener.SERVICE_RESOLUTION_FAILED, null /* serviceName */, null /* serviceType */, @@ -620,9 +626,11 @@ public class NsdServiceTest { 0 /* port */, new byte[0] /* txtRecord */, IFACE_IDX_ANY); + doReturn(TEST_TIME_MS + 10L).when(mClock).elapsedRealtime(); eventListener.onServiceResolutionStatus(resolutionFailedInfo); verify(resolveListener, timeout(TIMEOUT_MS)) .onResolveFailed(any(), eq(FAILURE_INTERNAL_ERROR)); + verify(mMetrics).reportServiceResolutionFailed(resolvId, 10L /* durationMs */); } @Test @@ -660,16 +668,19 @@ public class NsdServiceTest { eq(IFACE_IDX_ANY)); // Fail to get service address. + final int getAddrId = getAddrIdCaptor.getValue(); final GetAddressInfo gettingAddrFailedInfo = new GetAddressInfo( - getAddrIdCaptor.getValue(), + getAddrId, IMDnsEventListener.SERVICE_GET_ADDR_FAILED, null /* hostname */, null /* address */, IFACE_IDX_ANY, 0 /* netId */); + doReturn(TEST_TIME_MS + 10L).when(mClock).elapsedRealtime(); eventListener.onGettingServiceAddressStatus(gettingAddrFailedInfo); verify(resolveListener, timeout(TIMEOUT_MS)) .onResolveFailed(any(), eq(FAILURE_INTERNAL_ERROR)); + verify(mMetrics).reportServiceResolutionFailed(getAddrId, 10L /* durationMs */); } @Test @@ -828,7 +839,7 @@ public class NsdServiceTest { network); // Verify onServiceFound callback - listener.onServiceFound(mdnsServiceInfo); + listener.onServiceFound(mdnsServiceInfo, false /* isServiceFromCache */); final ArgumentCaptor updateInfoCaptor = ArgumentCaptor.forClass(NsdServiceInfo.class); verify(serviceInfoCallback, timeout(TIMEOUT_MS).times(1)) @@ -972,7 +983,7 @@ public class NsdServiceTest { network); // Verify onServiceNameDiscovered callback - listener.onServiceNameDiscovered(foundInfo); + listener.onServiceNameDiscovered(foundInfo, false /* isServiceFromCache */); verify(discListener, timeout(TIMEOUT_MS)).onServiceFound(argThat(info -> info.getServiceName().equals(SERVICE_NAME) // Service type in discovery callbacks has a dot at the end @@ -1081,8 +1092,8 @@ public class NsdServiceTest { final Network network = new Network(999); final String serviceType = "_nsd._service._tcp"; final String constructedServiceType = "_service._tcp.local"; - final ArgumentCaptor listenerCaptor = - ArgumentCaptor.forClass(MdnsServiceBrowserListener.class); + final ArgumentCaptor listenerCaptor = + ArgumentCaptor.forClass(MdnsListener.class); final NsdServiceInfo request = new NsdServiceInfo(SERVICE_NAME, serviceType); request.setNetwork(network); client.resolveService(request, resolveListener); @@ -1097,7 +1108,7 @@ public class NsdServiceTest { // Subtypes are not used for resolution, only for discovery assertEquals(Collections.emptyList(), optionsCaptor.getValue().getSubtypes()); - final MdnsServiceBrowserListener listener = listenerCaptor.getValue(); + final MdnsListener listener = listenerCaptor.getValue(); final MdnsServiceInfo mdnsServiceInfo = new MdnsServiceInfo( SERVICE_NAME, constructedServiceType.split("\\."), @@ -1113,10 +1124,14 @@ public class NsdServiceTest { network); // Verify onServiceFound callback - listener.onServiceFound(mdnsServiceInfo); + doReturn(TEST_TIME_MS + 10L).when(mClock).elapsedRealtime(); + listener.onServiceFound(mdnsServiceInfo, true /* isServiceFromCache */); final ArgumentCaptor infoCaptor = ArgumentCaptor.forClass(NsdServiceInfo.class); verify(resolveListener, timeout(TIMEOUT_MS)).onServiceResolved(infoCaptor.capture()); + verify(mMetrics).reportServiceResolved( + listener.mTransactionId, 10 /* durationMs */, true /* isServiceFromCache */); + final NsdServiceInfo info = infoCaptor.getValue(); assertEquals(SERVICE_NAME, info.getServiceName()); assertEquals("._service._tcp", info.getServiceType()); 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 1fdfe0916e..fde5abdc46 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java +++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java @@ -26,6 +26,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.argThat; @@ -645,14 +646,14 @@ public class MdnsServiceTypeClientTests { SERVICE_TYPE_LABELS, Collections.emptyMap(), TEST_TTL), socketKey); - verify(mockListenerOne).onServiceNameDiscovered(any()); - verify(mockListenerOne).onServiceFound(any()); + verify(mockListenerOne).onServiceNameDiscovered(any(), eq(false) /* isServiceFromCache */); + verify(mockListenerOne).onServiceFound(any(), eq(false) /* isServiceFromCache */); // File another identical query startSendAndReceive(mockListenerTwo, searchOptions); - verify(mockListenerTwo).onServiceNameDiscovered(any()); - verify(mockListenerTwo).onServiceFound(any()); + verify(mockListenerTwo).onServiceNameDiscovered(any(), eq(true) /* isServiceFromCache */); + verify(mockListenerTwo).onServiceFound(any(), eq(true) /* isServiceFromCache */); // This time no query is submitted, only scheduled assertNull(currentThreadExecutor.getAndClearSubmittedRunnable()); @@ -686,7 +687,8 @@ public class MdnsServiceTypeClientTests { "service-instance-1", null /* host */, 0 /* port */, SERVICE_TYPE_LABELS, Collections.emptyMap(), TEST_TTL), socketKey); - verify(mockListenerOne).onServiceNameDiscovered(serviceInfoCaptor.capture()); + verify(mockListenerOne).onServiceNameDiscovered( + serviceInfoCaptor.capture(), eq(false) /* isServiceFromCache */); verifyServiceInfo(serviceInfoCaptor.getAllValues().get(0), "service-instance-1", SERVICE_TYPE_LABELS, @@ -697,7 +699,7 @@ public class MdnsServiceTypeClientTests { Collections.emptyMap(), socketKey); - verify(mockListenerOne, never()).onServiceFound(any(MdnsServiceInfo.class)); + verify(mockListenerOne, never()).onServiceFound(any(MdnsServiceInfo.class), anyBoolean()); verify(mockListenerOne, never()).onServiceUpdated(any(MdnsServiceInfo.class)); } @@ -718,7 +720,8 @@ public class MdnsServiceTypeClientTests { socketKey); // Verify onServiceNameDiscovered was called once for the initial response. - verify(mockListenerOne).onServiceNameDiscovered(serviceInfoCaptor.capture()); + verify(mockListenerOne).onServiceNameDiscovered( + serviceInfoCaptor.capture(), eq(false) /* isServiceFromCache */); verifyServiceInfo(serviceInfoCaptor.getAllValues().get(0), "service-instance-1", SERVICE_TYPE_LABELS, @@ -730,7 +733,8 @@ public class MdnsServiceTypeClientTests { socketKey); // Verify onServiceFound was called once for the initial response. - verify(mockListenerOne).onServiceFound(serviceInfoCaptor.capture()); + verify(mockListenerOne).onServiceFound( + serviceInfoCaptor.capture(), eq(false) /* isServiceFromCache */); MdnsServiceInfo initialServiceInfo = serviceInfoCaptor.getAllValues().get(1); assertEquals(initialServiceInfo.getServiceInstanceName(), "service-instance-1"); assertEquals(initialServiceInfo.getIpv4Address(), ipV4Address); @@ -770,7 +774,8 @@ public class MdnsServiceTypeClientTests { socketKey); // Verify onServiceNameDiscovered was called once for the initial response. - verify(mockListenerOne).onServiceNameDiscovered(serviceInfoCaptor.capture()); + verify(mockListenerOne).onServiceNameDiscovered( + serviceInfoCaptor.capture(), eq(false) /* isServiceFromCache */); verifyServiceInfo(serviceInfoCaptor.getAllValues().get(0), "service-instance-1", SERVICE_TYPE_LABELS, @@ -782,7 +787,8 @@ public class MdnsServiceTypeClientTests { socketKey); // Verify onServiceFound was called once for the initial response. - verify(mockListenerOne).onServiceFound(serviceInfoCaptor.capture()); + verify(mockListenerOne).onServiceFound( + serviceInfoCaptor.capture(), eq(false) /* isServiceFromCache */); MdnsServiceInfo initialServiceInfo = serviceInfoCaptor.getAllValues().get(1); assertEquals(initialServiceInfo.getServiceInstanceName(), "service-instance-1"); assertEquals(initialServiceInfo.getIpv6Address(), ipV6Address); @@ -867,7 +873,8 @@ public class MdnsServiceTypeClientTests { startSendAndReceive(mockListenerOne, MdnsSearchOptions.getDefaultOptions()); // Verify onServiceNameDiscovered was called once for the existing response. - verify(mockListenerOne).onServiceNameDiscovered(serviceInfoCaptor.capture()); + verify(mockListenerOne).onServiceNameDiscovered( + serviceInfoCaptor.capture(), eq(true) /* isServiceFromCache */); verifyServiceInfo(serviceInfoCaptor.getAllValues().get(0), "service-instance-1", SERVICE_TYPE_LABELS, @@ -879,7 +886,8 @@ public class MdnsServiceTypeClientTests { socketKey); // Verify onServiceFound was called once for the existing response. - verify(mockListenerOne).onServiceFound(serviceInfoCaptor.capture()); + verify(mockListenerOne).onServiceFound( + serviceInfoCaptor.capture(), eq(true) /* isServiceFromCache */); MdnsServiceInfo existingServiceInfo = serviceInfoCaptor.getAllValues().get(1); assertEquals(existingServiceInfo.getServiceInstanceName(), "service-instance-1"); assertEquals(existingServiceInfo.getIpv4Address(), "192.168.1.1"); @@ -897,8 +905,9 @@ public class MdnsServiceTypeClientTests { // Verify onServiceFound was not called on the newly registered listener after the existing // response is gone. - verify(mockListenerTwo, never()).onServiceNameDiscovered(any(MdnsServiceInfo.class)); - verify(mockListenerTwo, never()).onServiceFound(any(MdnsServiceInfo.class)); + verify(mockListenerTwo, never()).onServiceNameDiscovered( + any(MdnsServiceInfo.class), eq(false)); + verify(mockListenerTwo, never()).onServiceFound(any(MdnsServiceInfo.class), anyBoolean()); } @Test @@ -1044,7 +1053,8 @@ public class MdnsServiceTypeClientTests { socketKey); // Verify onServiceNameDiscovered was first called for the initial response. - inOrder.verify(mockListenerOne).onServiceNameDiscovered(serviceInfoCaptor.capture()); + inOrder.verify(mockListenerOne).onServiceNameDiscovered( + serviceInfoCaptor.capture(), eq(false) /* isServiceFromCache */); verifyServiceInfo(serviceInfoCaptor.getAllValues().get(0), serviceName, SERVICE_TYPE_LABELS, @@ -1056,7 +1066,8 @@ public class MdnsServiceTypeClientTests { socketKey); // Verify onServiceFound was second called for the second response. - inOrder.verify(mockListenerOne).onServiceFound(serviceInfoCaptor.capture()); + inOrder.verify(mockListenerOne).onServiceFound( + serviceInfoCaptor.capture(), eq(false) /* isServiceFromCache */); verifyServiceInfo(serviceInfoCaptor.getAllValues().get(1), serviceName, SERVICE_TYPE_LABELS, @@ -1183,10 +1194,11 @@ public class MdnsServiceTypeClientTests { Collections.emptyList() /* authorityRecords */, Collections.emptyList() /* additionalRecords */); - inOrder.verify(mockListenerOne, never()).onServiceNameDiscovered(any()); + inOrder.verify(mockListenerOne, never()).onServiceNameDiscovered(any(), anyBoolean()); processResponse(addressResponse, socketKey); - inOrder.verify(mockListenerOne).onServiceFound(serviceInfoCaptor.capture()); + inOrder.verify(mockListenerOne).onServiceFound( + serviceInfoCaptor.capture(), eq(false) /* isServiceFromCache */); verifyServiceInfo(serviceInfoCaptor.getValue(), instanceName, SERVICE_TYPE_LABELS, @@ -1253,8 +1265,10 @@ public class MdnsServiceTypeClientTests { Collections.emptyList() /* additionalRecords */); processResponse(srvTxtResponse, socketKey); dispatchMessage(); - inOrder.verify(mockListenerOne).onServiceNameDiscovered(any()); - inOrder.verify(mockListenerOne).onServiceFound(any()); + inOrder.verify(mockListenerOne).onServiceNameDiscovered( + any(), eq(false) /* isServiceFromCache */); + inOrder.verify(mockListenerOne).onServiceFound( + any(), eq(false) /* isServiceFromCache */); // Expect no query on the next run currentThreadExecutor.getAndClearLastScheduledRunnable().run(); @@ -1355,24 +1369,29 @@ public class MdnsServiceTypeClientTests { // mockListenerOne gets notified for the requested instance verify(mockListenerOne).onServiceNameDiscovered( - matchServiceName(capitalizedRequestInstance)); - verify(mockListenerOne).onServiceFound(matchServiceName(capitalizedRequestInstance)); + matchServiceName(capitalizedRequestInstance), eq(false) /* isServiceFromCache */); + verify(mockListenerOne).onServiceFound( + matchServiceName(capitalizedRequestInstance), eq(false) /* isServiceFromCache */); // ...but does not get any callback for the other instance - verify(mockListenerOne, never()).onServiceFound(matchServiceName(otherInstance)); - verify(mockListenerOne, never()).onServiceNameDiscovered(matchServiceName(otherInstance)); + verify(mockListenerOne, never()).onServiceFound( + matchServiceName(otherInstance), anyBoolean()); + verify(mockListenerOne, never()).onServiceNameDiscovered( + matchServiceName(otherInstance), anyBoolean()); verify(mockListenerOne, never()).onServiceUpdated(matchServiceName(otherInstance)); verify(mockListenerOne, never()).onServiceRemoved(matchServiceName(otherInstance)); // mockListenerTwo gets notified for both though final InOrder inOrder = inOrder(mockListenerTwo); inOrder.verify(mockListenerTwo).onServiceNameDiscovered( - matchServiceName(capitalizedRequestInstance)); + matchServiceName(capitalizedRequestInstance), eq(false) /* isServiceFromCache */); inOrder.verify(mockListenerTwo).onServiceFound( - matchServiceName(capitalizedRequestInstance)); + matchServiceName(capitalizedRequestInstance), eq(false) /* isServiceFromCache */); - inOrder.verify(mockListenerTwo).onServiceNameDiscovered(matchServiceName(otherInstance)); - inOrder.verify(mockListenerTwo).onServiceFound(matchServiceName(otherInstance)); + inOrder.verify(mockListenerTwo).onServiceNameDiscovered( + matchServiceName(otherInstance), eq(false) /* isServiceFromCache */); + inOrder.verify(mockListenerTwo).onServiceFound( + matchServiceName(otherInstance), eq(false) /* isServiceFromCache */); inOrder.verify(mockListenerTwo).onServiceUpdated(matchServiceName(otherInstance)); inOrder.verify(mockListenerTwo).onServiceRemoved(matchServiceName(otherInstance)); } @@ -1439,22 +1458,30 @@ public class MdnsServiceTypeClientTests { final ArgumentMatcher subtypeInstanceMatcher = info -> info.getServiceInstanceName().equals(matchingInstance) && info.getSubtypes().equals(Collections.singletonList(subtype)); - verify(mockListenerOne).onServiceNameDiscovered(argThat(subtypeInstanceMatcher)); - verify(mockListenerOne).onServiceFound(argThat(subtypeInstanceMatcher)); + verify(mockListenerOne).onServiceNameDiscovered( + argThat(subtypeInstanceMatcher), eq(false) /* isServiceFromCache */); + verify(mockListenerOne).onServiceFound( + argThat(subtypeInstanceMatcher), eq(false) /* isServiceFromCache */); // ...but does not get any callback for the other instance - verify(mockListenerOne, never()).onServiceFound(matchServiceName(otherInstance)); - verify(mockListenerOne, never()).onServiceNameDiscovered(matchServiceName(otherInstance)); + verify(mockListenerOne, never()).onServiceFound( + matchServiceName(otherInstance), anyBoolean()); + verify(mockListenerOne, never()).onServiceNameDiscovered( + matchServiceName(otherInstance), anyBoolean()); verify(mockListenerOne, never()).onServiceUpdated(matchServiceName(otherInstance)); verify(mockListenerOne, never()).onServiceRemoved(matchServiceName(otherInstance)); // mockListenerTwo gets notified for both though final InOrder inOrder = inOrder(mockListenerTwo); - inOrder.verify(mockListenerTwo).onServiceNameDiscovered(argThat(subtypeInstanceMatcher)); - inOrder.verify(mockListenerTwo).onServiceFound(argThat(subtypeInstanceMatcher)); + inOrder.verify(mockListenerTwo).onServiceNameDiscovered( + argThat(subtypeInstanceMatcher), eq(false) /* isServiceFromCache */); + inOrder.verify(mockListenerTwo).onServiceFound( + argThat(subtypeInstanceMatcher), eq(false) /* isServiceFromCache */); - inOrder.verify(mockListenerTwo).onServiceNameDiscovered(matchServiceName(otherInstance)); - inOrder.verify(mockListenerTwo).onServiceFound(matchServiceName(otherInstance)); + inOrder.verify(mockListenerTwo).onServiceNameDiscovered( + matchServiceName(otherInstance), eq(false) /* isServiceFromCache */); + inOrder.verify(mockListenerTwo).onServiceFound( + matchServiceName(otherInstance), eq(false) /* isServiceFromCache */); inOrder.verify(mockListenerTwo).onServiceUpdated(matchServiceName(otherInstance)); inOrder.verify(mockListenerTwo).onServiceRemoved(matchServiceName(otherInstance)); } @@ -1508,24 +1535,30 @@ public class MdnsServiceTypeClientTests { // mockListenerOne gets notified for the requested instance final InOrder inOrder1 = inOrder(mockListenerOne); inOrder1.verify(mockListenerOne).onServiceNameDiscovered( - matchServiceName(requestedInstance)); - inOrder1.verify(mockListenerOne).onServiceFound(matchServiceName(requestedInstance)); + matchServiceName(requestedInstance), eq(false) /* isServiceFromCache */); + inOrder1.verify(mockListenerOne).onServiceFound( + matchServiceName(requestedInstance), eq(false) /* isServiceFromCache */); inOrder1.verify(mockListenerOne).onServiceRemoved(matchServiceName(requestedInstance)); inOrder1.verify(mockListenerOne).onServiceNameRemoved(matchServiceName(requestedInstance)); - verify(mockListenerOne, never()).onServiceFound(matchServiceName(otherInstance)); - verify(mockListenerOne, never()).onServiceNameDiscovered(matchServiceName(otherInstance)); + verify(mockListenerOne, never()).onServiceFound( + matchServiceName(otherInstance), anyBoolean()); + verify(mockListenerOne, never()).onServiceNameDiscovered( + matchServiceName(otherInstance), anyBoolean()); verify(mockListenerOne, never()).onServiceRemoved(matchServiceName(otherInstance)); verify(mockListenerOne, never()).onServiceNameRemoved(matchServiceName(otherInstance)); // mockListenerTwo gets notified for both though final InOrder inOrder2 = inOrder(mockListenerTwo); inOrder2.verify(mockListenerTwo).onServiceNameDiscovered( - matchServiceName(requestedInstance)); - inOrder2.verify(mockListenerTwo).onServiceFound(matchServiceName(requestedInstance)); + matchServiceName(requestedInstance), eq(false) /* isServiceFromCache */); + inOrder2.verify(mockListenerTwo).onServiceFound( + matchServiceName(requestedInstance), eq(false) /* isServiceFromCache */); inOrder2.verify(mockListenerTwo).onServiceRemoved(matchServiceName(requestedInstance)); inOrder2.verify(mockListenerTwo).onServiceNameRemoved(matchServiceName(requestedInstance)); - verify(mockListenerTwo).onServiceNameDiscovered(matchServiceName(otherInstance)); - verify(mockListenerTwo).onServiceFound(matchServiceName(otherInstance)); + verify(mockListenerTwo).onServiceNameDiscovered( + matchServiceName(otherInstance), eq(false) /* isServiceFromCache */); + verify(mockListenerTwo).onServiceFound( + matchServiceName(otherInstance), eq(false) /* isServiceFromCache */); verify(mockListenerTwo).onServiceRemoved(matchServiceName(otherInstance)); verify(mockListenerTwo).onServiceNameRemoved(matchServiceName(otherInstance)); } @@ -1547,7 +1580,8 @@ public class MdnsServiceTypeClientTests { socketKey); // Verify that onServiceNameDiscovered is called. - inOrder.verify(mockListenerOne).onServiceNameDiscovered(serviceInfoCaptor.capture()); + inOrder.verify(mockListenerOne).onServiceNameDiscovered( + serviceInfoCaptor.capture(), eq(false) /* isServiceFromCache */); verifyServiceInfo(serviceInfoCaptor.getAllValues().get(0), serviceName, SERVICE_TYPE_LABELS, @@ -1559,7 +1593,8 @@ public class MdnsServiceTypeClientTests { socketKey); // Verify that onServiceFound is called. - inOrder.verify(mockListenerOne).onServiceFound(serviceInfoCaptor.capture()); + inOrder.verify(mockListenerOne).onServiceFound( + serviceInfoCaptor.capture(), eq(false) /* isServiceFromCache */); verifyServiceInfo(serviceInfoCaptor.getAllValues().get(1), serviceName, SERVICE_TYPE_LABELS, @@ -1581,7 +1616,8 @@ public class MdnsServiceTypeClientTests { // The services are cached in MdnsServiceCache, verify that onServiceNameDiscovered is // called immediately. - inOrder2.verify(mockListenerTwo).onServiceNameDiscovered(serviceInfoCaptor.capture()); + inOrder2.verify(mockListenerTwo).onServiceNameDiscovered( + serviceInfoCaptor.capture(), eq(true) /* isServiceFromCache */); verifyServiceInfo(serviceInfoCaptor.getAllValues().get(2), serviceName, SERVICE_TYPE_LABELS, @@ -1594,7 +1630,8 @@ public class MdnsServiceTypeClientTests { // The services are cached in MdnsServiceCache, verify that onServiceFound is // called immediately. - inOrder2.verify(mockListenerTwo).onServiceFound(serviceInfoCaptor.capture()); + inOrder2.verify(mockListenerTwo).onServiceFound( + serviceInfoCaptor.capture(), eq(true) /* isServiceFromCache */); verifyServiceInfo(serviceInfoCaptor.getAllValues().get(3), serviceName, SERVICE_TYPE_LABELS,