Skip to content

Commit

Permalink
[DeviceSync v2] Fix feature status setter metrics
Browse files Browse the repository at this point in the history
Record BatchSetFeatureStatuses RPC metrics after success, failure, and
timeout. Previously, we were only logging metrics on timeout. Oops.

Manually verified that metrics are now being properly recorded.

Change-Id: If8897a34529cadbc25d844977c7ce6f9edf3196b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2744125
Auto-Submit: Josh Nohle <nohle@chromium.org>
Commit-Queue: James Vecore <vecore@google.com>
Reviewed-by: James Vecore <vecore@google.com>
Cr-Commit-Position: refs/heads/master@{#861050}
  • Loading branch information
nohle authored and Chromium LUCI CQ committed Mar 9, 2021
1 parent e126a50 commit 01f5433
Showing 1 changed file with 13 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -178,21 +178,26 @@ void CryptAuthFeatureStatusSetterImpl::ProcessRequestQueue() {
CryptAuthFeatureTypeToString(CryptAuthFeatureTypeFromSoftwareFeature(
pending_requests_.front().feature)));

std::string status;
switch (pending_requests_.front().status_change) {
case FeatureStatusChange::kEnableExclusively:
status = "exclusively enable";
feature_status->set_enabled(true);
feature_status->set_enable_exclusively(true);
break;
case FeatureStatusChange::kEnableNonExclusively:
status = "enable";
feature_status->set_enabled(true);
feature_status->set_enable_exclusively(false);
break;
case FeatureStatusChange::kDisable:
status = "disable";
feature_status->set_enabled(false);
feature_status->set_enable_exclusively(false);
break;
}

PA_LOG(INFO) << "Attempting to " << status << " feature "
<< pending_requests_.front().feature;
SetState(State::kWaitingForBatchSetFeatureStatusesResponse);
cryptauth_client_ = client_factory_->CreateInstance();
cryptauth_client_->BatchSetFeatureStatuses(
Expand All @@ -208,6 +213,10 @@ void CryptAuthFeatureStatusSetterImpl::ProcessRequestQueue() {
void CryptAuthFeatureStatusSetterImpl::OnBatchSetFeatureStatusesSuccess(
const cryptauthv2::BatchSetFeatureStatusesResponse& response) {
DCHECK_EQ(State::kWaitingForBatchSetFeatureStatusesResponse, state_);
PA_LOG(VERBOSE) << "SetFeatureStatus attempt succeeded.";
RecordBatchSetFeatureStatusesMetrics(
base::TimeTicks::Now() - last_state_change_timestamp_,
CryptAuthApiCallResult::kSuccess);
FinishAttempt(base::nullopt /* error */);
}

Expand All @@ -216,6 +225,9 @@ void CryptAuthFeatureStatusSetterImpl::OnBatchSetFeatureStatusesFailure(
DCHECK_EQ(State::kWaitingForBatchSetFeatureStatusesResponse, state_);
PA_LOG(ERROR) << "BatchSetFeatureStatuses call failed with error " << error
<< ".";
RecordBatchSetFeatureStatusesMetrics(
base::TimeTicks::Now() - last_state_change_timestamp_,
CryptAuthApiCallResultFromNetworkRequestError(error));
FinishAttempt(error);
}

Expand All @@ -231,7 +243,6 @@ void CryptAuthFeatureStatusSetterImpl::FinishAttempt(
if (error) {
std::move(current_request.error_callback).Run(*error);
} else {
PA_LOG(VERBOSE) << "SetFeatureStatus attempt succeeded.";
std::move(current_request.success_callback).Run();
}

Expand Down

0 comments on commit 01f5433

Please sign in to comment.