Merge "dumpstate: attach more than 1 trace to BR" into main

Author: primiano

cmds/dumpstate/Android.bp

@@ -117,6 +117,7 @@ cc_defaults {
         "libdumpsys",
         "libserviceutils",
         "android.tracing.flags_c_lib",
+        "perfetto_flags_c_lib",
     ],
 }
 

cmds/dumpstate/dumpstate.cpp

@@ -57,6 +57,7 @@
 #include <log/log_read.h>
 #include <math.h>
 #include <openssl/sha.h>
+#include <perfetto_flags.h>
 #include <poll.h>
 #include <private/android_filesystem_config.h>
 #include <private/android_logger.h>
@@ -190,7 +191,7 @@ void add_mountinfo();
 #define SNAPSHOTCTL_LOG_DIR "/data/misc/snapshotctl_log"
 #define LINKERCONFIG_DIR "/linkerconfig"
 #define PACKAGE_DEX_USE_LIST "/data/system/package-dex-usage.list"
-#define SYSTEM_TRACE_SNAPSHOT "/data/misc/perfetto-traces/bugreport/systrace.pftrace"
+#define SYSTEM_TRACE_DIR "/data/misc/perfetto-traces/bugreport"
 #define CGROUPFS_DIR "/sys/fs/cgroup"
 #define SDK_EXT_INFO "/apex/com.android.sdkext/bin/derive_sdk"
 #define DROPBOX_DIR "/data/system/dropbox"
@@ -359,6 +360,31 @@ static bool CopyFileToFile(const std::string& input_file, const std::string& out
     return CopyFileToFd(input_file, out_fd.get());
 }
 
+template <typename Func>
+size_t ForEachTrace(Func func) {
+    std::unique_ptr<DIR, decltype(&closedir)> traces_dir(opendir(SYSTEM_TRACE_DIR), closedir);
+
+    if (traces_dir == nullptr) {
+        MYLOGW("Unable to open directory %s: %s\n", SYSTEM_TRACE_DIR, strerror(errno));
+        return 0;
+    }
+
+    size_t traces_found = 0;
+    struct dirent* entry = nullptr;
+    while ((entry = readdir(traces_dir.get()))) {
+        if (entry->d_type != DT_REG) {
+            continue;
+        }
+        std::string trace_path = std::string(SYSTEM_TRACE_DIR) + "/" + entry->d_name;
+        if (access(trace_path.c_str(), F_OK) != 0) {
+            continue;
+        }
+        ++traces_found;
+        func(trace_path);
+    }
+    return traces_found;
+}
+
 }  // namespace
 }  // namespace os
 }  // namespace android
