[tracing] Fix combining untyped and typed args in TRACE_EVENT.
Fix the compilation of passing a typed argument to TRACE_EVENT macro
after and untyped one (i.e. TRACE_EVENT(..., "arg", "value",
TrackEvent::kFoo, foo);) discovered in crrev.com/c/3178983.
This patch refactores WriteTrackEventArgs to fix that (using a
forward-declaration of untyped version) and adds integration tests
covering both untyped-after-typed and typed-after-untyped scenarios.
R=eseckler@google.com
CC=hbolaria@google.com
Change-Id: I61d3a9da31f93af6c6e01f4e76e812682cbed59e
diff --git a/include/perfetto/tracing/internal/write_track_event_args.h b/include/perfetto/tracing/internal/write_track_event_args.h
index 2f84878..ab215ae 100644
--- a/include/perfetto/tracing/internal/write_track_event_args.h
+++ b/include/perfetto/tracing/internal/write_track_event_args.h
@@ -24,19 +24,7 @@
namespace perfetto {
namespace internal {
-// Helper function handling filling provided |EventContext| from the provided
-// arguments, which include:
-// - Lambda functions,
-// - Debug annotations.
-//
-// TRACE_EVENT parameters which do not translate to directly writing something
-// into TrackEvent proto (like tracks and timestamps are _not_ covered by this
-// function).
-template <typename... Args, typename TypeCheck = void>
-void WriteTrackEventArgs(EventContext event_context, Args&&... args);
-
// No arguments means that we don't have to write anything.
-template <>
PERFETTO_ALWAYS_INLINE inline void WriteTrackEventArgs(EventContext) {}
namespace {
@@ -80,30 +68,25 @@
arg_function(std::move(event_ctx));
}
-// Write one debug annotation and recursively write the rest of the arguments.
+// Forward-declare the specification for writing untyped arguments to ensure
+// that typed specification could recursively pick it up.
template <typename ArgValue, typename... Args>
PERFETTO_ALWAYS_INLINE void WriteTrackEventArgs(EventContext event_ctx,
const char* arg_name,
ArgValue&& arg_value,
- Args&&... args) {
- TrackEventInternal::AddDebugAnnotation(&event_ctx, arg_name,
- std::forward<ArgValue>(arg_value));
- WriteTrackEventArgs(std::move(event_ctx), std::forward<Args>(args)...);
-}
+ Args&&... args);
// Write one typed message and recursively write the rest of the arguments.
-template <typename FieldMetadataType,
- typename ArgValue,
- typename... Args,
- typename Check = base::enable_if_t<
- std::is_base_of<protozero::proto_utils::FieldMetadataBase,
- FieldMetadataType>::value>>
+template <typename FieldMetadataType, typename ArgValue, typename... Args>
PERFETTO_ALWAYS_INLINE void WriteTrackEventArgs(
EventContext event_ctx,
protozero::proto_utils::internal::FieldMetadataHelper<FieldMetadataType>
field_name,
ArgValue&& arg_value,
Args&&... args) {
+ static_assert(std::is_base_of<protozero::proto_utils::FieldMetadataBase,
+ FieldMetadataType>::value,
+ "");
static_assert(
std::is_base_of<protos::pbzero::TrackEvent,
typename FieldMetadataType::message_type>::value,
@@ -116,6 +99,17 @@
WriteTrackEventArgs(std::move(event_ctx), std::forward<Args>(args)...);
}
+// Write one debug annotation and recursively write the rest of the arguments.
+template <typename ArgValue, typename... Args>
+PERFETTO_ALWAYS_INLINE void WriteTrackEventArgs(EventContext event_ctx,
+ const char* arg_name,
+ ArgValue&& arg_value,
+ Args&&... args) {
+ TrackEventInternal::AddDebugAnnotation(&event_ctx, arg_name,
+ std::forward<ArgValue>(arg_value));
+ WriteTrackEventArgs(std::move(event_ctx), std::forward<Args>(args)...);
+}
+
} // namespace internal
} // namespace perfetto
diff --git a/src/tracing/test/api_integrationtest.cc b/src/tracing/test/api_integrationtest.cc
index ed08a17..5e93578 100644
--- a/src/tracing/test/api_integrationtest.cc
+++ b/src/tracing/test/api_integrationtest.cc
@@ -498,7 +498,11 @@
std::vector<std::string> ReadSlicesFromTrace(
perfetto::TracingSession* tracing_session) {
- std::vector<char> raw_trace = tracing_session->ReadTraceBlocking();
+ return ReadSlicesFromTrace(tracing_session->ReadTraceBlocking());
+ }
+
+ std::vector<std::string> ReadSlicesFromTrace(
+ const std::vector<char>& raw_trace) {
EXPECT_GE(raw_trace.size(), 0u);
// Read back the trace, maintaining interning tables as we go.
@@ -1837,16 +1841,20 @@
EXPECT_TRUE(found_args);
}
-TEST_P(PerfettoApiTest, InlineTrackEventTypedArgs_NestedSingle) {
- struct LogMessage {
- void WriteIntoTrace(
- perfetto::TracedProto<perfetto::protos::pbzero::LogMessage> context)
- const {
- context->set_source_location_iid(1);
- context->set_body_iid(2);
- }
- };
+namespace {
+struct LogMessage {
+ void WriteIntoTrace(
+ perfetto::TracedProto<perfetto::protos::pbzero::LogMessage> context)
+ const {
+ context->set_source_location_iid(1);
+ context->set_body_iid(2);
+ }
+};
+
+} // namespace
+
+TEST_P(PerfettoApiTest, InlineTrackEventTypedArgs_NestedSingle) {
// Create a new trace session.
auto* tracing_session = NewTraceWithCategories({"foo"});
tracing_session->get()->StartBlocking();
@@ -1882,6 +1890,90 @@
EXPECT_TRUE(found_args);
}
+TEST_P(PerfettoApiTest, InlineTrackEventTypedAndUntypedArgs) {
+ // Create a new trace session.
+ auto* tracing_session = NewTraceWithCategories({"foo"});
+ tracing_session->get()->StartBlocking();
+
+ TRACE_EVENT_BEGIN("foo", "E",
+ perfetto::protos::pbzero::TrackEvent::kLogMessage,
+ LogMessage(), "arg", "value");
+ TRACE_EVENT_END("foo");
+
+ tracing_session->get()->StopBlocking();
+
+ std::vector<char> raw_trace = tracing_session->get()->ReadTraceBlocking();
+ std::string trace(raw_trace.data(), raw_trace.size());
+
+ perfetto::protos::gen::Trace parsed_trace;
+ ASSERT_TRUE(parsed_trace.ParseFromArray(raw_trace.data(), raw_trace.size()));
+
+ // Find typed argument.
+ bool found_args = false;
+ for (const auto& packet : parsed_trace.packet()) {
+ if (!packet.has_track_event())
+ continue;
+ const auto& track_event = packet.track_event();
+ if (track_event.type() !=
+ perfetto::protos::gen::TrackEvent::TYPE_SLICE_BEGIN) {
+ continue;
+ }
+
+ EXPECT_TRUE(track_event.has_log_message());
+ const auto& log = track_event.log_message();
+ EXPECT_EQ(1u, log.source_location_iid());
+ EXPECT_EQ(2u, log.body_iid());
+ found_args = true;
+ }
+ EXPECT_TRUE(found_args);
+
+ // Find untyped argument.
+ EXPECT_THAT(ReadSlicesFromTrace(raw_trace),
+ ElementsAre("B:foo.E(arg=(string)value)"));
+}
+
+TEST_P(PerfettoApiTest, InlineTrackEventUntypedAndTypedArgs) {
+ // Create a new trace session.
+ auto* tracing_session = NewTraceWithCategories({"foo"});
+ tracing_session->get()->StartBlocking();
+
+ TRACE_EVENT_BEGIN("foo", "E", "arg", "value",
+ perfetto::protos::pbzero::TrackEvent::kLogMessage,
+ LogMessage());
+ TRACE_EVENT_END("foo");
+
+ tracing_session->get()->StopBlocking();
+
+ std::vector<char> raw_trace = tracing_session->get()->ReadTraceBlocking();
+ std::string trace(raw_trace.data(), raw_trace.size());
+
+ perfetto::protos::gen::Trace parsed_trace;
+ ASSERT_TRUE(parsed_trace.ParseFromArray(raw_trace.data(), raw_trace.size()));
+
+ // Find typed argument.
+ bool found_args = false;
+ for (const auto& packet : parsed_trace.packet()) {
+ if (!packet.has_track_event())
+ continue;
+ const auto& track_event = packet.track_event();
+ if (track_event.type() !=
+ perfetto::protos::gen::TrackEvent::TYPE_SLICE_BEGIN) {
+ continue;
+ }
+
+ EXPECT_TRUE(track_event.has_log_message());
+ const auto& log = track_event.log_message();
+ EXPECT_EQ(1u, log.source_location_iid());
+ EXPECT_EQ(2u, log.body_iid());
+ found_args = true;
+ }
+ EXPECT_TRUE(found_args);
+
+ // Find untyped argument.
+ EXPECT_THAT(ReadSlicesFromTrace(raw_trace),
+ ElementsAre("B:foo.E(arg=(string)value)"));
+}
+
struct InternedLogMessageBody
: public perfetto::TrackEventInternedDataIndex<
InternedLogMessageBody,
