Skip to content

Commit

Permalink
Removed unneeded metadata fields from trace log
Browse files Browse the repository at this point in the history
This removes process_name and thread_name from trace log, these fields
are now recorded in perfetto, so it's not necessary to record them
client side.

Change-Id: I6591c25d572cd97b54d024ffcb3dcc00597d63fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2640819
Reviewed-by: ssid <[email protected]>
Commit-Queue: Salvador Guerrero <[email protected]>
Cr-Commit-Position: refs/heads/master@{#846445}
  • Loading branch information
Salvador Guerrero authored and Chromium LUCI CQ committed Jan 23, 2021
1 parent 3b567ed commit ab858cb
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 52 deletions.
37 changes: 0 additions & 37 deletions base/trace_event/trace_event_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1686,43 +1686,6 @@ TEST_F(TraceEventTestFixture, ThreadNames) {
}
}

TEST_F(TraceEventTestFixture, ThreadNameChanges) {
BeginTrace();

PlatformThread::SetName("");
TRACE_EVENT_INSTANT0("drink", "water", TRACE_EVENT_SCOPE_THREAD);

PlatformThread::SetName("cafe");
TRACE_EVENT_INSTANT0("drink", "coffee", TRACE_EVENT_SCOPE_THREAD);

PlatformThread::SetName("shop");
// No event here, so won't appear in combined name.

PlatformThread::SetName("pub");
TRACE_EVENT_INSTANT0("drink", "beer", TRACE_EVENT_SCOPE_THREAD);
TRACE_EVENT_INSTANT0("drink", "wine", TRACE_EVENT_SCOPE_THREAD);

PlatformThread::SetName(" bar");
TRACE_EVENT_INSTANT0("drink", "whisky", TRACE_EVENT_SCOPE_THREAD);

EndTraceAndFlush();

std::vector<const DictionaryValue*> items =
FindTraceEntries(trace_parsed_, "thread_name");
EXPECT_EQ(1u, items.size());
ASSERT_GT(items.size(), 0u);
const DictionaryValue* item = items[0];
ASSERT_TRUE(item);
int tid;
EXPECT_TRUE(item->GetInteger("tid", &tid));
EXPECT_EQ(PlatformThread::CurrentId(), static_cast<PlatformThreadId>(tid));

std::string expected_name = "cafe,pub, bar";
std::string tmp;
EXPECT_TRUE(item->GetString("args.name", &tmp));
EXPECT_EQ(expected_name, tmp);
}

// Test that the disabled trace categories are included/excluded from the
// trace output correctly.
TEST_F(TraceEventTestFixture, DisabledCategories) {
Expand Down
15 changes: 0 additions & 15 deletions base/trace_event/trace_log.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1579,11 +1579,6 @@ void TraceLog::AddMetadataEventsWhileLocked() {
"sort_index", process_sort_index_);
}

if (!process_name_.empty()) {
AddMetadataEventWhileLocked(current_thread_id, "process_name", "name",
process_name_);
}

TimeDelta process_uptime = TRACE_TIME_NOW() - process_creation_time_;
AddMetadataEventWhileLocked(current_thread_id, "process_uptime_seconds",
"uptime", process_uptime.InSeconds());
Expand Down Expand Up @@ -1617,16 +1612,6 @@ void TraceLog::AddMetadataEventsWhileLocked() {
it.second);
}

// TODO(ssid): Stop emitting and tracking thread names when perfetto is
// enabled and after crbug/978093 if fixed. The JSON exporter will emit thread
// names from thread descriptors.
AutoLock thread_info_lock(thread_info_lock_);
for (const auto& it : thread_names_) {
if (it.second.empty())
continue;
AddMetadataEventWhileLocked(it.first, "thread_name", "name", it.second);
}

// If buffer is full, add a metadata record to report this.
if (!buffer_limit_reached_timestamp_.is_null()) {
AddMetadataEventWhileLocked(current_thread_id, "trace_buffer_overflowed",
Expand Down
1 change: 1 addition & 0 deletions services/tracing/public/cpp/trace_event_args_allowlist.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const char* const kMemoryPressureEventsAllowedArgs[] = {
"level", "listener_creation_info", nullptr};

const AllowlistEntry kEventArgsAllowlist[] = {
// Thread and process names are now recorded in perfetto.
{"__metadata", "thread_name", nullptr},
{"__metadata", "process_name", nullptr},
{"__metadata", "process_uptime_seconds", nullptr},
Expand Down

0 comments on commit ab858cb

Please sign in to comment.