[Nearby] Add payload transfer size and rate metrics

Record metrics for
* total payload size in kB,
* total payload transfer rate in kB/s
* the number of file/text attachments per payload

We also slice the first two metrics by
* send/receive
* phone/Chromebook/tablet/unknown
* success/failure/cancelled

Manually tested and verified results at chrome://histograms.

Bug: 1105579
Change-Id: I94d6283157ca65bc4cd33fcf9d0c2074fb89db66
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2531899Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: James Vecore <vecore@google.com>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Commit-Queue: Josh Nohle <nohle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827064}

chrome/browser/nearby_sharing/nearby_share_metrics_logger.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/nearby_sharing/nearby_share_metrics_logger.h"
 
 #include "base/metrics/histogram_functions.h"
+#include "base/numerics/safe_conversions.h"
 #include "chrome/browser/nearby_sharing/common/nearby_share_prefs.h"
 #include "components/policy/core/common/policy_service.h"
 #include "components/policy/policy_constants.h"
@@ -12,6 +13,9 @@
 
 namespace {
 
+const char kTransferMetricPrefix[] = "Nearby.Share.Transfer";
+const size_t kBytesPerKilobyte = 1024;
+
 // These values are persisted to logs. Entries should not be renumbered and
 // numeric values should never be reused. If entries are added, kMaxValue should
 // be updated.
@@ -72,6 +76,39 @@ TransferNotCompletedReason TransferMetadataStatusToTransferNotCompletedReason(
   }
 }
 
+std::string GetDirectionSubcategoryName(bool is_incoming) {
+  return is_incoming ? ".Receive" : ".Send";
+}
+
+std::string GetShareTargetTypeSubcategoryName(
+    nearby_share::mojom::ShareTargetType type) {
+  switch (type) {
+    case nearby_share::mojom::ShareTargetType::kUnknown:
+      return ".Unknown";
+    case nearby_share::mojom::ShareTargetType::kPhone:
+      return ".Phone";
+    case nearby_share::mojom::ShareTargetType::kTablet:
+      return ".Tablet";
+    case nearby_share::mojom::ShareTargetType::kLaptop:
+      return ".Laptop";
+  }
+}
+
+std::string GetPayloadStatusSubcategoryName(
+    location::nearby::connections::mojom::PayloadStatus status) {
+  switch (status) {
+    case location::nearby::connections::mojom::PayloadStatus::kSuccess:
+      return ".Succeeded";
+    case location::nearby::connections::mojom::PayloadStatus::kFailure:
+      return ".Failed";
+    case location::nearby::connections::mojom::PayloadStatus::kCanceled:
+      return ".Cancelled";
+    case location::nearby::connections::mojom::PayloadStatus::kInProgress:
+      NOTREACHED();
+      return ".Cancelled";
+  }
+}
+
 }  // namespace
 
 void RecordNearbyShareEnabledMetric(const PrefService* pref_service) {
@@ -103,23 +140,10 @@ void RecordNearbyShareTransferCompletionStatusMetric(
     TransferMetadata::Status status) {
   DCHECK(TransferMetadata::IsFinalStatus(status));
 
-  const std::string kPrefix = "Nearby.Share.Transfer.CompletionStatus";
-  std::string send_or_receive = is_incoming ? ".Receive" : ".Send";
-  std::string share_target_type;
-  switch (type) {
-    case nearby_share::mojom::ShareTargetType::kUnknown:
-      share_target_type = ".Unknown";
-      break;
-    case nearby_share::mojom::ShareTargetType::kPhone:
-      share_target_type = ".Phone";
-      break;
-    case nearby_share::mojom::ShareTargetType::kTablet:
-      share_target_type = ".Tablet";
-      break;
-    case nearby_share::mojom::ShareTargetType::kLaptop:
-      share_target_type = ".Laptop";
-      break;
-  }
+  const std::string kPrefix =
+      kTransferMetricPrefix + std::string(".CompletionStatus");
+  std::string send_or_receive = GetDirectionSubcategoryName(is_incoming);
+  std::string share_target_type = GetShareTargetTypeSubcategoryName(type);
 
   bool is_complete = status == TransferMetadata::Status::kComplete;
   base::UmaHistogramBoolean(kPrefix, is_complete);
@@ -140,3 +164,67 @@ void RecordNearbyShareTransferCompletionStatusMetric(
         kPrefix + kReasonInfix + send_or_receive + share_target_type, reason);
   }
 }
