Add Installer.PayloadFormat metric
Adds a new PayloadFormat metric with three different values: Delta, Full
and ForcedFull. The metric is send whenever an update is applied successfully
indicating the type of the used payload. The difference between Full and
ForcedFull is that in the ForcedFull, the request sent to omaha included a
directive saying that a delta payload wasn't accepted, where a Full payload
is one where a delta payload was accepted but a Full payload was provided.
BUG=chromium:225977
TEST=cros_workon_make update_engine --test # Three tests added, one for each condition.
Change-Id: If44cd96df325d320ed327114cc5e5de3d34a5c62
Reviewed-on: https://gerrit.chromium.org/gerrit/60318
Reviewed-by: David Zeuthen <[email protected]>
Commit-Queue: Alex Deymo <[email protected]>
Tested-by: Alex Deymo <[email protected]>
diff --git a/constants.h b/constants.h
index dc39d5a..c6ffb74 100644
--- a/constants.h
+++ b/constants.h
@@ -71,6 +71,19 @@
kNumDownloadSources
} DownloadSource;
+// A payload can be a Full or Delta payload. In some cases, a Full payload is
+// used even when a Delta payload was available for the update, called here
+// ForcedFull. The PayloadType enum is only used to send UMA metrics about the
+// successfully applied payload.
+typedef enum {
+ kPayloadTypeFull,
+ kPayloadTypeDelta,
+ kPayloadTypeForcedFull,
+
+ // Note: Add new payload types only above this line.
+ kNumPayloadTypes
+} PayloadType;
+
// The default number of UMA buckets for metrics.
const int kNumDefaultUmaBuckets = 50;
diff --git a/payload_state.cc b/payload_state.cc
index 20e2cac..a933ea5 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -156,6 +156,7 @@
ReportRebootMetrics();
ReportDurationMetrics();
ReportUpdatesAbandonedCountMetric();
+ ReportPayloadTypeMetric();
// Reset the number of responses seen since it counts from the last
// successful update, e.g. now.
@@ -877,6 +878,28 @@
prefs_->Delete(kPrefsUpdateDurationUptime);
}
+void PayloadState::ReportPayloadTypeMetric() {
+ string metric;
+ PayloadType uma_payload_type;
+ OmahaRequestParams* params = system_state_->request_params();
+
+ if (response_.is_delta_payload) {
+ uma_payload_type = kPayloadTypeDelta;
+ } else if (params->delta_okay()) {
+ uma_payload_type = kPayloadTypeFull;
+ } else { // Full payload, delta was not allowed by request.
+ uma_payload_type = kPayloadTypeForcedFull;
+ }
+
+ metric = "Installer.PayloadFormat";
+ system_state_->metrics_lib()->SendEnumToUMA(
+ metric,
+ uma_payload_type,
+ kNumPayloadTypes);
+ LOG(INFO) << "Uploading " << utils::ToString(uma_payload_type)
+ << " for metric " << metric;
+}
+
string PayloadState::GetPrefsKey(const string& prefix, DownloadSource source) {
return prefix + "-from-" + utils::ToString(source);
}
diff --git a/payload_state.h b/payload_state.h
index 3c88622..c4651e9 100644
--- a/payload_state.h
+++ b/payload_state.h
@@ -134,6 +134,9 @@
// Reports the various metrics related to update duration.
void ReportDurationMetrics();
+ // Reports the metric related to the applied payload type.
+ void ReportPayloadTypeMetric();
+
// Resets all the persisted state values which are maintained relative to the
// current response signature. The response signature itself is not reset.
void ResetPersistedState();
diff --git a/payload_state_unittest.cc b/payload_state_unittest.cc
index dcdcd11..9bd4c08 100644
--- a/payload_state_unittest.cc
+++ b/payload_state_unittest.cc
@@ -714,6 +714,8 @@
"Installer.DownloadSourcesUsed", 3, _, _, _));
EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendToUMA(
"Installer.DownloadOverheadPercentage", 542, _, _, _));
+ EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendEnumToUMA(
+ "Installer.PayloadFormat", kPayloadTypeFull, kNumPayloadTypes));
payload_state.UpdateSucceeded();
@@ -989,4 +991,80 @@
EXPECT_EQ(0, payload_state.GetUrlFailureCount());
}
+TEST(PayloadStateTest, PayloadTypeMetricWhenTypeIsDelta) {
+ OmahaResponse response;
+ response.is_delta_payload = true;
+ PayloadState payload_state;
+ MockSystemState mock_system_state;
+
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
+ SetupPayloadStateWith2Urls("Hash6437", true, &payload_state, &response);
+
+ // Simulate a successful download and update.
+ payload_state.DownloadComplete();
+
+ EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendEnumToUMA(
+ "Installer.PayloadFormat", kPayloadTypeDelta, kNumPayloadTypes));
+ payload_state.UpdateSucceeded();
+
+ // Mock the request to a request where the delta was disabled but Omaha sends
+ // a delta anyway and test again.
+ OmahaRequestParams params(&mock_system_state);
+ params.set_delta_okay(false);
+ mock_system_state.set_request_params(¶ms);
+
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
+ SetupPayloadStateWith2Urls("Hash6437", true, &payload_state, &response);
+
+ payload_state.DownloadComplete();
+
+ EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendEnumToUMA(
+ "Installer.PayloadFormat", kPayloadTypeDelta, kNumPayloadTypes));
+ payload_state.UpdateSucceeded();
+}
+
+TEST(PayloadStateTest, PayloadTypeMetricWhenTypeIsForcedFull) {
+ OmahaResponse response;
+ response.is_delta_payload = false;
+ PayloadState payload_state;
+ MockSystemState mock_system_state;
+
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
+ SetupPayloadStateWith2Urls("Hash6437", true, &payload_state, &response);
+
+ // Mock the request to a request where the delta was disabled.
+ OmahaRequestParams params(&mock_system_state);
+ params.set_delta_okay(false);
+ mock_system_state.set_request_params(¶ms);
+
+ // Simulate a successful download and update.
+ payload_state.DownloadComplete();
+
+ EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendEnumToUMA(
+ "Installer.PayloadFormat", kPayloadTypeForcedFull, kNumPayloadTypes));
+ payload_state.UpdateSucceeded();
+}
+
+TEST(PayloadStateTest, PayloadTypeMetricWhenTypeIsFull) {
+ OmahaResponse response;
+ response.is_delta_payload = false;
+ PayloadState payload_state;
+ MockSystemState mock_system_state;
+
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
+ SetupPayloadStateWith2Urls("Hash6437", true, &payload_state, &response);
+
+ // Mock the request to a request where the delta was disabled.
+ OmahaRequestParams params(&mock_system_state);
+ params.set_delta_okay(true);
+ mock_system_state.set_request_params(¶ms);
+
+ // Simulate a successful download and update.
+ payload_state.DownloadComplete();
+
+ EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendEnumToUMA(
+ "Installer.PayloadFormat", kPayloadTypeFull, kNumPayloadTypes));
+ payload_state.UpdateSucceeded();
+}
+
}
diff --git a/utils.cc b/utils.cc
index 62581f4..2bcc311 100644
--- a/utils.cc
+++ b/utils.cc
@@ -802,7 +802,7 @@
return (b ? "true" : "false");
}
-std::string ToString(DownloadSource source) {
+string ToString(DownloadSource source) {
switch (source) {
case kDownloadSourceHttpsServer: return "HttpsServer";
case kDownloadSourceHttpServer: return "HttpServer";
@@ -814,6 +814,19 @@
return "Unknown";
}
+string ToString(PayloadType payload_type) {
+ switch (payload_type) {
+ case kPayloadTypeDelta: return "Delta";
+ case kPayloadTypeFull: return "Full";
+ case kPayloadTypeForcedFull: return "ForcedFull";
+ case kNumPayloadTypes: return "Unknown";
+ // Don't add a default case to let the compiler warn about newly added
+ // payload types which should be added here.
+ }
+
+ return "Unknown";
+}
+
ErrorCode GetBaseErrorCode(ErrorCode code) {
// Ignore the higher order bits in the code by applying the mask as
// we want the enumerations to be in the small contiguous range
diff --git a/utils.h b/utils.h
index 0fe2948..bd4d2d6 100644
--- a/utils.h
+++ b/utils.h
@@ -168,6 +168,9 @@
// Returns a string representation of the given enum.
std::string ToString(DownloadSource source);
+// Returns a string representation of the given enum.
+std::string ToString(PayloadType payload_type);
+
enum BootLoader {
BootLoader_SYSLINUX = 0,
BootLoader_CHROME_FIRMWARE = 1
