Merge "tp: Enable DISTINCT in-house DISTINCT in Perfetto" into main
diff --git a/Android.bp b/Android.bp
index 73b4ae5..4fe5a6a 100644
--- a/Android.bp
+++ b/Android.bp
@@ -5237,6 +5237,7 @@
"protos/perfetto/metrics/android/android_trusty_workqueues.proto",
"protos/perfetto/metrics/android/anr_metric.proto",
"protos/perfetto/metrics/android/app_process_starts_metric.proto",
+ "protos/perfetto/metrics/android/auto_metric.proto",
"protos/perfetto/metrics/android/batt_metric.proto",
"protos/perfetto/metrics/android/binder_metric.proto",
"protos/perfetto/metrics/android/camera_metric.proto",
@@ -5328,6 +5329,7 @@
"protos/perfetto/metrics/android/android_trusty_workqueues.proto",
"protos/perfetto/metrics/android/anr_metric.proto",
"protos/perfetto/metrics/android/app_process_starts_metric.proto",
+ "protos/perfetto/metrics/android/auto_metric.proto",
"protos/perfetto/metrics/android/batt_metric.proto",
"protos/perfetto/metrics/android/binder_metric.proto",
"protos/perfetto/metrics/android/camera_metric.proto",
@@ -5403,6 +5405,7 @@
"protos/perfetto/metrics/android/android_trusty_workqueues.proto",
"protos/perfetto/metrics/android/anr_metric.proto",
"protos/perfetto/metrics/android/app_process_starts_metric.proto",
+ "protos/perfetto/metrics/android/auto_metric.proto",
"protos/perfetto/metrics/android/batt_metric.proto",
"protos/perfetto/metrics/android/binder_metric.proto",
"protos/perfetto/metrics/android/camera_metric.proto",
@@ -12961,6 +12964,7 @@
srcs: [
"src/trace_processor/perfetto_sql/stdlib/android/anrs.sql",
"src/trace_processor/perfetto_sql/stdlib/android/app_process_starts.sql",
+ "src/trace_processor/perfetto_sql/stdlib/android/auto/multiuser.sql",
"src/trace_processor/perfetto_sql/stdlib/android/battery.sql",
"src/trace_processor/perfetto_sql/stdlib/android/battery_stats.sql",
"src/trace_processor/perfetto_sql/stdlib/android/binder.sql",
@@ -13463,6 +13467,7 @@
"src/trace_redaction/filter_sched_waking_events.cc",
"src/trace_redaction/filter_task_rename.cc",
"src/trace_redaction/find_package_uid.cc",
+ "src/trace_redaction/modify_process_trees.cc",
"src/trace_redaction/optimize_timeline.cc",
"src/trace_redaction/populate_allow_lists.cc",
"src/trace_redaction/process_thread_timeline.cc",
diff --git a/BUILD b/BUILD
index 2f53ea5..5d1e5be 100644
--- a/BUILD
+++ b/BUILD
@@ -2412,6 +2412,14 @@
],
)
+# GN target: //src/trace_processor/perfetto_sql/stdlib/android/auto:auto
+perfetto_filegroup(
+ name = "src_trace_processor_perfetto_sql_stdlib_android_auto_auto",
+ srcs = [
+ "src/trace_processor/perfetto_sql/stdlib/android/auto/multiuser.sql",
+ ],
+)
+
# GN target: //src/trace_processor/perfetto_sql/stdlib/android/frames:frames
perfetto_filegroup(
name = "src_trace_processor_perfetto_sql_stdlib_android_frames_frames",
@@ -2662,6 +2670,7 @@
name = "src_trace_processor_perfetto_sql_stdlib_stdlib",
deps = [
":src_trace_processor_perfetto_sql_stdlib_android_android",
+ ":src_trace_processor_perfetto_sql_stdlib_android_auto_auto",
":src_trace_processor_perfetto_sql_stdlib_android_frames_frames",
":src_trace_processor_perfetto_sql_stdlib_android_startup_startup",
":src_trace_processor_perfetto_sql_stdlib_chrome_chrome_sql",
@@ -4462,6 +4471,7 @@
"protos/perfetto/metrics/android/android_trusty_workqueues.proto",
"protos/perfetto/metrics/android/anr_metric.proto",
"protos/perfetto/metrics/android/app_process_starts_metric.proto",
+ "protos/perfetto/metrics/android/auto_metric.proto",
"protos/perfetto/metrics/android/batt_metric.proto",
"protos/perfetto/metrics/android/binder_metric.proto",
"protos/perfetto/metrics/android/camera_metric.proto",
diff --git a/include/perfetto/ext/base/flat_hash_map.h b/include/perfetto/ext/base/flat_hash_map.h
index 0be192d..f4046ea 100644
--- a/include/perfetto/ext/base/flat_hash_map.h
+++ b/include/perfetto/ext/base/flat_hash_map.h
@@ -23,7 +23,6 @@
#include "perfetto/ext/base/utils.h"
#include <algorithm>
-#include <functional>
#include <limits>
namespace perfetto {
@@ -45,6 +44,11 @@
// tsl::robin_map: 931,403,397 ns 243.622M insertions/s
// absl::flat_hash_map: 998,013,459 ns 227.379M insertions/s
// FollyF14FastMap: 1,181,480,602 ns 192.074M insertions/s
+//
+// TODO(primiano): the table regresses for heavy insert+erase workloads since we
+// don't clean up tombstones outside of resizes. In the limit, the entire
+// table's capacity is made up of values/tombstones, so each search has to
+// exhaustively scan the full capacity.
// The structs below define the probing algorithm used to probe slots upon a
// collision. They are guaranteed to visit all slots as our table size is always
diff --git a/include/perfetto/ext/base/hash.h b/include/perfetto/ext/base/hash.h
index 548003e..7809ed7 100644
--- a/include/perfetto/ext/base/hash.h
+++ b/include/perfetto/ext/base/hash.h
@@ -62,7 +62,7 @@
// Allow hashing anything that has a |data| field, a |size| field,
// and has the kHashable trait (e.g., base::StringView).
- template <typename T, typename = std::enable_if<T::kHashable>>
+ template <typename T, typename = std::enable_if_t<T::kHashable>>
void Update(const T& t) {
Update(t.data(), t.size());
}
diff --git a/protos/perfetto/metrics/android/BUILD.gn b/protos/perfetto/metrics/android/BUILD.gn
index 2bb8c0e..af7aa8f 100644
--- a/protos/perfetto/metrics/android/BUILD.gn
+++ b/protos/perfetto/metrics/android/BUILD.gn
@@ -33,6 +33,7 @@
"android_trusty_workqueues.proto",
"anr_metric.proto",
"app_process_starts_metric.proto",
+ "auto_metric.proto",
"batt_metric.proto",
"binder_metric.proto",
"camera_metric.proto",
diff --git a/protos/perfetto/metrics/android/auto_metric.proto b/protos/perfetto/metrics/android/auto_metric.proto
new file mode 100644
index 0000000..d7043fe
--- /dev/null
+++ b/protos/perfetto/metrics/android/auto_metric.proto
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2024 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.
+ */
+
+syntax = "proto2";
+
+package perfetto.protos;
+
+// Metrics for Multiuser events, such as switching users.
+message AndroidAutoMultiuserMetric {
+ message EventData {
+ // Id of the user the system has been switched to
+ optional int32 user_id = 1;
+
+ // The end event name for which the duration_ms is measured
+ optional string start_event = 2;
+
+ // The end event name for which the duration_ms is measured
+ optional string end_event = 3;
+
+ // Duration of the event (in milliseconds).
+ optional int64 duration_ms = 4;
+
+ // Previous user resource usage info during user switch
+ message UserData {
+ // Id of the user the system has been switched from
+ optional int32 user_id = 1;
+
+ optional int64 total_cpu_time_ms = 2;
+
+ optional int64 total_memory_usage_kb = 3;
+ }
+
+ optional UserData previous_user_info = 5;
+ }
+
+ // Metrics for a user switch.
+ repeated EventData user_switch = 1;
+}
diff --git a/protos/perfetto/metrics/metrics.proto b/protos/perfetto/metrics/metrics.proto
index e5a9553..4348187 100644
--- a/protos/perfetto/metrics/metrics.proto
+++ b/protos/perfetto/metrics/metrics.proto
@@ -22,6 +22,7 @@
import "protos/perfetto/metrics/android/android_boot.proto";
import "protos/perfetto/metrics/android/android_boot_unagg.proto";
import "protos/perfetto/metrics/android/android_garbage_collection_unagg_metric.proto";
+import "protos/perfetto/metrics/android/auto_metric.proto";
import "protos/perfetto/metrics/android/sysui_notif_shade_list_builder_metric.proto";
import "protos/perfetto/metrics/android/sysui_update_notif_on_ui_mode_changed_metric.proto";
import "protos/perfetto/metrics/android/android_frame_timeline_metric.proto";
@@ -298,7 +299,7 @@
// Multiuser - metrics for switching users.
// Specific for Android Auto
- optional AndroidMultiuserMetric android_auto_multiuser = 64;
+ optional AndroidAutoMultiuserMetric android_auto_multiuser = 64;
// All blocking calls (e.g. binder calls) for a trace.
optional AndroidBlockingCallsUnagg android_blocking_calls_unagg = 65;
diff --git a/protos/perfetto/metrics/perfetto_merged_metrics.proto b/protos/perfetto/metrics/perfetto_merged_metrics.proto
index a4da778..8f736b3 100644
--- a/protos/perfetto/metrics/perfetto_merged_metrics.proto
+++ b/protos/perfetto/metrics/perfetto_merged_metrics.proto
@@ -474,6 +474,42 @@
}
// End of protos/perfetto/metrics/android/anr_metric.proto
+// Begin of protos/perfetto/metrics/android/auto_metric.proto
+
+// Metrics for Multiuser events, such as switching users.
+message AndroidAutoMultiuserMetric {
+ message EventData {
+ // Id of the user the system has been switched to
+ optional int32 user_id = 1;
+
+ // The end event name for which the duration_ms is measured
+ optional string start_event = 2;
+
+ // The end event name for which the duration_ms is measured
+ optional string end_event = 3;
+
+ // Duration of the event (in milliseconds).
+ optional int64 duration_ms = 4;
+
+ // Previous user resource usage info during user switch
+ message UserData {
+ // Id of the user the system has been switched from
+ optional int32 user_id = 1;
+
+ optional int64 total_cpu_time_ms = 2;
+
+ optional int64 total_memory_usage_kb = 3;
+ }
+
+ optional UserData previous_user_info = 5;
+ }
+
+ // Metrics for a user switch.
+ repeated EventData user_switch = 1;
+}
+
+// End of protos/perfetto/metrics/android/auto_metric.proto
+
// Begin of protos/perfetto/metrics/android/batt_metric.proto
message AndroidBatteryMetric {
@@ -2822,7 +2858,7 @@
// Multiuser - metrics for switching users.
// Specific for Android Auto
- optional AndroidMultiuserMetric android_auto_multiuser = 64;
+ optional AndroidAutoMultiuserMetric android_auto_multiuser = 64;
// All blocking calls (e.g. binder calls) for a trace.
optional AndroidBlockingCallsUnagg android_blocking_calls_unagg = 65;
diff --git a/python/perfetto/trace_processor/metrics.descriptor b/python/perfetto/trace_processor/metrics.descriptor
index 59b0f2a..9c1dfa0 100644
--- a/python/perfetto/trace_processor/metrics.descriptor
+++ b/python/perfetto/trace_processor/metrics.descriptor
Binary files differ
diff --git a/src/trace_processor/importers/ftrace/binder_tracker.cc b/src/trace_processor/importers/ftrace/binder_tracker.cc
index 29d0c2c..48cdf58 100644
--- a/src/trace_processor/importers/ftrace/binder_tracker.cc
+++ b/src/trace_processor/importers/ftrace/binder_tracker.cc
@@ -296,7 +296,7 @@
transaction.args_inserter = args_inserter;
transaction.send_track_id = track_id;
transaction.send_slice_id = insert_slice();
- outstanding_transactions_.Insert(transaction_id, std::move(transaction));
+ outstanding_transactions_[transaction_id] = std::move(transaction);
auto* frame = GetTidTopFrame(tid);
if (frame) {
if (frame->state == TxnFrame::kSndAfterBC_TRANSACTION) {
@@ -315,18 +315,16 @@
void BinderTracker::TransactionReceived(int64_t ts,
uint32_t pid,
int32_t transaction_id) {
- const OutstandingTransaction* opt_transaction =
- outstanding_transactions_.Find(transaction_id);
- if (!opt_transaction) {
+ auto it = outstanding_transactions_.find(transaction_id);
+ if (it == outstanding_transactions_.end()) {
// If we don't know what type of transaction it is, we don't know how to
// insert the slice.
// TODO(lalitm): maybe we should insert a dummy slice anyway - seems like
// a questionable idea to just ignore these completely.
return;
}
-
- OutstandingTransaction transaction(std::move(*opt_transaction));
- outstanding_transactions_.Erase(transaction_id);
+ OutstandingTransaction transaction(std::move(it->second));
+ outstanding_transactions_.erase(it);
UniqueTid utid = context_->process_tracker->GetOrCreateThread(pid);
TrackId track_id = context_->track_tracker->InternThreadTrack(utid);
diff --git a/src/trace_processor/importers/ftrace/binder_tracker.h b/src/trace_processor/importers/ftrace/binder_tracker.h
index f1d682c..5912904 100644
--- a/src/trace_processor/importers/ftrace/binder_tracker.h
+++ b/src/trace_processor/importers/ftrace/binder_tracker.h
@@ -95,6 +95,8 @@
bool utid_stacks_empty() const { return utid_stacks_.size() == 0; }
private:
+ TraceProcessorContext* const context_;
+
struct OutstandingTransaction {
bool is_reply = false;
bool is_oneway = false;
@@ -102,10 +104,10 @@
std::optional<TrackId> send_track_id;
std::optional<SliceId> send_slice_id;
};
+ // TODO(rsavitski): switch back to FlatHashMap once the latter's perf is fixed
+ // for insert+erase heavy workfloads.
+ std::unordered_map<int32_t, OutstandingTransaction> outstanding_transactions_;
- TraceProcessorContext* const context_;
-
- base::FlatHashMap<int32_t, OutstandingTransaction> outstanding_transactions_;
struct TxnFrame {
// The state of this thread at this stack level.
enum State : uint32_t;
diff --git a/src/trace_processor/metrics/sql/android/android_auto_multiuser.sql b/src/trace_processor/metrics/sql/android/android_auto_multiuser.sql
index 3b12305..2be5ef0 100644
--- a/src/trace_processor/metrics/sql/android/android_auto_multiuser.sql
+++ b/src/trace_processor/metrics/sql/android/android_auto_multiuser.sql
@@ -14,51 +14,26 @@
-- limitations under the License.
--
-INCLUDE PERFETTO MODULE android.startup.startups;
-
--- Collect the last ts for user switch event.
--- The metric should represent time elapsed between
--- the latest user start and the latest carlauncher startup.
-DROP VIEW IF EXISTS auto_multiuser_events;
-CREATE PERFETTO VIEW auto_multiuser_events AS
-SELECT
- user_start_time_ns AS event_start_time_ns,
- launcher_end_time_ns AS event_end_time_ns
-FROM
- (
- SELECT MAX(slice.ts) as user_start_time_ns
- FROM slice
- WHERE (
- slice.name GLOB "UserController.startUser*"
- AND slice.name NOT GLOB "UserController.startUser-10*"
- )
- ),
- (
- SELECT ts_end AS launcher_end_time_ns
- FROM android_startups
- WHERE (package = 'com.android.car.carlauncher')
- );
-
--- Precompute user switch duration time.
--- Take only positive duration values(user start ts < carlauncher start ts)
--- If there are potential duplicates in carlauncher startup,
--- take the smallest value. It represents the closest carlaucnher startup
-DROP TABLE IF EXISTS android_auto_multiuser_timing;
-CREATE PERFETTO TABLE android_auto_multiuser_timing AS
-SELECT
- cast_int!((event_end_time_ns - event_start_time_ns) / 1e6 + 0.5) as duration_ms
-FROM
- auto_multiuser_events
-WHERE duration_ms > 0
-ORDER BY duration_ms ASC
-LIMIT 1;
+INCLUDE PERFETTO MODULE android.auto.multiuser;
+INCLUDE PERFETTO MODULE time.conversion;
DROP VIEW IF EXISTS android_auto_multiuser_output;
CREATE PERFETTO VIEW android_auto_multiuser_output AS
-SELECT AndroidMultiuserMetric (
- 'user_switch', AndroidMultiuserMetric_EventData(
- 'duration_ms', (
- SELECT duration_ms FROM android_auto_multiuser_timing
+SELECT AndroidAutoMultiuserMetric(
+ 'user_switch', (
+ SELECT RepeatedField(
+ AndroidAutoMultiuserMetric_EventData(
+ 'user_id', cast_int!(event_start_user_id),
+ 'start_event', event_start_name,
+ 'end_event', event_end_name,
+ 'duration_ms', time_to_ms(duration),
+ 'previous_user_info', AndroidAutoMultiuserMetric_EventData_UserData(
+ 'user_id', user_id,
+ 'total_cpu_time_ms', time_to_ms(total_cpu_time),
+ 'total_memory_usage_kb', total_memory_usage_kb
+ )
+ )
)
+ FROM android_auto_multiuser_timing_with_previous_user_resource_usage
)
);
\ No newline at end of file
diff --git a/src/trace_processor/perfetto_sql/stdlib/android/BUILD.gn b/src/trace_processor/perfetto_sql/stdlib/android/BUILD.gn
index 786882e..bd2f18d 100644
--- a/src/trace_processor/perfetto_sql/stdlib/android/BUILD.gn
+++ b/src/trace_processor/perfetto_sql/stdlib/android/BUILD.gn
@@ -16,6 +16,7 @@
perfetto_sql_source_set("android") {
deps = [
+ "auto",
"frames",
"startup",
]
diff --git a/src/trace_processor/perfetto_sql/stdlib/android/auto/BUILD.gn b/src/trace_processor/perfetto_sql/stdlib/android/auto/BUILD.gn
new file mode 100644
index 0000000..bf52d69
--- /dev/null
+++ b/src/trace_processor/perfetto_sql/stdlib/android/auto/BUILD.gn
@@ -0,0 +1,21 @@
+# Copyright (C) 2024 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.
+
+import("../../../../../../gn/perfetto_sql.gni")
+
+perfetto_sql_source_set("auto") {
+ sources = [
+ "multiuser.sql",
+ ]
+}
diff --git a/src/trace_processor/perfetto_sql/stdlib/android/auto/multiuser.sql b/src/trace_processor/perfetto_sql/stdlib/android/auto/multiuser.sql
new file mode 100644
index 0000000..ea95dcc
--- /dev/null
+++ b/src/trace_processor/perfetto_sql/stdlib/android/auto/multiuser.sql
@@ -0,0 +1,173 @@
+--
+-- Copyright 2024 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
+--
+-- https://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.
+--
+
+INCLUDE PERFETTO MODULE android.startup.startups;
+
+-- Time elapsed between the latest user start
+-- and the specific end event
+-- like package startup(ex carlauncher) or previous user stop.
+CREATE PERFETTO TABLE android_auto_multiuser_timing(
+ -- Id of the started android user
+ event_start_user_id STRING,
+ -- Start event time
+ event_start_time INT,
+ -- End event time
+ event_end_time INT,
+ -- End event name
+ event_end_name STRING,
+ -- Start event name
+ event_start_name STRING,
+ -- User switch duration from start event
+ -- to end event
+ duration LONG
+) AS
+-- The last ts for user switch event.
+WITH auto_multiuser_user_start AS (
+ SELECT
+ slice.name as event_start_name,
+ slice.ts AS user_start_time,
+ -- Ex.: UserController.startUser-11-fg-start-mode-1
+ -- User is enclosed in dashes and will be at most 2 characters(10, 11, etc.)
+ SUBSTR(name, INSTR(name, '-') + 1, 2) as started_user_id
+ FROM slice
+ WHERE (
+ slice.name GLOB "UserController.startUser*"
+ AND slice.name NOT GLOB "UserController.startUser-10*"
+ )
+ ORDER BY ts DESC
+ LIMIT 1
+)
+SELECT
+ started_user_id AS event_start_user_id,
+ user_start_time AS event_start_time,
+ event_end_time,
+ event_end_name,
+ event_start_name,
+ (event_end_time - user_start_time) as duration
+FROM (
+ SELECT
+ ts_end AS event_end_time,
+ package as event_end_name
+ FROM android_startups
+ UNION
+ SELECT
+ slice.ts as event_end_time,
+ slice.name as event_end_name
+ FROM slice
+ WHERE slice.name GLOB "finishUserStopped-10*"
+) as a
+JOIN auto_multiuser_user_start as b
+ON a.event_end_time > b.user_start_time;
+
+-- Previous user(user 10) total CPU time
+CREATE PERFETTO VIEW _android_auto_user_10_total_cpu_time AS
+SELECT
+ SUM(dur) as total_cpu_time,
+ (uid - android_appid) / 100000 as user_id,
+ event_end_name
+FROM sched_slice
+ JOIN thread USING (utid)
+ JOIN process USING (upid),
+ android_auto_multiuser_timing
+WHERE
+ user_id = 10
+ AND ts >= android_auto_multiuser_timing.event_start_time
+ AND ts <= android_auto_multiuser_timing.event_end_time
+GROUP BY event_end_name;
+
+-- Previous user(user 10) total memory usage
+CREATE PERFETTO VIEW _android_auto_user_10_total_memory AS
+WITH filtered_process AS (
+ SELECT
+ c.ts,
+ c.value,
+ p.name AS proc_name,
+ (uid - android_appid) / 100000 as user_id,
+ event_end_name
+ FROM counter AS c
+ LEFT JOIN process_counter_track AS t
+ ON c.track_id = t.id
+ LEFT JOIN process AS p USING (upid),
+ android_auto_multiuser_timing
+ WHERE
+ t.name GLOB "mem.rss"
+ AND user_id = 10
+ AND c.ts >= android_auto_multiuser_timing.event_start_time
+ AND c.ts <= android_auto_multiuser_timing.event_end_time
+),
+process_rss AS (
+ SELECT
+ *,
+ ifnull(
+ lag(value) OVER (PARTITION BY proc_name, event_end_name ORDER BY ts), value
+ ) AS prev_value
+ FROM filtered_process
+),
+per_process_allocations AS (
+ SELECT
+ proc_name,
+ SUM(value - prev_value) / 1e3 AS alloc_value_kb,
+ user_id,
+ event_end_name
+ FROM process_rss
+ WHERE value - prev_value > 0
+ GROUP BY proc_name, event_end_name
+ ORDER BY alloc_value_kb DESC
+)
+SELECT
+ cast_int!(SUM(alloc_value_kb)) AS total_memory_usage_kb,
+ user_id,
+ event_end_name
+FROM per_process_allocations
+GROUP BY event_end_name;
+
+-- This table extends `android_auto_multiuser_timing` table with previous user resource usage.
+CREATE PERFETTO VIEW android_auto_multiuser_timing_with_previous_user_resource_usage(
+ -- Start user id
+ event_start_user_id STRING,
+ -- Start event time
+ event_start_time INT,
+ -- End event time
+ event_end_time INT,
+ -- End event name
+ event_end_name STRING,
+ -- Start event name
+ event_start_name STRING,
+ -- User switch duration from start event
+ -- to end event
+ duration LONG,
+ -- User id
+ user_id INT,
+ -- Total CPU time for a user
+ total_cpu_time LONG,
+ -- Total memory user for a user
+ total_memory_usage_kb LONG
+) AS
+SELECT
+ a.event_start_user_id,
+ a.event_start_time,
+ a.event_end_time,
+ a.event_end_name,
+ a.event_start_name,
+ a.duration,
+ b.user_id,
+ b.total_cpu_time,
+ c.total_memory_usage_kb
+FROM android_auto_multiuser_timing as a
+LEFT JOIN _android_auto_user_10_total_cpu_time as b
+ ON a.event_end_name = b.event_end_name
+LEFT JOIN _android_auto_user_10_total_memory as c
+ ON a.event_end_name = c.event_end_name;
\ No newline at end of file
diff --git a/src/trace_redaction/BUILD.gn b/src/trace_redaction/BUILD.gn
index 037545d..8ff31a3 100644
--- a/src/trace_redaction/BUILD.gn
+++ b/src/trace_redaction/BUILD.gn
@@ -47,6 +47,8 @@
"find_package_uid.cc",
"find_package_uid.h",
"frame_cookie.h",
+ "modify_process_trees.cc",
+ "modify_process_trees.h",
"optimize_timeline.cc",
"optimize_timeline.h",
"populate_allow_lists.cc",
diff --git a/src/trace_redaction/modify_process_trees.cc b/src/trace_redaction/modify_process_trees.cc
new file mode 100644
index 0000000..099890b
--- /dev/null
+++ b/src/trace_redaction/modify_process_trees.cc
@@ -0,0 +1,114 @@
+/*
+ * Copyright (C) 2024 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.
+ */
+
+#include "src/trace_redaction/modify_process_trees.h"
+
+#include <string>
+
+#include "perfetto/base/status.h"
+#include "perfetto/protozero/field.h"
+#include "perfetto/protozero/scattered_heap_buffer.h"
+#include "src/trace_redaction/proto_util.h"
+#include "src/trace_redaction/trace_redaction_framework.h"
+
+#include "protos/perfetto/trace/ps/process_tree.pbzero.h"
+#include "protos/perfetto/trace/trace_packet.pbzero.h"
+
+namespace perfetto::trace_redaction {
+
+base::Status ModifyProcessTree::VerifyContext(const Context&) const {
+ return base::OkStatus();
+}
+
+base::Status ModifyProcessTree::Transform(const Context& context,
+ std::string* packet) const {
+ protozero::ProtoDecoder decoder(*packet);
+
+ auto process_tree =
+ decoder.FindField(protos::pbzero::TracePacket::kProcessTreeFieldNumber);
+
+ if (!process_tree.valid()) {
+ return base::OkStatus();
+ }
+
+ auto timestamp =
+ decoder.FindField(protos::pbzero::TracePacket::kTimestampFieldNumber);
+
+ protozero::HeapBuffered<protos::pbzero::TracePacket> packet_message;
+
+ for (auto field = decoder.ReadField(); field.valid();
+ field = decoder.ReadField()) {
+ if (field.id() == protos::pbzero::TracePacket::kProcessTreeFieldNumber) {
+ TransformProcessTree(context, timestamp, field,
+ packet_message->set_process_tree());
+ } else {
+ proto_util::AppendField(field, packet_message.get());
+ }
+ }
+
+ packet->assign(packet_message.SerializeAsString());
+
+ return base::OkStatus();
+}
+
+void ModifyProcessTree::TransformProcess(
+ const Context&,
+ const protozero::Field&,
+ const protozero::Field& process,
+ protos::pbzero::ProcessTree* process_tree) const {
+ PERFETTO_DCHECK(process.id() ==
+ protos::pbzero::ProcessTree::kProcessesFieldNumber);
+ proto_util::AppendField(process, process_tree);
+}
+
+void ModifyProcessTree::TransformThread(
+ const Context&,
+ const protozero::Field&,
+ const protozero::Field& thread,
+ protos::pbzero::ProcessTree* process_tree) const {
+ PERFETTO_DCHECK(thread.id() ==
+ protos::pbzero::ProcessTree::kThreadsFieldNumber);
+ proto_util::AppendField(thread, process_tree);
+}
+
+void ModifyProcessTree::TransformProcessTree(
+ const Context& context,
+ const protozero::Field& timestamp,
+ const protozero::Field& process_tree,
+ protos::pbzero::ProcessTree* message) const {
+ protozero::ProtoDecoder decoder(process_tree.as_bytes());
+
+ for (auto field = decoder.ReadField(); field.valid();
+ field = decoder.ReadField()) {
+ switch (field.id()) {
+ case protos::pbzero::ProcessTree::kProcessesFieldNumber:
+ TransformProcess(context, timestamp, field, message);
+ break;
+
+ case protos::pbzero::ProcessTree::kThreadsFieldNumber:
+ TransformThread(context, timestamp, field, message);
+ break;
+
+ default:
+ proto_util::AppendField(field, message);
+ break;
+ }
+ }
+
+ // TODO(vaage): Call the handler to add extra fields to the process tree.
+}
+
+} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/modify_process_trees.h b/src/trace_redaction/modify_process_trees.h
new file mode 100644
index 0000000..e36223e
--- /dev/null
+++ b/src/trace_redaction/modify_process_trees.h
@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2024 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_TRACE_REDACTION_MODIFY_PROCESS_TREES_H_
+#define SRC_TRACE_REDACTION_MODIFY_PROCESS_TREES_H_
+
+#include <string>
+
+#include "perfetto/base/status.h"
+#include "src/trace_redaction/trace_redaction_framework.h"
+
+#include "protos/perfetto/trace/ps/process_tree.pbzero.h"
+
+namespace perfetto::trace_redaction {
+
+// Walk through process trees, calling process and thread handlers to add new
+// process and threads messages to the process tree. If the default handler is
+// not replaced, the thread/process will be added to the parent.
+class ModifyProcessTree : public TransformPrimitive {
+ public:
+ base::Status Transform(const Context& context,
+ std::string* packet) const override;
+
+ protected:
+ // Verifies that the context contains required values. No-op by default.
+ virtual base::Status VerifyContext(const Context& context) const;
+
+ // Modifies a process before adding it back to the process tree. Appends the
+ // field to the process tree without modification by default.
+ virtual void TransformProcess(
+ const Context& context,
+ const protozero::Field& timestamp,
+ const protozero::Field& process,
+ protos::pbzero::ProcessTree* process_tree) const;
+
+ // Modifies a thread before adding it back to the process tree. Appends the
+ // field to the process tree without modification by default.
+ virtual void TransformThread(
+ const Context& context,
+ const protozero::Field& timestamp,
+ const protozero::Field& thread,
+ protos::pbzero::ProcessTree* process_trees) const;
+
+ // TODO(vaage): Add a handler that is called the process tree is populated so
+ // that fields can be added to process tree (e.g. creating new threads -
+ // needed for thread merging).
+
+ private:
+ void TransformProcessTree(const Context& context,
+ const protozero::Field& timestamp,
+ const protozero::Field& process_tree,
+ protos::pbzero::ProcessTree* message) const;
+};
+
+} // namespace perfetto::trace_redaction
+
+#endif // SRC_TRACE_REDACTION_MODIFY_PROCESS_TREES_H_
diff --git a/src/trace_redaction/populate_allow_lists.cc b/src/trace_redaction/populate_allow_lists.cc
index b2c9431..11d895b 100644
--- a/src/trace_redaction/populate_allow_lists.cc
+++ b/src/trace_redaction/populate_allow_lists.cc
@@ -84,24 +84,38 @@
context->trace_packet_allow_list.insert(item);
}
+ // FTRACE EVENT NOTES
+ //
+ // Dma events (kDmaHeapStatFieldNumber) are global events and are not
+ // emitted within a process context (they are centrally allocated by the
+ // HAL process). We drop them for now as we don't have the required
+ // attribution info in the trace.
+ //
+ // ION events (e.g. kIonBufferCreateFieldNumber, kIonHeapGrowFieldNumber,
+ // etc.) are global events are not emitted within a process context (they
+ // are centrally allocated by the HAL process). We drop them for now as we
+ // don't have the required attribution info in the trace.
+ //
+ // TODO(vaage): The allowed rss stat events (i.e. kRssStatFieldNumber,
+ // kRssStatThrottledFieldNumber) are process-scoped. It is non-trivial to
+ // merge events, so all events outside of the target package should be
+ // dropped.
+ //
+ // TODO(vaage): kSchedBlockedReasonFieldNumber contains two pids, an outer
+ // and inner pid. A primitive is needed to further redact these events.
+
std::initializer_list<uint32_t> ftrace_events = {
- protos::pbzero::FtraceEvent::kSchedSwitchFieldNumber,
protos::pbzero::FtraceEvent::kCpuFrequencyFieldNumber,
protos::pbzero::FtraceEvent::kCpuIdleFieldNumber,
+ protos::pbzero::FtraceEvent::kPrintFieldNumber,
+ protos::pbzero::FtraceEvent::kRssStatFieldNumber,
+ protos::pbzero::FtraceEvent::kRssStatThrottledFieldNumber,
protos::pbzero::FtraceEvent::kSchedBlockedReasonFieldNumber,
+ protos::pbzero::FtraceEvent::kSchedProcessFreeFieldNumber,
+ protos::pbzero::FtraceEvent::kSchedSwitchFieldNumber,
protos::pbzero::FtraceEvent::kSchedWakingFieldNumber,
protos::pbzero::FtraceEvent::kTaskNewtaskFieldNumber,
protos::pbzero::FtraceEvent::kTaskRenameFieldNumber,
- protos::pbzero::FtraceEvent::kSchedProcessFreeFieldNumber,
- protos::pbzero::FtraceEvent::kRssStatFieldNumber,
- protos::pbzero::FtraceEvent::kIonHeapShrinkFieldNumber,
- protos::pbzero::FtraceEvent::kIonHeapGrowFieldNumber,
- protos::pbzero::FtraceEvent::kIonStatFieldNumber,
- protos::pbzero::FtraceEvent::kIonBufferCreateFieldNumber,
- protos::pbzero::FtraceEvent::kIonBufferDestroyFieldNumber,
- protos::pbzero::FtraceEvent::kDmaHeapStatFieldNumber,
- protos::pbzero::FtraceEvent::kRssStatThrottledFieldNumber,
- protos::pbzero::FtraceEvent::kPrintFieldNumber,
};
for (auto item : ftrace_events) {
diff --git a/src/trace_redaction/redact_ftrace_event.cc b/src/trace_redaction/redact_ftrace_event.cc
index 6c09236..a16a90d 100644
--- a/src/trace_redaction/redact_ftrace_event.cc
+++ b/src/trace_redaction/redact_ftrace_event.cc
@@ -30,68 +30,76 @@
base::Status RedactFtraceEvent::Transform(const Context& context,
std::string* packet) const {
- protozero::ConstBytes packet_bytes = {
- reinterpret_cast<const uint8_t*>(packet->data()), packet->size()};
-
protozero::HeapBuffered<protos::pbzero::TracePacket> message;
- RedactPacket(context, packet_bytes, message.get());
+ protozero::ProtoDecoder decoder(*packet);
+
+ // Treat FtraceEvents (bundle) as a special case.
+ for (auto f = decoder.ReadField(); f.valid(); f = decoder.ReadField()) {
+ if (f.id() == protos::pbzero::TracePacket::kFtraceEventsFieldNumber) {
+ RedactEvents(context, f, message->set_ftrace_events());
+ } else {
+ proto_util::AppendField(f, message.get());
+ }
+ }
+
packet->assign(message.SerializeAsString());
return base::OkStatus();
}
-// Iterate over every field in a packet, treating FtraceEvents (bundle) as a
-// special case.
-void RedactFtraceEvent::RedactPacket(
- const Context& context,
- protozero::ConstBytes bytes,
- protos::pbzero::TracePacket* message) const {
- protozero::ProtoDecoder decoder(bytes);
-
- for (auto field = decoder.ReadField(); field.valid();
- field = decoder.ReadField()) {
- if (field.id() == protos::pbzero::TracePacket::kFtraceEventsFieldNumber) {
- RedactEvents(context, field.as_bytes(), message->set_ftrace_events());
- } else {
- proto_util::AppendField(field, message);
- }
- }
-}
-
// Iterate over every field in FtraceEvents (bundle), treating FtraceEvent as a
// special case (calls the correct redaction).
void RedactFtraceEvent::RedactEvents(
const Context& context,
- protozero::ConstBytes bytes,
+ protozero::Field bundle,
protos::pbzero::FtraceEventBundle* message) const {
- protozero::ProtoDecoder decoder(bytes);
+ PERFETTO_DCHECK(bundle.id() ==
+ protos::pbzero::TracePacket::kFtraceEventsFieldNumber);
- for (auto field = decoder.ReadField(); field.valid();
- field = decoder.ReadField()) {
- if (field.id() == protos::pbzero::FtraceEventBundle::kEventFieldNumber) {
- RedactEvent(context, field.as_bytes(), message->add_event());
+ // There is only one bundle per packet, so creating the bundle decoder is an
+ // "okay" expense.
+ protos::pbzero::FtraceEventBundle::Decoder bundle_decoder(bundle.as_bytes());
+
+ // Even through we have `bundle_decoder` create a simpler decoder to iterate
+ // over every field.
+ protozero::ProtoDecoder decoder(bundle.as_bytes());
+
+ // Treat FtraceEvent as a special case.
+ for (auto f = decoder.ReadField(); f.valid(); f = decoder.ReadField()) {
+ if (f.id() == protos::pbzero::FtraceEventBundle::kEventFieldNumber) {
+ RedactEvent(context, bundle_decoder, f, message->add_event());
} else {
- proto_util::AppendField(field, message);
+ proto_util::AppendField(f, message);
}
}
}
void RedactFtraceEvent::RedactEvent(
const Context& context,
- protozero::ConstBytes bytes,
+ const protos::pbzero::FtraceEventBundle::Decoder& bundle,
+ protozero::Field event,
protos::pbzero::FtraceEvent* message) const {
- protozero::ProtoDecoder event(bytes);
+ PERFETTO_DCHECK(event.id() ==
+ protos::pbzero::FtraceEventBundle::kEventFieldNumber);
- for (auto field = event.ReadField(); field.valid();
- field = event.ReadField()) {
- auto* mod = redactions_.Find(field.id());
+ // A modifier can/will change the decoder by calling ReadField(). To avoid a
+ // modifier from interfering with the this function's loop, a reusable decoder
+ // is used for each modifier call.
+ protozero::ProtoDecoder outer_decoder(event.as_bytes());
+ protozero::ProtoDecoder inner_decoder(event.as_bytes());
+ // If there is a handler for a field, treat it as a special case.
+ for (auto f = outer_decoder.ReadField(); f.valid();
+ f = outer_decoder.ReadField()) {
+ auto* mod = redactions_.Find(f.id());
if (mod && mod->get()) {
- protos::pbzero::FtraceEvent::Decoder event_decoder(bytes);
- mod->get()->Redact(context, event_decoder, field.as_bytes(), message);
+ // Reset the decoder so that it appears like a "new" decoder to the
+ // modifier.
+ inner_decoder.Reset();
+ mod->get()->Redact(context, bundle, inner_decoder, message);
} else {
- proto_util::AppendField(field, message);
+ proto_util::AppendField(f, message);
}
}
}
diff --git a/src/trace_redaction/redact_ftrace_event.h b/src/trace_redaction/redact_ftrace_event.h
index fe282cf..9de6cc6 100644
--- a/src/trace_redaction/redact_ftrace_event.h
+++ b/src/trace_redaction/redact_ftrace_event.h
@@ -18,11 +18,16 @@
#define SRC_TRACE_REDACTION_REDACT_FTRACE_EVENT_H_
#include <cstdint>
+#include <memory>
+#include <string>
+#include "perfetto/base/status.h"
#include "perfetto/ext/base/flat_hash_map.h"
+#include "perfetto/protozero/field.h"
#include "src/trace_redaction/trace_redaction_framework.h"
#include "protos/perfetto/trace/ftrace/ftrace_event.pbzero.h"
+#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.pbzero.h"
namespace perfetto::trace_redaction {
@@ -33,11 +38,14 @@
public:
virtual ~FtraceEventRedaction();
- // Write a new version of the event to the message.
+ // Write at most one field from `event` to `event_message`. This relies on the
+ // honor system; other redactions may be registered on other values.
+ //
+ // - event: effectively "protos::pbzero::FtraceEvent::Decoder"
virtual base::Status Redact(
const Context& context,
- const protos::pbzero::FtraceEvent::Decoder& event,
- protozero::ConstBytes bytes,
+ const protos::pbzero::FtraceEventBundle::Decoder& bundle,
+ protozero::ProtoDecoder& event,
protos::pbzero::FtraceEvent* event_message) const = 0;
};
@@ -46,23 +54,21 @@
base::Status Transform(const Context& context,
std::string* packet) const override;
- // Add a new redaction. T must extend FtraceEventRedaction.
- template <uint32_t field_id, typename T>
+ // Add a new redaction. T must extend FtraceEventRedaction. This relies on the
+ // honor system; no more than one redaction can be mapped to a field.
+ template <uint32_t field_id, class T>
void emplace_back() {
redactions_.Insert(field_id, std::make_unique<T>());
}
private:
- void RedactPacket(const Context& context,
- protozero::ConstBytes bytes,
- protos::pbzero::TracePacket* message) const;
-
void RedactEvents(const Context& context,
- protozero::ConstBytes bytes,
+ protozero::Field bundle,
protos::pbzero::FtraceEventBundle* message) const;
void RedactEvent(const Context& context,
- protozero::ConstBytes bytes,
+ const protos::pbzero::FtraceEventBundle::Decoder& bundle,
+ protozero::Field event,
protos::pbzero::FtraceEvent* message) const;
base::FlatHashMap<uint32_t, std::unique_ptr<FtraceEventRedaction>>
diff --git a/src/trace_redaction/redact_process_free.cc b/src/trace_redaction/redact_process_free.cc
index 7294d68..8f5a83e 100644
--- a/src/trace_redaction/redact_process_free.cc
+++ b/src/trace_redaction/redact_process_free.cc
@@ -44,20 +44,19 @@
// of this, the timeline is not needed.
base::Status RedactProcessFree::Redact(
const Context&,
- const protos::pbzero::FtraceEvent::Decoder&,
- protozero::ConstBytes bytes,
+ const protos::pbzero::FtraceEventBundle::Decoder&,
+ protozero::ProtoDecoder& event,
protos::pbzero::FtraceEvent* event_message) const {
- // SchedProcessFreeFtraceEvent
- protozero::ProtoDecoder process_free_decoder(bytes);
-
- // There must be pid. If there's no pid, the safest option is to drop it.
- auto pid = process_free_decoder.FindField(
- protos::pbzero::SchedProcessFreeFtraceEvent::kPidFieldNumber);
-
- if (!pid.valid()) {
- return base::OkStatus();
+ auto sched_process_free = event.FindField(
+ protos::pbzero::FtraceEvent::kSchedProcessFreeFieldNumber);
+ if (!sched_process_free.valid()) {
+ return base::ErrStatus(
+ "RedactProcessFree: was used for unsupported field type");
}
+ // SchedProcessFreeFtraceEvent
+ protozero::ProtoDecoder process_free_decoder(sched_process_free.as_bytes());
+
auto* process_free_message = event_message->set_sched_process_free();
// Replace the comm with an empty string instead of dropping the comm field.
diff --git a/src/trace_redaction/redact_process_free.h b/src/trace_redaction/redact_process_free.h
index 8d24153..19e954c 100644
--- a/src/trace_redaction/redact_process_free.h
+++ b/src/trace_redaction/redact_process_free.h
@@ -31,8 +31,8 @@
base::Status Redact(
const Context& context,
- const protos::pbzero::FtraceEvent::Decoder& event,
- protozero::ConstBytes bytes,
+ const protos::pbzero::FtraceEventBundle::Decoder& bundle,
+ protozero::ProtoDecoder& event,
protos::pbzero::FtraceEvent* event_message) const override;
};
diff --git a/src/trace_redaction/redact_process_free_unittest.cc b/src/trace_redaction/redact_process_free_unittest.cc
index ee119ca..04064ae 100644
--- a/src/trace_redaction/redact_process_free_unittest.cc
+++ b/src/trace_redaction/redact_process_free_unittest.cc
@@ -29,66 +29,72 @@
namespace perfetto::trace_redaction {
-class RedactProcessFreeTest : public testing::Test {};
+class RedactProcessFreeTest : public testing::Test {
+ protected:
+ void SetUp() override {
+ auto* source_event = bundle.add_event();
+ source_event->set_timestamp(123456789);
+ source_event->set_pid(10);
+ }
-TEST_F(RedactProcessFreeTest, ClearsComm) {
- protos::gen::FtraceEvent source_event;
- source_event.set_timestamp(123456789);
- source_event.set_pid(10);
+ base::Status Redact(protos::pbzero::FtraceEvent* event_message) {
+ RedactProcessFree redact;
+ Context context;
- auto* process_free = source_event.mutable_sched_process_free();
+ auto bundle_str = bundle.SerializeAsString();
+ protos::pbzero::FtraceEventBundle::Decoder bundle_decoder(bundle_str);
+
+ auto event_str = bundle.event().back().SerializeAsString();
+ protos::pbzero::FtraceEvent::Decoder event_decoder(event_str);
+
+ return redact.Redact(context, bundle_decoder, event_decoder, event_message);
+ }
+
+ protos::gen::FtraceEventBundle bundle;
+};
+
+// A free event will always test as "not active". So the comm value should
+// always be replaced with an empty string.
+TEST_F(RedactProcessFreeTest, ClearsCommValue) {
+ auto* process_free =
+ bundle.mutable_event()->back().mutable_sched_process_free();
process_free->set_comm("comm-a");
process_free->set_pid(11);
- RedactProcessFree redact;
- Context context;
-
- protos::pbzero::FtraceEvent::Decoder event_decoder(
- source_event.SerializeAsString());
protozero::HeapBuffered<protos::pbzero::FtraceEvent> event_message;
- auto result =
- redact.Redact(context, event_decoder, event_decoder.sched_switch(),
- event_message.get());
+ auto result = Redact(event_message.get());
ASSERT_OK(result) << result.c_message();
protos::gen::FtraceEvent redacted_event;
redacted_event.ParseFromString(event_message.SerializeAsString());
// No process free event should have been added to the ftrace event.
- ASSERT_FALSE(redacted_event.has_sched_process_free());
+ ASSERT_TRUE(redacted_event.has_sched_process_free());
+ ASSERT_TRUE(redacted_event.sched_process_free().has_comm());
+ ASSERT_TRUE(redacted_event.sched_process_free().comm().empty());
}
-// Even if there is no pid in the process free event, the process free event
-// should still exist but no comm value should be present.
+// Even if there is no pid in the process free event, the comm value should be
+// replaced with an empty string.
TEST_F(RedactProcessFreeTest, NoPidClearsEvent) {
- protos::gen::FtraceEvent source_event;
- source_event.set_timestamp(123456789);
- source_event.set_pid(10);
-
- // Don't add a pid. This should stop the process free event from being added
- // to the event message.
- auto* process_free = source_event.mutable_sched_process_free();
+ // Don't add a pid. This should have no change in behaviour.
+ auto* process_free =
+ bundle.mutable_event()->back().mutable_sched_process_free();
process_free->set_comm("comm-a");
- RedactProcessFree redact;
- Context context;
-
- protos::pbzero::FtraceEvent::Decoder event_decoder(
- source_event.SerializeAsString());
protozero::HeapBuffered<protos::pbzero::FtraceEvent> event_message;
- // Even if the process free event has been dropped, there should be no
- // resulting error.
- auto result =
- redact.Redact(context, event_decoder, event_decoder.sched_switch(),
- event_message.get());
+ auto result = Redact(event_message.get());
ASSERT_OK(result) << result.c_message();
protos::gen::FtraceEvent redacted_event;
redacted_event.ParseFromString(event_message.SerializeAsString());
- ASSERT_FALSE(redacted_event.has_sched_process_free());
+ // No process free event should have been added to the ftrace event.
+ ASSERT_TRUE(redacted_event.has_sched_process_free());
+ ASSERT_TRUE(redacted_event.sched_process_free().has_comm());
+ ASSERT_TRUE(redacted_event.sched_process_free().comm().empty());
}
} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/redact_sched_switch.cc b/src/trace_redaction/redact_sched_switch.cc
index 361993d..8dca030 100644
--- a/src/trace_redaction/redact_sched_switch.cc
+++ b/src/trace_redaction/redact_sched_switch.cc
@@ -24,6 +24,20 @@
namespace perfetto::trace_redaction {
+namespace {
+
+protozero::ConstChars SanitizeCommValue(const Context& context,
+ ProcessThreadTimeline::Slice slice,
+ protozero::Field field) {
+ if (NormalizeUid(slice.uid) == NormalizeUid(context.package_uid.value())) {
+ return field.as_string();
+ }
+
+ return {};
+}
+
+} // namespace
+
// Redact sched switch trace events in an ftrace event bundle:
//
// event {
@@ -49,8 +63,8 @@
base::Status RedactSchedSwitch::Redact(
const Context& context,
- const protos::pbzero::FtraceEvent::Decoder& event,
- protozero::ConstBytes bytes,
+ const protos::pbzero::FtraceEventBundle::Decoder&,
+ protozero::ProtoDecoder& event,
protos::pbzero::FtraceEvent* event_message) const {
if (!context.package_uid.has_value()) {
return base::ErrStatus("RedactSchedSwitch: missing package uid");
@@ -60,11 +74,31 @@
return base::ErrStatus("RedactSchedSwitch: missing timeline");
}
- protos::pbzero::SchedSwitchFtraceEvent::Decoder sched_switch(bytes);
+ // The timestamp is needed to do the timeline look-up. If the packet has no
+ // timestamp, don't add the sched switch event. This is the safest option.
+ auto timestamp =
+ event.FindField(protos::pbzero::FtraceEvent::kTimestampFieldNumber);
+ if (!timestamp.valid()) {
+ return base::OkStatus();
+ }
+
+ auto sched_switch =
+ event.FindField(protos::pbzero::FtraceEvent::kSchedSwitchFieldNumber);
+ if (!sched_switch.valid()) {
+ return base::ErrStatus(
+ "RedactSchedSwitch: was used for unsupported field type");
+ }
+
+ protozero::ProtoDecoder sched_switch_decoder(sched_switch.as_bytes());
+
+ auto prev_pid = sched_switch_decoder.FindField(
+ protos::pbzero::SchedSwitchFtraceEvent::kPrevPidFieldNumber);
+ auto next_pid = sched_switch_decoder.FindField(
+ protos::pbzero::SchedSwitchFtraceEvent::kNextPidFieldNumber);
// There must be a prev pid and a next pid. Otherwise, the event is invalid.
// Dropping the event is the safest option.
- if (!sched_switch.has_prev_pid() || !sched_switch.has_next_pid()) {
+ if (!prev_pid.valid() || !next_pid.valid()) {
return base::OkStatus();
}
@@ -72,30 +106,21 @@
auto sched_switch_message = event_message->set_sched_switch();
auto prev_slice =
- context.timeline->Search(event.timestamp(), sched_switch.prev_pid());
+ context.timeline->Search(timestamp.as_uint64(), prev_pid.as_int32());
auto next_slice =
- context.timeline->Search(event.timestamp(), sched_switch.next_pid());
+ context.timeline->Search(timestamp.as_uint64(), next_pid.as_int32());
- // To read the fields, move the read head back to the start.
- sched_switch.Reset();
-
- for (auto field = sched_switch.ReadField(); field.valid();
- field = sched_switch.ReadField()) {
+ for (auto field = sched_switch_decoder.ReadField(); field.valid();
+ field = sched_switch_decoder.ReadField()) {
switch (field.id()) {
case protos::pbzero::SchedSwitchFtraceEvent::kNextCommFieldNumber:
- if (next_slice.uid == context.package_uid) {
- sched_switch_message->set_next_comm(field.as_string());
- } else {
- sched_switch_message->set_next_comm("");
- }
+ sched_switch_message->set_next_comm(
+ SanitizeCommValue(context, next_slice, field));
break;
case protos::pbzero::SchedSwitchFtraceEvent::kPrevCommFieldNumber:
- if (prev_slice.uid == context.package_uid) {
- sched_switch_message->set_prev_comm(field.as_string());
- } else {
- sched_switch_message->set_prev_comm("");
- }
+ sched_switch_message->set_prev_comm(
+ SanitizeCommValue(context, prev_slice, field));
break;
default:
diff --git a/src/trace_redaction/redact_sched_switch.h b/src/trace_redaction/redact_sched_switch.h
index e9d70cb..bcfa30d 100644
--- a/src/trace_redaction/redact_sched_switch.h
+++ b/src/trace_redaction/redact_sched_switch.h
@@ -31,8 +31,8 @@
base::Status Redact(
const Context& context,
- const protos::pbzero::FtraceEvent::Decoder& event,
- protozero::ConstBytes bytes,
+ const protos::pbzero::FtraceEventBundle::Decoder& bundle,
+ protozero::ProtoDecoder& event,
protos::pbzero::FtraceEvent* event_message) const override;
};
diff --git a/src/trace_redaction/redact_sched_switch_unittest.cc b/src/trace_redaction/redact_sched_switch_unittest.cc
index f6a4c13..72b65f4 100644
--- a/src/trace_redaction/redact_sched_switch_unittest.cc
+++ b/src/trace_redaction/redact_sched_switch_unittest.cc
@@ -22,7 +22,6 @@
#include "protos/perfetto/trace/ftrace/ftrace_event.gen.h"
#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.gen.h"
#include "protos/perfetto/trace/ftrace/sched.gen.h"
-#include "protos/perfetto/trace/ftrace/sched.pbzero.h"
#include "protos/perfetto/trace/trace.gen.h"
#include "protos/perfetto/trace/trace_packet.gen.h"
@@ -46,36 +45,54 @@
class RedactSchedSwitchTest : public testing::Test {
protected:
void SetUp() override {
- timeline_ = std::make_unique<ProcessThreadTimeline>();
- timeline_->Append(
- ProcessThreadTimeline::Event::Open(0, kPidA, kNoParent, kUidA));
- timeline_->Append(
- ProcessThreadTimeline::Event::Open(0, kPidB, kNoParent, kUidB));
- timeline_->Sort();
+ auto* event = bundle_.add_event();
- protozero::HeapBuffered<protos::pbzero::FtraceEvent> event;
event->set_timestamp(123456789);
event->set_pid(kPidA);
- auto* sched_switch = event->set_sched_switch();
- sched_switch->set_prev_comm(kCommA.data(), kCommA.size());
+ auto* sched_switch = event->mutable_sched_switch();
+ sched_switch->set_prev_comm(std::string(kCommA));
sched_switch->set_prev_pid(kPidA);
- sched_switch->set_next_comm(kCommB.data(), kCommB.size());
+ sched_switch->set_next_comm(std::string(kCommB));
sched_switch->set_next_pid(kPidB);
+ }
- event_string_ = event.SerializeAsString();
+ base::Status Redact(const Context& context,
+ protos::pbzero::FtraceEvent* event_message) {
+ RedactSchedSwitch redact;
+
+ auto bundle_str = bundle_.SerializeAsString();
+ protos::pbzero::FtraceEventBundle::Decoder bundle_decoder(bundle_str);
+
+ auto event_str = bundle_.event().back().SerializeAsString();
+ protos::pbzero::FtraceEvent::Decoder event_decoder(event_str);
+
+ return redact.Redact(context, bundle_decoder, event_decoder, event_message);
}
const std::string& event_string() const { return event_string_; }
- std::unique_ptr<ProcessThreadTimeline> timeline() {
- return std::move(timeline_);
+ // This test breaks the rules for task_newtask and the timeline. The
+ // timeline will report the task existing before the new task event. This
+ // should not happen in the field, but it makes the test more robust.
+ std::unique_ptr<ProcessThreadTimeline> CreatePopulatedTimeline() {
+ auto timeline = std::make_unique<ProcessThreadTimeline>();
+
+ timeline->Append(
+ ProcessThreadTimeline::Event::Open(0, kPidA, kNoParent, kUidA));
+ timeline->Append(
+ ProcessThreadTimeline::Event::Open(0, kPidB, kNoParent, kUidB));
+ timeline->Sort();
+
+ return timeline;
}
private:
std::string event_string_;
std::unique_ptr<ProcessThreadTimeline> timeline_;
+
+ protos::gen::FtraceEventBundle bundle_;
};
TEST_F(RedactSchedSwitchTest, RejectMissingPackageUid) {
@@ -84,12 +101,8 @@
Context context;
context.timeline = std::make_unique<ProcessThreadTimeline>();
- protos::pbzero::FtraceEvent::Decoder event_decoder(event_string());
protozero::HeapBuffered<protos::pbzero::FtraceEvent> event_message;
-
- auto result =
- redact.Redact(context, event_decoder, event_decoder.sched_switch(),
- event_message.get());
+ auto result = Redact(context, event_message.get());
ASSERT_FALSE(result.ok());
}
@@ -99,12 +112,8 @@
Context context;
context.package_uid = kUidA;
- protos::pbzero::FtraceEvent::Decoder event_decoder(event_string());
protozero::HeapBuffered<protos::pbzero::FtraceEvent> event_message;
-
- auto result =
- redact.Redact(context, event_decoder, event_decoder.sched_switch(),
- event_message.get());
+ auto result = Redact(context, event_message.get());
ASSERT_FALSE(result.ok());
}
@@ -112,18 +121,14 @@
RedactSchedSwitch redact;
Context context;
- context.timeline = timeline();
+ context.timeline = CreatePopulatedTimeline();
// Neither pid is connected to the target package (see timeline
// initialization).
context.package_uid = kUidC;
- protos::pbzero::FtraceEvent::Decoder event_decoder(event_string());
protozero::HeapBuffered<protos::pbzero::FtraceEvent> event_message;
-
- auto result =
- redact.Redact(context, event_decoder, event_decoder.sched_switch(),
- event_message.get());
+ auto result = Redact(context, event_message.get());
ASSERT_OK(result) << result.c_message();
protos::gen::FtraceEvent event;
@@ -143,18 +148,15 @@
RedactSchedSwitch redact;
Context context;
- context.timeline = timeline();
+ context.timeline = CreatePopulatedTimeline();
// Only next pid is connected to the target package (see timeline
// initialization).
context.package_uid = kUidB;
- protos::pbzero::FtraceEvent::Decoder event_decoder(event_string());
protozero::HeapBuffered<protos::pbzero::FtraceEvent> event_message;
+ auto result = Redact(context, event_message.get());
- auto result =
- redact.Redact(context, event_decoder, event_decoder.sched_switch(),
- event_message.get());
ASSERT_OK(result) << result.c_message();
protos::gen::FtraceEvent event;
@@ -174,18 +176,14 @@
RedactSchedSwitch redact;
Context context;
- context.timeline = timeline();
+ context.timeline = CreatePopulatedTimeline();
// Only prev pid is connected to the target package (see timeline
// initialization).
context.package_uid = kUidA;
- protos::pbzero::FtraceEvent::Decoder event_decoder(event_string());
protozero::HeapBuffered<protos::pbzero::FtraceEvent> event_message;
-
- auto result =
- redact.Redact(context, event_decoder, event_decoder.sched_switch(),
- event_message.get());
+ auto result = Redact(context, event_message.get());
ASSERT_OK(result) << result.c_message();
protos::gen::FtraceEvent event;
diff --git a/src/trace_redaction/redact_task_newtask.cc b/src/trace_redaction/redact_task_newtask.cc
index cb6a41c..c942612 100644
--- a/src/trace_redaction/redact_task_newtask.cc
+++ b/src/trace_redaction/redact_task_newtask.cc
@@ -16,8 +16,6 @@
#include "src/trace_redaction/redact_task_newtask.h"
-#include <string>
-
#include "src/trace_redaction/proto_util.h"
#include "protos/perfetto/trace/ftrace/ftrace_event.pbzero.h"
@@ -26,6 +24,20 @@
namespace perfetto::trace_redaction {
+namespace {
+
+protozero::ConstChars SanitizeCommValue(const Context& context,
+ ProcessThreadTimeline::Slice slice,
+ protozero::Field field) {
+ if (NormalizeUid(slice.uid) == NormalizeUid(context.package_uid.value())) {
+ return field.as_string();
+ }
+
+ return {};
+}
+
+} // namespace
+
// Redact sched switch trace events in an ftrace event bundle:
//
// event {
@@ -43,8 +55,8 @@
// equal to "event.task_newtask.pid" (a thread cannot start itself).
base::Status RedactTaskNewTask::Redact(
const Context& context,
- const protos::pbzero::FtraceEvent::Decoder& event,
- protozero::ConstBytes bytes,
+ const protos::pbzero::FtraceEventBundle::Decoder&,
+ protozero::ProtoDecoder& event,
protos::pbzero::FtraceEvent* event_message) const {
if (!context.package_uid.has_value()) {
return base::ErrStatus("RedactTaskNewTask: missing package uid");
@@ -54,14 +66,24 @@
return base::ErrStatus("RedactTaskNewTask: missing timeline");
}
- // There must be a pid. If not, the message is meaningless and can be dropped.
- if (!event.has_timestamp()) {
+ // The timestamp is needed to do the timeline look-up. If the packet has no
+ // timestamp, don't add the sched switch event. This is the safest option.
+ auto timestamp =
+ event.FindField(protos::pbzero::FtraceEvent::kTimestampFieldNumber);
+ if (!timestamp.valid()) {
return base::OkStatus();
}
- protozero::ProtoDecoder new_task(bytes);
+ auto new_task =
+ event.FindField(protos::pbzero::FtraceEvent::kTaskNewtaskFieldNumber);
+ if (!new_task.valid()) {
+ return base::ErrStatus(
+ "RedactTaskNewTask: was used for unsupported field type");
+ }
- auto pid = new_task.FindField(
+ protozero::ProtoDecoder new_task_decoder(new_task.as_bytes());
+
+ auto pid = new_task_decoder.FindField(
protos::pbzero::TaskNewtaskFtraceEvent::kPidFieldNumber);
if (!pid.valid()) {
@@ -71,20 +93,16 @@
// Avoid making the message until we know that we have prev and next pids.
auto* new_task_message = event_message->set_task_newtask();
- auto slice = context.timeline->Search(event.timestamp(), pid.as_int32());
+ auto slice = context.timeline->Search(timestamp.as_uint64(), pid.as_int32());
- for (auto field = new_task.ReadField(); field.valid();
- field = new_task.ReadField()) {
+ for (auto field = new_task_decoder.ReadField(); field.valid();
+ field = new_task_decoder.ReadField()) {
+ // Perfetto view (ui.perfetto.dev) crashes if the comm value is missing.
+ // To work around this, the comm value is replaced with an empty string.
+ // This appears to work.
if (field.id() ==
protos::pbzero::TaskNewtaskFtraceEvent::kCommFieldNumber) {
- if (slice.uid == context.package_uid) {
- proto_util::AppendField(field, new_task_message);
- } else {
- // Perfetto view (ui.perfetto.dev) crashes if the comm value is missing.
- // To work around this, the comm value is replaced with an empty string.
- // This appears to work.
- new_task_message->set_comm("");
- }
+ new_task_message->set_comm(SanitizeCommValue(context, slice, field));
} else {
proto_util::AppendField(field, new_task_message);
}
diff --git a/src/trace_redaction/redact_task_newtask.h b/src/trace_redaction/redact_task_newtask.h
index 51d384c..51c1124 100644
--- a/src/trace_redaction/redact_task_newtask.h
+++ b/src/trace_redaction/redact_task_newtask.h
@@ -31,8 +31,8 @@
base::Status Redact(
const Context& context,
- const protos::pbzero::FtraceEvent::Decoder& event,
- protozero::ConstBytes bytes,
+ const protos::pbzero::FtraceEventBundle::Decoder& bundle,
+ protozero::ProtoDecoder& event,
protos::pbzero::FtraceEvent* event_message) const override;
};
diff --git a/src/trace_redaction/redact_task_newtask_unittest.cc b/src/trace_redaction/redact_task_newtask_unittest.cc
index d09f044..33886f6 100644
--- a/src/trace_redaction/redact_task_newtask_unittest.cc
+++ b/src/trace_redaction/redact_task_newtask_unittest.cc
@@ -16,11 +16,15 @@
#include "src/trace_redaction/redact_task_newtask.h"
#include "perfetto/protozero/scattered_heap_buffer.h"
+#include "protos/perfetto/trace/ftrace/task.gen.h"
#include "test/gtest_and_gmock.h"
#include "protos/perfetto/trace/ftrace/ftrace_event.gen.h"
-#include "protos/perfetto/trace/ftrace/task.gen.h"
-#include "protos/perfetto/trace/ftrace/task.pbzero.h"
+#include "protos/perfetto/trace/ftrace/ftrace_event.pbzero.h"
+#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.gen.h"
+#include "protos/perfetto/trace/ftrace/sched.gen.h"
+#include "protos/perfetto/trace/trace.gen.h"
+#include "protos/perfetto/trace/trace_packet.gen.h"
namespace perfetto::trace_redaction {
@@ -40,37 +44,52 @@
class RedactTaskNewTaskTest : public testing::Test {
protected:
void SetUp() override {
- timeline_ = std::make_unique<ProcessThreadTimeline>();
- timeline_->Append(
- ProcessThreadTimeline::Event::Open(0, kPidA, kNoParent, kUidA));
- timeline_->Append(
- ProcessThreadTimeline::Event::Open(0, kPidB, kNoParent, kUidB));
- timeline_->Sort();
+ auto* event = bundle_.add_event();
- // This test breaks the rules for task_newtask and the timeline. The
- // timeline will report the task existing before the new task event. This
- // should not happen in the field, but it makes the test more robust.
- protozero::HeapBuffered<protos::pbzero::FtraceEvent> event;
event->set_timestamp(123456789);
event->set_pid(kPidA);
- auto* new_task = event->set_task_newtask();
- new_task->set_comm(kCommA.data(), kCommA.size());
+ auto* new_task = event->mutable_task_newtask();
+ new_task->set_comm(std::string(kCommA));
new_task->set_pid(kPidA);
+ }
- event_string_ = event.SerializeAsString();
+ base::Status Redact(const Context& context,
+ protos::pbzero::FtraceEvent* event_message) {
+ RedactTaskNewTask redact;
+
+ auto bundle_str = bundle_.SerializeAsString();
+ protos::pbzero::FtraceEventBundle::Decoder bundle_decoder(bundle_str);
+
+ auto event_str = bundle_.event().back().SerializeAsString();
+ protos::pbzero::FtraceEvent::Decoder event_decoder(event_str);
+
+ return redact.Redact(context, bundle_decoder, event_decoder, event_message);
}
const std::string& event_string() const { return event_string_; }
- std::unique_ptr<ProcessThreadTimeline> timeline() {
- return std::move(timeline_);
+ // This test breaks the rules for task_newtask and the timeline. The
+ // timeline will report the task existing before the new task event. This
+ // should not happen in the field, but it makes the test more robust.
+ std::unique_ptr<ProcessThreadTimeline> CreatePopulatedTimeline() {
+ auto timeline = std::make_unique<ProcessThreadTimeline>();
+
+ timeline->Append(
+ ProcessThreadTimeline::Event::Open(0, kPidA, kNoParent, kUidA));
+ timeline->Append(
+ ProcessThreadTimeline::Event::Open(0, kPidB, kNoParent, kUidB));
+ timeline->Sort();
+
+ return timeline;
}
private:
std::string event_string_;
std::unique_ptr<ProcessThreadTimeline> timeline_;
+
+ protos::gen::FtraceEventBundle bundle_;
};
TEST_F(RedactTaskNewTaskTest, RejectMissingPackageUid) {
@@ -82,9 +101,7 @@
protos::pbzero::FtraceEvent::Decoder event_decoder(event_string());
protozero::HeapBuffered<protos::pbzero::FtraceEvent> event_message;
- auto result =
- redact.Redact(context, event_decoder, event_decoder.task_newtask(),
- event_message.get());
+ auto result = Redact(context, event_message.get());
ASSERT_FALSE(result.ok());
}
@@ -97,9 +114,7 @@
protos::pbzero::FtraceEvent::Decoder event_decoder(event_string());
protozero::HeapBuffered<protos::pbzero::FtraceEvent> event_message;
- auto result =
- redact.Redact(context, event_decoder, event_decoder.task_newtask(),
- event_message.get());
+ auto result = Redact(context, event_message.get());
ASSERT_FALSE(result.ok());
}
@@ -110,14 +125,12 @@
// keep its comm value.
Context context;
context.package_uid = kUidA;
- context.timeline = timeline();
+ context.timeline = CreatePopulatedTimeline();
protos::pbzero::FtraceEvent::Decoder event_decoder(event_string());
protozero::HeapBuffered<protos::pbzero::FtraceEvent> event_message;
- auto result =
- redact.Redact(context, event_decoder, event_decoder.task_newtask(),
- event_message.get());
+ auto result = Redact(context, event_message.get());
ASSERT_TRUE(result.ok());
protos::gen::FtraceEvent redacted_event;
@@ -135,14 +148,12 @@
// lose its comm value.
Context context;
context.package_uid = kUidB;
- context.timeline = timeline();
+ context.timeline = CreatePopulatedTimeline();
protos::pbzero::FtraceEvent::Decoder event_decoder(event_string());
protozero::HeapBuffered<protos::pbzero::FtraceEvent> event_message;
- auto result =
- redact.Redact(context, event_decoder, event_decoder.task_newtask(),
- event_message.get());
+ auto result = Redact(context, event_message.get());
ASSERT_TRUE(result.ok());
protos::gen::FtraceEvent redacted_event;
diff --git a/src/trace_redaction/scrub_process_trees.cc b/src/trace_redaction/scrub_process_trees.cc
index d8baf99..989d094 100644
--- a/src/trace_redaction/scrub_process_trees.cc
+++ b/src/trace_redaction/scrub_process_trees.cc
@@ -16,9 +16,6 @@
#include "src/trace_redaction/scrub_process_trees.h"
-#include <cstdint>
-#include <string>
-
#include "perfetto/base/status.h"
#include "perfetto/protozero/field.h"
#include "perfetto/protozero/scattered_heap_buffer.h"
@@ -29,149 +26,90 @@
#include "protos/perfetto/trace/trace_packet.pbzero.h"
namespace perfetto::trace_redaction {
+
namespace {
-constexpr auto kThreadsFieldNumber =
- protos::pbzero::ProcessTree::kThreadsFieldNumber;
-constexpr auto kTimestampFieldNumber =
- protos::pbzero::TracePacket::kTimestampFieldNumber;
-constexpr auto kProcessTreeFieldNumber =
- protos::pbzero::TracePacket::kProcessTreeFieldNumber;
-constexpr auto kProcessesFieldNumber =
- protos::pbzero::ProcessTree::kProcessesFieldNumber;
-
-// Skips the cmdline fields.
-void ClearProcessName(protozero::ConstBytes bytes,
- protos::pbzero::ProcessTree::Process* message) {
- protozero::ProtoDecoder decoder(bytes);
-
- for (auto field = decoder.ReadField(); field; field = decoder.ReadField()) {
- if (field.id() !=
- protos::pbzero::ProcessTree::Process::kCmdlineFieldNumber) {
- proto_util::AppendField(field, message);
- }
- }
-}
-
-void ScrubProcess(protozero::Field field,
- const ProcessThreadTimeline& timeline,
- uint64_t now,
- uint64_t uid,
- protos::pbzero::ProcessTree* message) {
- if (field.id() != kProcessesFieldNumber) {
- PERFETTO_FATAL(
- "ScrubProcess() should only be called with a ProcessTree::Processes");
+// Appends a value to the message if (and only if) the pid belongs to the target
+// package.
+void TryAppendPid(const Context& context,
+ const protozero::Field& timestamp,
+ const protozero::Field& pid,
+ const protozero::Field& value,
+ protozero::Message* message) {
+ // All valid processes with have a time and pid/tid values. However, if
+ // they're missing values, the trace is corrupt. To avoid making this work by
+ // dropping too much data, drop the cmdline for all processes.
+ if (!timestamp.valid() || !pid.valid()) {
+ return;
}
- protos::pbzero::ProcessTree::Process::Decoder decoder(field.as_bytes());
- auto slice = timeline.Search(now, decoder.pid());
+ auto slice = context.timeline->Search(timestamp.as_uint64(), pid.as_int32());
- if (NormalizeUid(slice.uid) == NormalizeUid(uid)) {
- proto_util::AppendField(field, message);
- } else {
- ClearProcessName(field.as_bytes(), message->add_processes());
- }
-}
-
-// The thread name is unused, but it's safer to remove it.
-void ClearThreadName(protozero::ConstBytes bytes,
- protos::pbzero::ProcessTree::Thread* message) {
- protozero::ProtoDecoder decoder(bytes);
-
- for (auto field = decoder.ReadField(); field; field = decoder.ReadField()) {
- if (field.id() != protos::pbzero::ProcessTree::Thread::kNameFieldNumber) {
- proto_util::AppendField(field, message);
- }
- }
-}
-
-void ScrubThread(protozero::Field field,
- const ProcessThreadTimeline& timeline,
- uint64_t now,
- uint64_t uid,
- protos::pbzero::ProcessTree* message) {
- if (field.id() != kThreadsFieldNumber) {
- PERFETTO_FATAL(
- "ScrubThread() should only be called with a ProcessTree::Threads");
+ // Only keep the target process cmdline.
+ if (NormalizeUid(slice.uid) != NormalizeUid(context.package_uid.value())) {
+ return;
}
- protos::pbzero::ProcessTree::Thread::Decoder thread_decoder(field.as_bytes());
- auto slice = timeline.Search(now, thread_decoder.tid());
-
- if (NormalizeUid(slice.uid) == NormalizeUid(uid)) {
- proto_util::AppendField(field, message);
- } else {
- ClearThreadName(field.as_bytes(), message->add_threads());
- }
+ proto_util::AppendField(value, message);
}
} // namespace
-base::Status ScrubProcessTrees::Transform(const Context& context,
- std::string* packet) const {
+base::Status ScrubProcessTrees::VerifyContext(const Context& context) const {
if (!context.package_uid.has_value()) {
return base::ErrStatus("ScrubProcessTrees: missing package uid.");
}
- if (context.timeline == nullptr) {
+ if (!context.timeline) {
return base::ErrStatus("ScrubProcessTrees: missing timeline.");
}
- protozero::ProtoDecoder decoder(*packet);
-
- if (!decoder.FindField(kProcessTreeFieldNumber).valid()) {
- return base::OkStatus();
- }
-
- auto timestamp_field = decoder.FindField(kTimestampFieldNumber);
-
- if (!timestamp_field.valid()) {
- return base::ErrStatus("ScrubProcessTrees: trace packet missing timestamp");
- }
-
- auto timestamp = timestamp_field.as_uint64();
-
- auto uid = context.package_uid.value();
-
- const auto& timeline = *context.timeline.get();
-
- protozero::HeapBuffered<protos::pbzero::TracePacket> message;
-
- for (auto packet_field = decoder.ReadField(); packet_field.valid();
- packet_field = decoder.ReadField()) {
- if (packet_field.id() != kProcessTreeFieldNumber) {
- proto_util::AppendField(packet_field, message.get());
- continue;
- }
-
- auto* process_tree_message = message->set_process_tree();
-
- protozero::ProtoDecoder process_tree_decoder(packet_field.as_bytes());
-
- for (auto process_tree_field = process_tree_decoder.ReadField();
- process_tree_field.valid();
- process_tree_field = process_tree_decoder.ReadField()) {
- switch (process_tree_field.id()) {
- case kProcessesFieldNumber:
- ScrubProcess(process_tree_field, timeline, timestamp, uid,
- process_tree_message);
- break;
-
- case kThreadsFieldNumber:
- ScrubThread(process_tree_field, timeline, timestamp, uid,
- process_tree_message);
- break;
-
- default:
- proto_util::AppendField(process_tree_field, process_tree_message);
- break;
- }
- }
- }
-
- packet->assign(message.SerializeAsString());
-
return base::OkStatus();
}
+void ScrubProcessTrees::TransformProcess(
+ const Context& context,
+ const protozero::Field& timestamp,
+ const protozero::Field& process,
+ protos::pbzero::ProcessTree* process_tree) const {
+ protozero::ProtoDecoder decoder(process.as_bytes());
+
+ auto pid =
+ decoder.FindField(protos::pbzero::ProcessTree::Process::kPidFieldNumber);
+
+ auto* process_message = process_tree->add_processes();
+
+ for (auto field = decoder.ReadField(); field.valid();
+ field = decoder.ReadField()) {
+ if (field.id() ==
+ protos::pbzero::ProcessTree::Process::kCmdlineFieldNumber) {
+ TryAppendPid(context, timestamp, pid, field, process_message);
+ } else {
+ proto_util::AppendField(field, process_message);
+ }
+ }
+}
+
+void ScrubProcessTrees::TransformThread(
+ const Context& context,
+ const protozero::Field& timestamp,
+ const protozero::Field& thread,
+ protos::pbzero::ProcessTree* process_tree) const {
+ protozero::ProtoDecoder decoder(thread.as_bytes());
+
+ auto tid =
+ decoder.FindField(protos::pbzero::ProcessTree::Thread::kTidFieldNumber);
+
+ auto* thread_message = process_tree->add_threads();
+
+ for (auto field = decoder.ReadField(); field.valid();
+ field = decoder.ReadField()) {
+ if (field.id() == protos::pbzero::ProcessTree::Thread::kNameFieldNumber) {
+ TryAppendPid(context, timestamp, tid, field, thread_message);
+ } else {
+ proto_util::AppendField(field, thread_message);
+ }
+ }
+}
+
} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/scrub_process_trees.h b/src/trace_redaction/scrub_process_trees.h
index 61cb85a..7c2b07a 100644
--- a/src/trace_redaction/scrub_process_trees.h
+++ b/src/trace_redaction/scrub_process_trees.h
@@ -17,19 +17,32 @@
#ifndef SRC_TRACE_REDACTION_SCRUB_PROCESS_TREES_H_
#define SRC_TRACE_REDACTION_SCRUB_PROCESS_TREES_H_
-#include <string>
-
#include "perfetto/base/status.h"
+#include "perfetto/protozero/field.h"
+#include "src/trace_redaction/modify_process_trees.h"
#include "src/trace_redaction/trace_redaction_framework.h"
+#include "protos/perfetto/trace/ps/process_tree.pbzero.h"
+
namespace perfetto::trace_redaction {
// Removes process names and thread names from process_trees if their pids/tids
// are not connected to the target package.
-class ScrubProcessTrees final : public TransformPrimitive {
- public:
- base::Status Transform(const Context& context,
- std::string* packet) const override;
+class ScrubProcessTrees : public ModifyProcessTree {
+ protected:
+ base::Status VerifyContext(const Context& context) const override;
+
+ void TransformProcess(
+ const Context& context,
+ const protozero::Field& timestamp,
+ const protozero::Field& process,
+ protos::pbzero::ProcessTree* process_trees) const override;
+
+ void TransformThread(
+ const Context& context,
+ const protozero::Field& timestamp,
+ const protozero::Field& thread,
+ protos::pbzero::ProcessTree* process_tree) const override;
};
} // namespace perfetto::trace_redaction
diff --git a/test/trace_processor/diff_tests/metrics/android/android_auto_multiuser.textproto b/test/trace_processor/diff_tests/metrics/android/android_auto_multiuser.textproto
new file mode 100644
index 0000000..c33839e
--- /dev/null
+++ b/test/trace_processor/diff_tests/metrics/android/android_auto_multiuser.textproto
@@ -0,0 +1,127 @@
+packet {
+ timestamp: 1
+ process_tree {
+ processes {
+ pid: 10
+ uid: 1000010
+ cmdline: "dummy:1"
+ }
+ processes {
+ pid: 11
+ uid: 1000010
+ cmdline: "dummy:2"
+ }
+ }
+}
+packet {
+ ftrace_events {
+ event {
+ timestamp: 1000000000
+ print {
+ buf: "S|5993|UserController.startUser-10-fg-start-mode-1|0\n"
+ }
+ }
+ }
+}
+packet {
+ ftrace_events {
+ event {
+ timestamp: 2000000000
+ print {
+ buf: "S|2608|launching: com.android.car.carlauncher|0\n"
+ }
+ }
+ }
+}
+packet {
+ ftrace_events {
+ cpu: 1
+ event {
+ timestamp: 3000000001
+ pid: 10
+ sched_switch {
+ prev_comm: "dummy:1"
+ prev_pid: 10
+ prev_state: 2
+ next_comm: "dummy:2"
+ next_pid: 11
+ }
+ }
+ }
+}
+packet {
+ ftrace_events {
+ cpu: 1
+ event {
+ timestamp: 3010000000
+ pid: 11
+ sched_switch {
+ prev_comm: "dummy:2"
+ prev_pid: 11
+ prev_state: 2
+ next_comm: "dummy:1"
+ next_pid: 10
+ }
+ }
+ }
+}
+packet {
+ timestamp: 3000000001
+ process_stats {
+ processes {
+ pid: 10
+ vm_rss_kb: 1000
+ }
+ }
+}
+packet {
+ timestamp: 3000000002
+ process_stats {
+ processes {
+ pid: 10
+ vm_rss_kb: 2000
+ }
+ }
+}
+packet {
+ timestamp: 3000000001
+ process_stats {
+ processes {
+ pid: 11
+ vm_rss_kb: 1000
+ }
+ }
+}
+packet {
+ timestamp: 3000000002
+ process_stats {
+ processes {
+ pid: 11
+ vm_rss_kb: 2000
+ }
+ }
+}
+packet {
+ ftrace_events {
+ cpu: 1
+ event {
+ timestamp: 3000000000
+ pid: 1
+ print {
+ buf: "S|5993|UserController.startUser-11-fg-start-mode-1|0\n"
+ }
+ }
+ }
+}
+packet {
+ ftrace_events {
+ cpu: 1
+ event {
+ timestamp: 4000000000
+ pid: 1
+ print {
+ buf: "S|2609|launching: com.android.car.carlauncher|0\n"
+ }
+ }
+ }
+}
\ No newline at end of file
diff --git a/test/trace_processor/diff_tests/metrics/android/tests.py b/test/trace_processor/diff_tests/metrics/android/tests.py
index da5559e..a1c2448 100644
--- a/test/trace_processor/diff_tests/metrics/android/tests.py
+++ b/test/trace_processor/diff_tests/metrics/android/tests.py
@@ -299,10 +299,36 @@
out=TextProto(r"""
android_auto_multiuser {
user_switch {
- duration_ms: 3878
+ user_id: 11
+ start_event: "UserController.startUser-11-fg-start-mode-1"
+ end_event: "com.android.car.carlauncher"
+ duration_ms: 3877
+ previous_user_info {
+ }
}
}
"""))
+
+ def test_android_auto_multiuser_switch_with_previous_user_data(self):
+ return DiffTestBlueprint(
+ trace=Path("android_auto_multiuser.textproto"),
+ query=Metric('android_auto_multiuser'),
+ out=TextProto(r"""
+ android_auto_multiuser {
+ user_switch {
+ user_id: 11
+ start_event: "UserController.startUser-11-fg-start-mode-1"
+ end_event: "com.android.car.carlauncher"
+ duration_ms: 999
+ previous_user_info {
+ user_id: 10
+ total_cpu_time_ms: 9
+ total_memory_usage_kb: 2048
+ }
+ }
+ }
+ """))
+
def test_android_oom_adjuster(self):
return DiffTestBlueprint(
trace=DataPath('android_postboot_unlock.pftrace'),
diff --git a/ui/src/frontend/base_slice_track.ts b/ui/src/frontend/base_slice_track.ts
index 329e441..39c38e5 100644
--- a/ui/src/frontend/base_slice_track.ts
+++ b/ui/src/frontend/base_slice_track.ts
@@ -356,8 +356,8 @@
id
${extraCols ? ',' + extraCols : ''}
from (${this.getSqlSource()})
- where dur = -1
group by 1
+ having dur = -1
`);
}
const incomplete = new Array<CastInternal<T['slice']>>(queryRes.numRows());
