Merge "Fix incidentd skip collecting timedout of a section." into pi-dev

am: 0f0c9243be

Change-Id: Ib27dd0d695ecf43dfb781e0c8a3374b97b2ca15e

cmds/incidentd/src/Section.cpp

@@ -70,6 +70,13 @@ static status_t write_section_header(int fd, int sectionId, size_t size) {
     return WriteFully(fd, buf, p - buf) ? NO_ERROR : -errno;
 }
 
+static void write_section_stats(IncidentMetadata::SectionStats* stats, const FdBuffer& buffer) {
+    stats->set_dump_size_bytes(buffer.data().size());
+    stats->set_dump_duration_ms(buffer.durationMs());
+    stats->set_timed_out(buffer.timedOut());
+    stats->set_is_truncated(buffer.truncated());
+}
+
 // Reads data from FdBuffer and writes it to the requests file descriptor.
 static status_t write_report_requests(const int id, const FdBuffer& buffer,
                                       ReportRequestSet* requests) {
@@ -77,12 +84,6 @@ static status_t write_report_requests(const int id, const FdBuffer& buffer,
     EncodedBuffer::iterator data = buffer.data();
     PrivacyBuffer privacyBuffer(get_privacy_of_section(id), data);
     int writeable = 0;
-    IncidentMetadata::SectionStats* stats = requests->sectionStats(id);
-
-    stats->set_dump_size_bytes(data.size());
-    stats->set_dump_duration_ms(buffer.durationMs());
-    stats->set_timed_out(buffer.timedOut());
-    stats->set_is_truncated(buffer.truncated());
 
     // The streaming ones, group requests by spec in order to save unnecessary strip operations
     map<PrivacySpec, vector<sp<ReportRequest>>> requestsBySpec;
@@ -140,7 +141,8 @@ static status_t write_report_requests(const int id, const FdBuffer& buffer,
         writeable++;
         VLOG("Section %d flushed %zu bytes to dropbox %d with spec %d", id, privacyBuffer.size(),
              requests->mainFd(), spec.dest);
-        stats->set_report_size_bytes(privacyBuffer.size());
+        // Reports bytes of the section uploaded via dropbox after filtering.
+        requests->sectionStats(id)->set_report_size_bytes(privacyBuffer.size());
     }
 
 DONE:
@@ -270,7 +272,7 @@ status_t FileSection::Execute(ReportRequestSet* requests) const {
     status_t readStatus = buffer.readProcessedDataInStream(fd.get(), std::move(p2cPipe.writeFd()),
                                                            std::move(c2pPipe.readFd()),
                                                            this->timeoutMs, mIsSysfs);
-
+    write_section_stats(requests->sectionStats(this->id), buffer);
     if (readStatus != NO_ERROR || buffer.timedOut()) {
         ALOGW("FileSection '%s' failed to read data from incident helper: %s, timedout: %s",
               this->name.string(), strerror(-readStatus), buffer.timedOut() ? "true" : "false");
@@ -363,7 +365,7 @@ status_t GZipSection::Execute(ReportRequestSet* requests) const {
     status_t readStatus = buffer.readProcessedDataInStream(
             fd.get(), std::move(p2cPipe.writeFd()), std::move(c2pPipe.readFd()), this->timeoutMs,
             isSysfs(mFilenames[index]));
-
+    write_section_stats(requests->sectionStats(this->id), buffer);
     if (readStatus != NO_ERROR || buffer.timedOut()) {
         ALOGW("GZipSection '%s' failed to read data from gzip: %s, timedout: %s",
               this->name.string(), strerror(-readStatus), buffer.timedOut() ? "true" : "false");
@@ -499,7 +501,7 @@ status_t WorkerThreadSection::Execute(ReportRequestSet* requests) const {
             }
         }
     }
-
+    write_section_stats(requests->sectionStats(this->id), buffer);
     if (timedOut || buffer.timedOut()) {
         ALOGW("WorkerThreadSection '%s' timed out", this->name.string());
         return NO_ERROR;
@@ -580,6 +582,7 @@ status_t CommandSection::Execute(ReportRequestSet* requests) const {
 
     cmdPipe.writeFd().reset();
     status_t readStatus = buffer.read(ihPipe.readFd().get(), this->timeoutMs);
+    write_section_stats(requests->sectionStats(this->id), buffer);
     if (readStatus != NO_ERROR || buffer.timedOut()) {
         ALOGW("CommandSection '%s' failed to read data from incident helper: %s, timedout: %s",
               this->name.string(), strerror(-readStatus), buffer.timedOut() ? "true" : "false");
@@ -927,4 +930,4 @@ status_t TombstoneSection::BlockingCall(int pipeWriteFd) const {
 
 }  // namespace incidentd
 }  // namespace os
-}  // namespace android
+}  // namespace android

cmds/incidentd/tests/Section_test.cpp

@@ -146,6 +146,7 @@ TEST_F(SectionTest, FileSection) {
 TEST_F(SectionTest, FileSectionTimeout) {
     FileSection fs(TIMEOUT_PARSER, tf.path, QUICK_TIMEOUT_MS);
     ASSERT_EQ(NO_ERROR, fs.Execute(&requests));
+    ASSERT_TRUE(requests.sectionStats(TIMEOUT_PARSER)->timed_out());
 }
 
 TEST_F(SectionTest, GZipSection) {
@@ -204,12 +205,14 @@ TEST_F(SectionTest, CommandSectionEcho) {
 TEST_F(SectionTest, CommandSectionCommandTimeout) {
     CommandSection cs(NOOP_PARSER, QUICK_TIMEOUT_MS, "/system/bin/yes", NULL);
     ASSERT_EQ(NO_ERROR, cs.Execute(&requests));
+    ASSERT_TRUE(requests.sectionStats(NOOP_PARSER)->timed_out());
 }
 
 TEST_F(SectionTest, CommandSectionIncidentHelperTimeout) {
     CommandSection cs(TIMEOUT_PARSER, QUICK_TIMEOUT_MS, "/system/bin/echo", "about", NULL);
     requests.setMainFd(STDOUT_FILENO);
     ASSERT_EQ(NO_ERROR, cs.Execute(&requests));
+    ASSERT_TRUE(requests.sectionStats(TIMEOUT_PARSER)->timed_out());
 }
 
 TEST_F(SectionTest, CommandSectionBadCommand) {
@@ -221,6 +224,7 @@ TEST_F(SectionTest, CommandSectionBadCommandAndTimeout) {
     CommandSection cs(TIMEOUT_PARSER, QUICK_TIMEOUT_MS, "nonexistcommand", "-opt", NULL);
     // timeout will return first
     ASSERT_EQ(NO_ERROR, cs.Execute(&requests));
+    ASSERT_TRUE(requests.sectionStats(TIMEOUT_PARSER)->timed_out());
 }
 
 TEST_F(SectionTest, LogSectionBinary) {