Skip to content

Commit

Permalink
Add UMA histograms to measure impact of preserved file optimization.
Browse files Browse the repository at this point in the history 
Add histograms around reading/writing to preserved file over DBus.
Records the count of successful/unsuccessful preserved file R/W.

The UMA histograms are used to better diagnose flow of method calls,
and if a dropoff happens at a specific method over the large number of
devices running ChromeOS.

BUG=chromium:1363237

(cherry picked from commit 4bb47c7)

Change-Id: Id187e396c85272931f00ce4907c48bd306bc1763
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4020908
Commit-Queue: Hirthanan Subenderan <hirthanan@google.com>
Reviewed-by: Gavin Williams <gavinwill@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1070108}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4024628
Commit-Queue: Gavin Williams <gavinwill@chromium.org>
Auto-Submit: Hirthanan Subenderan <hirthanan@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5414@{#29}
Cr-Branched-From: 4417ee5-refs/heads/main@{#1070088}
  • Loading branch information
Hirthanan Subenderan authored and Chromium LUCI CQ committed Nov 14, 2022
1 parent 6437b4d commit 35a45b8
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 1 deletion.
48 changes: 48 additions & 0 deletions chromeos/ash/components/device_activity/device_activity_client.cc
View file
Expand Up @@ -57,6 +57,10 @@ const char kFresnelQueryRequestEndpoint[] = "/v1/fresnel/psmRlweQuery";
// Count number of times a state has been entered.
const char kHistogramStateCount[] = "Ash.DeviceActiveClient.StateCount";

// Record the preserved file state.
const char kHistogramsPreservedFileState[] =
"Ash.DeviceActiveClient.PreservedFileState";

// Duration histogram uses State variant in order to create
// unique histograms measuring durations by State.
const char kHistogramDurationPrefix[] = "Ash.DeviceActiveClient.Duration";
Expand All @@ -69,6 +73,10 @@ const char kHistogramResponsePrefix[] = "Ash.DeviceActiveClient.Response";
const char kDeviceActiveClientQueryMembershipResult[] =
"Ash.DeviceActiveClient.QueryMembershipResult";

// Record number of successful saves of the preserved file content.
const char kDeviceActiveClientSavePreservedFileSuccess[] =
"Ash.DeviceActiveClient.SavePreservedFileSuccess";

// Record the minute the device activity client transitions out of idle.
const char kDeviceActiveClientTransitionOutOfIdleMinute[] =
"Ash.DeviceActiveClient.RecordedTransitionOutOfIdleMinute";
Expand Down Expand Up @@ -127,6 +135,11 @@ void RecordQueryMembershipResultBoolean(bool is_member) {
is_member);
}

void RecordSavePreservedFile(bool success) {
base::UmaHistogramBoolean(kDeviceActiveClientSavePreservedFileSuccess,
success);
}

// Return the minute of the current UTC time.
int GetCurrentMinute() {
base::Time cur_time = base::Time::Now();
Expand Down Expand Up @@ -182,6 +195,13 @@ void RecordResponseStateMetric(DeviceActivityClient::State state,
HistogramVariantName(kHistogramResponsePrefix, state), response);
}

// Histogram to record number of each PreservedFileState.
void RecordPreservedFileState(
DeviceActivityClient::PreservedFileState preserved_file_state) {
base::UmaHistogramEnumeration(kHistogramsPreservedFileState,
preserved_file_state);
}

std::unique_ptr<network::ResourceRequest> GenerateResourceRequest(
const std::string& request_method,
const GURL& url,
Expand Down Expand Up @@ -330,6 +350,9 @@ DeviceActiveUseCase* DeviceActivityClient::GetUseCasePtr(
}

void DeviceActivityClient::SaveLastPingDatesStatus() {
RecordDeviceActivityMethodCalled(
DeviceActivityClient::DeviceActivityMethod::
kDeviceActivityClientSaveLastPingDatesStatus);
private_computing::SaveStatusRequest request = GetSaveStatusRequest();

// Call DBus method with callback to |OnSaveLastPingDatesStatusComplete|.
Expand All @@ -341,22 +364,33 @@ void DeviceActivityClient::SaveLastPingDatesStatus() {

void DeviceActivityClient::OnSaveLastPingDatesStatusComplete(
private_computing::SaveStatusResponse response) {
RecordDeviceActivityMethodCalled(
DeviceActivityClient::DeviceActivityMethod::
kDeviceActivityClientOnSaveLastPingDatesStatusComplete);
if (response.has_error_message()) {
LOG(ERROR) << "Failed to store last ping timestamps with error message: "
<< response.error_message();
RecordSavePreservedFile(false);
} else {
VLOG(1) << "Successfully stored last ping timestamp to preserved file";
RecordSavePreservedFile(true);
}
}

void DeviceActivityClient::GetLastPingDatesStatus() {
RecordDeviceActivityMethodCalled(
DeviceActivityClient::DeviceActivityMethod::
kDeviceActivityClientGetLastPingDatesStatus);
PrivateComputingClient::Get()->GetLastPingDatesStatus(
base::BindOnce(&DeviceActivityClient::OnGetLastPingDatesStatusFetched,
weak_factory_.GetWeakPtr()));
}

void DeviceActivityClient::OnGetLastPingDatesStatusFetched(
private_computing::GetStatusResponse response) {
RecordDeviceActivityMethodCalled(
DeviceActivityClient::DeviceActivityMethod::
kDeviceActivityClientOnGetLastPingDatesStatusFetched);
// Update the last ping timestamps if the preserved file has a valid
// timestamp value for the use case.
if (!response.has_error_message()) {
Expand Down Expand Up @@ -396,12 +430,26 @@ void DeviceActivityClient::OnGetLastPingDatesStatusFetched(
}

if (!device_active_use_case_ptr->IsLastKnownPingTimestampSet()) {
RecordPreservedFileState(
DeviceActivityClient::PreservedFileState::kReadOkLocalStateEmpty);
VLOG(1) << "Updating local pref timestamp value with file timestamp = "
<< last_ping_utc_time;
device_active_use_case_ptr->SetLastKnownPingTimestamp(
last_ping_utc_time);
} else {
RecordPreservedFileState(
DeviceActivityClient::PreservedFileState::kReadOkLocalStateSet);
VLOG(1) << "Preserved File was read successfully but local state is "
"already set. "
<< "Device was most likely restarted and not powerwashed, so "
"no need to update local state.";
}
}
} else {
RecordPreservedFileState(
DeviceActivityClient::PreservedFileState::kReadFail);
LOG(ERROR)
<< "Preserved File read failed. State of local states is not checked.";
}

// Always trigger step to check for network status changing after reading the
Expand Down
22 changes: 21 additions & 1 deletion chromeos/ash/components/device_activity/device_activity_client.h
View file
Expand Up @@ -74,6 +74,22 @@ class COMPONENT_EXPORT(CHROMEOS_ASH_COMPONENTS_DEVICE_ACTIVITY)
kMaxValue = kTimeout,
};

// Categorize the preserved file state which will be used when bucketing UMA
// histograms.
//
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
enum class PreservedFileState {
kUnknown = 0, // Uncategorized state.
kReadOkLocalStateEmpty = 1, // Preserved file read successfully and local
// state empty.
kReadOkLocalStateSet = 2, // Preserved file read successfully but local
// state is already set.
kReadFail = 3, // Preserved file read failed and local state can either be
// empty or set.
kMaxValue = kReadFail,
};

// Categorize device activity methods which will be used when bucketing UMA
// histograms by number of calls to each method.
// Enum listed keys map to methods in |DeviceActivityController| and
Expand Down Expand Up @@ -106,7 +122,11 @@ class COMPONENT_EXPORT(CHROMEOS_ASH_COMPONENTS_DEVICE_ACTIVITY)
kDeviceActivityClientTransitionToIdle = 21,
kDeviceActivityClientOnSystemClockSyncResult = 22,
kDeviceActivityClientReportingTriggeredByTimer = 23,
kMaxValue = kDeviceActivityClientReportingTriggeredByTimer,
kDeviceActivityClientSaveLastPingDatesStatus = 24,
kDeviceActivityClientOnSaveLastPingDatesStatusComplete = 25,
kDeviceActivityClientGetLastPingDatesStatus = 26,
kDeviceActivityClientOnGetLastPingDatesStatusFetched = 27,
kMaxValue = kDeviceActivityClientOnGetLastPingDatesStatusFetched,
};

// Records UMA histogram for number of times various methods are called in
Expand Down
17 changes: 17 additions & 0 deletions tools/metrics/histograms/enums.xml
View file
Expand Up @@ -25709,6 +25709,16 @@ Called by update_debug_scenarios.py.-->
</int>
</enum>

<enum name="DeviceActiveClientPreservedFileState">
<!-- This must be kept current with DeviceActivityClient::PreservedFileState
located in chromeos/ash/components/device_activity/device_activity_client.h -->

<int value="0" label="Unknown"/>
<int value="1" label="Success read file and empty local state"/>
<int value="2" label="Success read file but local state already set"/>
<int value="3" label="Fail read file and local state is either set or unset"/>
</enum>

<enum name="DeviceActiveClientPsmResponse">
<!-- This must be kept current with DeviceActivityClient::PsmResponse
located in chromeos/ash/components/device_activity/device_activity_client.h -->
Expand Down Expand Up @@ -25758,6 +25768,13 @@ located in chromeos/ash/components/device_activity/device_activity_client.h -->
<int value="19" label="DeviceActivityClientTransitionToCheckIn"/>
<int value="20" label="DeviceActivityClientOnCheckInDone"/>
<int value="21" label="DeviceActivityClientTransitionToIdle"/>
<int value="22" label="DeviceActivityClientOnSystemClockSyncResult"/>
<int value="23" label="DeviceActivityClientReportingTriggeredByTimer"/>
<int value="24" label="DeviceActivityClientSaveLastPingDatesStatus"/>
<int value="25"
label="DeviceActivityClientOnSaveLastPingDatesStatusComplete"/>
<int value="26" label="DeviceActivityClientGetLastPingDatesStatus"/>
<int value="27" label="DeviceActivityClientOnGetLastPingDatesStatusFetched"/>
</enum>

<enum name="DeviceIdMismatch">
Expand Down
21 changes: 21 additions & 0 deletions tools/metrics/histograms/metadata/ash/histograms.xml
View file
Expand Up @@ -2140,6 +2140,17 @@ chromium-metrics-reviews@google.com.
<token key="DeviceActiveClientState" variants="DeviceActiveClientState"/>
</histogram>

<histogram name="Ash.DeviceActiveClient.PreservedFileState"
enum="DeviceActiveClientPreservedFileState" expires_after="2023-05-19">
<owner>hirthanan@google.com</owner>
<owner>chromeos-data-team@google.com</owner>
<summary>
Record the states after reading preserved file over DBus for last active utc
dates. PreservedFileState is defined in
//chromeos/ash/components/device_activity_client.h. ChromeOS only.
</summary>
</histogram>

<histogram name="Ash.DeviceActiveClient.QueryMembershipResult"
enum="BooleanSuccess" expires_after="2023-03-19">
<owner>hirthanan@google.com</owner>
Expand Down Expand Up @@ -2177,6 +2188,16 @@ chromium-metrics-reviews@google.com.
<token key="DeviceActiveClientState" variants="DeviceActiveClientState"/>
</histogram>

<histogram name="Ash.DeviceActiveClient.SavePreservedFileSuccess"
enum="BooleanSuccess" expires_after="2023-05-19">
<owner>hirthanan@google.com</owner>
<owner>chromeos-data-team@google.com</owner>
<summary>
Record success boolean after attempting to write last active utc dates for
all use case to the preserved file, over DBus.
</summary>
</histogram>

<histogram name="Ash.DeviceActiveClient.StateCount"
enum="DeviceActiveClientState" expires_after="2023-03-19">
<owner>hirthanan@google.com</owner>
Expand Down

0 comments on commit 35a45b8

Please sign in to comment.