Send UMA Stats for update engine error codes.
With the latest addition of new optional security checks for the update
manifest and operations checked in, we now want to track the number of
failures, if any, over time to help us decide when it is safe to make
the new security checks mandatory. This CL adds the UMA metric for
reporting the new (as well as the old) error codes to UMA for the first
time.
There's no change to the existing errors that are being sent to Omaha.
Due to UMA restrictions, some Omaha codes will be aggregated when being
sent to UMA.
BUG=chromium-os:34299
TEST=Unit tests pass, tested on real ZGB, all stats show up in
chrome://histograms correctly for both dev mode and normal mode.
Change-Id: I3ce4645636311cedbb33f601e775951966c0a545
Reviewed-on: https://gerrit.chromium.org/gerrit/36408
Commit-Ready: Jay Srinivasan <jaysri@chromium.org>
Reviewed-by: Jay Srinivasan <jaysri@chromium.org>
Tested-by: Jay Srinivasan <jaysri@chromium.org>
diff --git a/delta_performer.cc b/delta_performer.cc
index bcba546..59311b9 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -252,8 +252,9 @@
LOG(ERROR) << "Invalid metadata size. Expected = "
<< install_plan_->metadata_size
<< "Actual = " << *metadata_size;
- // TODO(jaysri): Add a UMA Stat here to help with the decision to enforce
+ // Send a UMA Stat here to help with the decision to enforce
// this check in a future release, as mentioned below.
+ SendUMAStat(kActionCodeDownloadInvalidMetadataSize);
// TODO(jaysri): VALIDATION: Initially we don't want to make this a fatal
// error. But in the next release, we should uncomment the lines below.
@@ -281,8 +282,9 @@
// and authenticity based on the information we have in Omaha response.
*error = ValidateMetadataSignature(&payload[0], *metadata_size);
if (*error != kActionCodeSuccess) {
- // TODO(jaysri): Add a UMA Stat here to help with the decision to enforce
+ // Send a UMA Stat here to help with the decision to enforce
// this check in a future release, as mentioned below.
+ SendUMAStat(*error);
// TODO(jaysri): VALIDATION: Initially we don't want to make this a fatal
// error. But in the next release, we should remove the line below and
@@ -364,10 +366,11 @@
if (*error != kActionCodeSuccess) {
// Cannot proceed further as operation hash is invalid.
// Higher level code will take care of retrying appropriately.
- //
- // TODO(jaysri): Add a UMA stat to indicate that an operation hash
- // was failed to be validated as expected.
- //
+
+ // Send a UMA stat to indicate that an operation hash failed to be
+ // validated as expected.
+ SendUMAStat(*error);
+
// TODO(jaysri): VALIDATION: For now, we don't treat this as fatal.
// But once we're confident that the new code works fine in the field,
// we should uncomment the line below.
@@ -700,6 +703,11 @@
// that we remain robust even if the connection to Omaha is subjected to
// any SSL attack.
LOG(WARNING) << "Cannot validate metadata as the signature is empty";
+
+ // Send a UMA stat here so we're aware of any man-in-the-middle attempts to
+ // bypass these checks.
+ SendUMAStat(kActionCodeDownloadMetadataSignatureMissingError);
+
return kActionCodeSuccess;
}
@@ -759,9 +767,10 @@
return kActionCodeSuccess;
}
- // TODO(jaysri): Add a UMA stat here so we're aware of any
- // man-in-the-middle attempts to bypass these checks.
- //
+ // Send a UMA stat here so we're aware of any man-in-the-middle attempts to
+ // bypass these checks.
+ SendUMAStat(kActionCodeDownloadOperationHashMissingError);
+
// TODO(jaysri): VALIDATION: no hash is present for the operation. This
// shouldn't happen normally for any client that has this code, because the
// corresponding update should have been produced with the operation
@@ -1100,4 +1109,10 @@
return true;
}
+void DeltaPerformer::SendUMAStat(ActionExitCode code) {
+ if (system_state_) {
+ utils::SendErrorCodeToUMA(system_state_->metrics_lib(), code);
+ }
+}
+
} // namespace chromeos_update_engine