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/action_processor.h b/action_processor.h
index 4de4f84..0dff359 100644
--- a/action_processor.h
+++ b/action_processor.h
@@ -23,7 +23,6 @@
// Action exit codes.
enum ActionExitCode {
- // use the 0xx and 1xx ranges for client errors.
kActionCodeSuccess = 0,
kActionCodeError = 1,
kActionCodeOmahaRequestError = 2,
@@ -54,26 +53,45 @@
kActionCodeDownloadOperationHashVerificationError = 27,
kActionCodeDownloadOperationExecutionError = 28,
kActionCodeDownloadOperationHashMismatch = 29,
+ kActionCodeOmahaRequestEmptyResponseError = 30,
+ kActionCodeOmahaRequestXMLParseError = 31,
+ kActionCodeOmahaRequestNoUpdateCheckNode = 32,
+ kActionCodeOmahaRequestNoUpdateCheckStatus = 33,
+ kActionCodeOmahaRequestBadUpdateCheckStatus = 34,
+ kActionCodeOmahaUpdateIgnoredPerPolicy = 35,
+ kActionCodeOmahaUpdateDeferredPerPolicy = 36,
+ kActionCodeOmahaErrorInHTTPResponse = 37,
+ kActionCodeDownloadOperationHashMissingError = 38,
+ kActionCodeDownloadInvalidMetadataSize = 39,
+ kActionCodeDownloadInvalidMetadataSignature = 39,
+ kActionCodeDownloadMetadataSignatureMissingError = 40,
- // use the 2xx range for errors in Omaha response.
- kActionCodeOmahaRequestEmptyResponseError = 200,
- kActionCodeOmahaRequestXMLParseError = 201,
- kActionCodeOmahaRequestNoUpdateCheckNode = 202,
- kActionCodeOmahaRequestNoUpdateCheckStatus = 203,
- kActionCodeOmahaRequestBadUpdateCheckStatus = 204,
+ // Any code above this is sent to both Omaha and UMA as-is.
+ // Codes/flags below this line is sent only to Omaha and not to UMA.
- // use the 2xxx range to encode HTTP errors.
+ // kNumBucketsForUMAMetrics is not an error code per se, it's just the count
+ // of the number of enums above. Add any new errors above this line if you
+ // want them to show up on UMA. Stuff below this line will not be sent to UMA
+ // but is used for other errors that are sent to Omaha. We don't assign any
+ // particular value for this enum so that it's just one more than the last
+ // one above and thus always represents the correct count of UMA metrics
+ // buckets, even when new enums are added above this line in future. See
+ // utils::SendErrorCodeToUMA on how this enum is used.
+ kNumBucketsForUMAMetrics,
+
+ // use the 2xxx range to encode HTTP errors. These errors are available in
+ // Dremel with the individual granularity. But for UMA purposes, all these
+ // errors are aggregated into one: kActionCodeOmahaErrorInHTTPResponse.
kActionCodeOmahaRequestHTTPResponseBase = 2000, // + HTTP response code
- // use the 5xxx range for return codes that are not really errors,
- // but deferred updates. these have to be logged with a different
- // result in Omaha so that they don't show up as errors in borgmon charts.
- kActionCodeOmahaUpdateIgnoredPerPolicy = 5000,
- kActionCodeOmahaUpdateDeferredPerPolicy = 5001,
-
- // Bit flags.
+ // Bit flags. Remember to update the mask below for new bits.
kActionCodeResumedFlag = 1 << 30, // Set if resuming an interruped update.
kActionCodeBootModeFlag = 1 << 31, // Set if boot mode not normal.
+
+ // Mask to be used to extract the actual code without the higher-order
+ // bit flags (for reporting to UMA which requires small contiguous
+ // enum values)
+ kActualCodeMask = ~(kActionCodeResumedFlag | kActionCodeBootModeFlag)
};
class AbstractAction;
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
diff --git a/delta_performer.h b/delta_performer.h
index a484096..0bdf7ca 100644
--- a/delta_performer.h
+++ b/delta_performer.h
@@ -15,6 +15,7 @@
#include "update_engine/file_writer.h"
#include "update_engine/install_plan.h"
#include "update_engine/omaha_hash_calculator.h"
+#include "update_engine/system_state.h"
#include "update_engine/update_metadata.pb.h"
namespace chromeos_update_engine {
@@ -36,8 +37,11 @@
static const uint64_t kDeltaManifestSizeSize;
static const char kUpdatePayloadPublicKeyPath[];
- DeltaPerformer(PrefsInterface* prefs, InstallPlan* install_plan)
+ DeltaPerformer(PrefsInterface* prefs,
+ SystemState* system_state,
+ InstallPlan* install_plan)
: prefs_(prefs),
+ system_state_(system_state),
install_plan_(install_plan),
fd_(-1),
kernel_fd_(-1),
@@ -212,9 +216,15 @@
// update. Returns false otherwise.
bool PrimeUpdateState();
+ // Sends UMA statistics for the given error code.
+ void SendUMAStat(ActionExitCode code);
+
// Update Engine preference store.
PrefsInterface* prefs_;
+ // Global context of the system.
+ SystemState* system_state_;
+
// Install Plan based on Omaha Response.
InstallPlan* install_plan_;
diff --git a/delta_performer_unittest.cc b/delta_performer_unittest.cc
index bfb57eb..7174780 100644
--- a/delta_performer_unittest.cc
+++ b/delta_performer_unittest.cc
@@ -21,6 +21,7 @@
#include "update_engine/extent_ranges.h"
#include "update_engine/full_update_generator.h"
#include "update_engine/graph_types.h"
+#include "update_engine/mock_system_state.h"
#include "update_engine/payload_signer.h"
#include "update_engine/prefs_mock.h"
#include "update_engine/test_utils.h"
@@ -457,7 +458,8 @@
// Update the A image in place.
InstallPlan install_plan;
- DeltaPerformer performer(&prefs, &install_plan);
+ MockSystemState mock_system_state;
+ DeltaPerformer performer(&prefs, &mock_system_state, &install_plan);
EXPECT_TRUE(utils::FileExists(kUnittestPublicKeyPath));
performer.set_public_key_path(kUnittestPublicKeyPath);
@@ -733,7 +735,8 @@
&install_plan.metadata_signature));
EXPECT_FALSE(install_plan.metadata_signature.empty());
- DeltaPerformer performer(&prefs, &install_plan);
+ MockSystemState mock_system_state;
+ DeltaPerformer performer(&prefs, &mock_system_state, &install_plan);
EXPECT_TRUE(utils::FileExists(kUnittestPublicKeyPath));
performer.set_public_key_path(kUnittestPublicKeyPath);
@@ -839,7 +842,8 @@
TEST(DeltaPerformerTest, BadDeltaMagicTest) {
PrefsMock prefs;
InstallPlan install_plan;
- DeltaPerformer performer(&prefs, &install_plan);
+ MockSystemState mock_system_state;
+ DeltaPerformer performer(&prefs, &mock_system_state, &install_plan);
EXPECT_EQ(0, performer.Open("/dev/null", 0, 0));
EXPECT_TRUE(performer.OpenKernel("/dev/null"));
EXPECT_TRUE(performer.Write("junk", 4));
diff --git a/download_action.cc b/download_action.cc
index 8a18f43..b2055c8 100644
--- a/download_action.cc
+++ b/download_action.cc
@@ -21,13 +21,15 @@
const size_t kFileWriterBufferSize = 128 * 1024; // 128 KiB
DownloadAction::DownloadAction(PrefsInterface* prefs,
+ SystemState* system_state,
HttpFetcher* http_fetcher)
: prefs_(prefs),
writer_(NULL),
http_fetcher_(http_fetcher),
code_(kActionCodeSuccess),
delegate_(NULL),
- bytes_received_(0) {}
+ bytes_received_(0),
+ system_state_(system_state) {}
DownloadAction::~DownloadAction() {}
@@ -44,7 +46,9 @@
if (writer_) {
LOG(INFO) << "Using writer for test.";
} else {
- delta_performer_.reset(new DeltaPerformer(prefs_, &install_plan_));
+ delta_performer_.reset(new DeltaPerformer(prefs_,
+ system_state_,
+ &install_plan_));
writer_ = delta_performer_.get();
}
int rc = writer_->Open(install_plan_.install_path.c_str(),
diff --git a/download_action.h b/download_action.h
index e743b41..98cde46 100644
--- a/download_action.h
+++ b/download_action.h
@@ -19,6 +19,7 @@
#include "update_engine/delta_performer.h"
#include "update_engine/http_fetcher.h"
#include "update_engine/install_plan.h"
+#include "update_engine/system_state.h"
// The Download Action downloads a specified url to disk. The url should point
// to an update in a delta payload format. The payload will be piped into a
@@ -57,8 +58,10 @@
public:
// Takes ownership of the passed in HttpFetcher. Useful for testing.
// A good calling pattern is:
- // DownloadAction(new WhateverHttpFetcher);
- DownloadAction(PrefsInterface* prefs, HttpFetcher* http_fetcher);
+ // DownloadAction(prefs, system_state, new WhateverHttpFetcher);
+ DownloadAction(PrefsInterface* prefs,
+ SystemState* system_state,
+ HttpFetcher* http_fetcher);
virtual ~DownloadAction();
typedef ActionTraits<DownloadAction>::InputObjectType InputObjectType;
typedef ActionTraits<DownloadAction>::OutputObjectType OutputObjectType;
@@ -114,6 +117,9 @@
DownloadActionDelegate* delegate_;
uint64_t bytes_received_;
+ // Global context for the system.
+ SystemState* system_state_;
+
DISALLOW_COPY_AND_ASSIGN(DownloadAction);
};
diff --git a/download_action_unittest.cc b/download_action_unittest.cc
index fb9acf7..81ea589 100644
--- a/download_action_unittest.cc
+++ b/download_action_unittest.cc
@@ -145,7 +145,7 @@
data.size(),
NULL);
// takes ownership of passed in HttpFetcher
- DownloadAction download_action(&prefs, http_fetcher);
+ DownloadAction download_action(&prefs, NULL, http_fetcher);
download_action.SetTestFileWriter(&writer);
BondActions(&feeder_action, &download_action);
DownloadActionDelegateMock download_delegate;
@@ -255,7 +255,7 @@
InstallPlan install_plan(false, "", 0, "", 0, "", temp_file.GetPath(), "");
feeder_action.set_obj(install_plan);
PrefsMock prefs;
- DownloadAction download_action(&prefs,
+ DownloadAction download_action(&prefs, NULL,
new MockHttpFetcher(&data[0],
data.size(),
NULL));
@@ -363,7 +363,8 @@
ObjectFeederAction<InstallPlan> feeder_action;
feeder_action.set_obj(install_plan);
PrefsMock prefs;
- DownloadAction download_action(&prefs, new MockHttpFetcher("x", 1, NULL));
+ DownloadAction download_action(&prefs, NULL,
+ new MockHttpFetcher("x", 1, NULL));
download_action.SetTestFileWriter(&writer);
DownloadActionTestAction test_action;
@@ -397,7 +398,8 @@
ObjectFeederAction<InstallPlan> feeder_action;
feeder_action.set_obj(install_plan);
PrefsMock prefs;
- DownloadAction download_action(&prefs, new MockHttpFetcher("x", 1, NULL));
+ DownloadAction download_action(&prefs, NULL,
+ new MockHttpFetcher("x", 1, NULL));
download_action.SetTestFileWriter(&writer);
BondActions(&feeder_action, &download_action);
diff --git a/generate_delta_main.cc b/generate_delta_main.cc
index 6991813..b651d00 100644
--- a/generate_delta_main.cc
+++ b/generate_delta_main.cc
@@ -191,7 +191,7 @@
kern_info.hash().end());
install_plan.rootfs_hash.assign(root_info.hash().begin(),
root_info.hash().end());
- DeltaPerformer performer(&prefs, &install_plan);
+ DeltaPerformer performer(&prefs, NULL, &install_plan);
CHECK_EQ(performer.Open(FLAGS_old_image.c_str(), 0, 0), 0);
CHECK(performer.OpenKernel(FLAGS_old_kernel.c_str()));
vector<char> buf(1024 * 1024);
diff --git a/main.cc b/main.cc
index 596b04d..43ee5b4 100644
--- a/main.cc
+++ b/main.cc
@@ -172,8 +172,11 @@
LOG_IF(ERROR, !prefs.Init(FilePath("/var/lib/update_engine/prefs")))
<< "Failed to initialize preferences.";
+ chromeos_update_engine::RealSystemState real_system_state;
+
MetricsLibrary metrics_lib;
metrics_lib.Init();
+ real_system_state.set_metrics_lib(&metrics_lib);
// Sets static members for the certificate checker.
chromeos_update_engine::CertificateChecker::set_metrics_lib(&metrics_lib);
@@ -193,9 +196,7 @@
// Create the update attempter:
chromeos_update_engine::ConcreteDbusGlib dbus;
- chromeos_update_engine::RealSystemState real_system_state;
chromeos_update_engine::UpdateAttempter update_attempter(&prefs,
- &metrics_lib,
&dbus,
&gpio_handler,
&real_system_state);
diff --git a/mock_system_state.h b/mock_system_state.h
index 7f71fa9..a2e5cd2 100644
--- a/mock_system_state.h
+++ b/mock_system_state.h
@@ -7,7 +7,9 @@
#include <gmock/gmock.h>
+#include <metrics/metrics_library_mock.h>
#include <policy/mock_device_policy.h>
+
#include "update_engine/system_state.h"
namespace chromeos_update_engine {
@@ -16,7 +18,11 @@
// OOBE is completed even when there's no such marker file, etc.
class MockSystemState : public SystemState {
public:
- MockSystemState() {}
+ MockSystemState() {
+ // By default, provide a mock metrics library. If the caller wants,
+ // they can override this by using set_metrics_lib() method.
+ metrics_lib_ = &mock_metrics_lib_;
+ }
virtual ~MockSystemState() {}
MOCK_METHOD0(IsOOBEComplete, bool());
@@ -31,8 +37,18 @@
return connection_manager_;
}
+ void set_metrics_lib(MetricsLibraryInterface* metrics_lib) {
+ metrics_lib_ = metrics_lib;
+ }
+ virtual MetricsLibraryInterface* metrics_lib() {
+ return metrics_lib_;
+ }
+
+
private:
ConnectionManager* connection_manager_;
+ MetricsLibraryMock mock_metrics_lib_;
+ MetricsLibraryInterface* metrics_lib_;
};
} // namespeace chromeos_update_engine
diff --git a/payload_signer.cc b/payload_signer.cc
index 1df710f..b75f393 100644
--- a/payload_signer.cc
+++ b/payload_signer.cc
@@ -130,7 +130,7 @@
LOG(INFO) << "Payload size: " << payload.size();
ActionExitCode error = kActionCodeSuccess;
InstallPlan install_plan;
- DeltaPerformer delta_performer(NULL, &install_plan);
+ DeltaPerformer delta_performer(NULL, NULL, &install_plan);
TEST_AND_RETURN_FALSE(delta_performer.ParsePayloadMetadata(
payload, out_manifest, out_metadata_size, &error) ==
DeltaPerformer::kMetadataParseSuccess);
diff --git a/system_state.cc b/system_state.cc
index 57483ea..0053fed 100644
--- a/system_state.cc
+++ b/system_state.cc
@@ -12,7 +12,8 @@
RealSystemState::RealSystemState()
: device_policy_(NULL),
- connection_manager_(this) {}
+ connection_manager_(this),
+ metrics_lib_(NULL) {}
bool RealSystemState::IsOOBEComplete() {
return file_util::PathExists(FilePath(kOOBECompletedMarker));
@@ -31,4 +32,12 @@
return &connection_manager_;
}
+void RealSystemState::set_metrics_lib(MetricsLibraryInterface* metrics_lib) {
+ metrics_lib_ = metrics_lib;
+}
+
+MetricsLibraryInterface* RealSystemState::metrics_lib() {
+ return metrics_lib_;
+}
+
} // namespace chromeos_update_engine
diff --git a/system_state.h b/system_state.h
index a7fc68c..7d7ac09 100644
--- a/system_state.h
+++ b/system_state.h
@@ -5,6 +5,7 @@
#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_SYSTEM_STATE_H_
#define CHROMEOS_PLATFORM_UPDATE_ENGINE_SYSTEM_STATE_H_
+#include "metrics/metrics_library.h"
#include <policy/device_policy.h>
#include <policy/libpolicy.h>
@@ -33,6 +34,10 @@
// Gets the connection manager object.
virtual ConnectionManager* GetConnectionManager() = 0;
+
+ // Sets or gets the Metrics Library interface for reporting UMA stats.
+ virtual void set_metrics_lib(MetricsLibraryInterface* metrics_lib) = 0;
+ virtual MetricsLibraryInterface* metrics_lib() = 0;
};
// A real implementation of the SystemStateInterface which is
@@ -50,6 +55,9 @@
virtual ConnectionManager* GetConnectionManager();
+ virtual void set_metrics_lib(MetricsLibraryInterface* metrics_lib);
+ virtual MetricsLibraryInterface* metrics_lib();
+
private:
// The latest device policy object from the policy provider.
const policy::DevicePolicy* device_policy_;
@@ -57,6 +65,9 @@
// The connection manager object that makes download
// decisions depending on the current type of connection.
ConnectionManager connection_manager_;
+
+ // The Metrics Library interface for reporting UMA stats.
+ MetricsLibraryInterface* metrics_lib_;
};
} // namespace chromeos_update_engine
diff --git a/update_attempter.cc b/update_attempter.cc
index ca93a9a..0c12ea7 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -108,14 +108,12 @@
}
UpdateAttempter::UpdateAttempter(PrefsInterface* prefs,
- MetricsLibraryInterface* metrics_lib,
DbusGlibInterface* dbus_iface,
GpioHandler* gpio_handler,
SystemState* system_state)
: processor_(new ActionProcessor()),
dbus_service_(NULL),
prefs_(prefs),
- metrics_lib_(metrics_lib),
update_check_scheduler_(NULL),
fake_update_success_(false),
http_response_code_(0),
@@ -441,6 +439,7 @@
download_fetcher->set_check_certificate(CertificateChecker::kDownload);
shared_ptr<DownloadAction> download_action(
new DownloadAction(prefs_,
+ system_state_,
new MultiRangeHttpFetcher(
download_fetcher))); // passes ownership
shared_ptr<OmahaRequestAction> download_finished_action(
@@ -589,6 +588,7 @@
prefs_->Delete(kPrefsUpdateCheckCount);
prefs_->Delete(kPrefsWallClockWaitPeriod);
prefs_->Delete(kPrefsUpdateFirstSeenAt);
+ LOG(INFO) << "Update successfully applied, waiting to reboot.";
SetStatusAndNotify(UPDATE_STATUS_UPDATED_NEED_REBOOT,
kUpdateNoticeUnspecified);
@@ -596,11 +596,16 @@
// Report the time it took to update the system.
int64_t update_time = time(NULL) - last_checked_time_;
if (!fake_update_success_)
- metrics_lib_->SendToUMA("Installer.UpdateTime",
- static_cast<int>(update_time), // sample
- 1, // min = 1 second
- 20 * 60, // max = 20 minutes
- 50); // buckets
+ system_state_->metrics_lib()->SendToUMA(
+ "Installer.UpdateTime",
+ static_cast<int>(update_time), // sample
+ 1, // min = 1 second
+ 20 * 60, // max = 20 minutes
+ 50); // buckets
+
+ // Also report the success code so that the percentiles can be
+ // interpreted properly for the remaining error codes in UMA.
+ utils::SendErrorCodeToUMA(system_state_->metrics_lib(), code);
return;
}
@@ -874,6 +879,8 @@
return false;
LOG(INFO) << "Update failed -- reporting the error event.";
+ utils::SendErrorCodeToUMA(system_state_->metrics_lib(),
+ error_event_->error_code);
shared_ptr<OmahaRequestAction> error_event_action(
new OmahaRequestAction(prefs_,
&omaha_request_params_,
diff --git a/update_attempter.h b/update_attempter.h
index 40cf796..c873aa9 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -61,7 +61,6 @@
static const int kMaxDeltaUpdateFailures;
UpdateAttempter(PrefsInterface* prefs,
- MetricsLibraryInterface* metrics_lib,
DbusGlibInterface* dbus_iface,
GpioHandler* gpio_handler,
SystemState* system_state);
@@ -289,9 +288,6 @@
// Pointer to the preferences store interface.
PrefsInterface* prefs_;
- // Pointer to the UMA metrics collection library.
- MetricsLibraryInterface* metrics_lib_;
-
// The current UpdateCheckScheduler to notify of state transitions.
UpdateCheckScheduler* update_check_scheduler_;
diff --git a/update_attempter_mock.h b/update_attempter_mock.h
index 5ea7f27..d23bdb6 100644
--- a/update_attempter_mock.h
+++ b/update_attempter_mock.h
@@ -15,7 +15,7 @@
class UpdateAttempterMock : public UpdateAttempter {
public:
explicit UpdateAttempterMock(MockDbusGlib* dbus)
- : UpdateAttempter(NULL, NULL, dbus, NULL, NULL) {}
+ : UpdateAttempter(NULL, dbus, NULL, NULL) {}
MOCK_METHOD6(Update, void(const std::string& app_version,
const std::string& omaha_url,
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 55be51d..43ed55c 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -38,7 +38,7 @@
class UpdateAttempterUnderTest : public UpdateAttempter {
public:
explicit UpdateAttempterUnderTest(MockDbusGlib* dbus)
- : UpdateAttempter(NULL, NULL, dbus, NULL, NULL) {}
+ : UpdateAttempter(NULL, dbus, NULL, NULL) {}
};
class UpdateAttempterTest : public ::testing::Test {
@@ -53,7 +53,6 @@
EXPECT_EQ(NULL, attempter_.dbus_service_);
EXPECT_EQ(NULL, attempter_.prefs_);
EXPECT_EQ(NULL, attempter_.system_state_);
- EXPECT_EQ(NULL, attempter_.metrics_lib_);
EXPECT_EQ(NULL, attempter_.update_check_scheduler_);
EXPECT_EQ(0, attempter_.http_response_code_);
EXPECT_EQ(utils::kProcessPriorityNormal, attempter_.priority_);
@@ -115,7 +114,7 @@
TEST_F(UpdateAttempterTest, ActionCompletedDownloadTest) {
scoped_ptr<MockHttpFetcher> fetcher(new MockHttpFetcher("", 0, NULL));
fetcher->FailTransfer(503); // Sets the HTTP response code.
- DownloadAction action(&prefs_, fetcher.release());
+ DownloadAction action(&prefs_, NULL, fetcher.release());
EXPECT_CALL(prefs_, GetInt64(kPrefsDeltaUpdateFailures, _)).Times(0);
attempter_.ActionCompleted(NULL, &action, kActionCodeSuccess);
EXPECT_EQ(503, attempter_.http_response_code());
diff --git a/utils.cc b/utils.cc
index 1e87262..1165db5 100644
--- a/utils.cc
+++ b/utils.cc
@@ -31,6 +31,7 @@
#include <rootdev/rootdev.h>
#include "update_engine/file_writer.h"
+#include "update_engine/install_plan.h"
#include "update_engine/omaha_request_params.h"
#include "update_engine/subprocess.h"
@@ -698,6 +699,29 @@
exp_time.second);
}
+void SendErrorCodeToUMA(MetricsLibraryInterface* metrics_lib,
+ ActionExitCode code)
+{
+ string metric = utils::IsNormalBootMode() ? "UpdateEngine.NormalErrorCodes" :
+ "UpdateEngine.DevModeErrorCodes";
+
+ // 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 kNumBucketsForUMAMetrics.
+ int actual_code = code & kActualCodeMask;
+
+ // Make additional adjustments required for UMA.
+ if (actual_code >= kActionCodeOmahaRequestHTTPResponseBase) {
+ // Since we want to keep the enums to a small value, aggregate all HTTP
+ // errors into this one bucket for UMA purposes.
+ actual_code = kActionCodeOmahaErrorInHTTPResponse;
+ }
+
+ LOG(INFO) << "Sending " << actual_code << " to UMA metric: " << metric;
+ metrics_lib->SendEnumToUMA(metric, actual_code, kNumBucketsForUMAMetrics);
+}
+
+
} // namespace utils
} // namespace chromeos_update_engine
diff --git a/utils.h b/utils.h
index 1b3095b..e7bf288 100644
--- a/utils.h
+++ b/utils.h
@@ -16,12 +16,15 @@
#include <base/time.h>
#include <ext2fs/ext2fs.h>
#include <glib.h>
+#include "metrics/metrics_library.h"
#include "update_engine/action.h"
#include "update_engine/action_processor.h"
namespace chromeos_update_engine {
+class InstallPlan;
+
namespace utils {
// Returns true if this is an official Chrome OS build, false otherwise.
@@ -275,6 +278,11 @@
// when applicable.
std::string FormatTimeDelta(base::TimeDelta delta);
+// Sends the error code to the appropriate bucket in UMA using the metrics_lib
+// interface. This method also massages the error code to be suitable for UMA
+// purposes.
+void SendErrorCodeToUMA(MetricsLibraryInterface* metrics_lib,
+ ActionExitCode code);
} // namespace utils