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; 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 {}; } 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..a37325cc4c9 --- /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 | jq --raw-output '.groups.[0].metrics_dir'` + if ! [[ -d "${metrics_dir}" ]]; then + echo "metrics directory not found" + exit + 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 + fi + if ! [[ -f "`ls ${metrics_dir}/device_boot_start*.txtpb`" ]]; then + echo "metrics not transmitted for the expected start event" + exit + fi + if ! [[ -f "`ls ${metrics_dir}/device_boot_complete*.txtpb`" ]]; then + echo "metrics not transmitted for the expected boot complete event" + exit + 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 + fi +) 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