Skip to content

Commit

Permalink
[CloudOpenMetrics] Add companion metrics
Browse files Browse the repository at this point in the history
Add companion metrics which log the MetricState of each of the existing
metrics from the cloud upload flow:
- Task Result
- Upload Result
- Copy/Move IO Task Error
- Source Volume
- TransferRequired
- Open Errors

The CloudOpenMetrics object updates the state of each metric throughout
the cloud upload flow. In the CloudOpenMetrics destructor, extra
business logic is applied to check if any metric is inconsistent (and
should take an inconsistent MetricState). Finally, each companion
metric is logged.

Add the MetricState enum to enums.xml.

Bug: b/300861997
Change-Id: I9d5d463e71603c159cb26fdd384d5c71b5bbd941
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4942614
Reviewed-by: Nigel Tao <nigeltao@chromium.org>
Commit-Queue: Cassy Chun-Crogan <cassycc@google.com>
Cr-Commit-Position: refs/heads/main@{#1211796}
  • Loading branch information
cassycc authored and Chromium LUCI CQ committed Oct 18, 2023
1 parent 4780e1d commit 2191905
Show file tree
Hide file tree
Showing 6 changed files with 165 additions and 6 deletions.
10 changes: 10 additions & 0 deletions chrome/browser/ui/webui/ash/cloud_upload/cloud_open_metrics.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,16 @@ CloudOpenMetrics::~CloudOpenMetrics() {
ExpectNotLogged(transfer_required_);
ExpectNotLogged(upload_result_);
}

// TODO(cassycc): Log the rest of the companion metrics once the rest of the
// consistency checks have been implemented.
if (google_drive) {
base::UmaHistogramEnumeration(kGoogleDriveTaskResultMetricStateMetricName,
task_result_.state);
} else {
base::UmaHistogramEnumeration(kOneDriveTaskResultMetricStateMetricName,
task_result_.state);
}
}

