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
diff --git a/Tethering/src/com/android/networkstack/tethering/metrics/TetheringMetrics.java b/Tethering/src/com/android/networkstack/tethering/metrics/TetheringMetrics.java
index fc50faf..32c1410 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 @@
private final SparseArray<NetworkTetheringReported.Builder> mBuilderMap = new SparseArray<>();
private final SparseArray<Long> mDownstreamStartTime = new SparseArray<Long>();
private final ArrayList<RecordUpstreamEvent> mUpstreamEventList = new ArrayList<>();
- private final ArrayMap<UpstreamType, DataUsage> 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<UpstreamType, DataUsage> mLastReportedUpstreamUsage = new ArrayMap<>();
private final Context mContext;
private final Dependencies mDependencies;
private final NetworkStatsManager mNetworkStatsManager;
@@ -282,22 +286,33 @@
* 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 @@
}
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 @@
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 34689bc..f736dbf 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 @@
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 @@
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 @@
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);