+
+void RecordNearbyShareTransferSizeMetric(
+    bool is_incoming,
+    nearby_share::mojom::ShareTargetType type,
+    location::nearby::connections::mojom::PayloadStatus status,
+    uint64_t payload_size_bytes) {
+  DCHECK_NE(status,
+            location::nearby::connections::mojom::PayloadStatus::kInProgress);
+
+  int kilobytes =
+      base::saturated_cast<int>(payload_size_bytes / kBytesPerKilobyte);
+  for (const std::string& direction_name :
+       {std::string(), GetDirectionSubcategoryName(is_incoming)}) {
+    for (const std::string& share_target_type_name :
+         {std::string(), GetShareTargetTypeSubcategoryName(type)}) {
+      for (const std::string& payload_status_name :
+           {std::string(), GetPayloadStatusSubcategoryName(status)}) {
+        base::UmaHistogramCounts1M(
+            kTransferMetricPrefix + std::string(".TotalSize") + direction_name +
+                share_target_type_name + payload_status_name,
+            kilobytes);
+      }
+    }
+  }
+}
+
+void RecordNearbyShareTransferRateMetric(
+    bool is_incoming,
+    nearby_share::mojom::ShareTargetType type,
+    location::nearby::connections::mojom::PayloadStatus status,
+    uint64_t transferred_payload_bytes,
+    base::TimeDelta time_elapsed) {
+  DCHECK_NE(status,
+            location::nearby::connections::mojom::PayloadStatus::kInProgress);
+
+  int kilobytes_per_second = base::saturated_cast<int>(base::ClampDiv(
+      base::ClampDiv(transferred_payload_bytes, time_elapsed.InSecondsF()),
+      kBytesPerKilobyte));
+  for (const std::string& direction_name :
+       {std::string(), GetDirectionSubcategoryName(is_incoming)}) {
+    for (const std::string& share_target_type_name :
+         {std::string(), GetShareTargetTypeSubcategoryName(type)}) {
+      for (const std::string& payload_status_name :
+           {std::string(), GetPayloadStatusSubcategoryName(status)}) {
+        base::UmaHistogramCounts100000(
+            kTransferMetricPrefix + std::string(".Rate") + direction_name +
+                share_target_type_name + payload_status_name,
+            kilobytes_per_second);
+      }
+    }
+  }
+}
+
+void RecordNearbyShareTransferNumAttachmentsMetric(
+    size_t num_text_attachments,
+    size_t num_file_attachments) {
+  const std::string kAttachmentInfix = ".NumAttachments";
+  base::UmaHistogramCounts100(kTransferMetricPrefix + kAttachmentInfix,
+                              num_text_attachments + num_file_attachments);
+  base::UmaHistogramCounts100(
+      kTransferMetricPrefix + kAttachmentInfix + ".Text", num_text_attachments);
+  base::UmaHistogramCounts100(
+      kTransferMetricPrefix + kAttachmentInfix + ".File", num_file_attachments);
+}

chrome/browser/nearby_sharing/nearby_share_metrics_logger.h

@@ -5,7 +5,9 @@
 #ifndef CHROME_BROWSER_NEARBY_SHARING_NEARBY_SHARE_METRICS_LOGGER_H_
 #define CHROME_BROWSER_NEARBY_SHARING_NEARBY_SHARE_METRICS_LOGGER_H_
 
+#include "base/time/time.h"
 #include "chrome/browser/nearby_sharing/transfer_metadata.h"
+#include "chromeos/services/nearby/public/mojom/nearby_connections_types.mojom.h"
 #include "chromeos/services/nearby/public/mojom/nearby_share_target_types.mojom.h"
 
 class PrefService;
@@ -16,4 +18,18 @@ void RecordNearbyShareTransferCompletionStatusMetric(
     nearby_share::mojom::ShareTargetType type,
     TransferMetadata::Status status);
 
+void RecordNearbyShareTransferSizeMetric(
+    bool is_incoming,
+    nearby_share::mojom::ShareTargetType type,
+    location::nearby::connections::mojom::PayloadStatus status,
+    uint64_t payload_size_bytes);
+void RecordNearbyShareTransferRateMetric(
+    bool is_incoming,
+    nearby_share::mojom::ShareTargetType type,
+    location::nearby::connections::mojom::PayloadStatus status,
+    uint64_t transferred_payload_bytes,
+    base::TimeDelta time_elapsed);
+void RecordNearbyShareTransferNumAttachmentsMetric(size_t num_text_attachments,
+                                                   size_t num_file_attachments);
+
 #endif  // CHROME_BROWSER_NEARBY_SHARING_NEARBY_SHARE_METRICS_LOGGER_H_

