perfetto_cmd: use CloneSession for --save-for-bugreport
Moves over the --save-for-bugreport cmdline option
to use the new CloneSession() IPC.
This moves the file writing from the tracing service
to the cmdline client.
Upcming CLs will remove the old
SaveTraceForBugreport IPC.
Bug: 260112703
Test: (1) perfetto_integrationtests --gtest_filter=PerfettoCmdlineTest.SaveForBugreport*
Test: (2) manual:
1. adb shell perfetto --background -c - --txt < bugreport.cfg
2. adb bugreport br.zip and check that the br contains a trace.
Change-Id: I34d1b2bb8c21aeedc24936fc48015fa38bb998c7
diff --git a/Android.bp b/Android.bp
index ad54f8d..55c57b0 100644
--- a/Android.bp
+++ b/Android.bp
@@ -1005,6 +1005,7 @@
":perfetto_src_base_version",
":perfetto_src_ipc_client",
":perfetto_src_ipc_common",
+ ":perfetto_src_perfetto_cmd_bugreport_path",
":perfetto_src_perfetto_cmd_perfetto_cmd",
":perfetto_src_perfetto_cmd_protos_cpp_gen",
":perfetto_src_perfetto_cmd_trigger_producer",
@@ -1191,6 +1192,7 @@
":perfetto_src_ipc_perfetto_ipc",
":perfetto_src_kallsyms_kallsyms",
":perfetto_src_kernel_utils_syscall_table",
+ ":perfetto_src_perfetto_cmd_bugreport_path",
":perfetto_src_protozero_filtering_bytecode_common",
":perfetto_src_protozero_filtering_bytecode_generator",
":perfetto_src_protozero_filtering_bytecode_parser",
@@ -1990,6 +1992,7 @@
":perfetto_src_ipc_perfetto_ipc",
":perfetto_src_kallsyms_kallsyms",
":perfetto_src_kernel_utils_syscall_table",
+ ":perfetto_src_perfetto_cmd_bugreport_path",
":perfetto_src_profiling_common_callstack_trie",
":perfetto_src_profiling_common_interner",
":perfetto_src_profiling_common_interning_output",
@@ -8467,6 +8470,11 @@
],
}
+// GN: //src/perfetto_cmd:bugreport_path
+filegroup {
+ name: "perfetto_src_perfetto_cmd_bugreport_path",
+}
+
// GN: //src/perfetto_cmd:gen_cc_config_descriptor
genrule {
name: "perfetto_src_perfetto_cmd_gen_cc_config_descriptor",
@@ -11623,6 +11631,7 @@
":perfetto_src_kallsyms_kallsyms",
":perfetto_src_kallsyms_unittests",
":perfetto_src_kernel_utils_syscall_table",
+ ":perfetto_src_perfetto_cmd_bugreport_path",
":perfetto_src_perfetto_cmd_perfetto_cmd",
":perfetto_src_perfetto_cmd_protos_cpp_gen",
":perfetto_src_perfetto_cmd_trigger_producer",
diff --git a/BUILD b/BUILD
index e5d3c9b..cb6b454 100644
--- a/BUILD
+++ b/BUILD
@@ -906,6 +906,14 @@
],
)
+# GN target: //src/perfetto_cmd:bugreport_path
+perfetto_filegroup(
+ name = "src_perfetto_cmd_bugreport_path",
+ srcs = [
+ "src/perfetto_cmd/bugreport_path.h",
+ ],
+)
+
# GN target: //src/perfetto_cmd:gen_cc_config_descriptor
perfetto_cc_proto_descriptor(
name = "src_perfetto_cmd_gen_cc_config_descriptor",
@@ -4537,6 +4545,7 @@
":include_perfetto_tracing_tracing",
":src_android_stats_android_stats",
":src_android_stats_perfetto_atoms",
+ ":src_perfetto_cmd_bugreport_path",
":src_perfetto_cmd_perfetto_cmd",
":src_perfetto_cmd_trigger_producer",
":src_tracing_common",
diff --git a/include/perfetto/ext/tracing/core/basic_types.h b/include/perfetto/ext/tracing/core/basic_types.h
index ac75b4d..0950fe8 100644
--- a/include/perfetto/ext/tracing/core/basic_types.h
+++ b/include/perfetto/ext/tracing/core/basic_types.h
@@ -76,6 +76,11 @@
constexpr uint32_t kDefaultFlushTimeoutMs = 5000;
+// The special id 0xffff..ffff represents the tracing session with the highest
+// bugreport score. This is used for CloneSession(kBugreportSessionId).
+constexpr TracingSessionID kBugreportSessionId =
+ static_cast<TracingSessionID>(-1);
+
} // namespace perfetto
#endif // INCLUDE_PERFETTO_EXT_TRACING_CORE_BASIC_TYPES_H_
diff --git a/include/perfetto/ext/tracing/core/tracing_service.h b/include/perfetto/ext/tracing/core/tracing_service.h
index 68dc086..82e53f7 100644
--- a/include/perfetto/ext/tracing/core/tracing_service.h
+++ b/include/perfetto/ext/tracing/core/tracing_service.h
@@ -42,9 +42,6 @@
class SharedMemoryArbiter;
class TraceWriter;
-// Exposed for testing.
-std::string GetBugreportPath();
-
// TODO: for the moment this assumes that all the calls happen on the same
// thread/sequence. Not sure this will be the case long term in Chrome.
@@ -192,6 +189,8 @@
// Clones an existing tracing session and attaches to it. The session is
// cloned in read-only mode and can only be used to read a snapshot of an
// existing tracing session. Will invoke Consumer::OnSessionCloned().
+ // If TracingSessionID == kBugreportSessionId (0xff...ff) the session with the
+ // highest bugreport score is cloned (if any exists).
// TODO(primiano): make pure virtual after various 3way patches.
virtual void CloneSession(TracingSessionID);
diff --git a/protos/perfetto/ipc/consumer_port.proto b/protos/perfetto/ipc/consumer_port.proto
index aa7fc93..d5a4025 100644
--- a/protos/perfetto/ipc/consumer_port.proto
+++ b/protos/perfetto/ipc/consumer_port.proto
@@ -270,7 +270,7 @@
// or something failed.
message SaveTraceForBugreportResponse {
// If true, an eligible the trace was saved into a known location (on Android
- // /data/misc/perfetto-traces, see GetBugreportPath()).
+ // /data/misc/perfetto-traces, see GetBugreportTracePath()).
// If false no trace with bugreport_score > 0 was found or an error occurred.
// see |msg| in that case for details about the failure.
optional bool success = 1;
@@ -279,6 +279,8 @@
// Arguments for rpc CloneSession.
message CloneSessionRequest {
+ // The session ID to clone. If session_id == kBugreportSessionId (0xff...ff)
+ // the session with the highest bugreport score is cloned (if any exists).
optional uint64 session_id = 1;
}
diff --git a/protos/perfetto/trace/perfetto/tracing_service_event.proto b/protos/perfetto/trace/perfetto/tracing_service_event.proto
index 04923dc..0573be0 100644
--- a/protos/perfetto/trace/perfetto/tracing_service_event.proto
+++ b/protos/perfetto/trace/perfetto/tracing_service_event.proto
@@ -60,6 +60,8 @@
// the contents are routed onto the bugreport file. This event flags the
// situation explicitly. Traces that contain this marker should be discarded
// by test infrastructures / pipelines.
+ // Deprecated since Android U, where --save-for-bugreport uses
+ // non-destructive cloning.
bool seized_for_bugreport = 6;
}
}
diff --git a/protos/perfetto/trace/perfetto_trace.proto b/protos/perfetto/trace/perfetto_trace.proto
index d8400fa..670267f 100644
--- a/protos/perfetto/trace/perfetto_trace.proto
+++ b/protos/perfetto/trace/perfetto_trace.proto
@@ -10372,6 +10372,8 @@
// the contents are routed onto the bugreport file. This event flags the
// situation explicitly. Traces that contain this marker should be discarded
// by test infrastructures / pipelines.
+ // Deprecated since Android U, where --save-for-bugreport uses
+ // non-destructive cloning.
bool seized_for_bugreport = 6;
}
}
diff --git a/src/perfetto_cmd/BUILD.gn b/src/perfetto_cmd/BUILD.gn
index 70c47fc..51486c0 100644
--- a/src/perfetto_cmd/BUILD.gn
+++ b/src/perfetto_cmd/BUILD.gn
@@ -49,6 +49,7 @@
"../../include/perfetto/ext/traced",
]
deps = [
+ ":bugreport_path",
":gen_cc_config_descriptor",
":trigger_producer",
"../../gn:default_deps",
@@ -83,6 +84,15 @@
}
}
+source_set("bugreport_path") {
+ sources = [ "bugreport_path.h" ]
+ deps = [ "../../gn:default_deps" ]
+ public_deps = [
+ "../../include/perfetto/base",
+ "../../include/perfetto/ext/base",
+ ]
+}
+
perfetto_cc_proto_descriptor("gen_cc_config_descriptor") {
descriptor_name = "config.descriptor"
descriptor_target = "../../protos/perfetto/config:descriptor"
diff --git a/src/perfetto_cmd/bugreport_path.h b/src/perfetto_cmd/bugreport_path.h
new file mode 100644
index 0000000..5bc7602
--- /dev/null
+++ b/src/perfetto_cmd/bugreport_path.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef SRC_PERFETTO_CMD_BUGREPORT_PATH_H_
+#define SRC_PERFETTO_CMD_BUGREPORT_PATH_H_
+
+#include <string>
+
+#include "perfetto/base/build_config.h"
+#include "perfetto/ext/base/temp_file.h"
+
+namespace perfetto {
+
+// Expose for testing
+inline std::string GetBugreportTracePath() {
+#if PERFETTO_BUILDFLAG(PERFETTO_OS_ANDROID) && \
+ PERFETTO_BUILDFLAG(PERFETTO_ANDROID_BUILD)
+ return "/data/misc/perfetto-traces/bugreport/systrace.pftrace";
+#else
+ // Only for tests, SaveTraceForBugreport is not used on other OSes.
+ return base::GetSysTempDir() + "/bugreport.pftrace";
+#endif
+}
+
+} // namespace perfetto
+
+#endif // SRC_PERFETTO_CMD_BUGREPORT_PATH_H_
diff --git a/src/perfetto_cmd/perfetto_cmd.cc b/src/perfetto_cmd/perfetto_cmd.cc
index 5818c59..a0234b8 100644
--- a/src/perfetto_cmd/perfetto_cmd.cc
+++ b/src/perfetto_cmd/perfetto_cmd.cc
@@ -51,6 +51,7 @@
#include "perfetto/ext/base/getopt.h"
#include "perfetto/ext/base/pipe.h"
#include "perfetto/ext/base/string_view.h"
+#include "perfetto/ext/base/temp_file.h"
#include "perfetto/ext/base/thread_utils.h"
#include "perfetto/ext/base/utils.h"
#include "perfetto/ext/base/uuid.h"
@@ -64,9 +65,11 @@
#include "perfetto/tracing/core/trace_config.h"
#include "perfetto/tracing/core/tracing_service_state.h"
#include "src/android_stats/statsd_logging_helper.h"
+#include "src/perfetto_cmd/bugreport_path.h"
#include "src/perfetto_cmd/config.h"
#include "src/perfetto_cmd/packet_writer.h"
#include "src/perfetto_cmd/pbtxt_to_pb.h"
+#include "src/perfetto_cmd/rate_limiter.h"
#include "src/perfetto_cmd/trigger_producer.h"
#include "protos/perfetto/common/ftrace_descriptor.gen.h"
@@ -78,7 +81,8 @@
perfetto::PerfettoCmd* g_perfetto_cmd;
-uint32_t kOnTraceDataTimeoutMs = 3000;
+const uint32_t kOnTraceDataTimeoutMs = 3000;
+const uint32_t kCloneTimeoutMs = 10000;
class LoggingErrorReporter : public ErrorReporter {
public:
@@ -552,6 +556,13 @@
return 1;
}
+ // --save-for-bugreport is the equivalent of:
+ // --clone kBugreportSessionId -o /data/misc/perfetto-traces/bugreport/...
+ if (bugreport_ && trace_out_path_.empty()) {
+ clone_tsid_ = kBugreportSessionId;
+ trace_out_path_ = GetBugreportTracePath();
+ }
+
// Parse the trace config. It can be either:
// 1) A proto-encoded file/stdin (-c ...).
// 2) A proto-text file/stdin (-c ... --txt).
@@ -561,8 +572,7 @@
trace_config_.reset(new TraceConfig());
bool parsed = false;
- const bool will_trace_or_trigger =
- !is_attach() && !query_service_ && !bugreport_;
+ const bool will_trace_or_trigger = !is_attach() && !query_service_;
if (!will_trace_or_trigger || clone_tsid_) {
if ((!trace_config_raw.empty() || has_config_options)) {
PERFETTO_ELOG("Cannot specify a trace config with this option");
@@ -904,12 +914,12 @@
return finished_with_success ? 0 : 1;
} // if (triggers_to_activate_)
- if (query_service_ || bugreport_) {
+ if (query_service_) {
consumer_endpoint_ =
ConsumerIPCClient::Connect(GetConsumerSocket(), this, &task_runner_);
task_runner_.Run();
return 1; // We can legitimately get here if the service disconnects.
- } // if (query_service || bugreport_)
+ }
RateLimiter::Args args{};
args.is_user_build = IsUserBuild();
@@ -997,25 +1007,14 @@
return;
}
- if (bugreport_) {
- consumer_endpoint_->SaveTraceForBugreport(
- [](bool success, const std::string& msg) {
- if (success) {
- PERFETTO_ILOG("Trace saved into %s", msg.c_str());
- exit(0);
- }
- PERFETTO_ELOG("%s", msg.c_str());
- exit(1);
- });
- return;
- }
-
if (is_attach()) {
consumer_endpoint_->Attach(attach_key_);
return;
}
if (clone_tsid_.has_value()) {
+ task_runner_.PostDelayedTask(std::bind(&PerfettoCmd::OnTimeout, this),
+ kCloneTimeoutMs);
consumer_endpoint_->CloneSession(*clone_tsid_);
return;
}
diff --git a/src/perfetto_cmd/perfetto_cmd.h b/src/perfetto_cmd/perfetto_cmd.h
index 842ddcc..bd4a8c4 100644
--- a/src/perfetto_cmd/perfetto_cmd.h
+++ b/src/perfetto_cmd/perfetto_cmd.h
@@ -33,7 +33,6 @@
#include "perfetto/ext/tracing/core/consumer.h"
#include "perfetto/ext/tracing/ipc/consumer_ipc_client.h"
#include "src/android_stats/perfetto_atoms.h"
-#include "src/perfetto_cmd/rate_limiter.h"
namespace perfetto {
diff --git a/src/tracing/core/tracing_service_impl.cc b/src/tracing/core/tracing_service_impl.cc
index 5277657..0ee3122 100644
--- a/src/tracing/core/tracing_service_impl.cc
+++ b/src/tracing/core/tracing_service_impl.cc
@@ -102,15 +102,6 @@
namespace perfetto {
-#if PERFETTO_BUILDFLAG(PERFETTO_OS_ANDROID) && \
- PERFETTO_BUILDFLAG(PERFETTO_ANDROID_BUILD)
-// These are the only SELinux approved dir for trace files that are created
-// directly by traced.
-const char* kTraceDirBasePath = "/data/misc/perfetto-traces/";
-const char* kAndroidProductionBugreportTracePath =
- "/data/misc/perfetto-traces/bugreport/systrace.pftrace";
-#endif
-
namespace {
constexpr int kMaxBuffersPerConsumer = 128;
constexpr uint32_t kDefaultSnapshotsIntervalMs = 10 * 1000;
@@ -241,9 +232,7 @@
return filter_matches || filter_regex_matches;
}
-// Used when:
-// 1. TraceConfig.write_into_file == true and output_path is not empty.
-// 2. Calling SaveTraceForBugreport(), from perfetto --save-for-bugreport.
+// Used when TraceConfig.write_into_file == true and output_path is not empty.
base::ScopedFile CreateTraceFile(const std::string& path, bool overwrite) {
#if PERFETTO_BUILDFLAG(PERFETTO_OS_ANDROID) && \
PERFETTO_BUILDFLAG(PERFETTO_ANDROID_BUILD)
@@ -251,6 +240,9 @@
// It just improves the actionability of the error when people try to save the
// trace in a location that is not SELinux-allowed (a generic "permission
// denied" vs "don't put it here, put it there").
+ // These are the only SELinux approved dir for trace files that are created
+ // directly by traced.
+ static const char* kTraceDirBasePath = "/data/misc/perfetto-traces/";
if (!base::StartsWith(path, kTraceDirBasePath)) {
PERFETTO_ELOG("Invalid output_path %s. On Android it must be within %s.",
path.c_str(), kTraceDirBasePath);
@@ -271,10 +263,6 @@
return fd;
}
-std::string GetBugreportTmpPath() {
- return GetBugreportPath() + ".tmp";
-}
-
bool ShouldLogEvent(const TraceConfig& cfg) {
switch (cfg.statsd_logging()) {
case TraceConfig::STATSD_LOGGING_ENABLED:
@@ -320,16 +308,6 @@
constexpr uint8_t TracingServiceImpl::kSyncMarker[];
#endif
-std::string GetBugreportPath() {
-#if PERFETTO_BUILDFLAG(PERFETTO_OS_ANDROID) && \
- PERFETTO_BUILDFLAG(PERFETTO_ANDROID_BUILD)
- return kAndroidProductionBugreportTracePath;
-#else
- // Only for tests, SaveTraceForBugreport is not used on other OSes.
- return base::GetSysTempDir() + "/bugreport.pftrace";
-#endif
-}
-
// static
std::unique_ptr<TracingService> TracingService::CreateInstance(
std::unique_ptr<SharedMemory::Factory> shm_factory,
@@ -1692,11 +1670,6 @@
ReadBuffersIntoFile(tracing_session->id);
}
- if (tracing_session->on_disable_callback_for_bugreport) {
- std::move(tracing_session->on_disable_callback_for_bugreport)();
- tracing_session->on_disable_callback_for_bugreport = nullptr;
- }
-
MaybeLogUploadEvent(tracing_session->config, tracing_session->trace_uuid,
PerfettoStatsdAtom::kTracedNotifyTracingDisabled);
@@ -2078,21 +2051,6 @@
return false;
}
- // If a bugreport request happened and the trace was stolen for that, give
- // an empty trace with a clear signal to the consumer. This deals only with
- // the case of readback-from-IPC. A similar code-path deals with the
- // write_into_file case in MaybeSaveTraceForBugreport().
- if (tracing_session->seized_for_bugreport) {
- std::vector<TracePacket> packets;
- if (!tracing_session->config.builtin_data_sources()
- .disable_service_events()) {
- EmitSeizedForBugreportLifecycleEvent(&packets);
- }
- EmitLifecycleEvents(tracing_session, &packets);
- consumer->consumer_->OnTraceData(std::move(packets), /*has_more=*/false);
- return true;
- }
-
if (IsWaitingForTrigger(tracing_session))
return false;
@@ -2141,8 +2099,7 @@
if (!tracing_session->write_into_file)
return false;
- if (!tracing_session->seized_for_bugreport &&
- IsWaitingForTrigger(tracing_session))
+ if (IsWaitingForTrigger(tracing_session))
return false;
// ReadBuffers() can allocate memory internally, for filtering. By limiting
@@ -2501,25 +2458,22 @@
bool is_long_trace =
(tracing_session->config.write_into_file() &&
tracing_session->config.file_write_period_ms() < kMillisPerDay);
- bool seized_for_bugreport = tracing_session->seized_for_bugreport;
tracing_sessions_.erase(tsid);
tracing_session = nullptr;
UpdateMemoryGuardrail();
PERFETTO_LOG("Tracing session %" PRIu64 " ended, total sessions:%zu", tsid,
tracing_sessions_.size());
-
#if PERFETTO_BUILDFLAG(PERFETTO_ANDROID_BUILD) && \
PERFETTO_BUILDFLAG(PERFETTO_OS_ANDROID)
- if (notify_traceur && (seized_for_bugreport || is_long_trace)) {
+ if (notify_traceur && is_long_trace) {
PERFETTO_LAZY_LOAD(android_internal::NotifyTraceSessionEnded, notify_fn);
- if (!notify_fn || !notify_fn(seized_for_bugreport))
+ if (!notify_fn || !notify_fn(/*session_stolen=*/false))
PERFETTO_ELOG("Failed to notify Traceur long tracing has ended");
}
#else
base::ignore_result(notify_traceur);
base::ignore_result(is_long_trace);
- base::ignore_result(seized_for_bugreport);
#endif
}
@@ -2983,6 +2937,23 @@
return &it->second;
}
+TracingServiceImpl::TracingSession*
+TracingServiceImpl::FindTracingSessionWithMaxBugreportScore() {
+ TracingSession* max_session = nullptr;
+ for (auto& session_id_and_session : tracing_sessions_) {
+ auto& session = session_id_and_session.second;
+ const int32_t score = session.config.bugreport_score();
+ // Exclude sessions with 0 (or below) score. By default tracing sessions
+ // should NOT be eligible to be attached to bugreports.
+ if (score <= 0 || session.state != TracingSession::STARTED)
+ continue;
+
+ if (!max_session || score > max_session->config.bugreport_score())
+ max_session = &session;
+ }
+ return max_session;
+}
+
ProducerID TracingServiceImpl::GetNextProducerID() {
PERFETTO_DCHECK_THREAD(thread_checker_);
PERFETTO_CHECK(producers_.size() < kMaxProducerID);
@@ -3411,18 +3382,6 @@
SerializeAndAppendPacket(packets, std::move(pair.second));
}
-void TracingServiceImpl::EmitSeizedForBugreportLifecycleEvent(
- std::vector<TracePacket>* packets) {
- protozero::HeapBuffered<protos::pbzero::TracePacket> packet;
- packet->set_timestamp(static_cast<uint64_t>(base::GetBootTimeNs().count()));
- packet->set_trusted_uid(static_cast<int32_t>(uid_));
- packet->set_trusted_packet_sequence_id(kServicePacketSequenceID);
- auto* service_event = packet->set_service_event();
- service_event->AppendVarInt(
- protos::pbzero::TracingServiceEvent::kSeizedForBugreportFieldNumber, 1);
- SerializeAndAppendPacket(packets, packet.SerializeAsArray());
-}
-
void TracingServiceImpl::MaybeEmitReceivedTriggers(
TracingSession* tracing_session,
std::vector<TracePacket>* packets) {
@@ -3445,91 +3404,6 @@
}
}
-bool TracingServiceImpl::MaybeSaveTraceForBugreport(
- std::function<void()> callback) {
- TracingSession* max_session = nullptr;
- TracingSessionID max_tsid = 0;
- for (auto& session_id_and_session : tracing_sessions_) {
- auto& session = session_id_and_session.second;
- const int32_t score = session.config.bugreport_score();
- // Exclude sessions with 0 (or below) score. By default tracing sessions
- // should NOT be eligible to be attached to bugreports.
- if (score <= 0 || session.state != TracingSession::STARTED)
- continue;
-
- // Also don't try to steal long traces with write_into_file if their content
- // has been already partially written into a file, as we would get partial
- // traces on both sides. We can't just copy the original file into the
- // bugreport because the file could be too big (GBs) for bugreports.
- // The only case where it's legit to steal traces with write_into_file, is
- // when the consumer specified a very large write_period_ms (e.g. 24h),
- // meaning that this is effectively a ring-buffer trace. Traceur (the
- // Android System Tracing app), which uses --detach, does this to have a
- // consistent invocation path for long-traces and ring-buffer-mode traces.
- if (session.write_into_file && session.bytes_written_into_file > 0)
- continue;
-
- // If we are already in the process of finalizing another trace for
- // bugreport, don't even start another one, as they would try to write onto
- // the same file.
- if (session.on_disable_callback_for_bugreport)
- return false;
-
- if (!max_session || score > max_session->config.bugreport_score()) {
- max_session = &session;
- max_tsid = session_id_and_session.first;
- }
- }
-
- // No eligible trace found.
- if (!max_session)
- return false;
-
- PERFETTO_LOG("Seizing trace for bugreport. tsid:%" PRIu64
- " state:%d wf:%d score:%d name:\"%s\"",
- max_tsid, max_session->state, !!max_session->write_into_file,
- max_session->config.bugreport_score(),
- max_session->config.unique_session_name().c_str());
-
- auto br_fd = CreateTraceFile(GetBugreportTmpPath(), /*overwrite=*/true);
- if (!br_fd)
- return false;
-
- if (max_session->write_into_file) {
- auto fd = *max_session->write_into_file;
- // If we are stealing a write_into_file session, add a marker that explains
- // why the trace has been stolen rather than creating an empty file. This is
- // only for write_into_file traces. A similar code path deals with the case
- // of reading-back a seized trace from IPC in ReadBuffersIntoConsumer().
- if (!max_session->config.builtin_data_sources().disable_service_events()) {
- std::vector<TracePacket> packets;
- EmitSeizedForBugreportLifecycleEvent(&packets);
- for (auto& packet : packets) {
- char* preamble;
- size_t preamble_size = 0;
- std::tie(preamble, preamble_size) = packet.GetProtoPreamble();
- base::WriteAll(fd, preamble, preamble_size);
- for (const Slice& slice : packet.slices()) {
- base::WriteAll(fd, slice.start, slice.size);
- }
- } // for (packets)
- } // if (!disable_service_events())
- } // if (max_session->write_into_file)
- max_session->write_into_file = std::move(br_fd);
- max_session->on_disable_callback_for_bugreport = std::move(callback);
- max_session->seized_for_bugreport = true;
-
- // Post a task to avoid that early FlushAndDisableTracing() failures invoke
- // the callback before we return. That would re-enter in a weird way the
- // callstack of the calling ConsumerEndpointImpl::SaveTraceForBugreport().
- auto weak_this = weak_ptr_factory_.GetWeakPtr();
- task_runner_->PostTask([weak_this, max_tsid] {
- if (weak_this)
- weak_this->FlushAndDisableTracing(max_tsid);
- });
- return true;
-}
-
void TracingServiceImpl::MaybeLogUploadEvent(const TraceConfig& cfg,
const base::Uuid& uuid,
PerfettoStatsdAtom atom,
@@ -3571,6 +3445,17 @@
void TracingServiceImpl::FlushAndCloneSession(ConsumerEndpointImpl* consumer,
TracingSessionID tsid) {
PERFETTO_DCHECK_THREAD(thread_checker_);
+
+ if (tsid == kBugreportSessionId) {
+ TracingSession* session = FindTracingSessionWithMaxBugreportScore();
+ if (!session) {
+ consumer->consumer_->OnSessionCloned(
+ false, "No tracing sessions eligible for bugreport found");
+ return;
+ }
+ tsid = session->id;
+ }
+
auto weak_this = weak_ptr_factory_.GetWeakPtr();
auto weak_consumer = consumer->GetWeakPtr();
Flush(tsid, 0, [weak_this, tsid, weak_consumer](bool final_flush_outcome) {
@@ -3589,6 +3474,7 @@
bool final_flush_outcome) {
PERFETTO_DLOG("CloneSession(%" PRIu64 ") started, consumer uid: %d", src_tsid,
static_cast<int>(consumer->uid_));
+
TracingSession* src = GetTracingSession(src_tsid);
// The session might be gone by the time we try to clone it.
@@ -3662,6 +3548,11 @@
new protozero::MessageFilter(*src->trace_filter));
}
+ SnapshotLifecyleEvent(
+ cloned_session,
+ protos::pbzero::TracingServiceEvent::kTracingDisabledFieldNumber,
+ true /* snapshot_clocks */);
+
PERFETTO_DLOG("Consumer (uid:%d) cloned tracing session %" PRIu64
" -> %" PRIu64,
static_cast<int>(consumer->uid_), src_tsid, tsid);
@@ -3982,21 +3873,9 @@
void TracingServiceImpl::ConsumerEndpointImpl::SaveTraceForBugreport(
SaveTraceForBugreportCallback consumer_callback) {
- PERFETTO_DCHECK_THREAD(thread_checker_);
- auto on_complete_callback = [consumer_callback] {
- if (rename(GetBugreportTmpPath().c_str(), GetBugreportPath().c_str())) {
- consumer_callback(false, "rename(" + GetBugreportTmpPath() + ", " +
- GetBugreportPath() + ") failed (" +
- strerror(errno) + ")");
- } else {
- consumer_callback(true, GetBugreportPath());
- }
- };
- if (!service_->MaybeSaveTraceForBugreport(std::move(on_complete_callback))) {
- consumer_callback(false,
- "No trace with TraceConfig.bugreport_score > 0 eligible "
- "for bug reporting was found");
- }
+ consumer_callback(false,
+ "SaveTraceForBugreport is deprecated. Use "
+ "CloneSession(kBugreportSessionId) instead.");
}
void TracingServiceImpl::ConsumerEndpointImpl::CloneSession(
diff --git a/src/tracing/core/tracing_service_impl.h b/src/tracing/core/tracing_service_impl.h
index 4800559..02e5639 100644
--- a/src/tracing/core/tracing_service_impl.h
+++ b/src/tracing/core/tracing_service_impl.h
@@ -632,11 +632,6 @@
uint64_t max_file_size_bytes = 0;
uint64_t bytes_written_into_file = 0;
- // Set when using SaveTraceForBugreport(). This callback will be called
- // when the tracing session ends and the data has been saved into the file.
- std::function<void()> on_disable_callback_for_bugreport;
- bool seized_for_bugreport = false;
-
// Periodic task for snapshotting service events (e.g. clocks, sync markers
// etc)
base::PeriodicTask snapshot_periodic_task;
@@ -680,6 +675,10 @@
// session doesn't exists.
TracingSession* GetTracingSession(TracingSessionID);
+ // Returns a pointer to the tracing session that has the highest
+ // TraceConfig.bugreport_score, if any, or nullptr.
+ TracingSession* FindTracingSessionWithMaxBugreportScore();
+
// Returns a pointer to the |tracing_sessions_| entry, matching the given
// uid and detach key, or nullptr if no such session exists.
TracingSession* GetDetachedSession(uid_t, const std::string& key);
@@ -708,12 +707,10 @@
void EmitStats(TracingSession*, std::vector<TracePacket>*);
TraceStats GetTraceStats(TracingSession*);
void EmitLifecycleEvents(TracingSession*, std::vector<TracePacket>*);
- void EmitSeizedForBugreportLifecycleEvent(std::vector<TracePacket>*);
void MaybeEmitUuidAndTraceConfig(TracingSession*, std::vector<TracePacket>*);
void MaybeEmitSystemInfo(TracingSession*, std::vector<TracePacket>*);
void MaybeEmitReceivedTriggers(TracingSession*, std::vector<TracePacket>*);
void MaybeNotifyAllDataSourcesStarted(TracingSession*);
- bool MaybeSaveTraceForBugreport(std::function<void()> callback);
void OnFlushTimeout(TracingSessionID, FlushRequestID);
void OnDisableTracingTimeout(TracingSessionID);
void DisableTracingNotifyConsumerAndFlushFile(TracingSession*);
diff --git a/test/BUILD.gn b/test/BUILD.gn
index 768b8ad..6b995ed 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -38,6 +38,7 @@
"../protos/perfetto/trace/sys_stats:cpp",
"../src/base:base",
"../src/base:test_support",
+ "../src/perfetto_cmd:bugreport_path",
]
if (enable_perfetto_traced_probes) {
deps += [
diff --git a/test/android_integrationtest.cc b/test/android_integrationtest.cc
index 19e894d..9d4e543 100644
--- a/test/android_integrationtest.cc
+++ b/test/android_integrationtest.cc
@@ -63,33 +63,6 @@
using ::testing::Property;
using ::testing::SizeIs;
-// For the SaveForBugreport* tests.
-void SetTraceConfigForBugreportTest(TraceConfig* trace_config) {
- trace_config->add_buffers()->set_size_kb(4096);
- trace_config->set_duration_ms(60000); // Will never hit this.
- trace_config->set_bugreport_score(10);
- auto* ds_config = trace_config->add_data_sources()->mutable_config();
- ds_config->set_name("android.perfetto.FakeProducer");
- ds_config->mutable_for_testing()->set_message_count(3);
- ds_config->mutable_for_testing()->set_message_size(10);
- ds_config->mutable_for_testing()->set_send_batch_on_register(true);
-}
-
-// For the SaveForBugreport* tests.
-static void VerifyBugreportTraceContents() {
- // Read the trace written in the fixed location (/data/misc/perfetto-traces/
- // on Android, /tmp/ on Linux/Mac) and make sure it has the right contents.
- std::string trace_str;
- base::ReadFile(GetBugreportPath(), &trace_str);
- ASSERT_FALSE(trace_str.empty());
- protos::gen::Trace trace;
- ASSERT_TRUE(trace.ParseFromString(trace_str));
- int test_packets = 0;
- for (const auto& p : trace.packet())
- test_packets += p.has_for_testing() ? 1 : 0;
- ASSERT_EQ(test_packets, 3);
-}
-
} // namespace
TEST(PerfettoAndroidIntegrationTest, TestKmemActivity) {
@@ -283,125 +256,6 @@
ASSERT_TRUE(has_battery_packet);
}
-TEST(PerfettoAndroidIntegrationTest, SaveForBugreport) {
- base::TestTaskRunner task_runner;
-
- TestHelper helper(&task_runner);
- helper.StartServiceIfRequired();
- helper.ConnectFakeProducer();
- helper.ConnectConsumer();
- helper.WaitForConsumerConnect();
-
- TraceConfig trace_config;
- SetTraceConfigForBugreportTest(&trace_config);
-
- helper.StartTracing(trace_config);
- helper.WaitForProducerEnabled();
-
- EXPECT_TRUE(helper.SaveTraceForBugreportAndWait());
- helper.WaitForTracingDisabled();
-
- VerifyBugreportTraceContents();
-
- // Now read the trace returned to the consumer via ReadBuffers. This should
- // be always empty because --save-for-bugreport takes it over and makes the
- // buffers unreadable by the consumer (by virtue of force-setting
- // write_into_file, which is incompatible with ReadBuffers()). The only
- // content should be the |seized_for_bugreport| flag.
- helper.ReadData();
- helper.WaitForReadData();
- const auto& packets = helper.full_trace();
- ASSERT_EQ(packets.size(), 1u);
- for (const auto& p : packets) {
- ASSERT_TRUE(p.has_service_event());
- ASSERT_TRUE(p.service_event().seized_for_bugreport());
- }
-}
-
-// Tests that the SaveForBugreport logic works also for traces with
-// write_into_file = true (with a passed file descriptor).
-TEST(PerfettoAndroidIntegrationTest, SaveForBugreport_WriteIntoFile) {
- base::TestTaskRunner task_runner;
-
- TestHelper helper(&task_runner);
- helper.StartServiceIfRequired();
- helper.ConnectFakeProducer();
- helper.ConnectConsumer();
- helper.WaitForConsumerConnect();
-
- TraceConfig trace_config;
- SetTraceConfigForBugreportTest(&trace_config);
- trace_config.set_file_write_period_ms(60000); // Will never hit this.
- trace_config.set_write_into_file(true);
-
- auto pipe_pair = base::Pipe::Create();
- helper.StartTracing(trace_config, std::move(pipe_pair.wr));
- helper.WaitForProducerEnabled();
-
- EXPECT_TRUE(helper.SaveTraceForBugreportAndWait());
- helper.WaitForTracingDisabled();
-
- VerifyBugreportTraceContents();
-
- // Now read the original file descriptor passed in.
- std::string trace_bytes;
- ASSERT_TRUE(base::ReadPlatformHandle(*pipe_pair.rd, &trace_bytes));
- protos::gen::Trace trace;
- ASSERT_TRUE(trace.ParseFromString(trace_bytes));
- ASSERT_EQ(trace.packet().size(), 1u);
- for (const auto& p : trace.packet()) {
- ASSERT_TRUE(p.has_service_event());
- ASSERT_TRUE(p.service_event().seized_for_bugreport());
- }
-}
-
-// Tests that SaveTraceForBugreport() works also if the trace has triggers
-// defined and those triggers have not been hit. This is a regression test for
-// b/188008375 .
-#if PERFETTO_BUILDFLAG(PERFETTO_ANDROID_BUILD)
-// Disabled due to b/191940560
-#define MAYBE_SaveForBugreport_Triggers DISABLED_SaveForBugreport_Triggers
-#else
-#define MAYBE_SaveForBugreport_Triggers SaveForBugreport_Triggers
-#endif
-TEST(PerfettoAndroidIntegrationTest, MAYBE_SaveForBugreport_Triggers) {
- base::TestTaskRunner task_runner;
-
- TestHelper helper(&task_runner);
- helper.StartServiceIfRequired();
- helper.ConnectFakeProducer();
- helper.ConnectConsumer();
- helper.WaitForConsumerConnect();
-
- TraceConfig trace_config;
- SetTraceConfigForBugreportTest(&trace_config);
- trace_config.set_duration_ms(0); // set_trigger_timeout_ms is used instead.
- auto* trigger_config = trace_config.mutable_trigger_config();
- trigger_config->set_trigger_timeout_ms(8.64e+7);
- trigger_config->set_trigger_mode(TraceConfig::TriggerConfig::STOP_TRACING);
- auto* trigger = trigger_config->add_triggers();
- trigger->set_name("trigger_name");
- trigger->set_stop_delay_ms(1);
-
- helper.StartTracing(trace_config);
- helper.WaitForProducerEnabled();
-
- EXPECT_TRUE(helper.SaveTraceForBugreportAndWait());
- helper.WaitForTracingDisabled();
-
- VerifyBugreportTraceContents();
-
- // Now read the original trace.
- helper.ReadData();
- helper.WaitForReadData();
- const auto& packets = helper.full_trace();
- ASSERT_EQ(packets.size(), 1u);
- for (const auto& p : packets) {
- ASSERT_TRUE(p.has_service_event());
- ASSERT_TRUE(p.service_event().seized_for_bugreport());
- }
-}
-
} // namespace perfetto
#endif // PERFETTO_OS_ANDROID
diff --git a/test/cmdline_integrationtest.cc b/test/cmdline_integrationtest.cc
index 0747789..cbc3555 100644
--- a/test/cmdline_integrationtest.cc
+++ b/test/cmdline_integrationtest.cc
@@ -27,6 +27,7 @@
#include "perfetto/protozero/scattered_heap_buffer.h"
#include "src/base/test/test_task_runner.h"
#include "src/base/test/utils.h"
+#include "src/perfetto_cmd/bugreport_path.h"
#include "test/gtest_and_gmock.h"
#include "test/test_helper.h"
@@ -66,6 +67,19 @@
return path;
}
+// For the SaveForBugreport* tests.
+TraceConfig CreateTraceConfigForBugreportTest() {
+ TraceConfig trace_config;
+ trace_config.add_buffers()->set_size_kb(4096);
+ trace_config.set_duration_ms(60000); // Will never hit this.
+ trace_config.set_bugreport_score(10);
+ auto* ds_config = trace_config.add_data_sources()->mutable_config();
+ ds_config->set_name("android.perfetto.FakeProducer");
+ ds_config->mutable_for_testing()->set_message_count(3);
+ ds_config->mutable_for_testing()->set_message_size(10);
+ return trace_config;
+}
+
class PerfettoCmdlineTest : public ::testing::Test {
public:
void SetUp() override {
@@ -120,6 +134,68 @@
return Exec("trigger_perfetto", std::move(args), std::move(std_in));
}
+ // This is in common to the 3 TEST_F SaveForBugreport* fixtures, which differ
+ // only in the config, passed here as input.
+ void RunBugreportTest(protos::gen::TraceConfig trace_config,
+ bool check_original_trace = true) {
+ const std::string path = RandomTraceFileName();
+
+ auto perfetto_proc = ExecPerfetto(
+ {
+ "-o",
+ path,
+ "-c",
+ "-",
+ },
+ trace_config.SerializeAsString());
+
+ auto perfetto_br_proc = ExecPerfetto({
+ "--save-for-bugreport",
+ });
+
+ // Start the service and connect a simple fake producer.
+ StartServiceIfRequiredNoNewExecsAfterThis();
+
+ auto* fake_producer = ConnectFakeProducer();
+ ASSERT_TRUE(fake_producer);
+
+ std::thread background_trace([&perfetto_proc]() {
+ std::string stderr_str;
+ ASSERT_EQ(0, perfetto_proc.Run(&stderr_str)) << stderr_str;
+ });
+
+ // Wait for the producer to start, and then write out packets.
+ WaitForProducerEnabled();
+ auto on_data_written = task_runner_.CreateCheckpoint("data_written");
+ fake_producer->ProduceEventBatch(WrapTask(on_data_written));
+ task_runner_.RunUntilCheckpoint("data_written");
+
+ ASSERT_EQ(0, perfetto_br_proc.Run(&stderr_)) << "stderr: " << stderr_;
+ perfetto_proc.SendSigterm();
+ background_trace.join();
+
+ auto check_trace_contents = [](std::string trace_path) {
+ // Read the trace written in the fixed location
+ // (/data/misc/perfetto-traces/ on Android, /tmp/ on Linux/Mac) and make
+ // sure it has the right contents.
+ std::string trace_str;
+ base::ReadFile(trace_path, &trace_str);
+ ASSERT_FALSE(trace_str.empty());
+ protos::gen::Trace trace;
+ ASSERT_TRUE(trace.ParseFromString(trace_str));
+ int test_packets = 0;
+ for (const auto& p : trace.packet())
+ test_packets += p.has_for_testing() ? 1 : 0;
+ ASSERT_EQ(test_packets, 3) << trace_path;
+ };
+
+ // Verify that both the original trace and the cloned bugreport contain
+ // the expected contents.
+ check_trace_contents(GetBugreportTracePath());
+ if (check_original_trace)
+ check_trace_contents(path);
+ }
+
// Tests are allowed to freely use these variables.
std::string stderr_;
base::TestTaskRunner task_runner_;
@@ -760,4 +836,37 @@
}
}
+TEST_F(PerfettoCmdlineTest, SaveForBugreport) {
+ TraceConfig trace_config = CreateTraceConfigForBugreportTest();
+ RunBugreportTest(std::move(trace_config));
+}
+
+TEST_F(PerfettoCmdlineTest, SaveForBugreport_WriteIntoFile) {
+ TraceConfig trace_config = CreateTraceConfigForBugreportTest();
+ trace_config.set_file_write_period_ms(60000); // Will never hit this.
+ trace_config.set_write_into_file(true);
+ RunBugreportTest(std::move(trace_config));
+}
+
+// Tests that SaveTraceForBugreport() works also if the trace has triggers
+// defined and those triggers have not been hit. This is a regression test for
+// b/188008375 .
+#if PERFETTO_BUILDFLAG(PERFETTO_ANDROID_BUILD)
+// Disabled due to b/191940560
+#define MAYBE_SaveForBugreport_Triggers DISABLED_SaveForBugreport_Triggers
+#else
+#define MAYBE_SaveForBugreport_Triggers SaveForBugreport_Triggers
+#endif
+TEST_F(PerfettoCmdlineTest, MAYBE_SaveForBugreport_Triggers) {
+ TraceConfig trace_config = CreateTraceConfigForBugreportTest();
+ trace_config.set_duration_ms(0); // set_trigger_timeout_ms is used instead.
+ auto* trigger_config = trace_config.mutable_trigger_config();
+ trigger_config->set_trigger_timeout_ms(8.64e+7);
+ trigger_config->set_trigger_mode(TraceConfig::TriggerConfig::STOP_TRACING);
+ auto* trigger = trigger_config->add_triggers();
+ trigger->set_name("trigger_name");
+ trigger->set_stop_delay_ms(1);
+ RunBugreportTest(std::move(trace_config), /*check_original_trace=*/false);
+}
+
} // namespace perfetto
diff --git a/test/configs/bugreport.cfg b/test/configs/bugreport.cfg
new file mode 100644
index 0000000..d42fd61
--- /dev/null
+++ b/test/configs/bugreport.cfg
@@ -0,0 +1,30 @@
+bugreport_score: 100
+duration_ms: 600000
+
+buffers {
+ size_kb: 32768
+ fill_policy: RING_BUFFER
+}
+
+data_sources {
+ config {
+ name: "linux.ftrace"
+ target_buffer: 0
+ ftrace_config {
+ ftrace_events: "sched/sched_switch"
+ ftrace_events: "power/suspend_resume"
+ ftrace_events: "sched/sched_process_exit"
+ ftrace_events: "sched/sched_process_free"
+ ftrace_events: "task/task_newtask"
+ ftrace_events: "task/task_rename"
+ ftrace_events: "sched/sched_wakeup"
+ }
+ }
+}
+
+data_sources {
+ config {
+ name: "linux.process_stats"
+ target_buffer: 0
+ }
+}
diff --git a/test/test_helper.cc b/test/test_helper.cc
index 2267dc0..cbc3aca 100644
--- a/test/test_helper.cc
+++ b/test/test_helper.cc
@@ -156,18 +156,6 @@
return success;
}
-bool TestHelper::SaveTraceForBugreportAndWait() {
- bool success = false;
- auto checkpoint = CreateCheckpoint("bugreport");
- auto callback = [&success, checkpoint](bool s, const std::string&) {
- success = s;
- checkpoint();
- };
- endpoint_->SaveTraceForBugreport(callback);
- RunUntilCheckpoint("bugreport");
- return success;
-}
-
void TestHelper::CreateProducerProvidedSmb() {
fake_producer_thread_.CreateProducerProvidedSmb();
}
diff --git a/test/test_helper.h b/test/test_helper.h
index fda54b0..c8d1b48 100644
--- a/test/test_helper.h
+++ b/test/test_helper.h
@@ -40,6 +40,8 @@
#if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
#include "src/tracing/ipc/shared_memory_windows.h"
#else
+#include <signal.h>
+
#include "src/traced/probes/probes_producer.h"
#include "src/tracing/ipc/posix_shared_memory.h"
#endif
@@ -309,7 +311,6 @@
void FreeBuffers();
void DetachConsumer(const std::string& key);
bool AttachConsumer(const std::string& key);
- bool SaveTraceForBugreportAndWait();
void CreateProducerProvidedSmb();
bool IsShmemProvidedByProducer();
void ProduceStartupEventBatch(const protos::gen::TestConfig& config);
@@ -427,6 +428,12 @@
constexpr bool kUseSystemBinaries = true;
#endif
+ auto pass_env = [](const std::string& var, base::Subprocess* proc) {
+ const char* val = getenv(var.c_str());
+ if (val)
+ proc->args.env.push_back(var + "=" + val);
+ };
+
std::vector<std::string>& cmd = subprocess_.args.exec_cmd;
if (kUseSystemBinaries) {
PERFETTO_CHECK(TestHelper::kDefaultMode ==
@@ -442,6 +449,9 @@
subprocess_.args.env.push_back(
std::string("PERFETTO_CONSUMER_SOCK_NAME=") +
TestHelper::GetDefaultModeConsumerSocketName());
+ pass_env("TMPDIR", &subprocess_);
+ pass_env("TMP", &subprocess_);
+ pass_env("TEMP", &subprocess_);
cmd.push_back(base::GetCurExecutableDir() + "/" + argv0);
cmd.insert(cmd.end(), args.begin(), args.end());
}
@@ -479,6 +489,16 @@
sync_pipe_.rd.reset();
}
+ void SendSigterm() {
+#ifdef SIGTERM
+ kill(subprocess_.pid(), SIGTERM);
+#else
+ // This code is never used on Windows tests, not bothering.
+ if (subprocess_.pid()) // Always true, but avoids Wnoreturn compile errors.
+ PERFETTO_FATAL("SendSigterm() not implemented on this platform");
+#endif
+ }
+
private:
base::Subprocess subprocess_;
base::Pipe sync_pipe_;
