From 573bc8ad2f31a2216f3796f090a7991878d9d9da Mon Sep 17 00:00:00 2001 From: Chad Reynolds Date: Wed, 7 Jan 2026 20:41:17 -0800 Subject: [PATCH 1/4] Only write metrics event files when enabled The main reason for the change is to use the presence of the event files as a signal that metrics transmissions are working. That signal will be used in an upcoming presubmit test to verify metrics functionality. This means the files will not be written out for debugging when metrics are disabled. However, that is offset for Googlers who will have the package installed by default and now have metrics enabled even for local development. Since the event files are only written after transmissions, I think there is less likelihood of confusion on whether metrics are being transmitted. This commit also removes the README that tried to explain this to users who stumbled upon the `metrics` directory. Bug: 453757034 --- base/cvd/cuttlefish/host/libs/metrics/enabled.h | 6 ------ .../cuttlefish/host/libs/metrics/metrics_orchestration.cc | 8 ++++---- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/base/cvd/cuttlefish/host/libs/metrics/enabled.h b/base/cvd/cuttlefish/host/libs/metrics/enabled.h index 92e2dc2ab60..f67d35215b3 100644 --- a/base/cvd/cuttlefish/host/libs/metrics/enabled.h +++ b/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"; diff --git a/base/cvd/cuttlefish/host/libs/metrics/metrics_orchestration.cc b/base/cvd/cuttlefish/host/libs/metrics/metrics_orchestration.cc index 83cc26f1b98..61ab01796a1 100644 --- a/base/cvd/cuttlefish/host/libs/metrics/metrics_orchestration.cc +++ b/base/cvd/cuttlefish/host/libs/metrics/metrics_orchestration.cc @@ -95,7 +95,6 @@ MetricsPaths GetMetricsPaths(const LocalInstanceGroup& instance_group) { Result 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 GatherMetrics(const MetricsPaths& metrics_paths, Result 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 {}; } From 455c90159083a7d145df9f3ace0c1581fd77a522 Mon Sep 17 00:00:00 2001 From: Chad Reynolds Date: Thu, 8 Jan 2026 12:28:55 -0800 Subject: [PATCH 2/4] Add group metrics directory to `cvd fleet` output To be used in the upcoming metrics transmission presubmit test Bug: 453757034 --- .../host/commands/cvd/instances/local_instance_group.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance_group.cpp b/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance_group.cpp index 8b88373c5c7..09e3d4aa464 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance_group.cpp +++ b/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance_group.cpp @@ -214,6 +214,7 @@ Result 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; From 65411d722e6ce81170474eafc5a5d0ad7d4772ba Mon Sep 17 00:00:00 2001 From: Chad Reynolds Date: Thu, 8 Jan 2026 10:25:32 -0800 Subject: [PATCH 3/4] Install `cuttlefish-metrics` to test containers Enable metrics for our Github presubmit tests by including the package in the container definition used by `presubmit.yaml`. Also add the package to the Kokoro presubmits install list, used for `.kokoro/presubmit.sh`. Bug: 453757034 --- tools/testutils/cw/Containerfile | 1 + tools/testutils/prepare_host.sh | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/testutils/cw/Containerfile b/tools/testutils/cw/Containerfile index 8465b7e0719..2b596fb4552 100644 --- a/tools/testutils/cw/Containerfile +++ b/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 diff --git a/tools/testutils/prepare_host.sh b/tools/testutils/prepare_host.sh index 215a90fd6c5..259af614652 100755 --- a/tools/testutils/prepare_host.sh +++ b/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 From 727260584ea74034d421f0d8283036d904c0ecac Mon Sep 17 00:00:00 2001 From: Chad Reynolds Date: Thu, 8 Jan 2026 11:51:56 -0800 Subject: [PATCH 4/4] Add presubmit test to verify metrics transmission This test works by using the existence of the metrics event files, which are now written AFTER transmission, as a signal metrics were transmitted successfully. Bug: 453757034 --- e2etests/cvd/BUILD.bazel | 8 ++- e2etests/cvd/boot_tests.bzl | 16 ++++++ e2etests/cvd/metrics_test.sh | 101 +++++++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 1 deletion(-) create mode 100755 e2etests/cvd/metrics_test.sh diff --git a/e2etests/cvd/BUILD.bazel b/e2etests/cvd/BUILD.bazel index 43fa9a4c318..f7629b04220 100644 --- a/e2etests/cvd/BUILD.bazel +++ b/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 diff --git a/e2etests/cvd/boot_tests.bzl b/e2etests/cvd/boot_tests.bzl index aa7264a6bbe..2aa8a2fbc5b 100644 --- a/e2etests/cvd/boot_tests.bzl +++ b/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", + ], + ) diff --git a/e2etests/cvd/metrics_test.sh b/e2etests/cvd/metrics_test.sh new file mode 100755 index 00000000000..d5ddb95dfe0 --- /dev/null +++ b/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 +)