trace_processor: refactor and simplify metrics function code
This CL works on simplifying some of the code for the metrics SQL
functions. This prepares for changing the implementation of the
RepeatedField function in a followup CL.
While doing this, since it's very helpful, also add some gtest matchers
for status OK/error checking which makes the tests nicer.
Change-Id: Icf1ad1417eab5e3d309f00fd340a22ba874ca7d2
diff --git a/Android.bp b/Android.bp
index 169c74f..9e001f0 100644
--- a/Android.bp
+++ b/Android.bp
@@ -1726,6 +1726,13 @@
":perfetto_src_tracing_ipc_service_service",
":perfetto_test_test_helper",
],
+ static_libs: [
+ "libgmock",
+ "libgtest",
+ ],
+ whole_static_libs: [
+ "perfetto_gtest_logcat_printer",
+ ],
generated_headers: [
"perfetto_protos_perfetto_common_cpp_gen_headers",
"perfetto_protos_perfetto_common_zero_gen_headers",
diff --git a/src/base/BUILD.gn b/src/base/BUILD.gn
index f9ecc20..094e69f 100644
--- a/src/base/BUILD.gn
+++ b/src/base/BUILD.gn
@@ -155,8 +155,10 @@
deps = [
":base",
"../../gn:default_deps",
+ "../../gn:gtest_and_gmock",
]
sources = [
+ "test/status_matchers.h",
"test/tmp_dir_tree.cc",
"test/tmp_dir_tree.h",
"test/utils.cc",
diff --git a/src/base/test/status_matchers.h b/src/base/test/status_matchers.h
new file mode 100644
index 0000000..ab16b99
--- /dev/null
+++ b/src/base/test/status_matchers.h
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef SRC_BASE_TEST_STATUS_MATCHERS_H_
+#define SRC_BASE_TEST_STATUS_MATCHERS_H_
+
+#include "test/gtest_and_gmock.h"
+
+namespace perfetto::base::gtest_matchers {
+
+// Returns a gMock matcher that matches a Status or StatusOr<> which is OK.
+MATCHER(IsOk, negation ? "is not OK" : "is OK") {
+ return arg.ok();
+}
+
+// Returns a gMock matcher that matches a Status or StatusOr<> which is an
+// error.
+MATCHER(IsError, negation ? "is not error" : "is error") {
+ return !arg.ok();
+}
+
+// Macros for testing the results of functions that return absl::Status or
+// absl::StatusOr<T> (for any type T).
+#define EXPECT_OK(expression) \
+ EXPECT_THAT(expression, ::perfetto::base::gtest_matchers::IsOk())
+#define ASSERT_OK(expression) \
+ ASSERT_THAT(expression, ::perfetto::base::gtest_matchers::IsOk())
+
+} // namespace perfetto::base::gtest_matchers
+
+#endif // SRC_BASE_TEST_STATUS_MATCHERS_H_
diff --git a/src/trace_processor/metrics/BUILD.gn b/src/trace_processor/metrics/BUILD.gn
index 6fe12ad..d3d2c9f 100644
--- a/src/trace_processor/metrics/BUILD.gn
+++ b/src/trace_processor/metrics/BUILD.gn
@@ -45,6 +45,7 @@
"../../../include/perfetto/trace_processor",
"../../../protos/perfetto/common:zero",
"../../../protos/perfetto/trace_processor:metrics_impl_zero",
+ "../../../protos/perfetto/trace_processor:zero",
"../../base",
"../../protozero:protozero",
"../perfetto_sql/engine",
@@ -71,5 +72,9 @@
"../../../gn:gtest_and_gmock",
"../../../gn:sqlite",
"../../../protos/perfetto/common:zero",
+ "../../../protos/perfetto/trace_processor:metrics_impl_zero",
+ "../../../protos/perfetto/trace_processor:zero",
+ "../../base:test_support",
+ "../../protozero",
]
}
diff --git a/src/trace_processor/metrics/metrics.cc b/src/trace_processor/metrics/metrics.cc
index ddd9e24..43f9383 100644
--- a/src/trace_processor/metrics/metrics.cc
+++ b/src/trace_processor/metrics/metrics.cc
@@ -16,15 +16,32 @@
#include "src/trace_processor/metrics/metrics.h"
+#include <algorithm>
+#include <cinttypes>
+#include <cstddef>
+#include <cstdint>
+#include <cstdlib>
+#include <cstring>
+#include <iterator>
+#include <memory>
+#include <optional>
#include <regex>
+#include <string>
#include <unordered_map>
+#include <utility>
#include <vector>
+#include <sqlite3.h>
+
+#include "perfetto/base/logging.h"
#include "perfetto/base/status.h"
-#include "perfetto/ext/base/string_utils.h"
+#include "perfetto/ext/base/status_or.h"
+#include "perfetto/ext/base/string_view.h"
#include "perfetto/ext/base/utils.h"
+#include "perfetto/protozero/field.h"
+#include "perfetto/protozero/proto_utils.h"
#include "perfetto/protozero/scattered_heap_buffer.h"
-#include "sqlite3.h"
+#include "perfetto/trace_processor/basic_types.h"
#include "src/trace_processor/perfetto_sql/engine/perfetto_sql_engine.h"
#include "src/trace_processor/sqlite/sql_source.h"
#include "src/trace_processor/sqlite/sqlite_utils.h"
@@ -33,6 +50,7 @@
#include "src/trace_processor/util/status_macros.h"
#include "protos/perfetto/common/descriptor.pbzero.h"
+#include "protos/perfetto/trace_processor/metatrace_categories.pbzero.h"
#include "protos/perfetto/trace_processor/metrics_impl.pbzero.h"
namespace perfetto {
@@ -41,11 +59,11 @@
namespace {
-base::Status ValidateSingleNonEmptyMessage(const uint8_t* ptr,
- size_t size,
- uint32_t schema_type,
- const std::string& message_type,
- protozero::ConstBytes* out) {
+base::StatusOr<protozero::ConstBytes> ValidateSingleNonEmptyMessage(
+ const uint8_t* ptr,
+ size_t size,
+ uint32_t schema_type,
+ const std::string& message_type) {
PERFETTO_DCHECK(size > 0);
if (size > protozero::proto_utils::kMaxMessageLength) {
@@ -82,11 +100,10 @@
// We disallow 0 size fields here as they should have been reported as null
// one layer down.
- *out = single.protobuf();
- if (out->size == 0) {
+ if (single.protobuf().size == 0) {
return base::ErrStatus("Field has zero size");
}
- return base::OkStatus();
+ return single.protobuf();
}
} // namespace
@@ -97,17 +114,38 @@
base::Status ProtoBuilder::AppendSqlValue(const std::string& field_name,
const SqlValue& value) {
+ base::StatusOr<const FieldDescriptor*> desc = FindFieldByName(field_name);
+ RETURN_IF_ERROR(desc.status());
switch (value.type) {
case SqlValue::kLong:
- return AppendLong(field_name, value.long_value);
+ if (desc.value()->is_repeated()) {
+ return base::ErrStatus(
+ "Unexpected long value for repeated field %s in proto type %s",
+ field_name.c_str(), descriptor_->full_name().c_str());
+ }
+ return AppendSingleLong(**desc, value.long_value);
case SqlValue::kDouble:
- return AppendDouble(field_name, value.double_value);
+ if (desc.value()->is_repeated()) {
+ return base::ErrStatus(
+ "Unexpected double value for repeated field %s in proto type %s",
+ field_name.c_str(), descriptor_->full_name().c_str());
+ }
+ return AppendSingleDouble(**desc, value.double_value);
case SqlValue::kString:
- return AppendString(field_name, value.string_value);
- case SqlValue::kBytes:
- return AppendBytes(field_name,
- static_cast<const uint8_t*>(value.bytes_value),
- value.bytes_count);
+ if (desc.value()->is_repeated()) {
+ return base::ErrStatus(
+ "Unexpected string value for repeated field %s in proto type %s",
+ field_name.c_str(), descriptor_->full_name().c_str());
+ }
+ return AppendSingleString(**desc, value.string_value);
+ case SqlValue::kBytes: {
+ const auto* ptr = static_cast<const uint8_t*>(value.bytes_value);
+ size_t size = value.bytes_count;
+ if (desc.value()->is_repeated()) {
+ return AppendRepeated(**desc, ptr, size);
+ }
+ return AppendSingleBytes(**desc, ptr, size);
+ }
case SqlValue::kNull:
// If the value is null, it's treated as the field being absent so we
// don't append anything.
@@ -116,38 +154,24 @@
PERFETTO_FATAL("For GCC");
}
-base::Status ProtoBuilder::AppendLong(const std::string& field_name,
- int64_t value,
- bool is_inside_repeated) {
- auto field = descriptor_->FindFieldByName(field_name);
- if (!field) {
- return base::ErrStatus("Field with name %s not found in proto type %s",
- field_name.c_str(),
- descriptor_->full_name().c_str());
- }
-
+base::Status ProtoBuilder::AppendSingleLong(const FieldDescriptor& field,
+ int64_t value) {
using FieldDescriptorProto = protos::pbzero::FieldDescriptorProto;
- if (field->is_repeated() && !is_inside_repeated) {
- return base::ErrStatus(
- "Unexpected long value for repeated field %s in proto type %s",
- field_name.c_str(), descriptor_->full_name().c_str());
- }
-
- switch (field->type()) {
+ switch (field.type()) {
case FieldDescriptorProto::TYPE_INT32:
case FieldDescriptorProto::TYPE_INT64:
case FieldDescriptorProto::TYPE_UINT32:
case FieldDescriptorProto::TYPE_BOOL:
- message_->AppendVarInt(field->number(), value);
+ message_->AppendVarInt(field.number(), value);
break;
case FieldDescriptorProto::TYPE_ENUM: {
auto opt_enum_descriptor_idx =
- pool_->FindDescriptorIdx(field->resolved_type_name());
+ pool_->FindDescriptorIdx(field.resolved_type_name());
if (!opt_enum_descriptor_idx) {
return base::ErrStatus(
"Unable to find enum type %s to fill field %s (in proto message "
"%s)",
- field->resolved_type_name().c_str(), field->name().c_str(),
+ field.resolved_type_name().c_str(), field.name().c_str(),
descriptor_->full_name().c_str());
}
const auto& enum_desc = pool_->descriptors()[*opt_enum_descriptor_idx];
@@ -157,64 +181,49 @@
" "
"in enum type %s; encountered while filling "
"field %s (in proto message %s)",
- value, field->resolved_type_name().c_str(),
- field->name().c_str(),
+ value, field.resolved_type_name().c_str(),
+ field.name().c_str(),
descriptor_->full_name().c_str());
}
- message_->AppendVarInt(field->number(), value);
+ message_->AppendVarInt(field.number(), value);
break;
}
case FieldDescriptorProto::TYPE_SINT32:
case FieldDescriptorProto::TYPE_SINT64:
- message_->AppendSignedVarInt(field->number(), value);
+ message_->AppendSignedVarInt(field.number(), value);
break;
case FieldDescriptorProto::TYPE_FIXED32:
case FieldDescriptorProto::TYPE_SFIXED32:
case FieldDescriptorProto::TYPE_FIXED64:
case FieldDescriptorProto::TYPE_SFIXED64:
- message_->AppendFixed(field->number(), value);
+ message_->AppendFixed(field.number(), value);
break;
case FieldDescriptorProto::TYPE_UINT64:
return base::ErrStatus(
"Field %s (in proto message %s) is using a uint64 type. uint64 in "
"metric messages is not supported by trace processor; use an int64 "
"field instead.",
- field->name().c_str(), descriptor_->full_name().c_str());
+ field.name().c_str(), descriptor_->full_name().c_str());
default: {
return base::ErrStatus(
"Tried to write value of type long into field %s (in proto type %s) "
"which has type %d",
- field->name().c_str(), descriptor_->full_name().c_str(),
- field->type());
+ field.name().c_str(), descriptor_->full_name().c_str(), field.type());
}
}
return base::OkStatus();
}
-base::Status ProtoBuilder::AppendDouble(const std::string& field_name,
- double value,
- bool is_inside_repeated) {
- auto field = descriptor_->FindFieldByName(field_name);
- if (!field) {
- return base::ErrStatus("Field with name %s not found in proto type %s",
- field_name.c_str(),
- descriptor_->full_name().c_str());
- }
-
+base::Status ProtoBuilder::AppendSingleDouble(const FieldDescriptor& field,
+ double value) {
using FieldDescriptorProto = protos::pbzero::FieldDescriptorProto;
- if (field->is_repeated() && !is_inside_repeated) {
- return base::ErrStatus(
- "Unexpected double value for repeated field %s in proto type %s",
- field_name.c_str(), descriptor_->full_name().c_str());
- }
-
- switch (field->type()) {
+ switch (field.type()) {
case FieldDescriptorProto::TYPE_FLOAT:
case FieldDescriptorProto::TYPE_DOUBLE: {
- if (field->type() == FieldDescriptorProto::TYPE_FLOAT) {
- message_->AppendFixed(field->number(), static_cast<float>(value));
+ if (field.type() == FieldDescriptorProto::TYPE_FLOAT) {
+ message_->AppendFixed(field.number(), static_cast<float>(value));
} else {
- message_->AppendFixed(field->number(), value);
+ message_->AppendFixed(field.number(), value);
}
break;
}
@@ -222,43 +231,28 @@
return base::ErrStatus(
"Tried to write value of type double into field %s (in proto type "
"%s) which has type %d",
- field->name().c_str(), descriptor_->full_name().c_str(),
- field->type());
+ field.name().c_str(), descriptor_->full_name().c_str(), field.type());
}
}
return base::OkStatus();
}
-base::Status ProtoBuilder::AppendString(const std::string& field_name,
- base::StringView data,
- bool is_inside_repeated) {
- const FieldDescriptor* field = descriptor_->FindFieldByName(field_name);
- if (!field) {
- return base::ErrStatus("Field with name %s not found in proto type %s",
- field_name.c_str(),
- descriptor_->full_name().c_str());
- }
-
+base::Status ProtoBuilder::AppendSingleString(const FieldDescriptor& field,
+ base::StringView data) {
using FieldDescriptorProto = protos::pbzero::FieldDescriptorProto;
- if (field->is_repeated() && !is_inside_repeated) {
- return base::ErrStatus(
- "Unexpected string value for repeated field %s in proto type %s",
- field_name.c_str(), descriptor_->full_name().c_str());
- }
-
- switch (field->type()) {
+ switch (field.type()) {
case FieldDescriptorProto::TYPE_STRING: {
- message_->AppendBytes(field->number(), data.data(), data.size());
+ message_->AppendBytes(field.number(), data.data(), data.size());
break;
}
case FieldDescriptorProto::TYPE_ENUM: {
auto opt_enum_descriptor_idx =
- pool_->FindDescriptorIdx(field->resolved_type_name());
+ pool_->FindDescriptorIdx(field.resolved_type_name());
if (!opt_enum_descriptor_idx) {
return base::ErrStatus(
"Unable to find enum type %s to fill field %s (in proto message "
"%s)",
- field->resolved_type_name().c_str(), field->name().c_str(),
+ field.resolved_type_name().c_str(), field.name().c_str(),
descriptor_->full_name().c_str());
}
const auto& enum_desc = pool_->descriptors()[*opt_enum_descriptor_idx];
@@ -269,40 +263,47 @@
"Invalid enum string %s "
"in enum type %s; encountered while filling "
"field %s (in proto message %s)",
- enum_str.c_str(), field->resolved_type_name().c_str(),
- field->name().c_str(), descriptor_->full_name().c_str());
+ enum_str.c_str(), field.resolved_type_name().c_str(),
+ field.name().c_str(), descriptor_->full_name().c_str());
}
- message_->AppendVarInt(field->number(), *opt_enum_value);
+ message_->AppendVarInt(field.number(), *opt_enum_value);
break;
}
default: {
return base::ErrStatus(
"Tried to write value of type string into field %s (in proto type "
"%s) which has type %d",
- field->name().c_str(), descriptor_->full_name().c_str(),
- field->type());
+ field.name().c_str(), descriptor_->full_name().c_str(), field.type());
}
}
return base::OkStatus();
}
-base::Status ProtoBuilder::AppendBytes(const std::string& field_name,
- const uint8_t* ptr,
- size_t size,
- bool is_inside_repeated) {
- const FieldDescriptor* field = descriptor_->FindFieldByName(field_name);
- if (!field) {
- return base::ErrStatus("Field with name %s not found in proto type %s",
- field_name.c_str(),
- descriptor_->full_name().c_str());
- }
-
+base::Status ProtoBuilder::AppendSingleBytes(const FieldDescriptor& field,
+ const uint8_t* ptr,
+ size_t size) {
using FieldDescriptorProto = protos::pbzero::FieldDescriptorProto;
- if (field->is_repeated() && !is_inside_repeated)
- return AppendRepeated(*field, ptr, size);
+ if (field.type() == FieldDescriptorProto::TYPE_MESSAGE) {
+ // If we have an zero sized bytes, we still want to propogate that the field
+ // message was set but empty.
+ if (size == 0) {
+ // ptr can be null and passing nullptr to AppendBytes feels dangerous so
+ // just pass an empty string (which will have a valid pointer always) and
+ // zero as the size.
+ message_->AppendBytes(field.number(), "", 0);
+ return base::OkStatus();
+ }
- if (field->type() == FieldDescriptorProto::TYPE_MESSAGE)
- return AppendSingleMessage(*field, ptr, size);
+ base::StatusOr<protozero::ConstBytes> bytes = ValidateSingleNonEmptyMessage(
+ ptr, size, field.type(), field.resolved_type_name());
+ if (!bytes.ok()) {
+ return base::ErrStatus(
+ "[Field %s in message %s]: %s", field.name().c_str(),
+ descriptor_->full_name().c_str(), bytes.status().c_message());
+ }
+ message_->AppendBytes(field.number(), bytes->data, bytes->size);
+ return base::OkStatus();
+ }
if (size == 0) {
return base::ErrStatus(
@@ -310,43 +311,20 @@
"%s). Nulls are only supported for message protos; all other types "
"should ensure that nulls are not passed to proto builder functions by "
"using the SQLite IFNULL/COALESCE functions.",
- field->name().c_str(), descriptor_->full_name().c_str());
+ field.name().c_str(), descriptor_->full_name().c_str());
}
return base::ErrStatus(
"Tried to write value of type bytes into field %s (in proto type %s) "
"which has type %d",
- field->name().c_str(), descriptor_->full_name().c_str(), field->type());
-}
-
-base::Status ProtoBuilder::AppendSingleMessage(const FieldDescriptor& field,
- const uint8_t* ptr,
- size_t size) {
- // If we have an zero sized bytes, we still want to propogate that the field
- // message was set but empty.
- if (size == 0) {
- // ptr can be null and passing nullptr to AppendBytes feels dangerous so
- // just pass an empty string (which will have a valid pointer always) and
- // zero as the size.
- message_->AppendBytes(field.number(), "", 0);
- return base::OkStatus();
- }
-
- protozero::ConstBytes bytes;
- base::Status validation = ValidateSingleNonEmptyMessage(
- ptr, size, field.type(), field.resolved_type_name(), &bytes);
- if (!validation.ok()) {
- return util::ErrStatus("[Field %s in message %s]: %s", field.name().c_str(),
- descriptor_->full_name().c_str(),
- validation.c_message());
- }
- message_->AppendBytes(field.number(), bytes.data, bytes.size);
- return base::OkStatus();
+ field.name().c_str(), descriptor_->full_name().c_str(), field.type());
}
base::Status ProtoBuilder::AppendRepeated(const FieldDescriptor& field,
const uint8_t* ptr,
size_t size) {
+ PERFETTO_DCHECK(field.is_repeated());
+
if (size > protozero::proto_utils::kMaxMessageLength) {
return base::ErrStatus(
"Message passed to field %s in proto message %s has size %zu which is "
@@ -362,36 +340,30 @@
field.name().c_str(), descriptor_->full_name().c_str());
}
- const auto& rep = decoder.repeated();
- protos::pbzero::RepeatedBuilderResult::Decoder repeated(rep.data, rep.size);
-
+ protos::pbzero::RepeatedBuilderResult::Decoder repeated(decoder.repeated());
for (auto it = repeated.value(); it; ++it) {
protos::pbzero::RepeatedBuilderResult::Value::Decoder value(*it);
- base::Status status;
if (value.has_int_value()) {
- status = AppendLong(field.name(), value.int_value(), true);
+ RETURN_IF_ERROR(AppendSingleLong(field, value.int_value()));
} else if (value.has_double_value()) {
- status = AppendDouble(field.name(), value.double_value(), true);
+ RETURN_IF_ERROR(AppendSingleDouble(field, value.double_value()));
} else if (value.has_string_value()) {
- status = AppendString(field.name(),
- base::StringView(value.string_value()), true);
+ RETURN_IF_ERROR(AppendSingleString(field, value.string_value()));
} else if (value.has_bytes_value()) {
const auto& bytes = value.bytes_value();
- status = AppendBytes(field.name(), bytes.data, bytes.size, true);
+ RETURN_IF_ERROR(AppendSingleBytes(field, bytes.data, bytes.size));
} else {
- status = base::ErrStatus("Unknown type in repeated field");
+ return base::ErrStatus("Unknown type in repeated field");
}
-
- if (!status.ok())
- return status;
}
return base::OkStatus();
}
std::vector<uint8_t> ProtoBuilder::SerializeToProtoBuilderResult() {
std::vector<uint8_t> serialized = SerializeRaw();
- if (serialized.empty())
+ if (serialized.empty()) {
return serialized;
+ }
const auto& type_name = descriptor_->full_name();
@@ -409,6 +381,17 @@
return message_.SerializeAsArray();
}
+base::StatusOr<const FieldDescriptor*> ProtoBuilder::FindFieldByName(
+ const std::string& field_name) {
+ const auto* field = descriptor_->FindFieldByName(field_name);
+ if (!field) {
+ return base::ErrStatus("Field with name %s not found in proto type %s",
+ field_name.c_str(),
+ descriptor_->full_name().c_str());
+ }
+ return field;
+}
+
RepeatedFieldBuilder::RepeatedFieldBuilder() {
repeated_ = message_->set_repeated();
}
@@ -457,9 +440,9 @@
std::vector<uint8_t> RepeatedFieldBuilder::SerializeToProtoBuilderResult() {
repeated_ = nullptr;
- if (!has_data_)
- return std::vector<uint8_t>();
-
+ if (!has_data_) {
+ return {};
+ }
message_->set_is_repeated(true);
return message_.SerializeAsArray();
}
@@ -477,8 +460,9 @@
out->insert(out->end(), start, raw_text.begin() + it->position(0));
auto value_it = substitutions.find(it->str(1));
- if (value_it == substitutions.end())
+ if (value_it == substitutions.end()) {
return 1;
+ }
const auto& value = value_it->second;
std::copy(value.begin(), value.end(), std::back_inserter(*out));
@@ -501,8 +485,9 @@
"NULL_IF_EMPTY: should only be called with bytes argument");
}
- if (sqlite3_value_bytes(argv[0]) == 0)
+ if (sqlite3_value_bytes(argv[0]) == 0) {
return base::OkStatus();
+ }
out = sqlite_utils::SqliteValueToSqlValue(argv[0]);
return base::OkStatus();
@@ -588,8 +573,9 @@
return base::ErrStatus("BuildProto: Invalid args");
}
- auto* key = reinterpret_cast<const char*>(sqlite3_value_text(argv[i]));
- auto value = sqlite_utils::SqliteValueToSqlValue(argv[i + 1]);
+ const char* key =
+ reinterpret_cast<const char*>(sqlite3_value_text(argv[i]));
+ SqlValue value = sqlite_utils::SqliteValueToSqlValue(argv[i + 1]);
RETURN_IF_ERROR(builder.AppendSqlValue(key, value));
}
@@ -618,8 +604,9 @@
sqlite3_value** argv,
SqlValue&,
Destructors&) {
- if (argc == 0 || sqlite3_value_type(argv[0]) != SQLITE_TEXT)
+ if (argc == 0 || sqlite3_value_type(argv[0]) != SQLITE_TEXT) {
return base::ErrStatus("RUN_METRIC: Invalid arguments");
+ }
const char* path = reinterpret_cast<const char*>(sqlite3_value_text(argv[0]));
auto metric_it = std::find_if(
@@ -631,8 +618,9 @@
std::unordered_map<std::string, std::string> substitutions;
for (size_t i = 1; i < argc; i += 2) {
- if (sqlite3_value_type(argv[i]) != SQLITE_TEXT)
+ if (sqlite3_value_type(argv[i]) != SQLITE_TEXT) {
return base::ErrStatus("RUN_METRIC: all keys must be strings");
+ }
std::optional<std::string> key_str = sqlite_utils::SqlValueToString(
sqlite_utils::SqliteValueToSqlValue(argv[i]));
@@ -672,11 +660,13 @@
SqlValue proto = sqlite_utils::SqliteValueToSqlValue(argv[0]);
SqlValue message_type = sqlite_utils::SqliteValueToSqlValue(argv[1]);
- if (proto.type != SqlValue::Type::kBytes)
+ if (proto.type != SqlValue::Type::kBytes) {
return base::ErrStatus("UNWRAP_METRIC_PROTO: proto is not a blob");
+ }
- if (message_type.type != SqlValue::Type::kString)
+ if (message_type.type != SqlValue::Type::kString) {
return base::ErrStatus("UNWRAP_METRIC_PROTO: message type is not string");
+ }
const uint8_t* ptr = static_cast<const uint8_t*>(proto.AsBytes());
size_t size = proto.bytes_count;
@@ -688,24 +678,25 @@
static constexpr uint32_t kMessageType =
static_cast<uint32_t>(protozero::proto_utils::ProtoSchemaType::kMessage);
- protozero::ConstBytes bytes;
- base::Status validation = ValidateSingleNonEmptyMessage(
- ptr, size, kMessageType, message_type.AsString(), &bytes);
- if (!validation.ok())
- return base::ErrStatus("UNWRAP_METRICS_PROTO: %s", validation.c_message());
+ base::StatusOr<protozero::ConstBytes> bytes = ValidateSingleNonEmptyMessage(
+ ptr, size, kMessageType, message_type.AsString());
+ if (!bytes.ok()) {
+ return base::ErrStatus("UNWRAP_METRICS_PROTO: %s",
+ bytes.status().c_message());
+ }
std::unique_ptr<uint8_t[], base::FreeDeleter> data(
- static_cast<uint8_t*>(malloc(bytes.size)));
- memcpy(data.get(), bytes.data, bytes.size);
+ static_cast<uint8_t*>(malloc(bytes->size)));
+ memcpy(data.get(), bytes->data, bytes->size);
destructors.bytes_destructor = free;
- out = SqlValue::Bytes(data.release(), bytes.size);
+ out = SqlValue::Bytes(data.release(), bytes->size);
return base::OkStatus();
}
base::Status ComputeMetrics(PerfettoSqlEngine* engine,
- const std::vector<std::string> metrics_to_compute,
+ const std::vector<std::string>& metrics_to_compute,
const std::vector<SqlMetricFile>& sql_metrics,
const DescriptorPool& pool,
const ProtoDescriptor& root_descriptor,
@@ -718,8 +709,9 @@
return metric.proto_field_name.has_value() &&
name == metric.proto_field_name.value();
});
- if (metric_it == sql_metrics.end())
+ if (metric_it == sql_metrics.end()) {
return base::ErrStatus("Unknown metric %s", name.c_str());
+ }
const SqlMetricFile& sql_metric = *metric_it;
auto prep_it =
@@ -740,7 +732,7 @@
// empty proto being returned.
const auto& field_name = sql_metric.proto_field_name.value();
if (it->stmt.IsDone()) {
- metric_builder.AppendBytes(field_name, nullptr, 0);
+ metric_builder.AppendSqlValue(field_name, SqlValue::Bytes(nullptr, 0));
continue;
}
diff --git a/src/trace_processor/metrics/metrics.h b/src/trace_processor/metrics/metrics.h
index 8a861c5..89dc3f7 100644
--- a/src/trace_processor/metrics/metrics.h
+++ b/src/trace_processor/metrics/metrics.h
@@ -19,13 +19,19 @@
#include <sqlite3.h>
+#include <cstddef>
+#include <cstdint>
+#include <optional>
+#include <string>
#include <unordered_map>
#include <vector>
+#include "perfetto/base/status.h"
+#include "perfetto/ext/base/status_or.h"
#include "perfetto/ext/base/string_view.h"
-#include "perfetto/protozero/field.h"
#include "perfetto/protozero/message.h"
#include "perfetto/protozero/scattered_heap_buffer.h"
+#include "perfetto/trace_processor/basic_types.h"
#include "perfetto/trace_processor/trace_processor.h"
#include "src/trace_processor/perfetto_sql/engine/perfetto_sql_engine.h"
#include "src/trace_processor/perfetto_sql/intrinsics/functions/sql_function.h"
@@ -33,9 +39,7 @@
#include "protos/perfetto/trace_processor/metrics_impl.pbzero.h"
-namespace perfetto {
-namespace trace_processor {
-namespace metrics {
+namespace perfetto::trace_processor::metrics {
// A description of a SQL metric in C++.
struct SqlMetricFile {
@@ -68,23 +72,6 @@
base::Status AppendSqlValue(const std::string& field_name,
const SqlValue& value);
- // Note: all external callers to these functions should not
- // |is_inside_repeated| to this function and instead rely on the default
- // value.
- base::Status AppendLong(const std::string& field_name,
- int64_t value,
- bool is_inside_repeated = false);
- base::Status AppendDouble(const std::string& field_name,
- double value,
- bool is_inside_repeated = false);
- base::Status AppendString(const std::string& field_name,
- base::StringView value,
- bool is_inside_repeated = false);
- base::Status AppendBytes(const std::string& field_name,
- const uint8_t* data,
- size_t size,
- bool is_inside_repeated = false);
-
// Returns the serialized |protos::ProtoBuilderResult| with the built proto
// as the nested |protobuf| message.
// Note: no other functions should be called on this class after this method
@@ -100,14 +87,20 @@
std::vector<uint8_t> SerializeRaw();
private:
- base::Status AppendSingleMessage(const FieldDescriptor& field,
- const uint8_t* ptr,
- size_t size);
-
+ base::Status AppendSingleLong(const FieldDescriptor& field, int64_t value);
+ base::Status AppendSingleDouble(const FieldDescriptor& field, double value);
+ base::Status AppendSingleString(const FieldDescriptor& field,
+ base::StringView data);
+ base::Status AppendSingleBytes(const FieldDescriptor& field,
+ const uint8_t* ptr,
+ size_t size);
base::Status AppendRepeated(const FieldDescriptor& field,
const uint8_t* ptr,
size_t size);
+ base::StatusOr<const FieldDescriptor*> FindFieldByName(
+ const std::string& field_name);
+
const DescriptorPool* pool_ = nullptr;
const ProtoDescriptor* descriptor_ = nullptr;
protozero::HeapBuffered<protozero::Message> message_;
@@ -201,14 +194,12 @@
void RepeatedFieldFinal(sqlite3_context* ctx);
base::Status ComputeMetrics(PerfettoSqlEngine*,
- const std::vector<std::string> metrics_to_compute,
+ const std::vector<std::string>& metrics_to_compute,
const std::vector<SqlMetricFile>& metrics,
const DescriptorPool& pool,
const ProtoDescriptor& root_descriptor,
std::vector<uint8_t>* metrics_proto);
-} // namespace metrics
-} // namespace trace_processor
-} // namespace perfetto
+} // namespace perfetto::trace_processor::metrics
#endif // SRC_TRACE_PROCESSOR_METRICS_METRICS_H_
diff --git a/src/trace_processor/metrics/metrics_unittest.cc b/src/trace_processor/metrics/metrics_unittest.cc
index 7b2d814..beaef2e 100644
--- a/src/trace_processor/metrics/metrics_unittest.cc
+++ b/src/trace_processor/metrics/metrics_unittest.cc
@@ -16,21 +16,31 @@
#include "src/trace_processor/metrics/metrics.h"
+#include <cstdint>
+#include <optional>
+#include <string>
+#include <unordered_map>
#include <vector>
+#include "perfetto/protozero/field.h"
+#include "perfetto/protozero/proto_decoder.h"
+#include "perfetto/protozero/proto_utils.h"
+#include "perfetto/trace_processor/basic_types.h"
#include "protos/perfetto/common/descriptor.pbzero.h"
+#include "src/base/test/status_matchers.h"
#include "src/trace_processor/util/descriptors.h"
#include "test/gtest_and_gmock.h"
-namespace perfetto {
-namespace trace_processor {
-namespace metrics {
+#include "protos/perfetto/trace_processor/metrics_impl.pbzero.h"
+
+namespace perfetto::trace_processor::metrics {
namespace {
+using base::gtest_matchers::IsError;
std::string RunTemplateReplace(
const std::string& str,
- std::unordered_map<std::string, std::string> subs) {
+ const std::unordered_map<std::string, std::string>& subs) {
std::string out;
EXPECT_EQ(TemplateReplace(str, subs, &out), 0);
return out;
@@ -58,10 +68,7 @@
const std::vector<uint8_t>& result_ser) {
protos::pbzero::ProtoBuilderResult::Decoder result(result_ser.data(),
result_ser.size());
- protozero::ConstBytes single_ser = result.single();
- protos::pbzero::SingleBuilderResult::Decoder single(single_ser.data,
- single_ser.size);
-
+ protos::pbzero::SingleBuilderResult::Decoder single(result.single());
protozero::ConstBytes proto_ser = single.protobuf();
return protozero::TypedProtoDecoder<1, repeated>(proto_ser.data,
proto_ser.size);
@@ -84,7 +91,7 @@
std::vector<uint8_t>(), false, false));
ProtoBuilder builder(&pool, &descriptor);
- ASSERT_TRUE(builder.AppendLong("int_value", 12345).ok());
+ ASSERT_OK(builder.AppendSqlValue("int_value", SqlValue::Long(12345)));
auto result_ser = builder.SerializeToProtoBuilderResult();
auto proto = DecodeSingleFieldProto<false>(result_ser);
@@ -108,7 +115,7 @@
std::vector<uint8_t>(), false, false));
ProtoBuilder builder(&pool, &descriptor);
- ASSERT_TRUE(builder.AppendDouble("double_value", 1.2345).ok());
+ ASSERT_OK(builder.AppendSqlValue("double_value", SqlValue::Double(1.2345)));
auto result_ser = builder.SerializeToProtoBuilderResult();
auto proto = DecodeSingleFieldProto<false>(result_ser);
@@ -132,7 +139,8 @@
std::vector<uint8_t>(), false, false));
ProtoBuilder builder(&pool, &descriptor);
- ASSERT_TRUE(builder.AppendString("string_value", "hello world!").ok());
+ ASSERT_OK(
+ builder.AppendSqlValue("string_value", SqlValue::String("hello world!")));
auto result_ser = builder.SerializeToProtoBuilderResult();
auto proto = DecodeSingleFieldProto<false>(result_ser);
@@ -169,14 +177,14 @@
descriptor.AddField(field);
ProtoBuilder nest_builder(&pool, &nested);
- ASSERT_TRUE(nest_builder.AppendLong("nested_int_value", 789).ok());
+ ASSERT_OK(
+ nest_builder.AppendSqlValue("nested_int_value", SqlValue::Long(789)));
auto nest_ser = nest_builder.SerializeToProtoBuilderResult();
ProtoBuilder builder(&pool, &descriptor);
- ASSERT_TRUE(
- builder.AppendBytes("nested_value", nest_ser.data(), nest_ser.size())
- .ok());
+ ASSERT_OK(builder.AppendSqlValue(
+ "nested_value", SqlValue::Bytes(nest_ser.data(), nest_ser.size())));
auto result_ser = builder.SerializeToProtoBuilderResult();
auto proto = DecodeSingleFieldProto<false>(result_ser);
@@ -215,9 +223,8 @@
std::vector<uint8_t> rep_ser = rep_builder.SerializeToProtoBuilderResult();
ProtoBuilder builder(&pool, &descriptor);
- ASSERT_TRUE(
- builder.AppendBytes("rep_int_value", rep_ser.data(), rep_ser.size())
- .ok());
+ ASSERT_OK(builder.AppendSqlValue(
+ "rep_int_value", SqlValue::Bytes(rep_ser.data(), rep_ser.size())));
auto result_ser = builder.SerializeToProtoBuilderResult();
auto proto = DecodeSingleFieldProto<true>(result_ser);
@@ -259,18 +266,25 @@
pool.AddProtoDescriptorForTesting(descriptor);
ProtoBuilder value_builder(&pool, &descriptor);
- ASSERT_FALSE(value_builder.AppendLong("enum_value", 4).ok());
- ASSERT_TRUE(value_builder.AppendLong("enum_value", 3).ok());
- ASSERT_FALSE(value_builder.AppendLong("enum_value", 6).ok());
+ ASSERT_THAT(value_builder.AppendSqlValue("enum_value", SqlValue::Long(4)),
+ IsError());
+ ASSERT_OK(value_builder.AppendSqlValue("enum_value", SqlValue::Long(3)));
+ ASSERT_THAT(value_builder.AppendSqlValue("enum_value", SqlValue::Long(6)),
+ IsError());
auto value_proto = DecodeSingleFieldProto<false>(
value_builder.SerializeToProtoBuilderResult());
ASSERT_EQ(value_proto.Get(1).as_int32(), 3);
ProtoBuilder str_builder(&pool, &descriptor);
- ASSERT_FALSE(str_builder.AppendString("enum_value", "FOURTH").ok());
- ASSERT_TRUE(str_builder.AppendString("enum_value", "SECOND").ok());
- ASSERT_FALSE(str_builder.AppendString("enum_value", "OTHER").ok());
+ ASSERT_THAT(
+ str_builder.AppendSqlValue("enum_value", SqlValue::String("FOURTH")),
+ IsError());
+ ASSERT_OK(
+ str_builder.AppendSqlValue("enum_value", SqlValue::String("SECOND")));
+ ASSERT_THAT(
+ str_builder.AppendSqlValue("enum_value", SqlValue::String("OTHER")),
+ IsError());
auto str_proto = DecodeSingleFieldProto<false>(
str_builder.SerializeToProtoBuilderResult());
@@ -278,7 +292,4 @@
}
} // namespace
-
-} // namespace metrics
-} // namespace trace_processor
-} // namespace perfetto
+} // namespace perfetto::trace_processor::metrics
diff --git a/test/gtest_and_gmock.h b/test/gtest_and_gmock.h
index 710d07d..c8d2540 100644
--- a/test/gtest_and_gmock.h
+++ b/test/gtest_and_gmock.h
@@ -46,8 +46,9 @@
#endif // defined(__clang__)
-#include <gmock/gmock.h>
-#include <gtest/gtest.h>
+#include <gmock/gmock-matchers.h> // IWYU pragma: export
+#include <gmock/gmock.h> // IWYU pragma: export
+#include <gtest/gtest.h> // IWYU pragma: export
#if defined(__GNUC__) || defined(__clang__)
#pragma GCC diagnostic pop
