Merge "Support writing untyped annotations into typed contexts."
diff --git a/include/perfetto/tracing/traced_proto.h b/include/perfetto/tracing/traced_proto.h
index 44e4fb2..a52972b 100644
--- a/include/perfetto/tracing/traced_proto.h
+++ b/include/perfetto/tracing/traced_proto.h
@@ -20,6 +20,7 @@
#include "perfetto/base/template_util.h"
#include "perfetto/protozero/field_writer.h"
#include "perfetto/protozero/proto_utils.h"
+#include "perfetto/tracing/traced_value.h"
namespace perfetto {
class EventContext;
@@ -45,7 +46,7 @@
// TracedValue::WriteProto<T>() or implicitly.
// - If a proto message has a repeating DebugAnnotation debug_annotations
// field, it can be filled using the TracedDictionary obtained from
-// TracedProto::WriteDebugAnnotations.
+// TracedProto::AddDebugAnnotations.
template <typename MessageType>
class TracedProto {
public:
@@ -61,6 +62,31 @@
EventContext& context() const { return context_; }
+ // Write additional untyped values into the same context, which is useful
+ // when a given C++ class has a typed representation, but also either has
+ // members which can only be written into an untyped context (e.g. they are
+ // autogenerated) or it's desirable to have a way to quickly extend the
+ // trace representation of this class (e.g. for debugging).
+ //
+ // The usage of the returned TracedDictionary should not be interleaved with
+ // writing into |message| as this results in an inefficient proto layout. To
+ // enforce this, AddDebugAnnotations should be called on TracedProto&&, i.e.
+ // std::move(message).AddDebugAnnotations().
+ //
+ // This requires a 'repeated DebugAnnotations debug_annotations' field in
+ // MessageType.
+ template <typename Check = void>
+ TracedDictionary AddDebugAnnotations() && {
+ static_assert(
+ std::is_base_of<
+ protozero::proto_utils::FieldMetadataBase,
+ typename MessageType::FieldMetadata_DebugAnnotations>::value,
+ "This message does not have a |debug_annotations| field. Please add a"
+ "'repeated perfetto.protos.DebugAnnotation debug_annnotations = N;' "
+ "field to your message.");
+ return TracedDictionary(message_, MessageType::kDebugAnnotations, nullptr);
+ }
+
private:
friend class EventContext;
diff --git a/include/perfetto/tracing/traced_value.h b/include/perfetto/tracing/traced_value.h
index b1a14d3..53f1cb6 100644
--- a/include/perfetto/tracing/traced_value.h
+++ b/include/perfetto/tracing/traced_value.h
@@ -20,6 +20,8 @@
#include "perfetto/base/compiler.h"
#include "perfetto/base/export.h"
#include "perfetto/base/template_util.h"
+#include "perfetto/protozero/message.h"
+#include "perfetto/protozero/proto_utils.h"
#include "perfetto/tracing/internal/checked_scope.h"
#include "perfetto/tracing/string_helpers.h"
#include "perfetto/tracing/traced_value_forward.h"
@@ -246,12 +248,39 @@
private:
friend class TracedValue;
+ template <typename T>
+ friend class TracedProto;
- inline explicit TracedDictionary(protos::pbzero::DebugAnnotation* context,
- internal::CheckedScope* parent_scope)
- : context_(context), checked_scope_(parent_scope) {}
+ // Create a |TracedDictionary| which will populate the given field of the
+ // given |message|.
+ template <typename MessageType, typename FieldMetadata>
+ inline explicit TracedDictionary(
+ MessageType* message,
+ protozero::proto_utils::internal::FieldMetadataHelper<FieldMetadata>,
+ internal::CheckedScope* parent_scope)
+ : message_(message),
+ field_id_(FieldMetadata::kFieldId),
+ checked_scope_(parent_scope) {
+ static_assert(std::is_base_of<protozero::Message, MessageType>::value,
+ "Message should be a subclass of protozero::Message");
+ static_assert(std::is_base_of<protozero::proto_utils::FieldMetadataBase,
+ FieldMetadata>::value,
+ "FieldMetadata should be a subclass of FieldMetadataBase");
+ static_assert(
+ std::is_same<typename FieldMetadata::message_type, MessageType>::value,
+ "Field does not belong to this message");
+ static_assert(
+ std::is_same<typename FieldMetadata::cpp_field_type,
+ ::perfetto::protos::pbzero::DebugAnnotation>::value,
+ "Field should be of DebugAnnotation type");
+ static_assert(
+ FieldMetadata::kRepetitionType ==
+ protozero::proto_utils::RepetitionType::kRepeatedNotPacked,
+ "Field should be non-packed repeated");
+ }
- protos::pbzero::DebugAnnotation* context_;
+ protozero::Message* const message_;
+ const uint32_t field_id_;
internal::CheckedScope checked_scope_;
};
diff --git a/protos/perfetto/trace/perfetto_trace.proto b/protos/perfetto/trace/perfetto_trace.proto
index 7fe24f3..4ee4df6 100644
--- a/protos/perfetto/trace/perfetto_trace.proto
+++ b/protos/perfetto/trace/perfetto_trace.proto
@@ -8397,6 +8397,8 @@
// When 0 this is the bottom-most nested message.
optional uint32 remaining_nesting_depth = 3;
+
+ repeated DebugAnnotation debug_annotations = 7;
}
optional TestPayload payload = 5;
}
diff --git a/protos/perfetto/trace/test_event.proto b/protos/perfetto/trace/test_event.proto
index 67a8f60..023343e 100644
--- a/protos/perfetto/trace/test_event.proto
+++ b/protos/perfetto/trace/test_event.proto
@@ -16,6 +16,8 @@
syntax = "proto2";
+import "protos/perfetto/trace/track_event/debug_annotation.proto";
+
package perfetto.protos;
// Event used by testing code.
@@ -43,6 +45,8 @@
// When 0 this is the bottom-most nested message.
optional uint32 remaining_nesting_depth = 3;
+
+ repeated DebugAnnotation debug_annotations = 7;
}
optional TestPayload payload = 5;
}
diff --git a/protos/perfetto/trace/test_extensions.proto b/protos/perfetto/trace/test_extensions.proto
index 94118dc..dca2db6 100644
--- a/protos/perfetto/trace/test_extensions.proto
+++ b/protos/perfetto/trace/test_extensions.proto
@@ -17,6 +17,7 @@
syntax = "proto2";
import public "protos/perfetto/trace/track_event/track_event.proto";
+import "protos/perfetto/trace/track_event/debug_annotation.proto";
package perfetto.protos;
@@ -35,4 +36,6 @@
message TestExtensionChild {
optional string child_field_for_testing = 1;
+
+ repeated DebugAnnotation debug_annotations = 99;
}
diff --git a/src/trace_processor/importers/proto/track_event_parser.cc b/src/trace_processor/importers/proto/track_event_parser.cc
index 8b93efd..7926bd3 100644
--- a/src/trace_processor/importers/proto/track_event_parser.cc
+++ b/src/trace_processor/importers/proto/track_event_parser.cc
@@ -1360,21 +1360,21 @@
context_->storage->InternString("count"),
context_->storage->InternString("bytes")}} {
// Switch |source_location_iid| into its interned data variant.
- args_parser_.AddParsingOverride(
+ args_parser_.AddParsingOverrideForField(
"begin_impl_frame_args.current_args.source_location_iid",
[](const protozero::Field& field,
util::ProtoToArgsParser::Delegate& delegate) {
return MaybeParseSourceLocation("begin_impl_frame_args.current_args",
field, delegate);
});
- args_parser_.AddParsingOverride(
+ args_parser_.AddParsingOverrideForField(
"begin_impl_frame_args.last_args.source_location_iid",
[](const protozero::Field& field,
util::ProtoToArgsParser::Delegate& delegate) {
return MaybeParseSourceLocation("begin_impl_frame_args.last_args",
field, delegate);
});
- args_parser_.AddParsingOverride(
+ args_parser_.AddParsingOverrideForField(
"begin_frame_observer_state.last_begin_frame_args.source_location_iid",
[](const protozero::Field& field,
util::ProtoToArgsParser::Delegate& delegate) {
@@ -1382,7 +1382,7 @@
"begin_frame_observer_state.last_begin_frame_args", field,
delegate);
});
- args_parser_.AddParsingOverride(
+ args_parser_.AddParsingOverrideForField(
"chrome_memory_pressure_notification.creation_location_iid",
[](const protozero::Field& field,
util::ProtoToArgsParser::Delegate& delegate) {
@@ -1390,6 +1390,18 @@
field, delegate);
});
+ // Parse DebugAnnotations.
+ args_parser_.AddParsingOverrideForType(
+ ".perfetto.protos.DebugAnnotation",
+ [&](util::ProtoToArgsParser::ScopedNestedKeyContext& key,
+ const protozero::ConstBytes& data,
+ util::ProtoToArgsParser::Delegate& delegate) {
+ // Do not add "debug_annotations" to the final key.
+ key.RemoveFieldSuffix();
+ util::DebugAnnotationParser annotation_parser(args_parser_);
+ return annotation_parser.Parse(data, delegate);
+ });
+
for (uint16_t index : kReflectFields) {
reflect_fields_.push_back(index);
}
diff --git a/src/trace_processor/util/debug_annotation_parser.cc b/src/trace_processor/util/debug_annotation_parser.cc
index eeaf39f..7fa359e 100644
--- a/src/trace_processor/util/debug_annotation_parser.cc
+++ b/src/trace_processor/util/debug_annotation_parser.cc
@@ -105,18 +105,15 @@
size_t index = delegate.GetArrayEntryIndex(context_name.key);
bool added_entry = false;
for (auto it = annotation.array_values(); it; ++it) {
+ std::string array_key = context_name.key;
protos::pbzero::DebugAnnotation::Decoder value(*it);
auto nested_key = proto_to_args_parser_.EnterArray(index);
ParseResult value_parse_result =
ParseDebugAnnotationValue(value, delegate, nested_key.key());
- // Reset the key here to ensure that we have the correct array key to
- // increment.
- nested_key.Reset();
-
if (value_parse_result.added_entry) {
- index = delegate.IncrementArrayEntryIndex(context_name.key);
+ index = delegate.IncrementArrayEntryIndex(array_key);
added_entry = true;
}
if (!value_parse_result.status.ok())
diff --git a/src/trace_processor/util/proto_to_args_parser.cc b/src/trace_processor/util/proto_to_args_parser.cc
index 8e4eb22..96e066d 100644
--- a/src/trace_processor/util/proto_to_args_parser.cc
+++ b/src/trace_processor/util/proto_to_args_parser.cc
@@ -26,34 +26,11 @@
namespace {
-// ScopedStringAppender will add |append| to |dest| when constructed and
-// erases the appended suffix from |dest| when it goes out of scope. Thus
-// |dest| must be valid for the entire lifetime of ScopedStringAppender.
-//
-// This is useful as we descend into a proto since the column names just
-// appended with ".field_name" as we go lower.
-//
-// I.E. message1.message2.field_name1 is a column, but we'll then need to
-// append message1.message2.field_name2 afterwards so we only need to append
-// "field_name1" within some scope.
-class ScopedStringAppender {
- public:
- ScopedStringAppender(const std::string& append, std::string* dest)
- : old_size_(dest->size()), dest_(dest) {
- if (dest->empty()) {
- dest_->reserve(append.size());
- } else {
- dest_->reserve(old_size_ + 1 + append.size());
- dest_->append(".");
- }
- dest_->append(append);
- }
- ~ScopedStringAppender() { dest_->erase(old_size_); }
-
- private:
- size_t old_size_;
- std::string* dest_;
-};
+void AppendProtoType(std::string& target, const std::string& value) {
+ if (!target.empty())
+ target += '.';
+ target += value;
+}
} // namespace
@@ -78,10 +55,10 @@
}
ProtoToArgsParser::ScopedNestedKeyContext::~ScopedNestedKeyContext() {
- Reset();
+ RemoveFieldSuffix();
}
-void ProtoToArgsParser::ScopedNestedKeyContext::Reset() {
+void ProtoToArgsParser::ScopedNestedKeyContext::RemoveFieldSuffix() {
if (old_flat_key_length_)
key_.flat_key.resize(old_flat_key_length_.value());
if (old_key_length_)
@@ -103,6 +80,21 @@
const std::string& type,
const std::vector<uint16_t>* allowed_fields,
Delegate& delegate) {
+ ScopedNestedKeyContext key_context(key_prefix_);
+ return ParseMessageInternal(key_context, cb, type, allowed_fields, delegate);
+}
+
+base::Status ProtoToArgsParser::ParseMessageInternal(
+ ScopedNestedKeyContext& key_context,
+ const protozero::ConstBytes& cb,
+ const std::string& type,
+ const std::vector<uint16_t>* allowed_fields,
+ Delegate& delegate) {
+ if (auto override_result =
+ MaybeApplyOverrideForType(type, key_context, cb, delegate)) {
+ return override_result.value();
+ }
+
auto idx = pool_.FindDescriptorIdx(type);
if (!idx) {
return base::Status("Failed to find proto descriptor");
@@ -159,14 +151,14 @@
// In the args table we build up message1.message2.field1 as the column
// name. This will append the ".field1" suffix to |key_prefix| and then
// remove it when it goes out of scope.
- ScopedStringAppender scoped_prefix(prefix_part, &key_prefix_.key);
- ScopedStringAppender scoped_flat_key_prefix(field_descriptor.name(),
- &key_prefix_.flat_key);
+ ScopedNestedKeyContext key_context(key_prefix_);
+ AppendProtoType(key_prefix_.flat_key, field_descriptor.name());
+ AppendProtoType(key_prefix_.key, prefix_part);
// If we have an override parser then use that instead and move onto the
// next loop.
if (base::Optional<base::Status> status =
- MaybeApplyOverride(field, delegate)) {
+ MaybeApplyOverrideForField(field, delegate)) {
return *status;
}
@@ -175,27 +167,45 @@
// recurse into it.
if (field_descriptor.type() ==
protos::pbzero::FieldDescriptorProto::TYPE_MESSAGE) {
- return ParseMessage(field.as_bytes(), field_descriptor.resolved_type_name(),
- nullptr, delegate);
+ return ParseMessageInternal(key_context, field.as_bytes(),
+ field_descriptor.resolved_type_name(), nullptr,
+ delegate);
}
return ParseSimpleField(field_descriptor, field, delegate);
}
-void ProtoToArgsParser::AddParsingOverride(std::string field,
- ParsingOverride func) {
- overrides_[std::move(field)] = std::move(func);
+void ProtoToArgsParser::AddParsingOverrideForField(
+ const std::string& field,
+ ParsingOverrideForField func) {
+ field_overrides_[field] = std::move(func);
}
-base::Optional<base::Status> ProtoToArgsParser::MaybeApplyOverride(
+void ProtoToArgsParser::AddParsingOverrideForType(const std::string& type,
+ ParsingOverrideForType func) {
+ type_overrides_[type] = std::move(func);
+}
+
+base::Optional<base::Status> ProtoToArgsParser::MaybeApplyOverrideForField(
const protozero::Field& field,
Delegate& delegate) {
- auto it = overrides_.find(key_prefix_.flat_key);
- if (it == overrides_.end())
+ auto it = field_overrides_.find(key_prefix_.flat_key);
+ if (it == field_overrides_.end())
return base::nullopt;
return it->second(field, delegate);
}
+base::Optional<base::Status> ProtoToArgsParser::MaybeApplyOverrideForType(
+ const std::string& message_type,
+ ScopedNestedKeyContext& key,
+ const protozero::ConstBytes& data,
+ Delegate& delegate) {
+ auto it = type_overrides_.find(message_type);
+ if (it == type_overrides_.end())
+ return base::nullopt;
+ return it->second(key, data, delegate);
+}
+
base::Status ProtoToArgsParser::ParseSimpleField(
const FieldDescriptor& descriptor,
const protozero::Field& field,
@@ -275,12 +285,8 @@
ProtoToArgsParser::ScopedNestedKeyContext ProtoToArgsParser::EnterDictionary(
const std::string& name) {
auto context = ScopedNestedKeyContext(key_prefix_);
- if (!key_prefix_.key.empty())
- key_prefix_.key += '.';
- key_prefix_.key += name;
- if (!key_prefix_.flat_key.empty())
- key_prefix_.flat_key += '.';
- key_prefix_.flat_key += name;
+ AppendProtoType(key_prefix_.key, name);
+ AppendProtoType(key_prefix_.flat_key, name);
return context;
}
diff --git a/src/trace_processor/util/proto_to_args_parser.h b/src/trace_processor/util/proto_to_args_parser.h
index 9110e91..e6934a6 100644
--- a/src/trace_processor/util/proto_to_args_parser.h
+++ b/src/trace_processor/util/proto_to_args_parser.h
@@ -110,7 +110,64 @@
uint64_t iid) = 0;
};
- using ParsingOverride =
+ // Given a view of bytes that represent a serialized protozero message of
+ // |type| we will parse each field.
+ //
+ // Returns on any error with a status describing the problem. However any
+ // added values before encountering the error will be parsed and forwarded to
+ // the delegate.
+ //
+ // Fields with ids given in |fields| are parsed using reflection, as well
+ // as known (previously registered) extension fields. If |allowed_fields| is a
+ // nullptr, all fields are going to be parsed.
+ //
+ // Note:
+ // |type| must be the fully qualified name, but with a '.' added to the
+ // beginning. I.E. ".perfetto.protos.TrackEvent". And must match one of the
+ // descriptors already added through |AddProtoFileDescriptor|.
+ //
+ // IMPORTANT: currently bytes fields are not supported.
+ //
+ // TODO(b/145578432): Add support for byte fields.
+ base::Status ParseMessage(const protozero::ConstBytes& cb,
+ const std::string& type,
+ const std::vector<uint16_t>* allowed_fields,
+ Delegate& delegate);
+
+ // This class is responsible for resetting the current key prefix to the old
+ // value when deleted or reset.
+ struct ScopedNestedKeyContext {
+ public:
+ ~ScopedNestedKeyContext();
+ ScopedNestedKeyContext(ScopedNestedKeyContext&&);
+ ScopedNestedKeyContext(const ScopedNestedKeyContext&) = delete;
+ ScopedNestedKeyContext& operator=(const ScopedNestedKeyContext&) = delete;
+
+ const Key& key() const { return key_; }
+
+ // Clear this context, which strips the latest suffix from |key_| and sets
+ // it to the state before the nested context was created.
+ void RemoveFieldSuffix();
+
+ private:
+ friend class ProtoToArgsParser;
+
+ ScopedNestedKeyContext(Key& old_value);
+
+ struct ScopedStringAppender;
+
+ Key& key_;
+ base::Optional<size_t> old_flat_key_length_ = base::nullopt;
+ base::Optional<size_t> old_key_length_ = base::nullopt;
+ };
+
+ // These methods can be called from parsing overrides to enter nested
+ // contexts. The contexts are left when the returned scope is destroyed or
+ // RemoveFieldSuffix() is called.
+ ScopedNestedKeyContext EnterDictionary(const std::string& key);
+ ScopedNestedKeyContext EnterArray(size_t index);
+
+ using ParsingOverrideForField =
std::function<base::Optional<base::Status>(const protozero::Field&,
Delegate& delegate)>;
@@ -136,63 +193,41 @@
// To override the handling of both SubMessage fields you must add two parsing
// overrides. One with a |field_path| == "field1.field" and another with
// "field2.field".
- void AddParsingOverride(std::string field_path,
- ParsingOverride parsing_override);
+ void AddParsingOverrideForField(const std::string& field_path,
+ ParsingOverrideForField parsing_override);
- // Given a view of bytes that represent a serialized protozero message of
- // |type| we will parse each field.
+ using ParsingOverrideForType = std::function<base::Optional<base::Status>(
+ ScopedNestedKeyContext& key,
+ const protozero::ConstBytes& data,
+ Delegate& delegate)>;
+
+ // Installs an override for all fields with the given type. We will invoke
+ // |parsing_override| when a field with the given message type is encountered.
+ // Note that the path-based overrides take precedence over type overrides.
//
- // Returns on any error with a status describing the problem. However any
- // added values before encountering the error will be parsed and forwarded to
- // the delegate.
+ // The return value of |parsing_override| indicates whether the override
+ // parsed the sub-message and ProtoToArgsParser should skip it (base::nullopt)
+ // or the sub-message should continue to be parsed by ProtoToArgsParser using
+ // the descriptor (base::Status).
//
- // Fields with ids given in |fields| are parsed using reflection, as well
- // as known (previously registered) extension fields. If |allowed_fields| is a
- // nullptr, all fields are going to be parsed.
//
- // Note:
- // |type| must be the fully qualified name, but with a '.' added to the
- // beginning. I.E. ".perfetto.protos.TrackEvent". And must match one of the
- // descriptors already added through |AddProtoFileDescriptor|.
+ // For example, given the following protos and a type override for SubMessage,
+ // all three fields will be parsed using this override.
//
- // IMPORTANT: currently bytes fields are not supported.
+ // message SubMessage {
+ // optional int32 value = 1;
+ // }
//
- // TODO(b/145578432): Add support for byte fields.
- base::Status ParseMessage(const protozero::ConstBytes& cb,
- const std::string& type,
- const std::vector<uint16_t>* allowed_fields,
- Delegate& delegate);
-
- struct ScopedNestedKeyContext {
- public:
- ~ScopedNestedKeyContext();
- ScopedNestedKeyContext(ScopedNestedKeyContext&&);
- ScopedNestedKeyContext(const ScopedNestedKeyContext&) = delete;
- ScopedNestedKeyContext& operator=(const ScopedNestedKeyContext&) = delete;
-
- const Key& key() const { return key_; }
-
- // Reset this context, which sets |key_| to the state before the nested
- // context was created.
- void Reset();
-
- private:
- friend class ProtoToArgsParser;
-
- ScopedNestedKeyContext(Key& old_value);
-
- struct ScopedStringAppender;
-
- Key& key_;
- base::Optional<size_t> old_flat_key_length_ = base::nullopt;
- base::Optional<size_t> old_key_length_ = base::nullopt;
- };
-
- // These methods can be called from parsing overrides to enter nested
- // contexts. The contexts are left when the returned scope is destroyed or
- // reset.
- ScopedNestedKeyContext EnterDictionary(const std::string& key);
- ScopedNestedKeyContext EnterArray(size_t index);
+ // message MainMessage1 {
+ // optional SubMessage field1 = 1;
+ // optional SubMessage field2 = 2;
+ // }
+ //
+ // message MainMessage2 {
+ // optional SubMessage field3 = 1;
+ // }
+ void AddParsingOverrideForType(const std::string& message_type,
+ ParsingOverrideForType parsing_override);
private:
base::Status ParseField(const FieldDescriptor& field_descriptor,
@@ -200,14 +235,30 @@
protozero::Field field,
Delegate& delegate);
- base::Optional<base::Status> MaybeApplyOverride(const protozero::Field&,
- Delegate& delegate);
+ base::Optional<base::Status> MaybeApplyOverrideForField(
+ const protozero::Field&,
+ Delegate& delegate);
+
+ base::Optional<base::Status> MaybeApplyOverrideForType(
+ const std::string& message_type,
+ ScopedNestedKeyContext& key,
+ const protozero::ConstBytes& data,
+ Delegate& delegate);
+
+ // A type override can call |key.RemoveFieldSuffix()| if it wants to exclude
+ // the overriden field's name from the parsed args' keys.
+ base::Status ParseMessageInternal(ScopedNestedKeyContext& key,
+ const protozero::ConstBytes& cb,
+ const std::string& type,
+ const std::vector<uint16_t>* fields,
+ Delegate& delegate);
base::Status ParseSimpleField(const FieldDescriptor& desciptor,
const protozero::Field& field,
Delegate& delegate);
- std::unordered_map<std::string, ParsingOverride> overrides_;
+ std::unordered_map<std::string, ParsingOverrideForField> field_overrides_;
+ std::unordered_map<std::string, ParsingOverrideForType> type_overrides_;
const DescriptorPool& pool_;
Key key_prefix_;
};
diff --git a/src/trace_processor/util/proto_to_args_parser_unittest.cc b/src/trace_processor/util/proto_to_args_parser_unittest.cc
index a8d17f6..8171742 100644
--- a/src/trace_processor/util/proto_to_args_parser_unittest.cc
+++ b/src/trace_processor/util/proto_to_args_parser_unittest.cc
@@ -35,10 +35,9 @@
constexpr size_t kChunkSize = 42;
-using ::testing::_;
-using ::testing::Eq;
-using ::testing::Invoke;
-using ::testing::NiceMock;
+protozero::ConstChars ToChars(const char* str) {
+ return protozero::ConstChars{str, strlen(str)};
+}
class ProtoToArgsParserTest : public ::testing::Test,
public ProtoToArgsParser::Delegate {
@@ -254,7 +253,7 @@
ASSERT_TRUE(status.ok()) << "Failed to parse kTestMessagesDescriptor: "
<< status.message();
- parser.AddParsingOverride(
+ parser.AddParsingOverrideForField(
"super_nested.value_c",
[](const protozero::Field& field, ProtoToArgsParser::Delegate& writer) {
EXPECT_EQ(field.type(), protozero::proto_utils::ProtoWireType::kVarInt);
@@ -290,7 +289,7 @@
ASSERT_TRUE(status.ok()) << "Failed to parse kTestMessagesDescriptor: "
<< status.message();
- parser.AddParsingOverride(
+ parser.AddParsingOverrideForField(
"super_nested.value_c",
[](const protozero::Field& field, ProtoToArgsParser::Delegate&) {
static int val = 0;
@@ -341,7 +340,7 @@
ProtoToArgsParser parser(pool);
// Now we override the behaviour of |value_c| so we can expand the iid into
// multiple args rows.
- parser.AddParsingOverride(
+ parser.AddParsingOverrideForField(
"super_nested.value_c",
[](const protozero::Field& field, ProtoToArgsParser::Delegate& delegate)
-> base::Optional<base::Status> {
@@ -367,6 +366,79 @@
"line_number line_number 2"));
}
+TEST_F(ProtoToArgsParserTest, OverrideForType) {
+ using namespace protozero::test::protos::pbzero;
+ protozero::HeapBuffered<NestedA> msg{kChunkSize, kChunkSize};
+ msg->set_super_nested()->set_value_c(3);
+
+ auto binary_proto = msg.SerializeAsArray();
+
+ DescriptorPool pool;
+ auto status = pool.AddFromFileDescriptorSet(kTestMessagesDescriptor.data(),
+ kTestMessagesDescriptor.size());
+ ASSERT_TRUE(status.ok()) << "Failed to parse kTestMessagesDescriptor: "
+ << status.message();
+
+ ProtoToArgsParser parser(pool);
+
+ parser.AddParsingOverrideForType(
+ ".protozero.test.protos.NestedA.NestedB.NestedC",
+ [](ProtoToArgsParser::ScopedNestedKeyContext&,
+ const protozero::ConstBytes&, Delegate& delegate) {
+ delegate.AddInteger(ProtoToArgsParser::Key("arg"), 42);
+ return base::OkStatus();
+ });
+
+ status = parser.ParseMessage(
+ protozero::ConstBytes{binary_proto.data(), binary_proto.size()},
+ ".protozero.test.protos.NestedA", nullptr, *this);
+ EXPECT_TRUE(status.ok())
+ << "InternProtoFieldsIntoArgsTable failed with error: "
+ << status.message();
+ EXPECT_THAT(args(), testing::ElementsAre("arg arg 42"));
+}
+
+TEST_F(ProtoToArgsParserTest, FieldOverrideTakesPrecedence) {
+ using namespace protozero::test::protos::pbzero;
+ protozero::HeapBuffered<NestedA> msg{kChunkSize, kChunkSize};
+ msg->set_super_nested()->set_value_c(3);
+
+ auto binary_proto = msg.SerializeAsArray();
+
+ DescriptorPool pool;
+ auto status = pool.AddFromFileDescriptorSet(kTestMessagesDescriptor.data(),
+ kTestMessagesDescriptor.size());
+ ASSERT_TRUE(status.ok()) << "Failed to parse kTestMessagesDescriptor: "
+ << status.message();
+
+ ProtoToArgsParser parser(pool);
+
+ parser.AddParsingOverrideForField(
+ "super_nested",
+ [](const protozero::Field&, ProtoToArgsParser::Delegate& writer) {
+ writer.AddString(ProtoToArgsParser::Key("arg"),
+ ToChars("override-for-field"));
+ return base::OkStatus();
+ });
+
+ parser.AddParsingOverrideForType(
+ ".protozero.test.protos.NestedA.NestedB.NestedC",
+ [](ProtoToArgsParser::ScopedNestedKeyContext&,
+ const protozero::ConstBytes&, Delegate& delegate) {
+ delegate.AddString(ProtoToArgsParser::Key("arg"),
+ ToChars("override-for-type"));
+ return base::OkStatus();
+ });
+
+ status = parser.ParseMessage(
+ protozero::ConstBytes{binary_proto.data(), binary_proto.size()},
+ ".protozero.test.protos.NestedA", nullptr, *this);
+ EXPECT_TRUE(status.ok())
+ << "InternProtoFieldsIntoArgsTable failed with error: "
+ << status.message();
+ EXPECT_THAT(args(), testing::ElementsAre("arg arg override-for-field"));
+}
+
} // namespace
} // namespace util
} // namespace trace_processor
diff --git a/src/tracing/traced_proto_unittest.cc b/src/tracing/traced_proto_unittest.cc
index e5c84f1..ac55400 100644
--- a/src/tracing/traced_proto_unittest.cc
+++ b/src/tracing/traced_proto_unittest.cc
@@ -16,6 +16,7 @@
#include "perfetto/tracing/traced_proto.h"
+#include "perfetto/test/traced_value_test_support.h"
#include "perfetto/tracing/track_event.h"
#include "protos/perfetto/trace/test_event.gen.h"
#include "protos/perfetto/trace/test_event.pb.h"
@@ -87,6 +88,9 @@
struct Foo {
void WriteIntoTrace(TracedProto<TestPayload> message) const {
message->set_single_int(42);
+
+ auto dict = std::move(message).AddDebugAnnotations();
+ dict.Add("arg", "value");
}
};
@@ -114,4 +118,21 @@
EXPECT_EQ(result.nested(1).single_int(), 42);
}
+TEST_F(TracedProtoTest, WriteDebugAnnotations) {
+ protozero::HeapBuffered<protos::pbzero::TestEvent> event;
+ WriteIntoTracedProto(context().Wrap(event.get()),
+ protos::pbzero::TestEvent::kPayload, Foo());
+
+ protos::TestEvent result;
+ result.ParseFromString(event.SerializeAsString());
+
+ protos::DebugAnnotation dict;
+ for (const auto& annotation : result.payload().debug_annotations()) {
+ *dict.add_dict_entries() = annotation;
+ }
+
+ EXPECT_EQ(internal::DebugAnnotationToString(dict.SerializeAsString()),
+ "{arg:value}");
+}
+
} // namespace perfetto
diff --git a/src/tracing/traced_value.cc b/src/tracing/traced_value.cc
index 9ac3fd2..bd8b1ad 100644
--- a/src/tracing/traced_value.cc
+++ b/src/tracing/traced_value.cc
@@ -84,7 +84,9 @@
checked_scope_.Reset();
PERFETTO_DCHECK(!context_->is_finalized());
- return TracedDictionary(context_, checked_scope_.parent_scope());
+ return TracedDictionary(context_,
+ protos::pbzero::DebugAnnotation::kDictEntries,
+ checked_scope_.parent_scope());
}
TracedArray TracedValue::WriteArray() && {
@@ -114,14 +116,16 @@
TracedValue TracedDictionary::AddItem(StaticString key) {
PERFETTO_DCHECK(checked_scope_.is_active());
- protos::pbzero::DebugAnnotation* item = context_->add_dict_entries();
+ protos::pbzero::DebugAnnotation* item =
+ message_->BeginNestedMessage<protos::pbzero::DebugAnnotation>(field_id_);
item->set_name(key.value);
return TracedValue(item, &checked_scope_);
}
TracedValue TracedDictionary::AddItem(DynamicString key) {
PERFETTO_DCHECK(checked_scope_.is_active());
- protos::pbzero::DebugAnnotation* item = context_->add_dict_entries();
+ protos::pbzero::DebugAnnotation* item =
+ message_->BeginNestedMessage<protos::pbzero::DebugAnnotation>(field_id_);
item->set_name(key.value);
return TracedValue(item, &checked_scope_);
}
diff --git a/test/trace_processor/track_event/track_event_typed_args.textproto b/test/trace_processor/track_event/track_event_typed_args.textproto
index 6ff29b1..b2d9208 100644
--- a/test/trace_processor/track_event/track_event_typed_args.textproto
+++ b/test/trace_processor/track_event/track_event_typed_args.textproto
@@ -78,6 +78,17 @@
"should be absent from result"
[perfetto.protos.TestExtension.nested_message_extension_for_testing] {
child_field_for_testing: "nesting test"
+ debug_annotations {
+ name: "arg1"
+ string_value: "value"
+ }
+ debug_annotations {
+ name: "arg2"
+ dict_entries {
+ name: "key"
+ string_value: "value"
+ }
+ }
}
}
}
@@ -120,6 +131,13 @@
type: TYPE_STRING
label: LABEL_OPTIONAL
}
+ field {
+ name: "debug_annotations"
+ number: 99
+ type: TYPE_MESSAGE
+ label: LABEL_REPEATED
+ type_name: ".perfetto.protos.DebugAnnotation"
+ }
}
}
}
diff --git a/test/trace_processor/track_event/track_event_typed_args_args.out b/test/trace_processor/track_event/track_event_typed_args_args.out
index ac7d608..fae18d7 100644
--- a/test/trace_processor/track_event/track_event_typed_args_args.out
+++ b/test/trace_processor/track_event/track_event_typed_args_args.out
@@ -13,6 +13,8 @@
"chrome_latency_info.trace_id","chrome_latency_info.trace_id",7,"[NULL]"
"int_extension_for_testing","int_extension_for_testing[0]",42,"[NULL]"
"int_extension_for_testing","int_extension_for_testing[1]",1337,"[NULL]"
+"nested_message_extension_for_testing.arg1","nested_message_extension_for_testing.arg1","[NULL]","value"
+"nested_message_extension_for_testing.arg2.key","nested_message_extension_for_testing.arg2.key","[NULL]","value"
"nested_message_extension_for_testing.child_field_for_testing","nested_message_extension_for_testing.child_field_for_testing","[NULL]","nesting test"
"string_extension_for_testing","string_extension_for_testing","[NULL]","an extension string!"
"chrome_app_state","chrome_app_state","[NULL]","APP_STATE_FOREGROUND"