chrome/browser/nearby_sharing/payload_tracker.cc

@@ -7,6 +7,7 @@
 #include "base/callback.h"
 #include "chrome/browser/nearby_sharing/constants.h"
 #include "chrome/browser/nearby_sharing/logging/logging.h"
+#include "chrome/browser/nearby_sharing/nearby_share_metrics_logger.h"
 #include "chrome/browser/nearby_sharing/transfer_metadata_builder.h"
 
 PayloadTracker::PayloadTracker(
@@ -16,7 +17,7 @@ PayloadTracker::PayloadTracker(
         update_callback)
     : share_target_(share_target),
       update_callback_(std::move(update_callback)) {
-  total_download_size_ = 0;
+  total_transfer_size_ = 0;
 
   for (const auto& file : share_target.file_attachments) {
     auto it = attachment_info_map.find(file.id());
@@ -29,7 +30,8 @@ PayloadTracker::PayloadTracker(
     }
 
     payload_state_.emplace(*it->second.payload_id, State(file.size()));
-    total_download_size_ += file.size();
+    ++num_file_attachments_;
+    total_transfer_size_ += file.size();
   }
 
   for (const auto& text : share_target.text_attachments) {
@@ -43,7 +45,8 @@ PayloadTracker::PayloadTracker(
     }
 
     payload_state_.emplace(*it->second.payload_id, State(text.size()));
-    total_download_size_ += text.size();
+    ++num_text_attachments_;
+    total_transfer_size_ += text.size();
   }
 }
 
@@ -54,7 +57,13 @@ void PayloadTracker::OnStatusUpdate(PayloadTransferUpdatePtr update) {
   if (it == payload_state_.end())
     return;
 
-  it->second.amount_downloaded = update->bytes_transferred;
+  // For metrics.
+  if (!first_update_timestamp_.has_value()) {
+    first_update_timestamp_ = base::TimeTicks::Now();
+    num_first_update_bytes_ = update->bytes_transferred;
+  }
+
+  it->second.amount_transferred = update->bytes_transferred;
   if (it->second.status != update->status) {
     it->second.status = update->status;
 
@@ -67,6 +76,8 @@ void PayloadTracker::OnStatusUpdate(PayloadTransferUpdatePtr update) {
 void PayloadTracker::OnTransferUpdate() {
   if (IsComplete()) {
     NS_LOG(VERBOSE) << __func__ << ": All payloads are complete.";
+    EmitFinalMetrics(
+        location::nearby::connections::mojom::PayloadStatus::kSuccess);
     update_callback_.Run(share_target_,
                          TransferMetadataBuilder()
                              .set_status(TransferMetadata::Status::kComplete)
@@ -77,6 +88,8 @@ void PayloadTracker::OnTransferUpdate() {
 
   if (IsCancelled()) {
     NS_LOG(VERBOSE) << __func__ << ": Payloads cancelled.";
+    EmitFinalMetrics(
+        location::nearby::connections::mojom::PayloadStatus::kCanceled);
     update_callback_.Run(share_target_,
                          TransferMetadataBuilder()
                              .set_status(TransferMetadata::Status::kCancelled)
@@ -86,6 +99,8 @@ void PayloadTracker::OnTransferUpdate() {
 
   if (HasFailed()) {
     NS_LOG(VERBOSE) << __func__ << ": Payloads failed.";
+    EmitFinalMetrics(
+        location::nearby::connections::mojom::PayloadStatus::kFailure);
     update_callback_.Run(share_target_,
                          TransferMetadataBuilder()
                              .set_status(TransferMetadata::Status::kFailed)
@@ -114,7 +129,7 @@ void PayloadTracker::OnTransferUpdate() {
                            .build());
 }
 
-bool PayloadTracker::IsComplete() {
+bool PayloadTracker::IsComplete() const {
   for (const auto& state : payload_state_) {
     if (state.second.status !=
         location::nearby::connections::mojom::PayloadStatus::kSuccess) {
@@ -124,7 +139,7 @@ bool PayloadTracker::IsComplete() {
   return true;
 }
 
-bool PayloadTracker::IsCancelled() {
+bool PayloadTracker::IsCancelled() const {
   for (const auto& state : payload_state_) {
     if (state.second.status ==
         location::nearby::connections::mojom::PayloadStatus::kCanceled) {
@@ -134,7 +149,7 @@ bool PayloadTracker::IsCancelled() {
   return false;
 }
 
-bool PayloadTracker::HasFailed() {
+bool PayloadTracker::HasFailed() const {
   for (const auto& state : payload_state_) {
     if (state.second.status ==
         location::nearby::connections::mojom::PayloadStatus::kFailure) {
@@ -144,15 +159,42 @@ bool PayloadTracker::HasFailed() {
   return false;
 }
 
-double PayloadTracker::CalculateProgressPercent() {
-  if (!total_download_size_) {
+uint64_t PayloadTracker::GetTotalTransferred() const {
+  uint64_t total_transferred = 0;
+  for (const auto& state : payload_state_)
+    total_transferred += state.second.amount_transferred;
+
+  return total_transferred;
+}
+
+double PayloadTracker::CalculateProgressPercent() const {
+  if (!total_transfer_size_) {
     NS_LOG(WARNING) << __func__ << ": Total attachment size is 0";
     return 100.0;
   }
 
-  uint64_t total_downloaded = 0;
-  for (const auto& state : payload_state_)
-    total_downloaded += state.second.amount_downloaded;
+  return (100.0 * GetTotalTransferred()) / total_transfer_size_;
+}
 
-  return (100.0 * total_downloaded) / total_download_size_;
+void PayloadTracker::EmitFinalMetrics(
+    location::nearby::connections::mojom::PayloadStatus status) const {
+  DCHECK_NE(status,
+            location::nearby::connections::mojom::PayloadStatus::kInProgress);
+  RecordNearbyShareTransferSizeMetric(share_target_.is_incoming,
+                                      share_target_.type, status,
+                                      total_transfer_size_);
+
+  RecordNearbyShareTransferNumAttachmentsMetric(num_text_attachments_,
+                                                num_file_attachments_);
+
+  // Because we only start tracking after receiving the first status update,
+  // subtract off that first transfer size.
+  uint64_t transferred_bytes_with_offset =
+      GetTotalTransferred() - num_first_update_bytes_;
+  if (first_update_timestamp_ && transferred_bytes_with_offset > 0) {
+    RecordNearbyShareTransferRateMetric(
+        share_target_.is_incoming, share_target_.type, status,
+        transferred_bytes_with_offset,
+        base::TimeTicks::Now() - *first_update_timestamp_);
+  }
 }

chrome/browser/nearby_sharing/payload_tracker.h

@@ -7,6 +7,7 @@
 
 #include "base/callback_forward.h"
 #include "base/containers/flat_map.h"
+#include "base/optional.h"
 #include "base/time/time.h"
 #include "chrome/browser/nearby_sharing/attachment_info.h"
 #include "chrome/browser/nearby_sharing/nearby_connections_manager.h"
@@ -14,6 +15,8 @@
 #include "chrome/browser/nearby_sharing/transfer_metadata.h"
 #include "chromeos/services/nearby/public/mojom/nearby_connections_types.mojom.h"
 
+// Listens for incoming or outgoing transfer updates from Nearby Connections and
+// forwards the transfer progress to the |update_callback|.
 class PayloadTracker : public NearbyConnectionsManager::PayloadStatusListener {
  public:
   PayloadTracker(
@@ -31,7 +34,7 @@ class PayloadTracker : public NearbyConnectionsManager::PayloadStatusListener {
     explicit State(int64_t total_size) : total_size(total_size) {}
     ~State() = default;
 
-    uint64_t amount_downloaded = 0;
+    uint64_t amount_transferred = 0;
     const uint64_t total_size;
     location::nearby::connections::mojom::PayloadStatus status =
         location::nearby::connections::mojom::PayloadStatus::kInProgress;
@@ -39,11 +42,15 @@ class PayloadTracker : public NearbyConnectionsManager::PayloadStatusListener {
 
   void OnTransferUpdate();
 
-  bool IsComplete();
-  bool IsCancelled();
-  bool HasFailed();
+  bool IsComplete() const;
+  bool IsCancelled() const;
+  bool HasFailed() const;
 
-  double CalculateProgressPercent();
+  uint64_t GetTotalTransferred() const;
+  double CalculateProgressPercent() const;
+
+  void EmitFinalMetrics(
+      location::nearby::connections::mojom::PayloadStatus status) const;
 
   ShareTarget share_target_;
   base::RepeatingCallback<void(ShareTarget, TransferMetadata)> update_callback_;
@@ -51,10 +58,16 @@ class PayloadTracker : public NearbyConnectionsManager::PayloadStatusListener {
   // Map of payload id to state of payload.
   std::map<int64_t, State> payload_state_;
 
-  uint64_t total_download_size_;
+  uint64_t total_transfer_size_;
 
   int last_update_progress_ = 0;
   base::Time last_update_timestamp_;
+
+  // For metrics.
+  size_t num_text_attachments_ = 0;
+  size_t num_file_attachments_ = 0;
+  uint64_t num_first_update_bytes_ = 0;
+  base::Optional<base::TimeTicks> first_update_timestamp_;
 };
 
 #endif  // CHROME_BROWSER_NEARBY_SHARING_PAYLOAD_TRACKER_H_

tools/metrics/histograms/histograms_xml/others/histograms.xml

@@ -9373,6 +9373,81 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </token>
 </histogram>
 
+<histogram name="Nearby.Share.Transfer.NumAttachments{Type}"
+    units="attachments" expires_after="2021-08-19">
+  <owner>nohle@chromium.org</owner>
+  <owner>better-together-dev@google.com</owner>
+  <summary>
+    Records the number of {Type} attachments intended to be sent in a Nearby
+    Share transfer. Emitted when the transfer concludes, successfully or not.
+  </summary>
+  <token key="Type">
+    <variant name="" summary="text or file"/>
+    <variant name=".File" summary="file"/>
+    <variant name=".Text" summary="text"/>
+  </token>
+</histogram>
+
+<histogram
+    name="Nearby.Share.Transfer.Rate{Direction}{ShareTargetType}{PayloadStatus}"
+    units="KB/s" expires_after="2021-08-19">
+  <owner>nohle@chromium.org</owner>
+  <owner>better-together-dev@google.com</owner>
+  <summary>
+    Records the transfer rate of a Nearby Share {Direction} transfer that
+    {PayloadStatus} when sharing with {ShareTargetType}. Emitted when the
+    transfer concludes.
+  </summary>
+  <token key="Direction">
+    <variant name="" summary="outgoing (send) or incoming (receive)"/>
+    <variant name=".Receive" summary="incoming (receive)"/>
+    <variant name=".Send" summary="outgoing (send)"/>
+  </token>
+  <token key="ShareTargetType">
+    <variant name="" summary="any device type"/>
+    <variant name=".Laptop" summary="a laptop"/>
+    <variant name=".Phone" summary="a phone"/>
+    <variant name=".Tablet" summary="a tablet"/>
+    <variant name=".Unknown" summary="an unknown device type"/>
+  </token>
+  <token key="PayloadStatus">
+    <variant name="" summary="succeeded, failed, or was cancelled"/>
+    <variant name=".Cancelled" summary="was cancelled"/>
+    <variant name=".Failed" summary="failed"/>
+    <variant name=".Succeeded" summary="succeeded"/>
+  </token>
+</histogram>
+
+<histogram
+    name="Nearby.Share.Transfer.TotalSize{Direction}{ShareTargetType}{PayloadStatus}"
+    units="KB" expires_after="2021-08-19">
+  <owner>nohle@chromium.org</owner>
+  <owner>better-together-dev@google.com</owner>
+  <summary>
+    Records the total payload size of a Nearby Share {Direction} transfer that
+    {PayloadStatus} when sharing with {ShareTargetType}. Emitted when the
+    transfer concludes.
+  </summary>
+  <token key="Direction">
+    <variant name="" summary="outgoing (send) or incoming (receive)"/>
+    <variant name=".Receive" summary="incoming (receive)"/>
+    <variant name=".Send" summary="outgoing (send)"/>
+  </token>
+  <token key="ShareTargetType">
+    <variant name="" summary="any device type"/>
+    <variant name=".Laptop" summary="a laptop"/>
+    <variant name=".Phone" summary="a phone"/>
+    <variant name=".Tablet" summary="a tablet"/>
+    <variant name=".Unknown" summary="an unknown device type"/>
+  </token>
+  <token key="PayloadStatus">
+    <variant name="" summary="succeeded, failed, or was cancelled"/>
+    <variant name=".Cancelled" summary="was cancelled"/>
+    <variant name=".Failed" summary="failed"/>
+    <variant name=".Succeeded" summary="succeeded"/>
+  </token>
+</histogram>
+
 <histogram name="net.HttpIdentSrcURL" units="requests" expires_after="M85">
   <owner>tsepez@chromium.org</owner>
   <summary>