void CloudOpenMetrics::LogCopyError(base::File::Error value) {
Expand Down
13 changes: 7 additions & 6 deletions chrome/browser/ui/webui/ash/cloud_upload/cloud_open_metrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,18 @@ namespace ash::cloud_upload {

enum class MetricState {
// Not logged and it shouldn’t have been.
kCorrectlyNotLogged,
kCorrectlyNotLogged = 0,
// Logged when it should have been.
kCorrectlyLogged,
kCorrectlyLogged = 1,
// Not logged when it should have been.
kIncorrectlyNotLogged,
kIncorrectlyNotLogged = 2,
// Logged when it shouldn’t have been.
kIncorrectlyLogged,
kIncorrectlyLogged = 3,
// Logged more than once.
kIncorrectlyLoggedMultipleTimes,
kIncorrectlyLoggedMultipleTimes = 4,
// An unexpected value was logged.
kWrongValueLogged,
kWrongValueLogged = 5,
kMaxValue = kWrongValueLogged,
};

// Represents a metric identified by `metric_name` that logs value of type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,31 @@ TEST_F(MetricTest, MakeInconsistentIfNotLoggedWithWhenLoggedWithWrongValue) {
ASSERT_EQ(metric_.state, MetricState::kWrongValueLogged);
}

class CloudOpenMetricsTest : public testing::Test {
public:
CloudOpenMetricsTest() = default;

protected:
base::HistogramTester histogram_;
};

// Tests that the TaskResult companion metric is set correctly when TaskResult
// is logged.
TEST_F(CloudOpenMetricsTest, TaskResultLogged) {
{
CloudOpenMetrics cloud_open_metrics(CloudProvider::kGoogleDrive);
cloud_open_metrics.LogTaskResult(OfficeTaskResult::kOpened);
}
histogram_.ExpectUniqueSample(kGoogleDriveTaskResultMetricStateMetricName,
MetricState::kCorrectlyLogged, 1);
}

// Tests that the TaskResult companion metric is set correctly when TaskResult
// is not logged.
TEST_F(CloudOpenMetricsTest, TaskResultNotLogged) {
{ CloudOpenMetrics cloud_open_metrics(CloudProvider::kGoogleDrive); }
histogram_.ExpectUniqueSample(kGoogleDriveTaskResultMetricStateMetricName,
MetricState::kIncorrectlyNotLogged, 1);
}

} // namespace ash::cloud_upload
37 changes: 37 additions & 0 deletions chrome/browser/ui/webui/ash/cloud_upload/cloud_upload_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,38 +192,75 @@ enum class OfficeFilesUploadResult {

constexpr char kGoogleDriveTaskResultMetricName[] =
"FileBrowser.OfficeFiles.TaskResult.Drive";
constexpr char kGoogleDriveTaskResultMetricStateMetricName[] =
"FileBrowser.OfficeFiles.TaskResult.GoogleDrive.MetricState";

constexpr char kOneDriveTaskResultMetricName[] =
"FileBrowser.OfficeFiles.TaskResult.OneDrive";
constexpr char kOneDriveTaskResultMetricStateMetricName[] =
"FileBrowser.OfficeFiles.TaskResult.OneDrive.MetricState";

constexpr char kGoogleDriveUploadResultMetricName[] =
"FileBrowser.OfficeFiles.Open.UploadResult.GoogleDrive";
constexpr char kGoogleDriveUploadResultMetricStateMetricName[] =
"FileBrowser.OfficeFiles.Open.UploadResult.GoogleDrive.MetricState";

constexpr char kOneDriveUploadResultMetricName[] =
"FileBrowser.OfficeFiles.Open.UploadResult.OneDrive";
constexpr char kOneDriveUploadResultMetricStateMetricName[] =
"FileBrowser.OfficeFiles.Open.UploadResult.OneDrive.MetricState";

constexpr char kGoogleDriveMoveErrorMetricName[] =
"FileBrowser.OfficeFiles.Open.IOTaskError.GoogleDrive.Move";
constexpr char kGoogleDriveMoveErrorMetricStateMetricName[] =
"FileBrowser.OfficeFiles.Open.IOTaskError.GoogleDrive.Move.MetricState";

constexpr char kGoogleDriveCopyErrorMetricName[] =
"FileBrowser.OfficeFiles.Open.IOTaskError.GoogleDrive.Copy";
constexpr char kGoogleDriveCopyErrorMetricStateMetricName[] =
"FileBrowser.OfficeFiles.Open.IOTaskError.GoogleDrive.Copy.MetricState";

constexpr char kOneDriveMoveErrorMetricName[] =
"FileBrowser.OfficeFiles.Open.IOTaskError.OneDrive.Move";
constexpr char kOneDriveMoveErrorMetricStateMetricName[] =
"FileBrowser.OfficeFiles.Open.IOTaskError.OneDrive.Move.MetricState";

constexpr char kOneDriveCopyErrorMetricName[] =
"FileBrowser.OfficeFiles.Open.IOTaskError.OneDrive.Copy";
constexpr char kOneDriveCopyErrorMetricStateMetricName[] =
"FileBrowser.OfficeFiles.Open.IOTaskError.OneDrive.Copy.MetricState";

constexpr char kDriveOpenSourceVolumeMetric[] =
"FileBrowser.OfficeFiles.Open.SourceVolume.GoogleDrive";
constexpr char kDriveOpenSourceVolumeMetricStateMetric[] =
"FileBrowser.OfficeFiles.Open.SourceVolume.GoogleDrive.MetricState";

constexpr char kOneDriveOpenSourceVolumeMetric[] =
"FileBrowser.OfficeFiles.Open.SourceVolume.MicrosoftOneDrive";
constexpr char kOneDriveOpenSourceVolumeMetricStateMetric[] =
"FileBrowser.OfficeFiles.Open.SourceVolume.OneDrive.MetricState";

constexpr char kOpenCloudProviderMetric[] =
"FileBrowser.OfficeFiles.Open.CloudProvider";

constexpr char kDriveTransferRequiredMetric[] =
"FileBrowser.OfficeFiles.Open.TransferRequired.GoogleDrive";
constexpr char kDriveTransferRequiredMetricStateMetric[] =
"FileBrowser.OfficeFiles.Open.TransferRequired.GoogleDrive.MetricState";

constexpr char kOneDriveTransferRequiredMetric[] =
"FileBrowser.OfficeFiles.Open.TransferRequired.OneDrive";
constexpr char kOneDriveTransferRequiredMetricStateMetric[] =
"FileBrowser.OfficeFiles.Open.TransferRequired.OneDrive.MetricState";

constexpr char kDriveErrorMetricName[] = "FileBrowser.OfficeFiles.Errors.Drive";
constexpr char kDriveErrorMetricStateMetricName[] =
"FileBrowser.OfficeFiles.Errors.GoogleDrive.MetricState";

constexpr char kOneDriveErrorMetricName[] =
"FileBrowser.OfficeFiles.Errors.OneDrive";
constexpr char kOneDriveErrorMetricStateMetricName[] =
"FileBrowser.OfficeFiles.Errors.OneDrive.MetricState";

// Query actions for this path to get ODFS Metadata.
const char kODFSMetadataQueryPath[] = "/";
Expand Down
9 changes: 9 additions & 0 deletions tools/metrics/histograms/enums.xml
Original file line number Diff line number Diff line change
Expand Up @@ -73070,6 +73070,15 @@ Called by update_use_counter_css.py.-->
<int value="9" label="Exceeded allocated time for background task"/>
</enum>

<enum name="MetricState">
<int value="0" label="Correctly not logged"/>
<int value="1" label="Correctly logged"/>
<int value="2" label="Incorrectly not logged"/>
<int value="3" label="Incorrectly logged"/>
<int value="4" label="Incorrectly logged multiple times"/>
<int value="5" label="Wrong value logged"/>
</enum>

<enum name="MhtmlGenerationFinalSaveStatus">
<int value="0" label="Success"/>
<int value="1" label="File closing error"/>
Expand Down
75 changes: 75 additions & 0 deletions tools/metrics/histograms/metadata/file/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,16 @@ chromium-metrics-reviews@google.com.
</summary>
</histogram>

<histogram name="FileBrowser.OfficeFiles.Errors.{CloudProvider}.MetricState"
enum="MetricState" expires_after="M124">
<owner>simmonsjosh@google.com</owner>
<owner>src/ui/file_manager/OWNERS</owner>
<summary>
The state of the FileBrowser.OfficeFiles.Errors.{CloudProvider} metric.
</summary>
<token key="CloudProvider" variants="CloudProvider"/>
</histogram>

<histogram
name="FileBrowser.OfficeFiles.FileHandler.{RootType}.{ConnectionStatus}"
enum="OfficeFileHandler" expires_after="M124">
Expand Down Expand Up @@ -1235,6 +1245,24 @@ chromium-metrics-reviews@google.com.
</token>
</histogram>

<histogram
name="FileBrowser.OfficeFiles.Open.IOTaskError.{CloudProvider}.{Transfer}.MetricState"
enum="MetricState" expires_after="M124">
<owner>simmonsjosh@google.com</owner>
<owner>src/ui/file_manager/OWNERS</owner>
<summary>
The state of the
FileBrowser.OfficeFiles.Open.IOTaskError.{CloudProvider}.{Transfer} metric.
</summary>
<token key="CloudProvider" variants="CloudProvider"/>
<token key="Transfer">
<variant name="Copy"
summary="The files have been copied to {CloudProvider}"/>
<variant name="Move"
summary="The files have been moved to {CloudProvider}"/>
</token>
</histogram>

<histogram name="FileBrowser.OfficeFiles.Open.SourceVolume.{CloudProvider}"
enum="OfficeFilesSourceVolume" expires_after="M124">
<owner>austinct@chromium.org</owner>
Expand All @@ -1256,6 +1284,18 @@ chromium-metrics-reviews@google.com.
</token>
</histogram>

<histogram
name="FileBrowser.OfficeFiles.Open.SourceVolume.{CloudProvider}.MetricState"
enum="MetricState" expires_after="M124">
<owner>simmonsjosh@google.com</owner>
<owner>src/ui/file_manager/OWNERS</owner>
<summary>
The state of the FileBrowser.OfficeFiles.Open.SourceVolume.{CloudProvider}
metric.
</summary>
<token key="CloudProvider" variants="CloudProvider"/>
</histogram>

<histogram name="FileBrowser.OfficeFiles.Open.TransferRequired.{CloudProvider}"
enum="OfficeFilesTransferRequired" expires_after="M124">
<owner>simmonsjosh@google.com</owner>
Expand All @@ -1267,6 +1307,18 @@ chromium-metrics-reviews@google.com.
<token key="CloudProvider" variants="CloudProvider"/>
</histogram>

<histogram
name="FileBrowser.OfficeFiles.Open.TransferRequired.{CloudProvider}.MetricState"
enum="MetricState" expires_after="M124">
<owner>simmonsjosh@google.com</owner>
<owner>src/ui/file_manager/OWNERS</owner>
<summary>
The state of the
FileBrowser.OfficeFiles.Open.TransferRequired.{CloudProvider} metric.
</summary>
<token key="CloudProvider" variants="CloudProvider"/>
</histogram>

<histogram name="FileBrowser.OfficeFiles.Open.UploadResult.{CloudProvider}"
enum="OfficeFilesUploadResult" expires_after="M124">
<owner>simmonsjosh@google.com</owner>
Expand All @@ -1278,6 +1330,18 @@ chromium-metrics-reviews@google.com.
<token key="CloudProvider" variants="CloudProvider"/>
</histogram>

<histogram
name="FileBrowser.OfficeFiles.Open.UploadResult.{CloudProvider}.MetricState"
enum="MetricState" expires_after="M124">
<owner>simmonsjosh@google.com</owner>
<owner>src/ui/file_manager/OWNERS</owner>
<summary>
The state of the FileBrowser.OfficeFiles.Open.UploadResult.{CloudProvider}
metric.
</summary>
<token key="CloudProvider" variants="CloudProvider"/>
</histogram>

<histogram name="FileBrowser.OfficeFiles.Setup.CancelPage"
enum="OfficeSetupPage" expires_after="M124">
<owner>austinct@chromium.org</owner>
Expand Down Expand Up @@ -1338,6 +1402,17 @@ chromium-metrics-reviews@google.com.
</token>
</histogram>

<histogram
name="FileBrowser.OfficeFiles.TaskResult.{CloudProvider}.MetricState"
enum="MetricState" expires_after="M124">
<owner>simmonsjosh@google.com</owner>
<owner>src/ui/file_manager/OWNERS</owner>
<summary>
The state of the FileBrowser.OfficeFiles.TaskResult.{CloudProvider} metric.
</summary>
<token key="CloudProvider" variants="CloudProvider"/>
</histogram>

<histogram name="FileBrowser.OfficeFiles.UseOutsideDrive"
enum="OfficeFilesUseOutsideDriveHook" expires_after="2024-02-25">
<owner>cassycc@google.com</owner>
Expand Down

0 comments on commit 2191905

Please sign in to comment.