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