From 7065213fcab18c0b8ded8a1ff57ae96f51fcb6d7 Mon Sep 17 00:00:00 2001 From: Paul Hu Date: Tue, 1 Oct 2024 02:48:41 +0000 Subject: [PATCH] Update the usage data when upstream is changed Need to refresh the usage data. If the network switches back to Wi-Fi after using cellular data (Wi-Fi -> Cellular -> Wi-Fi), the old baseline might be inaccurate, leading to incorrect delta calculations. Also address some leftover comments. Fix: 369306449 Fix: 370652336 Test: atest TetheringTests Change-Id: I1c14aac533cee1d989b7ab354111f071da018058 --- .../tethering/metrics/TetheringMetrics.java | 55 +++++++++++++------ .../metrics/TetheringMetricsTest.java | 14 ++++- 2 files changed, 49 insertions(+), 20 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/metrics/TetheringMetrics.java b/Tethering/src/com/android/networkstack/tethering/metrics/TetheringMetrics.java index fc50faf88e..32c1410e09 100644 --- a/Tethering/src/com/android/networkstack/tethering/metrics/TetheringMetrics.java +++ b/Tethering/src/com/android/networkstack/tethering/metrics/TetheringMetrics.java @@ -111,7 +111,11 @@ public class TetheringMetrics { private final SparseArray mBuilderMap = new SparseArray<>(); private final SparseArray mDownstreamStartTime = new SparseArray(); private final ArrayList mUpstreamEventList = new ArrayList<>(); - private final ArrayMap mUpstreamUsageBaseline = new ArrayMap<>(); + // Store the last reported data usage for each upstream type to be used for calculating the + // usage delta. The keys are the upstream types, and the values are the tethering UID data + // usage for the corresponding types. Retrieve the baseline data usage when tethering is + // enabled, update it when the upstream changes, and clear it when tethering is disabled. + private final ArrayMap mLastReportedUpstreamUsage = new ArrayMap<>(); private final Context mContext; private final Dependencies mDependencies; private final NetworkStatsManager mNetworkStatsManager; @@ -282,22 +286,33 @@ public class TetheringMetrics { * Calculates the data usage difference between the current and previous usage for the * specified upstream type. * + * Note: This must be called before updating mCurrentUpstream when changing the upstream. + * * @return A DataUsage object containing the calculated difference in transmitted (tx) and * received (rx) bytes. */ private DataUsage calculateDataUsageDelta(@Nullable UpstreamType upstream) { - if (upstream != null && mDependencies.isUpstreamDataUsageMetricsEnabled(mContext) - && isUsageSupportedForUpstreamType(upstream)) { - final DataUsage oldUsage = mUpstreamUsageBaseline.getOrDefault(upstream, EMPTY); - if (oldUsage.equals(EMPTY)) { - Log.d(TAG, "No usage baseline for the upstream=" + upstream); - return EMPTY; - } - // TODO(b/352537247): Fix data usage which might be incorrect if the device uses - // tethering with the same upstream for over 15 days. - return DataUsage.subtract(getCurrentDataUsageForUpstreamType(upstream), oldUsage); + if (!mDependencies.isUpstreamDataUsageMetricsEnabled(mContext)) { + return EMPTY; } - return EMPTY; + + if (upstream == null || !isUsageSupportedForUpstreamType(upstream)) { + return EMPTY; + } + + final DataUsage oldUsage = mLastReportedUpstreamUsage.getOrDefault(upstream, EMPTY); + if (oldUsage.equals(EMPTY)) { + Log.d(TAG, "No usage baseline for the upstream=" + upstream); + return EMPTY; + } + // TODO(b/370724247): Fix data usage which might be incorrect if the device uses + // tethering with the same upstream for over 15 days. + // Need to refresh the baseline usage data. If the network switches back to Wi-Fi after + // using cellular data (Wi-Fi -> Cellular -> Wi-Fi), the old baseline might be + // inaccurate, leading to incorrect delta calculations. + final DataUsage newUsage = getCurrentDataUsageForUpstreamType(upstream); + mLastReportedUpstreamUsage.put(upstream, newUsage); + return DataUsage.subtract(newUsage, oldUsage); } /** @@ -444,25 +459,29 @@ public class TetheringMetrics { } private void handleInitUpstreamUsageBaseline() { - if (!(mDependencies.isUpstreamDataUsageMetricsEnabled(mContext) - && mUpstreamUsageBaseline.isEmpty())) { + if (!mDependencies.isUpstreamDataUsageMetricsEnabled(mContext)) { + return; + } + + if (!mLastReportedUpstreamUsage.isEmpty()) { + Log.wtf(TAG, "The upstream usage baseline has been initialed."); return; } for (UpstreamType type : UpstreamType.values()) { if (!isUsageSupportedForUpstreamType(type)) continue; - mUpstreamUsageBaseline.put(type, getCurrentDataUsageForUpstreamType(type)); + mLastReportedUpstreamUsage.put(type, getCurrentDataUsageForUpstreamType(type)); } } @VisibleForTesting @NonNull - DataUsage getDataUsageFromUpstreamType(@NonNull UpstreamType type) { + DataUsage getLastReportedUsageFromUpstreamType(@NonNull UpstreamType type) { if (mHandler.getLooper().getThread() != Thread.currentThread()) { throw new IllegalStateException( "Not running on Handler thread: " + Thread.currentThread().getName()); } - return mUpstreamUsageBaseline.getOrDefault(type, EMPTY); + return mLastReportedUpstreamUsage.getOrDefault(type, EMPTY); } @@ -497,7 +516,7 @@ public class TetheringMetrics { mUpstreamEventList.clear(); mCurrentUpstream = null; mCurrentUpStreamStartTime = 0L; - mUpstreamUsageBaseline.clear(); + mLastReportedUpstreamUsage.clear(); } private DownstreamType downstreamTypeToEnum(final int ifaceType) { diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/metrics/TetheringMetricsTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/metrics/TetheringMetricsTest.java index 34689bcd54..f736dbf66e 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/metrics/TetheringMetricsTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/metrics/TetheringMetricsTest.java @@ -498,7 +498,7 @@ public final class TetheringMetricsTest { private void verifyEmptyUsageForAllUpstreamTypes() { mHandler.post(() -> { for (UpstreamType type : UpstreamType.values()) { - assertEquals(EMPTY, mTetheringMetrics.getDataUsageFromUpstreamType(type)); + assertEquals(EMPTY, mTetheringMetrics.getLastReportedUsageFromUpstreamType(type)); } }); HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT); @@ -555,7 +555,8 @@ public final class TetheringMetricsTest { mHandler.post(() -> { for (UpstreamType type : UpstreamType.values()) { - final DataUsage dataUsage = mTetheringMetrics.getDataUsageFromUpstreamType(type); + final DataUsage dataUsage = + mTetheringMetrics.getLastReportedUsageFromUpstreamType(type); if (TetheringMetrics.isUsageSupportedForUpstreamType(type)) { assertEquals(mMockUpstreamUsageBaseline.get(type), dataUsage); } else { @@ -610,12 +611,21 @@ public final class TetheringMetricsTest { incrementCurrentTime(cellDuration); updateUpstreamDataUsage(UT_CELLULAR, cellUsageDiff); + // Change the upstream back to Wi-FI and update the data usage + runAndWaitForIdle(() -> + mTetheringMetrics.maybeUpdateUpstreamType(buildUpstreamState(TRANSPORT_WIFI))); + final long wifiDuration2 = 50 * SECOND_IN_MILLIS; + final long wifiUsageDiff2 = 1000L; + incrementCurrentTime(wifiDuration2); + updateUpstreamDataUsage(UT_WIFI, wifiUsageDiff2); + // Stop tethering and verify that the data usage is uploaded. updateErrorAndSendReport(TETHERING_WIFI, TETHER_ERROR_NO_ERROR); UpstreamEvents.Builder upstreamEvents = UpstreamEvents.newBuilder(); addUpstreamEvent(upstreamEvents, UT_WIFI, wifiDuration, wifiUsageDiff, wifiUsageDiff); addUpstreamEvent(upstreamEvents, UT_BLUETOOTH, bluetoothDuration, btUsageDiff, btUsageDiff); addUpstreamEvent(upstreamEvents, UT_CELLULAR, cellDuration, cellUsageDiff, cellUsageDiff); + addUpstreamEvent(upstreamEvents, UT_WIFI, wifiDuration2, wifiUsageDiff2, wifiUsageDiff2); verifyReport(DownstreamType.DS_TETHERING_WIFI, ErrorCode.EC_NO_ERROR, UserType.USER_SETTINGS, upstreamEvents, currentTimeMillis() - wifiTetheringStartTime);