From b771670b5de29957caa86ec8644b5b5ca3e77eb1 Mon Sep 17 00:00:00 2001 From: Paul Hu Date: Wed, 29 May 2024 08:23:37 +0000 Subject: [PATCH] Report isKnownService in discovery stop metrics In the reportServiceDiscoveryStop function, the isKnownService value is not set. This means that it is not possible to track how many discovery queries are answered from cache. To track this information, add the isKnownService to reportServiceDiscoveryStop. Fix: 339738663 Test: atest FrameworksNetTests NsdManagerTest Change-Id: Idf9948826d3b8ffff9bce4f224543af87c653d80 --- .../metrics/NetworkNsdReportedMetrics.java | 4 +++- service-t/src/com/android/server/NsdService.java | 15 ++++++++++++--- .../metrics/NetworkNsdReportedMetricsTest.kt | 4 +++- .../java/com/android/server/NsdServiceTest.java | 4 ++-- 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/service-t/src/com/android/metrics/NetworkNsdReportedMetrics.java b/service-t/src/com/android/metrics/NetworkNsdReportedMetrics.java index 385adc6c70..a92dfaf7d6 100644 --- a/service-t/src/com/android/metrics/NetworkNsdReportedMetrics.java +++ b/service-t/src/com/android/metrics/NetworkNsdReportedMetrics.java @@ -193,7 +193,8 @@ public class NetworkNsdReportedMetrics { * @param sentQueryCount The count of sent queries before stop discovery. */ public void reportServiceDiscoveryStop(boolean isLegacy, int transactionId, long durationMs, - int foundCallbackCount, int lostCallbackCount, int servicesCount, int sentQueryCount) { + int foundCallbackCount, int lostCallbackCount, int servicesCount, int sentQueryCount, + boolean isServiceFromCache) { final Builder builder = makeReportedBuilder(isLegacy, transactionId); builder.setType(NsdEventType.NET_DISCOVER); builder.setQueryResult(MdnsQueryResult.MQR_SERVICE_DISCOVERY_STOP); @@ -202,6 +203,7 @@ public class NetworkNsdReportedMetrics { builder.setLostCallbackCount(lostCallbackCount); builder.setFoundServiceCount(servicesCount); builder.setSentQueryCount(sentQueryCount); + builder.setIsKnownService(isServiceFromCache); 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 0a8adf0f65..f8b0d531b6 100644 --- a/service-t/src/com/android/server/NsdService.java +++ b/service-t/src/com/android/server/NsdService.java @@ -1634,6 +1634,12 @@ public class NsdService extends INsdManager.Stub { NsdManager.nameOf(code), transactionId)); switch (code) { case NsdManager.SERVICE_FOUND: + // Set the ServiceFromCache flag only if the service is actually being + // retrieved from the cache. This flag should not be overridden by later + // service found event, which may not be cached. + if (event.mIsServiceFromCache) { + request.setServiceFromCache(true); + } clientInfo.onServiceFound(clientRequestId, info, request); break; case NsdManager.SERVICE_LOST: @@ -2887,7 +2893,8 @@ public class NsdService extends INsdManager.Stub { request.getFoundServiceCount(), request.getLostServiceCount(), request.getServicesCount(), - request.getSentQueryCount()); + request.getSentQueryCount(), + request.isServiceFromCache()); } else if (listener instanceof ResolutionListener) { mMetrics.reportServiceResolutionStop(false /* isLegacy */, transactionId, request.calculateRequestDurationMs(mClock.elapsedRealtime()), @@ -2927,7 +2934,8 @@ public class NsdService extends INsdManager.Stub { request.getFoundServiceCount(), request.getLostServiceCount(), request.getServicesCount(), - NO_SENT_QUERY_COUNT); + NO_SENT_QUERY_COUNT, + request.isServiceFromCache()); break; case NsdManager.RESOLVE_SERVICE: stopResolveService(transactionId); @@ -3050,7 +3058,8 @@ public class NsdService extends INsdManager.Stub { request.getFoundServiceCount(), request.getLostServiceCount(), request.getServicesCount(), - request.getSentQueryCount()); + request.getSentQueryCount(), + request.isServiceFromCache()); try { mCb.onStopDiscoverySucceeded(listenerKey); } catch (RemoteException e) { diff --git a/tests/unit/java/com/android/metrics/NetworkNsdReportedMetricsTest.kt b/tests/unit/java/com/android/metrics/NetworkNsdReportedMetricsTest.kt index aa28e5a5a2..10ba6a458e 100644 --- a/tests/unit/java/com/android/metrics/NetworkNsdReportedMetricsTest.kt +++ b/tests/unit/java/com/android/metrics/NetworkNsdReportedMetricsTest.kt @@ -163,7 +163,8 @@ class NetworkNsdReportedMetricsTest { val sentQueryCount = 150 val metrics = NetworkNsdReportedMetrics(clientId, deps) metrics.reportServiceDiscoveryStop(true /* isLegacy */, transactionId, durationMs, - foundCallbackCount, lostCallbackCount, servicesCount, sentQueryCount) + foundCallbackCount, lostCallbackCount, servicesCount, sentQueryCount, + true /* isServiceFromCache */) val eventCaptor = ArgumentCaptor.forClass(NetworkNsdReported::class.java) verify(deps).statsWrite(eventCaptor.capture()) @@ -179,6 +180,7 @@ class NetworkNsdReportedMetricsTest { assertEquals(servicesCount, it.foundServiceCount) assertEquals(durationMs, it.eventDurationMillisec) assertEquals(sentQueryCount, it.sentQueryCount) + assertTrue(it.isKnownService) } } diff --git a/tests/unit/java/com/android/server/NsdServiceTest.java b/tests/unit/java/com/android/server/NsdServiceTest.java index aece3f7adc..979e0a154a 100644 --- a/tests/unit/java/com/android/server/NsdServiceTest.java +++ b/tests/unit/java/com/android/server/NsdServiceTest.java @@ -1196,7 +1196,7 @@ public class NsdServiceTest { Instant.MAX /* expirationTime */); // Verify onServiceNameDiscovered callback - listener.onServiceNameDiscovered(foundInfo, false /* isServiceFromCache */); + listener.onServiceNameDiscovered(foundInfo, true /* 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 @@ -1232,7 +1232,7 @@ public class NsdServiceTest { verify(mSocketProvider, timeout(CLEANUP_DELAY_MS + TIMEOUT_MS)).requestStopWhenInactive(); verify(mMetrics).reportServiceDiscoveryStop(false /* isLegacy */, discId, 10L /* durationMs */, 1 /* foundCallbackCount */, 1 /* lostCallbackCount */, - 1 /* servicesCount */, 3 /* sentQueryCount */); + 1 /* servicesCount */, 3 /* sentQueryCount */, true /* isServiceFromCache */); } @Test