From 289eae6dbdbc20fa9212108c1c53a9d63676e8ad Mon Sep 17 00:00:00 2001
From: Jeffrey Huang <jeffreyhuang@google.com>
Date: Wed, 10 Jun 2020 16:28:23 -0700
Subject: [PATCH] Avoid timestamp update when data is kept on dump

Bug: 158703584
Test: atest statsd_test
Change-Id: Ia6814c2cdb67dde2fd790ddc18fc785b1bba062c
---
 cmds/statsd/src/metrics/MetricsManager.cpp   |  8 +++-
 cmds/statsd/tests/StatsLogProcessor_test.cpp | 42 ++++++++++++++++++++
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/cmds/statsd/src/metrics/MetricsManager.cpp b/cmds/statsd/src/metrics/MetricsManager.cpp
index e8c575a1adea7..7e825efddb759 100644
--- a/cmds/statsd/src/metrics/MetricsManager.cpp
+++ b/cmds/statsd/src/metrics/MetricsManager.cpp
@@ -361,8 +361,12 @@ void MetricsManager::onDumpReport(const int64_t dumpTimeStampNs,
         protoOutput->end(token);
     }
 
-    mLastReportTimeNs = dumpTimeStampNs;
-    mLastReportWallClockNs = getWallClockNs();
+    // Do not update the timestamps when data is not cleared to avoid timestamps from being
+    // misaligned.
+    if (erase_data) {
+        mLastReportTimeNs = dumpTimeStampNs;
+        mLastReportWallClockNs = getWallClockNs();
+    }
     VLOG("=========================Metric Reports End==========================");
 }
 
diff --git a/cmds/statsd/tests/StatsLogProcessor_test.cpp b/cmds/statsd/tests/StatsLogProcessor_test.cpp
index 076f32752223e..7529fbb8d9656 100644
--- a/cmds/statsd/tests/StatsLogProcessor_test.cpp
+++ b/cmds/statsd/tests/StatsLogProcessor_test.cpp
@@ -1796,6 +1796,48 @@ TEST(StatsLogProcessorTest_mapIsolatedUidToHostUid, LogIsolatedUidAttributionCha
     EXPECT_EQ(field2, actualFieldValues->at(5).mValue.int_value);
 }
 
+TEST(StatsLogProcessorTest, TestDumpReportWithoutErasingDataDoesNotUpdateTimestamp) {
+    int hostUid = 20;
+    int isolatedUid = 30;
+    sp<MockUidMap> mockUidMap = makeMockUidMapForOneHost(hostUid, {isolatedUid});
+    ConfigKey key(3, 4);
+    StatsdConfig config = MakeConfig(false);
+    sp<StatsLogProcessor> processor =
+            CreateStatsLogProcessor(1, 1, config, key, nullptr, 0, mockUidMap);
+    vector<uint8_t> bytes;
+
+    int64_t dumpTime1Ns = 1 * NS_PER_SEC;
+    processor->onDumpReport(key, dumpTime1Ns, false /* include_current_bucket */,
+            true /* erase_data */, ADB_DUMP, FAST, &bytes);
+
+    ConfigMetricsReportList output;
+    output.ParseFromArray(bytes.data(), bytes.size());
+    EXPECT_EQ(output.reports_size(), 1);
+    EXPECT_EQ(output.reports(0).current_report_elapsed_nanos(), dumpTime1Ns);
+
+    int64_t dumpTime2Ns = 5 * NS_PER_SEC;
+    processor->onDumpReport(key, dumpTime2Ns, false /* include_current_bucket */,
+            false /* erase_data */, ADB_DUMP, FAST, &bytes);
+
+    // Check that the dump report without clearing data is successful.
+    output.ParseFromArray(bytes.data(), bytes.size());
+    EXPECT_EQ(output.reports_size(), 1);
+    EXPECT_EQ(output.reports(0).current_report_elapsed_nanos(), dumpTime2Ns);
+    EXPECT_EQ(output.reports(0).last_report_elapsed_nanos(), dumpTime1Ns);
+
+    int64_t dumpTime3Ns = 10 * NS_PER_SEC;
+    processor->onDumpReport(key, dumpTime3Ns, false /* include_current_bucket */,
+            true /* erase_data */, ADB_DUMP, FAST, &bytes);
+
+    // Check that the previous dump report that didn't clear data did not overwrite the first dump's
+    // timestamps.
+    output.ParseFromArray(bytes.data(), bytes.size());
+    EXPECT_EQ(output.reports_size(), 1);
+    EXPECT_EQ(output.reports(0).current_report_elapsed_nanos(), dumpTime3Ns);
+    EXPECT_EQ(output.reports(0).last_report_elapsed_nanos(), dumpTime1Ns);
+
+}
+
 #else
 GTEST_LOG_(INFO) << "This test does nothing.\n";
 #endif