Segregate UMA metrics for production scenarios from test scenarios.
Currently we separate the UMA metrics only by one category: whether the
device is in dev mode or not. In addition, we need to exclude the noise
from these two categories:
1. Most of our testing on MP-signed images which are performed
with autest.
2. All our hwlab tests run in non-dev mode but they use dev-signed images
with dev-firmware keys.
So this CL defines additional bit fields to represent these states and
if any of these three flags are set, the UMA metric is sent to a
DevModeErrorCodes bucket. Thus the NormalErrorCodes bucket will have only
the production errors and thus we can monitor more effectively.
BUG=chromium-os:37613
TEST=Updated unit tests, ran on ZGB for all scenarios.
Change-Id: Id9cce33f09d1cc50cb15e67c731f7548940cbc24
Reviewed-on: https://gerrit.chromium.org/gerrit/41103
Reviewed-by: Chris Sosa <[email protected]>
Commit-Queue: Jay Srinivasan <[email protected]>
Tested-by: Jay Srinivasan <[email protected]>
diff --git a/utils.cc b/utils.cc
index e5d7290..fee3439 100644
--- a/utils.cc
+++ b/utils.cc
@@ -34,6 +34,8 @@
#include "update_engine/file_writer.h"
#include "update_engine/omaha_request_params.h"
#include "update_engine/subprocess.h"
+#include "update_engine/system_state.h"
+#include "update_engine/update_attempter.h"
using base::Time;
using base::TimeDelta;
@@ -715,8 +717,7 @@
// Ignore the higher order bits in the code by applying the mask as
// we want the enumerations to be in the small contiguous range
// with values less than kActionCodeUmaReportedMax.
- ActionExitCode base_code = static_cast<ActionExitCode>(
- code & kActualCodeMask);
+ ActionExitCode base_code = static_cast<ActionExitCode>(code & ~kSpecialFlags);
// Make additional adjustments required for UMA and error classification.
// TODO(jaysri): Move this logic to UeErrorCode.cc when we fix
@@ -724,26 +725,168 @@
if (base_code >= kActionCodeOmahaRequestHTTPResponseBase) {
// Since we want to keep the enums to a small value, aggregate all HTTP
// errors into this one bucket for UMA and error classification purposes.
+ LOG(INFO) << "Converting error code " << base_code
+ << " to kActionCodeOmahaErrorInHTTPResponse";
base_code = kActionCodeOmahaErrorInHTTPResponse;
}
return base_code;
}
+// Returns a printable version of the various flags denoted in the higher order
+// bits of the given code. Returns an empty string if none of those bits are
+// set.
+string GetFlagNames(uint32_t code) {
+ uint32_t flags = code & kSpecialFlags;
+ string flag_names;
+ string separator = "";
+ for(size_t i = 0; i < sizeof(flags) * 8; i++) {
+ uint32_t flag = flags & (1 << i);
+ if (flag) {
+ flag_names += separator + CodeToString(static_cast<ActionExitCode>(flag));
+ separator = ", ";
+ }
+ }
-
-void SendErrorCodeToUma(MetricsLibraryInterface* metrics_lib,
- ActionExitCode code) {
- string metric = utils::IsNormalBootMode() ? "Installer.NormalErrorCodes" :
- "Installer.DevModeErrorCodes";
-
- ActionExitCode reported_code = GetBaseErrorCode(code);
-
- LOG(INFO) << "Sending error code " << reported_code
- << " to UMA metric: " << metric;
- metrics_lib->SendEnumToUMA(metric, reported_code, kActionCodeUmaReportedMax);
+ return flag_names;
}
+void SendErrorCodeToUma(SystemState* system_state, ActionExitCode code) {
+ if (!system_state)
+ return;
+
+ ActionExitCode uma_error_code = GetBaseErrorCode(code);
+
+ // If the code doesn't have flags computed already, compute them now based on
+ // the state of the current update attempt.
+ uint32_t flags = code & kSpecialFlags;
+ if (!flags)
+ flags = system_state->update_attempter()->GetErrorCodeFlags();
+
+ // Determine the UMA bucket depending on the flags. But, ignore the resumed
+ // flag, as it's perfectly normal for production devices to resume their
+ // downloads and so we want to record those cases also in NormalErrorCodes
+ // bucket.
+ string metric = (flags & ~kActionCodeResumedFlag) ?
+ "Installer.DevModeErrorCodes" : "Installer.NormalErrorCodes";
+
+ LOG(INFO) << "Sending error code " << uma_error_code
+ << " (" << CodeToString(uma_error_code) << ")"
+ << " to UMA metric: " << metric
+ << ". Flags = " << (flags ? GetFlagNames(flags) : "None");
+
+ system_state->metrics_lib()->SendEnumToUMA(metric,
+ uma_error_code,
+ kActionCodeUmaReportedMax);
+}
+
+string CodeToString(ActionExitCode code) {
+ // If the given code has both parts (i.e. the error code part and the flags
+ // part) then strip off the flags part since the switch statement below
+ // has case statements only for the base error code or a single flag but
+ // doesn't support any combinations of those.
+ if ((code & kSpecialFlags) && (code & ~kSpecialFlags))
+ code = static_cast<ActionExitCode>(code & ~kSpecialFlags);
+ switch (code) {
+ case kActionCodeSuccess: return "kActionCodeSuccess";
+ case kActionCodeError: return "kActionCodeError";
+ case kActionCodeOmahaRequestError: return "kActionCodeOmahaRequestError";
+ case kActionCodeOmahaResponseHandlerError:
+ return "kActionCodeOmahaResponseHandlerError";
+ case kActionCodeFilesystemCopierError:
+ return "kActionCodeFilesystemCopierError";
+ case kActionCodePostinstallRunnerError:
+ return "kActionCodePostinstallRunnerError";
+ case kActionCodeSetBootableFlagError:
+ return "kActionCodeSetBootableFlagError";
+ case kActionCodeInstallDeviceOpenError:
+ return "kActionCodeInstallDeviceOpenError";
+ case kActionCodeKernelDeviceOpenError:
+ return "kActionCodeKernelDeviceOpenError";
+ case kActionCodeDownloadTransferError:
+ return "kActionCodeDownloadTransferError";
+ case kActionCodePayloadHashMismatchError:
+ return "kActionCodePayloadHashMismatchError";
+ case kActionCodePayloadSizeMismatchError:
+ return "kActionCodePayloadSizeMismatchError";
+ case kActionCodeDownloadPayloadVerificationError:
+ return "kActionCodeDownloadPayloadVerificationError";
+ case kActionCodeDownloadNewPartitionInfoError:
+ return "kActionCodeDownloadNewPartitionInfoError";
+ case kActionCodeDownloadWriteError:
+ return "kActionCodeDownloadWriteError";
+ case kActionCodeNewRootfsVerificationError:
+ return "kActionCodeNewRootfsVerificationError";
+ case kActionCodeNewKernelVerificationError:
+ return "kActionCodeNewKernelVerificationError";
+ case kActionCodeSignedDeltaPayloadExpectedError:
+ return "kActionCodeSignedDeltaPayloadExpectedError";
+ case kActionCodeDownloadPayloadPubKeyVerificationError:
+ return "kActionCodeDownloadPayloadPubKeyVerificationError";
+ case kActionCodePostinstallBootedFromFirmwareB:
+ return "kActionCodePostinstallBootedFromFirmwareB";
+ case kActionCodeDownloadStateInitializationError:
+ return "kActionCodeDownloadStateInitializationError";
+ case kActionCodeDownloadInvalidMetadataMagicString:
+ return "kActionCodeDownloadInvalidMetadataMagicString";
+ case kActionCodeDownloadSignatureMissingInManifest:
+ return "kActionCodeDownloadSignatureMissingInManifest";
+ case kActionCodeDownloadManifestParseError:
+ return "kActionCodeDownloadManifestParseError";
+ case kActionCodeDownloadMetadataSignatureError:
+ return "kActionCodeDownloadMetadataSignatureError";
+ case kActionCodeDownloadMetadataSignatureVerificationError:
+ return "kActionCodeDownloadMetadataSignatureVerificationError";
+ case kActionCodeDownloadMetadataSignatureMismatch:
+ return "kActionCodeDownloadMetadataSignatureMismatch";
+ case kActionCodeDownloadOperationHashVerificationError:
+ return "kActionCodeDownloadOperationHashVerificationError";
+ case kActionCodeDownloadOperationExecutionError:
+ return "kActionCodeDownloadOperationExecutionError";
+ case kActionCodeDownloadOperationHashMismatch:
+ return "kActionCodeDownloadOperationHashMismatch";
+ case kActionCodeOmahaRequestEmptyResponseError:
+ return "kActionCodeOmahaRequestEmptyResponseError";
+ case kActionCodeOmahaRequestXMLParseError:
+ return "kActionCodeOmahaRequestXMLParseError";
+ case kActionCodeDownloadInvalidMetadataSize:
+ return "kActionCodeDownloadInvalidMetadataSize";
+ case kActionCodeDownloadInvalidMetadataSignature:
+ return "kActionCodeDownloadInvalidMetadataSignature";
+ case kActionCodeOmahaResponseInvalid:
+ return "kActionCodeOmahaResponseInvalid";
+ case kActionCodeOmahaUpdateIgnoredPerPolicy:
+ return "kActionCodeOmahaUpdateIgnoredPerPolicy";
+ case kActionCodeOmahaUpdateDeferredPerPolicy:
+ return "kActionCodeOmahaUpdateDeferredPerPolicy";
+ case kActionCodeOmahaErrorInHTTPResponse:
+ return "kActionCodeOmahaErrorInHTTPResponse";
+ case kActionCodeDownloadOperationHashMissingError:
+ return "kActionCodeDownloadOperationHashMissingError";
+ case kActionCodeDownloadMetadataSignatureMissingError:
+ return "kActionCodeDownloadMetadataSignatureMissingError";
+ case kActionCodeOmahaUpdateDeferredForBackoff:
+ return "kActionCodeOmahaUpdateDeferredForBackoff";
+ case kActionCodeUmaReportedMax:
+ return "kActionCodeUmaReportedMax";
+ case kActionCodeOmahaRequestHTTPResponseBase:
+ return "kActionCodeOmahaRequestHTTPResponseBase";
+ case kActionCodeResumedFlag:
+ return "Resumed";
+ case kActionCodeDevModeFlag:
+ return "DevMode";
+ case kActionCodeTestImageFlag:
+ return "TestImage";
+ case kActionCodeTestOmahaUrlFlag:
+ return "TestOmahaUrl";
+ case kSpecialFlags:
+ return "kSpecialFlags";
+ // Don't add a default case to let the compiler warn about newly added
+ // error codes which should be added here.
+ }
+
+ return "Unknown error: " + base::UintToString(static_cast<unsigned>(code));
+}
} // namespace utils
