TrackEvent: Make process/thread naming less of a footgun
When calling TrackEvent::Set{Process,Thread}Descriptor, it's tempting to
set the |name| field of the returned TrackDescriptor to record the name
of the thread or process, but this only ends up setting the name of the
track object which is associated with the thread or process, and is not
directly displayed by the UI.
Instead, one should name the thread or process by filling in the
{Process,Thread}Descriptor message inside the TrackDescriptor. This
patch makes the API more conducive to doing that by accepting
protos::gen track descriptors -- which are more easily mutated by the
user -- while maintaining backwards compatibility with the original
behavior.
Bug: 155076522
Change-Id: I7c8c9032c8e7d59a28c90c81a2b11424a7bce5e2diff --git a/Android.bp b/Android.bp
index 5230e56..d3b7f3f 100644
--- a/Android.bp
+++ b/Android.bp
@@ -114,6 +114,7 @@
":perfetto_protos_perfetto_trace_ps_zero_gen",
":perfetto_protos_perfetto_trace_sys_stats_zero_gen",
":perfetto_protos_perfetto_trace_system_info_zero_gen",
+ ":perfetto_protos_perfetto_trace_track_event_cpp_gen",
":perfetto_protos_perfetto_trace_track_event_zero_gen",
":perfetto_src_base_base",
":perfetto_src_base_unix_socket",
@@ -184,6 +185,7 @@
"perfetto_protos_perfetto_trace_ps_zero_gen_headers",
"perfetto_protos_perfetto_trace_sys_stats_zero_gen_headers",
"perfetto_protos_perfetto_trace_system_info_zero_gen_headers",
+ "perfetto_protos_perfetto_trace_track_event_cpp_gen_headers",
"perfetto_protos_perfetto_trace_track_event_zero_gen_headers",
],
defaults: [
@@ -313,6 +315,7 @@
":perfetto_protos_perfetto_trace_ps_zero_gen",
":perfetto_protos_perfetto_trace_sys_stats_zero_gen",
":perfetto_protos_perfetto_trace_system_info_zero_gen",
+ ":perfetto_protos_perfetto_trace_track_event_cpp_gen",
":perfetto_protos_perfetto_trace_track_event_zero_gen",
":perfetto_src_android_internal_headers",
":perfetto_src_android_internal_lazy_library_loader",
@@ -396,6 +399,7 @@
"perfetto_protos_perfetto_trace_ps_zero_gen_headers",
"perfetto_protos_perfetto_trace_sys_stats_zero_gen_headers",
"perfetto_protos_perfetto_trace_system_info_zero_gen_headers",
+ "perfetto_protos_perfetto_trace_track_event_cpp_gen_headers",
"perfetto_protos_perfetto_trace_track_event_zero_gen_headers",
],
defaults: [
@@ -499,6 +503,7 @@
":perfetto_protos_perfetto_trace_ps_zero_gen",
":perfetto_protos_perfetto_trace_sys_stats_zero_gen",
":perfetto_protos_perfetto_trace_system_info_zero_gen",
+ ":perfetto_protos_perfetto_trace_track_event_cpp_gen",
":perfetto_protos_perfetto_trace_track_event_zero_gen",
":perfetto_src_base_base",
":perfetto_src_base_unix_socket",
@@ -562,6 +567,7 @@
"perfetto_protos_perfetto_trace_ps_zero_gen_headers",
"perfetto_protos_perfetto_trace_sys_stats_zero_gen_headers",
"perfetto_protos_perfetto_trace_system_info_zero_gen_headers",
+ "perfetto_protos_perfetto_trace_track_event_cpp_gen_headers",
"perfetto_protos_perfetto_trace_track_event_zero_gen_headers",
],
export_generated_headers: [
@@ -604,6 +610,7 @@
"perfetto_protos_perfetto_trace_ps_zero_gen_headers",
"perfetto_protos_perfetto_trace_sys_stats_zero_gen_headers",
"perfetto_protos_perfetto_trace_system_info_zero_gen_headers",
+ "perfetto_protos_perfetto_trace_track_event_cpp_gen_headers",
"perfetto_protos_perfetto_trace_track_event_zero_gen_headers",
],
defaults: [
@@ -673,6 +680,7 @@
":perfetto_protos_perfetto_trace_ps_zero_gen",
":perfetto_protos_perfetto_trace_sys_stats_zero_gen",
":perfetto_protos_perfetto_trace_system_info_zero_gen",
+ ":perfetto_protos_perfetto_trace_track_event_cpp_gen",
":perfetto_protos_perfetto_trace_track_event_zero_gen",
":perfetto_src_android_internal_headers",
":perfetto_src_android_internal_lazy_library_loader",
@@ -736,6 +744,7 @@
"perfetto_protos_perfetto_trace_ps_zero_gen_headers",
"perfetto_protos_perfetto_trace_sys_stats_zero_gen_headers",
"perfetto_protos_perfetto_trace_system_info_zero_gen_headers",
+ "perfetto_protos_perfetto_trace_track_event_cpp_gen_headers",
"perfetto_protos_perfetto_trace_track_event_zero_gen_headers",
"perfetto_src_perfetto_cmd_protos_gen_headers",
],
@@ -8154,6 +8163,7 @@
":perfetto_protos_perfetto_trace_ps_zero_gen",
":perfetto_protos_perfetto_trace_sys_stats_zero_gen",
":perfetto_protos_perfetto_trace_system_info_zero_gen",
+ ":perfetto_protos_perfetto_trace_track_event_cpp_gen",
":perfetto_protos_perfetto_trace_track_event_zero_gen",
":perfetto_src_base_base",
":perfetto_src_base_unix_socket",
@@ -8227,6 +8237,7 @@
"perfetto_protos_perfetto_trace_ps_zero_gen_headers",
"perfetto_protos_perfetto_trace_sys_stats_zero_gen_headers",
"perfetto_protos_perfetto_trace_system_info_zero_gen_headers",
+ "perfetto_protos_perfetto_trace_track_event_cpp_gen_headers",
"perfetto_protos_perfetto_trace_track_event_zero_gen_headers",
],
defaults: [
@@ -8315,6 +8326,7 @@
":perfetto_protos_perfetto_trace_ps_zero_gen",
":perfetto_protos_perfetto_trace_sys_stats_zero_gen",
":perfetto_protos_perfetto_trace_system_info_zero_gen",
+ ":perfetto_protos_perfetto_trace_track_event_cpp_gen",
":perfetto_protos_perfetto_trace_track_event_zero_gen",
":perfetto_src_base_base",
":perfetto_src_base_unix_socket",
@@ -8373,6 +8385,7 @@
"perfetto_protos_perfetto_trace_ps_zero_gen_headers",
"perfetto_protos_perfetto_trace_sys_stats_zero_gen_headers",
"perfetto_protos_perfetto_trace_system_info_zero_gen_headers",
+ "perfetto_protos_perfetto_trace_track_event_cpp_gen_headers",
"perfetto_protos_perfetto_trace_track_event_zero_gen_headers",
"perfetto_src_perfetto_cmd_protos_gen_headers",
],
diff --git a/BUILD b/BUILD
index 9696008..d609792 100644
--- a/BUILD
+++ b/BUILD
@@ -154,6 +154,7 @@
":protos_perfetto_trace_ps_zero",
":protos_perfetto_trace_sys_stats_zero",
":protos_perfetto_trace_system_info_zero",
+ ":protos_perfetto_trace_track_event_cpp",
":protos_perfetto_trace_track_event_zero",
],
)
@@ -250,6 +251,7 @@
":protos_perfetto_trace_ps_zero",
":protos_perfetto_trace_sys_stats_zero",
":protos_perfetto_trace_system_info_zero",
+ ":protos_perfetto_trace_track_event_cpp",
":protos_perfetto_trace_track_event_zero",
],
linkstatic = True,
@@ -2591,6 +2593,14 @@
],
)
+# GN target: //protos/perfetto/trace/track_event:cpp
+perfetto_cc_protocpp_library(
+ name = "protos_perfetto_trace_track_event_cpp",
+ deps = [
+ ":protos_perfetto_trace_track_event_protos",
+ ],
+)
+
# GN target: //protos/perfetto/trace/track_event:lite
perfetto_cc_proto_library(
name = "protos_perfetto_trace_track_event_lite",
@@ -2753,6 +2763,7 @@
":protos_perfetto_trace_ps_zero",
":protos_perfetto_trace_sys_stats_zero",
":protos_perfetto_trace_system_info_zero",
+ ":protos_perfetto_trace_track_event_cpp",
":protos_perfetto_trace_track_event_zero",
],
linkstatic = True,
@@ -2832,6 +2843,7 @@
":protos_perfetto_trace_ps_zero",
":protos_perfetto_trace_sys_stats_zero",
":protos_perfetto_trace_system_info_zero",
+ ":protos_perfetto_trace_track_event_cpp",
":protos_perfetto_trace_track_event_zero",
":src_perfetto_cmd_protos",
] + PERFETTO_CONFIG.deps.zlib,
diff --git a/docs/app-instrumentation.md b/docs/app-instrumentation.md
index 697ee49..922f366 100644
--- a/docs/app-instrumentation.md
+++ b/docs/app-instrumentation.md
@@ -499,10 +499,18 @@
Tracks can also optionally be annotated with metadata:
```C++
+auto desc = track.Serialize();
+desc.set_name("MyTrack");
+perfetto::TrackEvent::SetTrackDescriptor(track, desc);
+```
+
+Threads and processes can also be named in a similar way, e.g.:
+
+```C++
+auto desc = perfetto::ProcessTrack::Current().Serialize();
+desc.mutable_process()->set_process_name("MyProcess");
perfetto::TrackEvent::SetTrackDescriptor(
- track, [](perfetto::protos::pbzero::TrackDescriptor* desc) {
- desc->set_name("MyTrack");
-});
+ perfetto::ProcessTrack::Current(), desc);
```
The metadata remains valid between tracing sessions. To free up data for a
diff --git a/include/perfetto/tracing/BUILD.gn b/include/perfetto/tracing/BUILD.gn
index 2fd0810..401833d 100644
--- a/include/perfetto/tracing/BUILD.gn
+++ b/include/perfetto/tracing/BUILD.gn
@@ -18,6 +18,7 @@
"../../../protos/perfetto/config/track_event:cpp",
"../../../protos/perfetto/trace:zero",
"../../../protos/perfetto/trace/interned_data:zero",
+ "../../../protos/perfetto/trace/track_event:cpp",
"../../../protos/perfetto/trace/track_event:zero",
"../base",
"../protozero",
diff --git a/include/perfetto/tracing/internal/track_event_data_source.h b/include/perfetto/tracing/internal/track_event_data_source.h
index 1bdffec..f737054 100644
--- a/include/perfetto/tracing/internal/track_event_data_source.h
+++ b/include/perfetto/tracing/internal/track_event_data_source.h
@@ -406,18 +406,31 @@
}
// Record metadata about different types of timeline tracks. See Track.
+ static void SetTrackDescriptor(const Track& track,
+ const protos::gen::TrackDescriptor& desc) {
+ PERFETTO_DCHECK(track.uuid == desc.uuid());
+ TrackRegistry::Get()->UpdateTrack(track, desc.SerializeAsString());
+ Base::template Trace([&](typename Base::TraceContext ctx) {
+ TrackEventInternal::WriteTrackDescriptor(
+ track, ctx.tls_inst_->trace_writer.get());
+ });
+ }
+
+ // DEPRECATED. Only kept for backwards compatibility.
static void SetTrackDescriptor(
const Track& track,
std::function<void(protos::pbzero::TrackDescriptor*)> callback) {
SetTrackDescriptorImpl(track, std::move(callback));
}
+ // DEPRECATED. Only kept for backwards compatibility.
static void SetProcessDescriptor(
std::function<void(protos::pbzero::TrackDescriptor*)> callback,
const ProcessTrack& track = ProcessTrack::Current()) {
SetTrackDescriptorImpl(std::move(track), std::move(callback));
}
+ // DEPRECATED. Only kept for backwards compatibility.
static void SetThreadDescriptor(
std::function<void(protos::pbzero::TrackDescriptor*)> callback,
const ThreadTrack& track = ThreadTrack::Current()) {
@@ -529,8 +542,7 @@
static void SetTrackDescriptorImpl(
const TrackType& track,
std::function<void(protos::pbzero::TrackDescriptor*)> callback) {
- TrackRegistry::Get()->UpdateTrack(
- track, [&](protos::pbzero::TrackDescriptor* desc) { callback(desc); });
+ TrackRegistry::Get()->UpdateTrack(track, std::move(callback));
Base::template Trace([&](typename Base::TraceContext ctx) {
TrackEventInternal::WriteTrackDescriptor(
track, ctx.tls_inst_->trace_writer.get());
diff --git a/include/perfetto/tracing/track.h b/include/perfetto/tracing/track.h
index f297b08..0a4697b 100644
--- a/include/perfetto/tracing/track.h
+++ b/include/perfetto/tracing/track.h
@@ -23,6 +23,7 @@
#include "perfetto/protozero/message_handle.h"
#include "perfetto/protozero/scattered_heap_buffer.h"
#include "protos/perfetto/trace/trace_packet.pbzero.h"
+#include "protos/perfetto/trace/track_event/track_descriptor.gen.h"
#include "protos/perfetto/trace/track_event/track_descriptor.pbzero.h"
#include <stdint.h>
@@ -56,10 +57,16 @@
//
// Tracks can also be annotated with metadata:
//
+// auto desc = track.Serialize();
+// desc.set_name("MyTrack");
+// perfetto::TrackEvent::SetTrackDescriptor(track, desc);
+//
+// Threads and processes can also be named in a similar way, e.g.:
+//
+// auto desc = perfetto::ProcessTrack::Current().Serialize();
+// desc.mutable_process()->set_process_name("MyProcess");
// perfetto::TrackEvent::SetTrackDescriptor(
-// track, [](perfetto::protos::pbzero::TrackDescriptor* desc) {
-// desc->set_name("MyTrack");
-// });
+// perfetto::ProcessTrack::Current(), desc);
//
// The metadata remains valid between tracing sessions. To free up data for a
// track, call EraseTrackDescriptor:
@@ -83,6 +90,7 @@
explicit operator bool() const { return uuid; }
void Serialize(protos::pbzero::TrackDescriptor*) const;
+ protos::gen::TrackDescriptor Serialize() const;
// Construct a global track with identifier |id|.
//
@@ -114,6 +122,7 @@
static ProcessTrack Current() { return ProcessTrack(); }
void Serialize(protos::pbzero::TrackDescriptor*) const;
+ protos::gen::TrackDescriptor Serialize() const;
private:
ProcessTrack() : Track(MakeProcessTrack()), pid(base::GetProcessId()) {}
@@ -133,6 +142,7 @@
}
void Serialize(protos::pbzero::TrackDescriptor*) const;
+ protos::gen::TrackDescriptor Serialize() const;
private:
explicit ThreadTrack(base::PlatformThreadId tid_)
@@ -177,6 +187,9 @@
});
}
+ // This variant lets the user supply a serialized track descriptor directly.
+ void UpdateTrack(Track, const std::string& serialized_desc);
+
// If |track| exists in the registry, write out the serialized track
// descriptor for it into |packet|. Otherwise just the ephemeral track object
// is serialized without any additional metadata.
diff --git a/src/tracing/test/api_integrationtest.cc b/src/tracing/test/api_integrationtest.cc
index 6eaa39c..1d48dd5 100644
--- a/src/tracing/test/api_integrationtest.cc
+++ b/src/tracing/test/api_integrationtest.cc
@@ -1140,6 +1140,49 @@
EXPECT_NE(0, thread_descs[1].thread().tid());
}
+TEST_F(PerfettoApiTest, CustomTrackDescriptor) {
+ // Setup the trace config.
+ perfetto::TraceConfig cfg;
+ cfg.set_duration_ms(500);
+ cfg.add_buffers()->set_size_kb(1024);
+ auto* ds_cfg = cfg.add_data_sources()->mutable_config();
+ ds_cfg->set_name("track_event");
+
+ // Create a new trace session.
+ auto* tracing_session = NewTrace(cfg);
+ tracing_session->get()->StartBlocking();
+
+ auto track = perfetto::ProcessTrack::Current();
+ auto desc = track.Serialize();
+ desc.mutable_process()->set_process_name("testing.exe");
+ desc.mutable_chrome_process()->set_process_priority(123);
+ perfetto::TrackEvent::SetTrackDescriptor(track, std::move(desc));
+ perfetto::TrackEvent::Flush();
+
+ tracing_session->get()->StopBlocking();
+
+ std::vector<char> raw_trace = tracing_session->get()->ReadTraceBlocking();
+ perfetto::protos::gen::Trace trace;
+ ASSERT_TRUE(trace.ParseFromArray(raw_trace.data(), raw_trace.size()));
+
+ constexpr uint32_t kMainThreadSequence = 2;
+ bool found_desc = false;
+ for (const auto& packet : trace.packet()) {
+ if (packet.trusted_packet_sequence_id() != kMainThreadSequence)
+ continue;
+ if (packet.has_track_descriptor()) {
+ auto td = packet.track_descriptor();
+ EXPECT_TRUE(td.has_process());
+ EXPECT_NE(0, td.process().pid());
+ EXPECT_TRUE(td.has_chrome_process());
+ EXPECT_EQ("testing.exe", td.process().process_name());
+ EXPECT_EQ(123, td.chrome_process().process_priority());
+ found_desc = true;
+ }
+ }
+ EXPECT_TRUE(found_desc);
+}
+
TEST_F(PerfettoApiTest, TrackEventCustomTrack) {
// Create a new trace session.
auto* tracing_session = NewTraceWithCategories({"bar"});
diff --git a/src/tracing/track.cc b/src/tracing/track.cc
index b5260e4..56525f7 100644
--- a/src/tracing/track.cc
+++ b/src/tracing/track.cc
@@ -18,7 +18,9 @@
#include "perfetto/ext/base/uuid.h"
#include "perfetto/tracing/internal/track_event_data_source.h"
+#include "protos/perfetto/trace/track_event/process_descriptor.gen.h"
#include "protos/perfetto/trace/track_event/process_descriptor.pbzero.h"
+#include "protos/perfetto/trace/track_event/thread_descriptor.gen.h"
#include "protos/perfetto/trace/track_event/thread_descriptor.pbzero.h"
namespace perfetto {
@@ -26,25 +28,44 @@
// static
uint64_t Track::process_uuid;
-void Track::Serialize(protos::pbzero::TrackDescriptor* desc) const {
- desc->set_uuid(uuid);
+protos::gen::TrackDescriptor Track::Serialize() const {
+ protos::gen::TrackDescriptor desc;
+ desc.set_uuid(uuid);
if (parent_uuid)
- desc->set_parent_uuid(parent_uuid);
+ desc.set_parent_uuid(parent_uuid);
+ return desc;
+}
+
+void Track::Serialize(protos::pbzero::TrackDescriptor* desc) const {
+ auto bytes = Serialize().SerializeAsString();
+ desc->AppendRawProtoBytes(bytes.data(), bytes.size());
+}
+
+protos::gen::TrackDescriptor ProcessTrack::Serialize() const {
+ auto desc = Track::Serialize();
+ auto pd = desc.mutable_process();
+ pd->set_pid(static_cast<int32_t>(pid));
+ // TODO(skyostil): Record command line.
+ return desc;
}
void ProcessTrack::Serialize(protos::pbzero::TrackDescriptor* desc) const {
- Track::Serialize(desc);
- auto pd = desc->set_process();
- pd->set_pid(static_cast<int32_t>(pid));
- // TODO(skyostil): Record command line.
+ auto bytes = Serialize().SerializeAsString();
+ desc->AppendRawProtoBytes(bytes.data(), bytes.size());
}
-void ThreadTrack::Serialize(protos::pbzero::TrackDescriptor* desc) const {
- Track::Serialize(desc);
- auto td = desc->set_thread();
+protos::gen::TrackDescriptor ThreadTrack::Serialize() const {
+ auto desc = Track::Serialize();
+ auto td = desc.mutable_thread();
td->set_pid(static_cast<int32_t>(pid));
td->set_tid(static_cast<int32_t>(tid));
// TODO(skyostil): Record thread name.
+ return desc;
+}
+
+void ThreadTrack::Serialize(protos::pbzero::TrackDescriptor* desc) const {
+ auto bytes = Serialize().SerializeAsString();
+ desc->AppendRawProtoBytes(bytes.data(), bytes.size());
}
namespace internal {
@@ -65,6 +86,12 @@
Track::process_uuid = static_cast<uint64_t>(base::Uuidv4().lsb());
}
+void TrackRegistry::UpdateTrack(Track track,
+ const std::string& serialized_desc) {
+ std::lock_guard<std::mutex> lock(mutex_);
+ tracks_[track.uuid] = std::move(serialized_desc);
+}
+
void TrackRegistry::UpdateTrackImpl(
Track track,
std::function<void(protos::pbzero::TrackDescriptor*)> fill_function) {
@@ -74,10 +101,7 @@
kInitialSliceSize, kMaximumSliceSize);
fill_function(new_descriptor.get());
auto serialized_desc = new_descriptor.SerializeAsString();
- {
- std::lock_guard<std::mutex> lock(mutex_);
- tracks_[track.uuid] = std::move(serialized_desc);
- }
+ UpdateTrack(track, serialized_desc);
}
void TrackRegistry::EraseTrack(Track track) {