Commit 8c251c3c authored by Siddhartha S's avatar Siddhartha S Committed by Commit Bot

tracing: Workaround to thread names from killed processes

The trace importer can't handle thread descriptors from killed
processes. So, do not emit thread names/types in thread descriptors when
privacy filtering is enabled. Metadata from trace log is sufficient.

BUG=978093

Change-Id: Iff2b870edff1f1c516eec3068489e67a66cc5b8a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1691008Reviewed-by: default avatarEric Seckler <eseckler@chromium.org>
Commit-Queue: ssid <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676158}
parent 7a7292b8
...@@ -1562,7 +1562,8 @@ void TraceLog::AddMetadataEventsWhileLocked() { ...@@ -1562,7 +1562,8 @@ void TraceLog::AddMetadataEventsWhileLocked() {
} }
// TODO(ssid): Stop emitting and tracking thread names when perfetto is // TODO(ssid): Stop emitting and tracking thread names when perfetto is
// enabled. The JSON exporter will emit thread names. // enabled and after crbug/978093 if fixed. The JSON exporter will emit thread
// names from thread descriptors.
AutoLock thread_info_lock(thread_info_lock_); AutoLock thread_info_lock(thread_info_lock_);
for (const auto& it : thread_names_) { for (const auto& it : thread_names_) {
if (it.second.empty()) if (it.second.empty())
......
...@@ -259,11 +259,9 @@ class TraceEventDataSourceTest : public testing::Test { ...@@ -259,11 +259,9 @@ class TraceEventDataSourceTest : public testing::Test {
} }
if (filtering_enabled) { if (filtering_enabled) {
EXPECT_FALSE(packet->thread_descriptor().has_thread_name()); EXPECT_FALSE(packet->thread_descriptor().has_thread_name());
} else { EXPECT_EQ(perfetto::protos::ThreadDescriptor::CHROME_THREAD_MAIN,
EXPECT_EQ(kTestThread, packet->thread_descriptor().thread_name()); packet->thread_descriptor().chrome_thread_type());
} }
EXPECT_EQ(perfetto::protos::ThreadDescriptor::CHROME_THREAD_MAIN,
packet->thread_descriptor().chrome_thread_type());
last_timestamp_ = packet->thread_descriptor().reference_timestamp_us(); last_timestamp_ = packet->thread_descriptor().reference_timestamp_us();
last_thread_time_ = packet->thread_descriptor().reference_thread_time_us(); last_thread_time_ = packet->thread_descriptor().reference_thread_time_us();
......
...@@ -553,10 +553,15 @@ void TrackEventThreadLocalEventSink::EmitThreadDescriptor( ...@@ -553,10 +553,15 @@ void TrackEventThreadLocalEventSink::EmitThreadDescriptor(
thread_name_ = maybe_new_name; thread_name_ = maybe_new_name;
thread_type_ = GetThreadType(maybe_new_name); thread_type_ = GetThreadType(maybe_new_name);
} }
if (!privacy_filtering_enabled_ && !thread_name_.empty()) { // TODO(ssid): Adding name and type to thread descriptor adds thread names
thread_descriptor->set_thread_name(thread_name_.c_str()); // from killed processes. The catapult trace importer can't handle different
// processes with same process ids. To workaround this issue, we do not emit
// name and type when filtering is enabled (when metadata is not emitted).
// Thread names will be emitted by trace log at metadata generation step when
// filtering is not enabled. See crbug/978093.
if (privacy_filtering_enabled_) {
thread_descriptor->set_chrome_thread_type(thread_type_);
} }
thread_descriptor->set_chrome_thread_type(thread_type_);
if (explicit_timestamp || !trace_event) { if (explicit_timestamp || !trace_event) {
// Don't use a user-provided timestamp as a reference timestamp. // Don't use a user-provided timestamp as a reference timestamp.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment