From 043bcd4537fd33c532e2d8dcf619e8f6495dd5ab Mon Sep 17 00:00:00 2001 From: Paul Hu Date: Fri, 14 Jul 2023 16:38:25 +0800 Subject: [PATCH] Report more advertising metrics data Report more advertising metrics data below when the service is unregistered. - Replied request count (sum across interfaces) - Sent packet count (including announcements and probes) - Number of conflicts during probing - Nubmer of conflicts after probing Bug: 287546772 Test: atest FrameworksNetTestCases NsdManagerTest Merged-In: I50c54a35dc523422e3a7302c059bbbc38eac5631 Change-Id: I50c54a35dc523422e3a7302c059bbbc38eac5631 --- .../metrics/NetworkNsdReportedMetrics.java | 34 +++++++++- .../src/com/android/server/NsdService.java | 32 +++++++-- .../connectivity/mdns/MdnsAdvertiser.java | 67 +++++++++++++++++++ .../connectivity/mdns/MdnsConstants.java | 1 + .../mdns/MdnsInterfaceAdvertiser.java | 29 ++++++-- .../connectivity/mdns/MdnsPacketRepeater.java | 7 +- .../mdns/MdnsRecordRepository.java | 66 +++++++++++++++--- .../connectivity/mdns/MdnsReplySender.java | 8 ++- service/src/com/android/metrics/stats.proto | 12 ++++ .../metrics/NetworkNsdReportedMetricsTest.kt | 20 +++++- .../com/android/server/NsdServiceTest.java | 10 ++- .../connectivity/mdns/MdnsAdvertiserTest.kt | 39 +++++++++++ .../connectivity/mdns/MdnsAnnouncerTest.kt | 2 +- .../connectivity/mdns/MdnsProberTest.kt | 6 +- .../mdns/MdnsRecordRepositoryTest.kt | 38 +++++++++-- 15 files changed, 333 insertions(+), 38 deletions(-) diff --git a/service-t/src/com/android/metrics/NetworkNsdReportedMetrics.java b/service-t/src/com/android/metrics/NetworkNsdReportedMetrics.java index 597c06ffed..6b03daaa7c 100644 --- a/service-t/src/com/android/metrics/NetworkNsdReportedMetrics.java +++ b/service-t/src/com/android/metrics/NetworkNsdReportedMetrics.java @@ -24,16 +24,22 @@ import android.stats.connectivity.NsdEventType; import com.android.internal.annotations.VisibleForTesting; import com.android.server.ConnectivityStatsLog; +import java.util.Random; + /** * Class to record the NetworkNsdReported into statsd. Each client should create this class to * report its data. */ public class NetworkNsdReportedMetrics { + // The upper bound for the random number used in metrics data sampling determines the possible + // sample rate. + private static final int RANDOM_NUMBER_UPPER_BOUND = 1000; // Whether this client is using legacy backend. private final boolean mIsLegacy; // The client id. private final int mClientId; private final Dependencies mDependencies; + private final Random mRandom; public NetworkNsdReportedMetrics(boolean isLegacy, int clientId) { this(isLegacy, clientId, new Dependencies()); @@ -44,6 +50,7 @@ public class NetworkNsdReportedMetrics { mIsLegacy = isLegacy; mClientId = clientId; mDependencies = dependencies; + mRandom = dependencies.makeRandomGenerator(); } /** @@ -67,7 +74,18 @@ public class NetworkNsdReportedMetrics { event.getFoundCallbackCount(), event.getLostCallbackCount(), event.getRepliedRequestsCount(), - event.getSentQueryCount()); + event.getSentQueryCount(), + event.getSentPacketCount(), + event.getConflictDuringProbingCount(), + event.getConflictAfterProbingCount(), + event.getRandomNumber()); + } + + /** + * @see Random + */ + public Random makeRandomGenerator() { + return new Random(); } } @@ -75,6 +93,7 @@ public class NetworkNsdReportedMetrics { final Builder builder = NetworkNsdReported.newBuilder(); builder.setIsLegacy(mIsLegacy); builder.setClientId(mClientId); + builder.setRandomNumber(mRandom.nextInt(RANDOM_NUMBER_UPPER_BOUND)); return builder; } @@ -113,14 +132,23 @@ public class NetworkNsdReportedMetrics { * * @param transactionId The transaction id of service registration. * @param durationMs The duration of service stayed registered. + * @param repliedRequestsCount The replied request count of this service before unregistered it. + * @param sentPacketCount The total sent packet count of this service before unregistered it. + * @param conflictDuringProbingCount The number of conflict during probing. + * @param conflictAfterProbingCount The number of conflict after probing. */ - public void reportServiceUnregistration(int transactionId, long durationMs) { + public void reportServiceUnregistration(int transactionId, long durationMs, + int repliedRequestsCount, int sentPacketCount, int conflictDuringProbingCount, + int conflictAfterProbingCount) { final Builder builder = makeReportedBuilder(); builder.setTransactionId(transactionId); builder.setType(NsdEventType.NET_REGISTER); builder.setQueryResult(MdnsQueryResult.MQR_SERVICE_UNREGISTERED); builder.setEventDurationMillisec(durationMs); - // TODO: Report repliedRequestsCount + builder.setRepliedRequestsCount(repliedRequestsCount); + builder.setSentPacketCount(sentPacketCount); + builder.setConflictDuringProbingCount(conflictDuringProbingCount); + builder.setConflictAfterProbingCount(conflictAfterProbingCount); 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 27e97f100f..b9acc48518 100644 --- a/service-t/src/com/android/server/NsdService.java +++ b/service-t/src/com/android/server/NsdService.java @@ -29,6 +29,8 @@ import static android.provider.DeviceConfig.NAMESPACE_TETHERING; import static com.android.modules.utils.build.SdkLevel.isAtLeastU; import static com.android.networkstack.apishim.ConstantsShim.REGISTER_NSD_OFFLOAD_ENGINE; +import static com.android.server.connectivity.mdns.MdnsAdvertiser.AdvertiserMetrics; +import static com.android.server.connectivity.mdns.MdnsConstants.NO_PACKET; import static com.android.server.connectivity.mdns.MdnsRecord.MAX_LABEL_LENGTH; import static com.android.server.connectivity.mdns.util.MdnsUtils.Clock; @@ -992,14 +994,20 @@ public class NsdService extends INsdManager.Stub { // instead of looking at the flag value. final long stopTimeMs = mClock.elapsedRealtime(); if (request instanceof AdvertiserClientRequest) { + final AdvertiserMetrics metrics = + mAdvertiser.getAdvertiserMetrics(transactionId); mAdvertiser.removeService(transactionId); clientInfo.onUnregisterServiceSucceeded(clientRequestId, transactionId, - request.calculateRequestDurationMs(stopTimeMs)); + request.calculateRequestDurationMs(stopTimeMs), metrics); } else { if (unregisterService(transactionId)) { clientInfo.onUnregisterServiceSucceeded(clientRequestId, transactionId, - request.calculateRequestDurationMs(stopTimeMs)); + request.calculateRequestDurationMs(stopTimeMs), + new AdvertiserMetrics(NO_PACKET /* repliedRequestsCount */, + NO_PACKET /* sentPacketCount */, + 0 /* conflictDuringProbingCount */, + 0 /* conflictAfterProbingCount */)); } else { clientInfo.onUnregisterServiceFailed( clientRequestId, NsdManager.FAILURE_INTERNAL_ERROR); @@ -2496,9 +2504,14 @@ public class NsdService extends INsdManager.Stub { } if (request instanceof AdvertiserClientRequest) { + final AdvertiserMetrics metrics = + mAdvertiser.getAdvertiserMetrics(transactionId); mAdvertiser.removeService(transactionId); mMetrics.reportServiceUnregistration(transactionId, - request.calculateRequestDurationMs(mClock.elapsedRealtime())); + request.calculateRequestDurationMs(mClock.elapsedRealtime()), + metrics.mRepliedRequestsCount, metrics.mSentPacketCount, + metrics.mConflictDuringProbingCount, + metrics.mConflictAfterProbingCount); continue; } @@ -2524,7 +2537,11 @@ public class NsdService extends INsdManager.Stub { case NsdManager.REGISTER_SERVICE: unregisterService(transactionId); mMetrics.reportServiceUnregistration(transactionId, - request.calculateRequestDurationMs(mClock.elapsedRealtime())); + request.calculateRequestDurationMs(mClock.elapsedRealtime()), + NO_PACKET /* repliedRequestsCount */, + NO_PACKET /* sentPacketCount */, + 0 /* conflictDuringProbingCount */, + 0 /* conflictAfterProbingCount */); break; default: break; @@ -2663,8 +2680,11 @@ public class NsdService extends INsdManager.Stub { } } - void onUnregisterServiceSucceeded(int listenerKey, int transactionId, long durationMs) { - mMetrics.reportServiceUnregistration(transactionId, durationMs); + void onUnregisterServiceSucceeded(int listenerKey, int transactionId, long durationMs, + AdvertiserMetrics metrics) { + mMetrics.reportServiceUnregistration(transactionId, durationMs, + metrics.mRepliedRequestsCount, metrics.mSentPacketCount, + metrics.mConflictDuringProbingCount, metrics.mConflictAfterProbingCount); try { mCb.onUnregisterServiceSucceeded(listenerKey); } catch (RemoteException e) { diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsAdvertiser.java b/service-t/src/com/android/server/connectivity/mdns/MdnsAdvertiser.java index ce05a84f8d..913d233ec8 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsAdvertiser.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsAdvertiser.java @@ -16,6 +16,7 @@ package com.android.server.connectivity.mdns; +import static com.android.server.connectivity.mdns.MdnsConstants.NO_PACKET; import static com.android.server.connectivity.mdns.MdnsRecord.MAX_LABEL_LENGTH; import android.annotation.NonNull; @@ -180,6 +181,7 @@ public class MdnsAdvertiser { // (with the old, conflicting, actually not used name as argument... The new // implementation will send callbacks with the new name). registration.mNotifiedRegistrationSuccess = false; + registration.mConflictAfterProbingCount++; // The service was done probing, just reset it to probing state (RFC6762 9.) forAllAdvertisers(a -> { @@ -195,6 +197,7 @@ public class MdnsAdvertiser { registration.updateForConflict( registration.makeNewServiceInfoForConflict(1 /* renameCount */), 1 /* renameCount */); + registration.mConflictDuringProbingCount++; // Keep renaming if the new name conflicts in local registrations updateRegistrationUntilNoConflict((net, adv) -> adv.hasRegistration(registration), @@ -360,6 +363,22 @@ public class MdnsAdvertiser { } } + int getServiceRepliedRequestsCount(int id) { + int repliedRequestsCount = NO_PACKET; + for (int i = 0; i < mAdvertisers.size(); i++) { + repliedRequestsCount += mAdvertisers.valueAt(i).getServiceRepliedRequestsCount(id); + } + return repliedRequestsCount; + } + + int getSentPacketCount(int id) { + int sentPacketCount = NO_PACKET; + for (int i = 0; i < mAdvertisers.size(); i++) { + sentPacketCount += mAdvertisers.valueAt(i).getSentPacketCount(id); + } + return sentPacketCount; + } + @Override public void onSocketCreated(@NonNull SocketKey socketKey, @NonNull MdnsInterfaceSocket socket, @@ -444,6 +463,8 @@ public class MdnsAdvertiser { private NsdServiceInfo mServiceInfo; @Nullable private final String mSubtype; + int mConflictDuringProbingCount; + int mConflictAfterProbingCount; private Registration(@NonNull NsdServiceInfo serviceInfo, @Nullable String subtype) { this.mOriginalName = serviceInfo.getServiceName(); @@ -555,6 +576,24 @@ public class MdnsAdvertiser { @NonNull OffloadServiceInfo offloadServiceInfo); } + /** + * Data class of avdverting metrics. + */ + public static class AdvertiserMetrics { + public final int mRepliedRequestsCount; + public final int mSentPacketCount; + public final int mConflictDuringProbingCount; + public final int mConflictAfterProbingCount; + + public AdvertiserMetrics(int repliedRequestsCount, int sentPacketCount, + int conflictDuringProbingCount, int conflictAfterProbingCount) { + mRepliedRequestsCount = repliedRequestsCount; + mSentPacketCount = sentPacketCount; + mConflictDuringProbingCount = conflictDuringProbingCount; + mConflictAfterProbingCount = conflictAfterProbingCount; + } + } + public MdnsAdvertiser(@NonNull Looper looper, @NonNull MdnsSocketProvider socketProvider, @NonNull AdvertiserCallback cb, @NonNull SharedLog sharedLog) { this(looper, socketProvider, cb, new Dependencies(), sharedLog); @@ -637,6 +676,34 @@ public class MdnsAdvertiser { } } + /** + * Get advertising metrics. + * + * @param id ID used when registering. + * @return The advertising metrics includes replied requests count, send packet count, conflict + * count during/after probing. + */ + public AdvertiserMetrics getAdvertiserMetrics(int id) { + checkThread(); + final Registration registration = mRegistrations.get(id); + if (registration == null) { + return new AdvertiserMetrics( + NO_PACKET /* repliedRequestsCount */, + NO_PACKET /* sentPacketCount */, + 0 /* conflictDuringProbingCount */, + 0 /* conflictAfterProbingCount */); + } + int repliedRequestsCount = NO_PACKET; + int sentPacketCount = NO_PACKET; + for (int i = 0; i < mAdvertiserRequests.size(); i++) { + repliedRequestsCount += + mAdvertiserRequests.valueAt(i).getServiceRepliedRequestsCount(id); + sentPacketCount += mAdvertiserRequests.valueAt(i).getSentPacketCount(id); + } + return new AdvertiserMetrics(repliedRequestsCount, sentPacketCount, + registration.mConflictDuringProbingCount, registration.mConflictAfterProbingCount); + } + private static boolean any(@NonNull ArrayMap map, @NonNull BiPredicate predicate) { for (int i = 0; i < map.size(); i++) { diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsConstants.java b/service-t/src/com/android/server/connectivity/mdns/MdnsConstants.java index 0c32cf1f94..1251170d4a 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsConstants.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsConstants.java @@ -37,6 +37,7 @@ public final class MdnsConstants { public static final int FLAG_TRUNCATED = 0x0200; public static final int QCLASS_INTERNET = 0x0001; public static final int QCLASS_UNICAST = 0x8000; + public static final int NO_PACKET = 0; public static final String SUBTYPE_LABEL = "_sub"; public static final String SUBTYPE_PREFIX = "_"; private static final String MDNS_IPV4_HOST_ADDRESS = "224.0.0.251"; diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiser.java b/service-t/src/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiser.java index 61a2c5e782..6454959d9d 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiser.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiser.java @@ -16,6 +16,8 @@ package com.android.server.connectivity.mdns; +import static com.android.server.connectivity.mdns.MdnsConstants.NO_PACKET; + import android.annotation.NonNull; import android.annotation.Nullable; import android.net.LinkAddress; @@ -93,8 +95,11 @@ public class MdnsInterfaceAdvertiser implements MulticastPacketReader.PacketHand /** * Callbacks from {@link MdnsProber}. */ - private class ProbingCallback implements - PacketRepeaterCallback { + private class ProbingCallback implements PacketRepeaterCallback { + @Override + public void onSent(int index, @NonNull MdnsProber.ProbingInfo info, int sentPacketCount) { + mRecordRepository.onProbingSent(info.getServiceId(), sentPacketCount); + } @Override public void onFinished(MdnsProber.ProbingInfo info) { final MdnsAnnouncer.AnnouncementInfo announcementInfo; @@ -118,8 +123,8 @@ public class MdnsInterfaceAdvertiser implements MulticastPacketReader.PacketHand */ private class AnnouncingCallback implements PacketRepeaterCallback { @Override - public void onSent(int index, @NonNull BaseAnnouncementInfo info) { - mRecordRepository.onAdvertisementSent(info.getServiceId()); + public void onSent(int index, @NonNull BaseAnnouncementInfo info, int sentPacketCount) { + mRecordRepository.onAdvertisementSent(info.getServiceId(), sentPacketCount); } @Override @@ -259,6 +264,22 @@ public class MdnsInterfaceAdvertiser implements MulticastPacketReader.PacketHand } } + /** + * Get the replied request count from given service id. + */ + public int getServiceRepliedRequestsCount(int id) { + if (!mRecordRepository.hasActiveService(id)) return NO_PACKET; + return mRecordRepository.getServiceRepliedRequestsCount(id); + } + + /** + * Get the total sent packet count from given service id. + */ + public int getSentPacketCount(int id) { + if (!mRecordRepository.hasActiveService(id)) return NO_PACKET; + return mRecordRepository.getSentPacketCount(id); + } + /** * Update interface addresses used to advertise. * diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsPacketRepeater.java b/service-t/src/com/android/server/connectivity/mdns/MdnsPacketRepeater.java index 644560cc1a..12ed1396fd 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsPacketRepeater.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsPacketRepeater.java @@ -59,7 +59,7 @@ public abstract class MdnsPacketRepeater { /** * Called when a packet was sent. */ - default void onSent(int index, @NonNull T info) {} + default void onSent(int index, @NonNull T info, int sentPacketCount) {} /** * Called when the {@link MdnsPacketRepeater} is done sending packets. @@ -114,9 +114,10 @@ public abstract class MdnsPacketRepeater { } // Send to both v4 and v6 addresses; the reply sender will take care of ignoring the // send when the socket has not joined the relevant group. + int sentPacketCount = 0; for (InetSocketAddress destination : ALL_ADDRS) { try { - mReplySender.sendNow(packet, destination); + sentPacketCount += mReplySender.sendNow(packet, destination); } catch (IOException e) { mSharedLog.e("Error sending packet to " + destination, e); } @@ -135,7 +136,7 @@ public abstract class MdnsPacketRepeater { // Call onSent after scheduling the next run, to allow the callback to cancel it if (mCb != null) { - mCb.onSent(index, request); + mCb.onSent(index, request, sentPacketCount); } } } diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java b/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java index e25d7e104f..1fb4d90e2c 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java @@ -16,6 +16,8 @@ package com.android.server.connectivity.mdns; +import static com.android.server.connectivity.mdns.MdnsConstants.NO_PACKET; + import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.TargetApi; @@ -174,6 +176,16 @@ public class MdnsRecordRepository { */ public boolean exiting = false; + /** + * The replied query packet count of this service. + */ + public int repliedServiceCount = NO_PACKET; + + /** + * The sent packet count of this service (including announcements and probes). + */ + public int sentPacketCount = NO_PACKET; + /** * Create a ServiceRegistration for dns-sd service registration (RFC6763). * @@ -181,7 +193,7 @@ public class MdnsRecordRepository { * @param serviceInfo Service to advertise */ ServiceRegistration(@NonNull String[] deviceHostname, @NonNull NsdServiceInfo serviceInfo, - @Nullable String subtype) { + @Nullable String subtype, int repliedServiceCount, int sentPacketCount) { this.serviceInfo = serviceInfo; this.subtype = subtype; @@ -254,6 +266,8 @@ public class MdnsRecordRepository { true /* sharedName */, true /* probing */)); this.allRecords = Collections.unmodifiableList(allRecords); + this.repliedServiceCount = repliedServiceCount; + this.sentPacketCount = sentPacketCount; } void setProbing(boolean probing) { @@ -316,7 +330,8 @@ public class MdnsRecordRepository { } final ServiceRegistration registration = new ServiceRegistration( - mDeviceHostname, serviceInfo, subtype); + mDeviceHostname, serviceInfo, subtype, NO_PACKET /* repliedServiceCount */, + NO_PACKET /* sentPacketCount */); mServices.put(serviceId, registration); // Remove existing exiting service @@ -405,6 +420,24 @@ public class MdnsRecordRepository { return mServices.size(); } + /** + * @return The replied request count of the service. + */ + public int getServiceRepliedRequestsCount(int id) { + final ServiceRegistration service = mServices.get(id); + if (service == null) return NO_PACKET; + return service.repliedServiceCount; + } + + /** + * @return The total sent packet count of the service. + */ + public int getSentPacketCount(int id) { + final ServiceRegistration service = mServices.get(id); + if (service == null) return NO_PACKET; + return service.sentPacketCount; + } + /** * Remove all services from the repository * @return IDs of the removed services @@ -472,9 +505,12 @@ public class MdnsRecordRepository { for (int i = 0; i < mServices.size(); i++) { final ServiceRegistration registration = mServices.valueAt(i); if (registration.exiting) continue; - addReplyFromService(question, registration.allRecords, registration.ptrRecords, + if (addReplyFromService(question, registration.allRecords, registration.ptrRecords, registration.srvRecord, registration.txtRecord, replyUnicast, now, - answerInfo, additionalAnswerRecords); + answerInfo, additionalAnswerRecords)) { + registration.repliedServiceCount++; + registration.sentPacketCount++; + } } } @@ -527,7 +563,7 @@ public class MdnsRecordRepository { /** * Add answers and additional answers for a question, from a ServiceRegistration. */ - private void addReplyFromService(@NonNull MdnsRecord question, + private boolean addReplyFromService(@NonNull MdnsRecord question, @NonNull List> serviceRecords, @Nullable List> servicePtrRecords, @Nullable RecordInfo serviceSrvRecord, @@ -596,7 +632,7 @@ public class MdnsRecordRepository { } // No more records to add if no answer - if (answerInfo.size() == answersStartIndex) return; + if (answerInfo.size() == answersStartIndex) return false; final List> additionalAnswerInfo = new ArrayList<>(); // RFC6763 12.1: if including PTR record, include the SRV and TXT records it names @@ -626,6 +662,7 @@ public class MdnsRecordRepository { addNsecRecordsForUniqueNames(additionalAnswerRecords, answerInfo.listIterator(answersStartIndex), additionalAnswerInfo.listIterator()); + return true; } /** @@ -862,8 +899,8 @@ public class MdnsRecordRepository { final ServiceRegistration existing = mServices.get(serviceId); if (existing == null) return null; - final ServiceRegistration newService = new ServiceRegistration( - mDeviceHostname, newInfo, existing.subtype); + final ServiceRegistration newService = new ServiceRegistration(mDeviceHostname, newInfo, + existing.subtype, existing.repliedServiceCount, existing.sentPacketCount); mServices.put(serviceId, newService); return makeProbingInfo(serviceId, newService.srvRecord.record); } @@ -871,7 +908,7 @@ public class MdnsRecordRepository { /** * Called when {@link MdnsAdvertiser} sent an advertisement for the given service. */ - public void onAdvertisementSent(int serviceId) { + public void onAdvertisementSent(int serviceId, int sentPacketCount) { final ServiceRegistration registration = mServices.get(serviceId); if (registration == null) return; @@ -880,8 +917,19 @@ public class MdnsRecordRepository { record.lastSentTimeMs = now; record.lastAdvertisedTimeMs = now; } + registration.sentPacketCount += sentPacketCount; } + /** + * Called when {@link MdnsAdvertiser} sent a probing for the given service. + */ + public void onProbingSent(int serviceId, int sentPacketCount) { + final ServiceRegistration registration = mServices.get(serviceId); + if (registration == null) return; + registration.sentPacketCount += sentPacketCount; + } + + /** * Compute: * 2001:db8::1 --> 1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.B.D.0.1.0.0.2.ip6.arpa diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsReplySender.java b/service-t/src/com/android/server/connectivity/mdns/MdnsReplySender.java index cf7a4640bb..71057fbbb2 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsReplySender.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsReplySender.java @@ -44,6 +44,9 @@ import java.util.Collections; public class MdnsReplySender { private static final boolean DBG = MdnsAdvertiser.DBG; private static final int MSG_SEND = 1; + private static final int PACKET_NOT_SENT = 0; + private static final int PACKET_SENT = 1; + @NonNull private final MdnsInterfaceSocket mSocket; @NonNull @@ -79,16 +82,17 @@ public class MdnsReplySender { * * Must be called on the looper thread used by the {@link MdnsReplySender}. */ - public void sendNow(@NonNull MdnsPacket packet, @NonNull InetSocketAddress destination) + public int sendNow(@NonNull MdnsPacket packet, @NonNull InetSocketAddress destination) throws IOException { ensureRunningOnHandlerThread(mHandler); if (!((destination.getAddress() instanceof Inet6Address && mSocket.hasJoinedIpv6()) || (destination.getAddress() instanceof Inet4Address && mSocket.hasJoinedIpv4()))) { // Skip sending if the socket has not joined the v4/v6 group (there was no address) - return; + return PACKET_NOT_SENT; } final byte[] outBuffer = MdnsUtils.createRawDnsPacket(mPacketCreationBuffer, packet); mSocket.send(new DatagramPacket(outBuffer, 0, outBuffer.length, destination)); + return PACKET_SENT; } /** Get the packetCreationBuffer */ diff --git a/service/src/com/android/metrics/stats.proto b/service/src/com/android/metrics/stats.proto index 99afb90715..ecc03779a7 100644 --- a/service/src/com/android/metrics/stats.proto +++ b/service/src/com/android/metrics/stats.proto @@ -64,6 +64,18 @@ message NetworkNsdReported { // Record sent query count before stopped discovery optional int32 sent_query_count = 12; + + // Record sent packet count before unregistered service + optional int32 sent_packet_count = 13; + + // Record number of conflict during probing + optional int32 conflict_during_probing_count = 14; + + // Record number of conflict after probing + optional int32 conflict_after_probing_count = 15; + + // The random number between 0 ~ 999 for sampling + optional int32 random_number = 16; } /** diff --git a/tests/unit/java/com/android/metrics/NetworkNsdReportedMetricsTest.kt b/tests/unit/java/com/android/metrics/NetworkNsdReportedMetricsTest.kt index 7f893df2c7..a82e29b194 100644 --- a/tests/unit/java/com/android/metrics/NetworkNsdReportedMetricsTest.kt +++ b/tests/unit/java/com/android/metrics/NetworkNsdReportedMetricsTest.kt @@ -21,12 +21,15 @@ import android.stats.connectivity.MdnsQueryResult import android.stats.connectivity.NsdEventType import com.android.testutils.DevSdkIgnoreRule import com.android.testutils.DevSdkIgnoreRunner +import java.util.Random import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertTrue +import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mockito.ArgumentCaptor +import org.mockito.Mockito.doReturn import org.mockito.Mockito.mock import org.mockito.Mockito.verify @@ -34,6 +37,12 @@ import org.mockito.Mockito.verify @DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) class NetworkNsdReportedMetricsTest { private val deps = mock(NetworkNsdReportedMetrics.Dependencies::class.java) + private val random = mock(Random::class.java) + + @Before + fun setUp() { + doReturn(random).`when`(deps).makeRandomGenerator() + } @Test fun testReportServiceRegistrationSucceeded() { @@ -80,8 +89,13 @@ class NetworkNsdReportedMetricsTest { val clientId = 99 val transactionId = 100 val durationMs = 10L + val repliedRequestsCount = 25 + val sentPacketCount = 50 + val conflictDuringProbingCount = 2 + val conflictAfterProbingCount = 1 val metrics = NetworkNsdReportedMetrics(true /* isLegacy */, clientId, deps) - metrics.reportServiceUnregistration(transactionId, durationMs) + metrics.reportServiceUnregistration(transactionId, durationMs, repliedRequestsCount, + sentPacketCount, conflictDuringProbingCount, conflictAfterProbingCount) val eventCaptor = ArgumentCaptor.forClass(NetworkNsdReported::class.java) verify(deps).statsWrite(eventCaptor.capture()) @@ -92,6 +106,10 @@ class NetworkNsdReportedMetricsTest { assertEquals(NsdEventType.NET_REGISTER, it.type) assertEquals(MdnsQueryResult.MQR_SERVICE_UNREGISTERED, it.queryResult) assertEquals(durationMs, it.eventDurationMillisec) + assertEquals(repliedRequestsCount, it.repliedRequestsCount) + assertEquals(sentPacketCount, it.sentPacketCount) + assertEquals(conflictDuringProbingCount, it.conflictDuringProbingCount) + assertEquals(conflictAfterProbingCount, it.conflictAfterProbingCount) } } diff --git a/tests/unit/java/com/android/server/NsdServiceTest.java b/tests/unit/java/com/android/server/NsdServiceTest.java index 695cfe8aec..44ed02a740 100644 --- a/tests/unit/java/com/android/server/NsdServiceTest.java +++ b/tests/unit/java/com/android/server/NsdServiceTest.java @@ -1222,6 +1222,8 @@ public class NsdServiceTest { verify(mMockMDnsM).stopOperation(legacyIdCaptor.getValue()); verify(mAdvertiser, never()).removeService(anyInt()); + doReturn(mock(MdnsAdvertiser.AdvertiserMetrics.class)) + .when(mAdvertiser).getAdvertiserMetrics(anyInt()); client.unregisterService(regListenerWithFeature); waitForIdle(); verify(mAdvertiser).removeService(serviceIdCaptor.getValue()); @@ -1312,14 +1314,20 @@ public class NsdServiceTest { new NsdServiceInfo(regInfo.getServiceName(), null)))); verify(mMetrics).reportServiceRegistrationSucceeded(regId, 10L /* durationMs */); + final MdnsAdvertiser.AdvertiserMetrics metrics = new MdnsAdvertiser.AdvertiserMetrics( + 50 /* repliedRequestCount */, 100 /* sentPacketCount */, + 3 /* conflictDuringProbingCount */, 2 /* conflictAfterProbingCount */); doReturn(TEST_TIME_MS + 100L).when(mClock).elapsedRealtime(); + doReturn(metrics).when(mAdvertiser).getAdvertiserMetrics(regId); client.unregisterService(regListener); waitForIdle(); verify(mAdvertiser).removeService(idCaptor.getValue()); verify(regListener, timeout(TIMEOUT_MS)).onServiceUnregistered( argThat(info -> matches(info, regInfo))); verify(mSocketProvider, timeout(TIMEOUT_MS)).requestStopWhenInactive(); - verify(mMetrics).reportServiceUnregistration(regId, 100L /* durationMs */); + verify(mMetrics).reportServiceUnregistration(regId, 100L /* durationMs */, + 50 /* repliedRequestCount */, 100 /* sentPacketCount */, + 3 /* conflictDuringProbingCount */, 2 /* conflictAfterProbingCount */); } @Test diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsAdvertiserTest.kt b/tests/unit/java/com/android/server/connectivity/mdns/MdnsAdvertiserTest.kt index 862a9ec5a6..8b10e0bd1f 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsAdvertiserTest.kt +++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsAdvertiserTest.kt @@ -33,7 +33,10 @@ import com.android.testutils.DevSdkIgnoreRunner import com.android.testutils.waitForIdle import java.net.NetworkInterface import java.util.Objects +import java.util.concurrent.CompletableFuture +import java.util.concurrent.TimeUnit import org.junit.After +import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -206,6 +209,18 @@ class MdnsAdvertiserTest { verify(cb).onRegisterServiceSucceeded(eq(SERVICE_ID_1), argThat { it.matches(SERVICE_1) }) verify(cb).onOffloadStartOrUpdate(eq(TEST_INTERFACE1), eq(OFFLOAD_SERVICEINFO_NO_SUBTYPE)) + // Service is conflicted. + postSync { intAdvCbCaptor.value.onServiceConflict(mockInterfaceAdvertiser1, SERVICE_ID_1) } + + // Verify the metrics data + doReturn(25).`when`(mockInterfaceAdvertiser1).getServiceRepliedRequestsCount(SERVICE_ID_1) + doReturn(40).`when`(mockInterfaceAdvertiser1).getSentPacketCount(SERVICE_ID_1) + val metrics = postReturn { advertiser.getAdvertiserMetrics(SERVICE_ID_1) } + assertEquals(25, metrics.mRepliedRequestsCount) + assertEquals(40, metrics.mSentPacketCount) + assertEquals(0, metrics.mConflictDuringProbingCount) + assertEquals(1, metrics.mConflictAfterProbingCount) + doReturn(TEST_OFFLOAD_PACKET2).`when`(mockInterfaceAdvertiser1) .getRawOffloadPayload( SERVICE_ID_1 @@ -265,6 +280,22 @@ class MdnsAdvertiserTest { verify(cb).onRegisterServiceSucceeded(eq(SERVICE_ID_1), argThat { it.matches(ALL_NETWORKS_SERVICE) }) + // Services are conflicted. + postSync { intAdvCbCaptor1.value.onServiceConflict(mockInterfaceAdvertiser1, SERVICE_ID_1) } + postSync { intAdvCbCaptor1.value.onServiceConflict(mockInterfaceAdvertiser1, SERVICE_ID_1) } + postSync { intAdvCbCaptor2.value.onServiceConflict(mockInterfaceAdvertiser2, SERVICE_ID_1) } + + // Verify the metrics data + doReturn(10).`when`(mockInterfaceAdvertiser1).getServiceRepliedRequestsCount(SERVICE_ID_1) + doReturn(5).`when`(mockInterfaceAdvertiser2).getServiceRepliedRequestsCount(SERVICE_ID_1) + doReturn(22).`when`(mockInterfaceAdvertiser1).getSentPacketCount(SERVICE_ID_1) + doReturn(12).`when`(mockInterfaceAdvertiser2).getSentPacketCount(SERVICE_ID_1) + val metrics = postReturn { advertiser.getAdvertiserMetrics(SERVICE_ID_1) } + assertEquals(15, metrics.mRepliedRequestsCount) + assertEquals(34, metrics.mSentPacketCount) + assertEquals(2, metrics.mConflictDuringProbingCount) + assertEquals(1, metrics.mConflictAfterProbingCount) + // Unregister the service postSync { advertiser.removeService(SERVICE_ID_1) } verify(mockInterfaceAdvertiser1).removeService(SERVICE_ID_1) @@ -376,6 +407,14 @@ class MdnsAdvertiserTest { handler.post(r) handler.waitForIdle(TIMEOUT_MS) } + + private fun postReturn(r: (() -> T)): T { + val future = CompletableFuture() + handler.post { + future.complete(r()) + } + return future.get(TIMEOUT_MS, TimeUnit.MILLISECONDS) + } } // NsdServiceInfo does not implement equals; this is useful to use in argument matchers diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsAnnouncerTest.kt b/tests/unit/java/com/android/server/connectivity/mdns/MdnsAnnouncerTest.kt index 12faa504b9..c39ee1e1c7 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsAnnouncerTest.kt +++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsAnnouncerTest.kt @@ -253,7 +253,7 @@ class MdnsAnnouncerTest { val captor = ArgumentCaptor.forClass(DatagramPacket::class.java) repeat(FIRST_ANNOUNCES_COUNT) { i -> - verify(cb, timeout(TEST_TIMEOUT_MS)).onSent(i, request) + verify(cb, timeout(TEST_TIMEOUT_MS)).onSent(i, request, 1 /* sentPacketCount */) verify(socket, atLeast(i + 1)).send(any()) val now = SystemClock.elapsedRealtime() assertTrue(now > timeStart + startDelay + i * FIRST_ANNOUNCES_DELAY) diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsProberTest.kt b/tests/unit/java/com/android/server/connectivity/mdns/MdnsProberTest.kt index 5ca4dd61fd..f28481995e 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsProberTest.kt +++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsProberTest.kt @@ -92,7 +92,7 @@ class MdnsProberTest { private fun assertProbesSent(probeInfo: TestProbeInfo, expectedHex: String) { repeat(probeInfo.numSends) { i -> - verify(cb, timeout(TEST_TIMEOUT_MS)).onSent(i, probeInfo) + verify(cb, timeout(TEST_TIMEOUT_MS)).onSent(i, probeInfo, 1 /* sentPacketCount */) // If the probe interval is short, more than (i+1) probes may have been sent already verify(socket, atLeast(i + 1)).send(any()) } @@ -190,7 +190,7 @@ class MdnsProberTest { prober.startProbing(probeInfo) // Expect the initial probe - verify(cb, timeout(TEST_TIMEOUT_MS)).onSent(0, probeInfo) + verify(cb, timeout(TEST_TIMEOUT_MS)).onSent(0, probeInfo, 1 /* sentPacketCount */) // Stop probing val stopResult = CompletableFuture() @@ -200,7 +200,7 @@ class MdnsProberTest { // Wait for a bit (more than the probe delay) to ensure no more probes were sent Thread.sleep(SHORT_TIMEOUT_MS * 2) - verify(cb, never()).onSent(1, probeInfo) + verify(cb, never()).onSent(1, probeInfo, 1 /* sentPacketCount */) verify(cb, never()).onFinished(probeInfo) // Only one sent packet diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt b/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt index 56fbdf04ff..af47b1c761 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt +++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt @@ -155,7 +155,7 @@ class MdnsRecordRepositoryTest { val probingInfo = repository.setServiceProbing(TEST_SERVICE_ID_1) repository.onProbingSucceeded(probingInfo) - repository.onAdvertisementSent(TEST_SERVICE_ID_1) + repository.onAdvertisementSent(TEST_SERVICE_ID_1, 2 /* sentPacketCount */) assertTrue(repository.hasActiveService(TEST_SERVICE_ID_1)) repository.exitService(TEST_SERVICE_ID_1) @@ -166,7 +166,7 @@ class MdnsRecordRepositoryTest { fun testExitAnnouncements() { val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME) repository.initWithService(TEST_SERVICE_ID_1, TEST_SERVICE_1) - repository.onAdvertisementSent(TEST_SERVICE_ID_1) + repository.onAdvertisementSent(TEST_SERVICE_ID_1, 2 /* sentPacketCount */) val exitAnnouncement = repository.exitService(TEST_SERVICE_ID_1) assertNotNull(exitAnnouncement) @@ -195,7 +195,7 @@ class MdnsRecordRepositoryTest { fun testExitAnnouncements_WithSubtype() { val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME) repository.initWithService(TEST_SERVICE_ID_1, TEST_SERVICE_1, TEST_SUBTYPE) - repository.onAdvertisementSent(TEST_SERVICE_ID_1) + repository.onAdvertisementSent(TEST_SERVICE_ID_1, 2 /* sentPacketCount */) val exitAnnouncement = repository.exitService(TEST_SERVICE_ID_1) assertNotNull(exitAnnouncement) @@ -230,7 +230,7 @@ class MdnsRecordRepositoryTest { fun testExitingServiceReAdded() { val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME) repository.initWithService(TEST_SERVICE_ID_1, TEST_SERVICE_1) - repository.onAdvertisementSent(TEST_SERVICE_ID_1) + repository.onAdvertisementSent(TEST_SERVICE_ID_1, 2 /* sentPacketCount */) repository.exitService(TEST_SERVICE_ID_1) assertEquals(TEST_SERVICE_ID_1, @@ -246,7 +246,7 @@ class MdnsRecordRepositoryTest { val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME) val announcementInfo = repository.initWithService(TEST_SERVICE_ID_1, TEST_SERVICE_1, TEST_SUBTYPE) - repository.onAdvertisementSent(TEST_SERVICE_ID_1) + repository.onAdvertisementSent(TEST_SERVICE_ID_1, 2 /* sentPacketCount */) val packet = announcementInfo.getPacket(0) assertEquals(0x8400 /* response, authoritative */, packet.flags) @@ -657,6 +657,34 @@ class MdnsRecordRepositoryTest { // Above records are identical to the actual registrations: no conflict assertEquals(emptySet(), repository.getConflictingServices(packet)) } + + @Test + fun testGetServiceRepliedRequestsCount() { + val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME) + repository.initWithService(TEST_SERVICE_ID_1, TEST_SERVICE_1) + // Verify that there is no packet replied. + assertEquals(MdnsConstants.NO_PACKET, + repository.getServiceRepliedRequestsCount(TEST_SERVICE_ID_1)) + + val questions = listOf(MdnsPointerRecord(arrayOf("_testservice", "_tcp", "local"), + 0L /* receiptTimeMillis */, + false /* cacheFlush */, + // TTL and data is empty for a question + 0L /* ttlMillis */, + null /* pointer */)) + val query = MdnsPacket(0 /* flags */, questions, listOf() /* answers */, + listOf() /* authorityRecords */, listOf() /* additionalRecords */) + val src = InetSocketAddress(parseNumericAddress("192.0.2.123"), 5353) + + // Reply to the question and verify there is one packet replied. + val reply = repository.getReply(query, src) + assertNotNull(reply) + assertEquals(1, repository.getServiceRepliedRequestsCount(TEST_SERVICE_ID_1)) + + // No package replied for unknown service. + assertEquals(MdnsConstants.NO_PACKET, + repository.getServiceRepliedRequestsCount(TEST_SERVICE_ID_2)) + } } private fun MdnsRecordRepository.initWithService(