Add presubmit test to verify v2 metrics are working when enabled

base/cvd/cuttlefish/host/commands/cvd/instances/local_instance_group.cpp

@@ -214,6 +214,7 @@ Result<Json::Value> LocalInstanceGroup::FetchStatus(
   }
   Json::Value group_json;
   group_json["group_name"] = GroupName();
+  group_json["metrics_dir"] = MetricsDir();
   group_json["start_time"] = Format(StartTime());
   group_json["instances"] = instances_json;
   return group_json;

base/cvd/cuttlefish/host/libs/metrics/enabled.h

@@ -20,12 +20,6 @@
 
 namespace cuttlefish {
 
-inline constexpr std::string_view kReadmeText =
-    "The existence of records in this directory does"
-    " not mean metrics are being transmitted.  The data is always gathered and "
-    "written out for debugging purposes.  To enable metrics transmission "
-    "the `cuttlefish-metrics` package must be installed.";
-
 inline constexpr char kTransmitterPath[] =
     "/usr/lib/cuttlefish-metrics/bin/metrics_transmitter";
 

base/cvd/cuttlefish/host/libs/metrics/metrics_orchestration.cc

@@ -95,7 +95,6 @@ MetricsPaths GetMetricsPaths(const LocalInstanceGroup& instance_group) {
 
 Result<void> SetUpMetrics(const std::string& metrics_directory) {
   CF_EXPECT(EnsureDirectoryExists(metrics_directory));
-  CF_EXPECT(WriteNewFile(metrics_directory + "/README", kReadmeText));
   CF_EXPECT(GenerateSessionIdFile(metrics_directory));
   return {};
 }
@@ -117,11 +116,12 @@ Result<MetricsData> GatherMetrics(const MetricsPaths& metrics_paths,
 Result<void> OutputMetrics(EventType event_type,
                            const MetricsPaths& metrics_paths,
                            const MetricsData& metrics_data) {
-  const CuttlefishLogEvent cf_log_event = BuildCuttlefishLogEvent(metrics_data);
-  CF_EXPECT(WriteMetricsEvent(event_type, metrics_paths.metrics_directory,
-                              cf_log_event));
   if (AreMetricsEnabled()) {
+    const CuttlefishLogEvent cf_log_event =
+        BuildCuttlefishLogEvent(metrics_data);
     CF_EXPECT(TransmitMetrics(kTransmitterPath, cf_log_event));
+    CF_EXPECT(WriteMetricsEvent(event_type, metrics_paths.metrics_directory,
+                                cf_log_event));
   }
   return {};
 }

e2etests/cvd/BUILD.bazel

@@ -1,4 +1,4 @@
-load("boot_tests.bzl", "cvd_command_boot_test", "cvd_load_boot_test", "launch_cvd_boot_test")
+load("boot_tests.bzl", "cvd_command_boot_test", "cvd_load_boot_test", "launch_cvd_boot_test", "metrics_test")
 
 # cvd fetch + launch_cvd tests
 launch_cvd_boot_test(
@@ -116,4 +116,10 @@ cvd_command_boot_test(
     tags = ["requires_gpu"],
 )
 
+metrics_test(
+    name = "verify_metrics_transmission",
+    branch = "aosp-android-latest-release",
+    target = "aosp_cf_x86_64_only_phone-userdebug",
+)
+
 # TODO(b/329100411) test loading older branches

e2etests/cvd/boot_tests.bzl

@@ -49,3 +49,19 @@ def cvd_command_boot_test(name, branch, target, cvd_command = [], credential_sou
             "no-sandbox",
         ] + tags,
     )
+
+def metrics_test(name, branch, target, credential_source = ""):
+    args = ["-b", branch, "-t", target]
+    if credential_source:
+        args += ["-c", credential_source]
+    native.sh_test(
+        name = name,
+        size = "medium",
+        srcs = ["metrics_test.sh"],
+        args = args,
+        tags = [
+            "exclusive",
+            "external",
+            "no-sandbox",
+        ],
+    )

e2etests/cvd/metrics_test.sh

@@ -0,0 +1,101 @@
+#!/usr/bin/env bash
+
+set -e -x
+
+BRANCH=""
+TARGET=""
+CREDENTIAL_SOURCE="${CREDENTIAL_SOURCE:-}"
+
+while getopts "b:c:t:" opt; do
+  case "${opt}" in
+    b)
+      BRANCH="${OPTARG}"
+      ;;
+    c)
+      CREDENTIAL_SOURCE="${OPTARG}"
+      ;;
+    t)
+      TARGET="${OPTARG}"
+      ;;
+    *)
+      echo "Unknown flag: -${opt}" >&2
+      echo "Usage: $0 -b BRANCH -t TARGET"
+      exit 1
+  esac
+done
+shift $((OPTIND-1))
+
+if [[ "${BRANCH}" == "" ]]; then
+  echo "Missing required -b argument"
+fi
+
+if [[ "${TARGET}" == "" ]]; then
+  echo "Missing required -t argument"
+fi
+
+workdir="$(mktemp -d -t cvd_command_test.XXXXXX)"
+
+function collect_logs_and_cleanup() {
+  # Don't immediately exit on failure anymore
+  set +e
+  if [[ -n "${TEST_UNDECLARED_OUTPUTS_DIR}" ]] && [[ -d "${TEST_UNDECLARED_OUTPUTS_DIR}" ]]; then
+    cp "${workdir}"/*.log "${TEST_UNDECLARED_OUTPUTS_DIR}"
+    cp "${workdir}"/cuttlefish_runtime/*.log "${TEST_UNDECLARED_OUTPUTS_DIR}"
+    cp "${workdir}"/cuttlefish_runtime/logcat "${TEST_UNDECLARED_OUTPUTS_DIR}"
+    cp "${workdir}"/cuttlefish_runtime/cuttlefish_config.json "${TEST_UNDECLARED_OUTPUTS_DIR}"
+  fi
+  rm -rf "${workdir}"
+  # Be nice, don't leave devices behind.
+  cvd reset -y
+}
+
+# Regardless of whether and when a failure occurs logs must collected
+trap collect_logs_and_cleanup EXIT
+
+# Make sure to run in a clean environment, without any devices running
+cvd reset -y
+
+credential_arg=""
+if [[ -n "$CREDENTIAL_SOURCE" ]]; then
+    credential_arg="--credential_source=${CREDENTIAL_SOURCE}"
+fi
+
+cvd fetch \
+  --default_build="${BRANCH}/${TARGET}" \
+  --target_directory="${workdir}" \
+  ${credential_arg}
+
+(
+  cd "${workdir}"
+
+  # generate instantiation, start, and boot complete events
+  cvd create --host_path="${workdir}" --product_path="${workdir}"
+
+  # verify transmission by detecting existence of the metrics directory and the debug event files
+  metrics_dir=`cvd fleet 2> /dev/null | jq --raw-output '.groups[0].metrics_dir'`
+  if ! [[ -d "${metrics_dir}" ]]; then
+    echo "metrics directory not found"
+    exit 1
+  fi
+
+  # file prefixes sourced from `cuttlefish/host/libs/metrics/event_type.cc::EventTypeString` function
+  if ! [[ -f "`ls ${metrics_dir}/device_instantiation*.txtpb`" ]]; then
+    echo "metrics not transmitted for the expected instantiation event"
+    exit 1
+  fi
+  if ! [[ -f "`ls ${metrics_dir}/device_boot_start*.txtpb`" ]]; then
+    echo "metrics not transmitted for the expected start event"
+    exit 1
+  fi
+  if ! [[ -f "`ls ${metrics_dir}/device_boot_complete*.txtpb`"  ]]; then
+    echo "metrics not transmitted for the expected boot complete event"
+    exit 1
+  fi
+
+  # generate stop event and verify transmission
+  cvd stop
+  if ! [[ -f "`ls ${metrics_dir}/device_stop*.txtpb`" ]]; then
+    echo "metrics not transmitted for the expected stop event"
+    exit 1
+  fi
+)

tools/testutils/cw/Containerfile

@@ -21,6 +21,7 @@ FROM with_bazel AS with_cuttlefish_packages
 
 RUN --mount=source=.,target=/mnt,type=bind apt-get install -y \
   /mnt/cuttlefish-base_*_*64.deb \
+  /mnt/cuttlefish-metrics_*_*64.deb \
   /mnt/cuttlefish-user_*_*64.deb \
   /mnt/cuttlefish-orchestration_*_*64.deb
 

tools/testutils/prepare_host.sh

@@ -69,7 +69,7 @@ if [[ "${PKG_DIR}" == "" ]] || ! [[ -d "${PKG_DIR}" ]]; then
 fi
 
 sudo apt-get update
-install_pkgs "${PKG_DIR}" cuttlefish-base cuttlefish-user
+install_pkgs "${PKG_DIR}" cuttlefish-base cuttlefish-metrics cuttlefish-user
 
 check_service_started cuttlefish-host-resources
 load_kernel_modules kvm vhost-vsock vhost-net bridge