@@ -1101,20 +1127,16 @@ static void MaybeAddSystemTraceToZip() {
     // This function copies into the .zip the system trace that was snapshotted
     // by the early call to MaybeSnapshotSystemTraceAsync(), if any background
     // tracing was happening.
-    bool system_trace_exists = access(SYSTEM_TRACE_SNAPSHOT, F_OK) == 0;
-    if (!system_trace_exists) {
-        // No background trace was happening at the time MaybeSnapshotSystemTraceAsync() was invoked
-        if (!PropertiesHelper::IsUserBuild()) {
-            MYLOGI(
-                "No system traces found. Check for previously uploaded traces by looking for "
-                "go/trace-uuid in logcat")
-        }
-        return;
+    size_t traces_found = android::os::ForEachTrace([&](const std::string& trace_path) {
+        ds.AddZipEntry(ZIP_ROOT_DIR + trace_path, trace_path);
+        android::os::UnlinkAndLogOnError(trace_path);
+    });
+
+    if (traces_found == 0 && !PropertiesHelper::IsUserBuild()) {
+        MYLOGI(
+            "No system traces found. Check for previously uploaded traces by looking for "
+            "go/trace-uuid in logcat")
     }
-    ds.AddZipEntry(
-            ZIP_ROOT_DIR + SYSTEM_TRACE_SNAPSHOT,
-            SYSTEM_TRACE_SNAPSHOT);
-    android::os::UnlinkAndLogOnError(SYSTEM_TRACE_SNAPSHOT);
 }
 
 static void DumpVisibleWindowViews() {
@@ -3412,8 +3434,8 @@ Dumpstate::RunStatus Dumpstate::RunInternal(int32_t calling_uid,
     // duration is logged into MYLOG instead.
     PrintHeader();
 
-    bool system_trace_exists = access(SYSTEM_TRACE_SNAPSHOT, F_OK) == 0;
-    if (options_->use_predumped_ui_data && !system_trace_exists) {
+    size_t trace_count = android::os::ForEachTrace([](const std::string&) {});
+    if (options_->use_predumped_ui_data && trace_count == 0) {
         MYLOGW("Ignoring 'use predumped data' flag because no predumped data is available");
         options_->use_predumped_ui_data = false;
     }
@@ -3560,20 +3582,24 @@ std::future<std::string> Dumpstate::MaybeSnapshotSystemTraceAsync() {
     }
 
     // If a stale file exists already, remove it.
-    unlink(SYSTEM_TRACE_SNAPSHOT);
+    android::os::ForEachTrace([&](const std::string& trace_path) { unlink(trace_path.c_str()); });
 
     MYLOGI("Launching async '%s'", SERIALIZE_PERFETTO_TRACE_TASK.c_str())
+
     return std::async(
         std::launch::async, [this, outPath = std::move(outPath), outFd = std::move(outFd)] {
-            // If a background system trace is happening and is marked as "suitable for
-            // bugreport" (i.e. bugreport_score > 0 in the trace config), this command
-            // will stop it and serialize into SYSTEM_TRACE_SNAPSHOT. In the (likely)
-            // case that no trace is ongoing, this command is a no-op.
+            // If one or more background system traces are happening and are marked as
+            // "suitable for bugreport" (bugreport_score > 0 in the trace config), this command
+            // will snapshot them into SYSTEM_TRACE_DIR.
+            // In the (likely) case that no trace is ongoing, this command is a no-op.
             // Note: this should not be enqueued as we need to freeze the trace before
             // dumpstate starts. Otherwise the trace ring buffers will contain mostly
             // the dumpstate's own activity which is irrelevant.
+            const char* cmd_arg = perfetto::flags::save_all_traces_in_bugreport()
+                                      ? "--save-all-for-bugreport"
+                                      : "--save-for-bugreport";
             RunCommand(
-                SERIALIZE_PERFETTO_TRACE_TASK, {"perfetto", "--save-for-bugreport"},
+                SERIALIZE_PERFETTO_TRACE_TASK, {"perfetto", cmd_arg},
                 CommandOptions::WithTimeout(30).DropRoot().CloseAllFileDescriptorsOnExec().Build(),
                 false, outFd);
             // MaybeAddSystemTraceToZip() will take care of copying the trace in the zip

cmds/dumpstate/dumpstate_smoke_test.xml

@@ -22,7 +22,9 @@
         <option name="cleanup" value="true" />
         <option name="push" value="dumpstate_smoke_test->/data/local/tmp/dumpstate_smoke_test" />
     </target_preparer>
-
+    <target_preparer class="com.android.tradefed.targetprep.FeatureFlagTargetPreparer">
+        <option name="flag-value" value="perfetto/perfetto.flags.save_all_traces_in_bugreport=true" />
+    </target_preparer>
     <test class="com.android.tradefed.testtype.GTest" >
         <option name="native-test-device-path" value="/data/local/tmp" />
         <option name="module-name" value="dumpstate_smoke_test" />

cmds/dumpstate/tests/dumpstate_smoke_test.cpp

@@ -24,8 +24,10 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 #include <libgen.h>
+#include <signal.h>
 #include <ziparchive/zip_archive.h>
 
+#include <cstdio>
 #include <fstream>
 #include <regex>
 
@@ -603,6 +605,93 @@ TEST_F(DumpstateBinderTest, SimultaneousBugreportsNotAllowed) {
         listener1->getErrorCode() == IDumpstateListener::BUGREPORT_ERROR_USER_CONSENT_TIMED_OUT);
 }
 
+class DumpstateTracingTest : public Test {
+  protected:
+    void TearDown() override {
+        for (int pid : bg_process_pids) {
+            kill(pid, SIGKILL);
+        }
+    }
+
+    void StartTracing(const std::string& config) {
+        // Write the perfetto config into a file.
+        const int id = static_cast<int>(bg_process_pids.size());
+        char cfg[64];
+        snprintf(cfg, sizeof(cfg), "/data/misc/perfetto-configs/br-%d", id);
+        unlink(cfg);  // Remove the config file if it exists already.
+        FILE* f = fopen(cfg, "w");
+        ASSERT_NE(f, nullptr);
+        fputs(config.c_str(), f);
+        fclose(f);
+
+        // Invoke perfetto to start tracing.
+        char cmd[255];
+        snprintf(cmd, sizeof(cmd), "perfetto --background-wait --txt -o /dev/null -c %s", cfg);
+        FILE* proc = popen(cmd, "r");
+        ASSERT_NE(proc, nullptr);
+
+        // Read back the PID of the background process. We will use it to kill
+        // all tracing sessions when the test ends or fails.
+        char pid_str[32]{};
+        ASSERT_NE(fgets(pid_str, sizeof(pid_str), proc), nullptr);
+        int pid = atoi(pid_str);
+        bg_process_pids.push_back(pid);
+
+        pclose(proc);
+        unlink(cfg);
+    }
+
+    std::vector<int> bg_process_pids;
+};
+
+TEST_F(DumpstateTracingTest, ManyTracesInBugreport) {
+    // Note the trace duration is irrelevant and is only an upper bound.
+    // Tracing is stopped as soon as the bugreport.zip creation ends.
+    StartTracing(R"(
+buffers { size_kb: 4096 }
+data_sources {
+  config {
+    name: "linux.ftrace"
+  }
+}
+
+duration_ms: 120000
+bugreport_filename: "sys.pftrace"
+bugreport_score: 100
+)");
+
+    StartTracing(R"(
+buffers { size_kb: 4096 }
+data_sources {
+  config {
+    name: "linux.ftrace"
+  }
+}
+
+duration_ms: 120000
+bugreport_score: 50
+bugreport_filename: "mem.pftrace"
+)");
+
+    ZippedBugreportGenerationTest::GenerateBugreport();
+    std::string zip_path = ZippedBugreportGenerationTest::getZipFilePath();
+    ZipArchiveHandle handle;
+    ASSERT_EQ(OpenArchive(zip_path.c_str(), &handle), 0);
+
+    const char* kExpectedEntries[]{
+        "FS/data/misc/perfetto-traces/bugreport/sys.pftrace",
+        "FS/data/misc/perfetto-traces/bugreport/mem.pftrace",
+    };
+
+    // Check that the bugreport contains both traces.
+    for (const char* file_path : kExpectedEntries) {
+        ZipEntry entry{};
+        GetEntry(handle, file_path, &entry);
+        EXPECT_GT(entry.uncompressed_length, 100);
+    }
+    CloseArchive(handle);
+}
+
 }  // namespace dumpstate
 }  // namespace os
 }  // namespace android