Split histogram FCMInvalidations.FCMMessageStatus by sender_id

This CL adds suffixes Sync, Drive, and Policy to the existing histogram
FCMInvalidations.FCMMessageStatus. Having all the different senders in
a single histogram made analysis tricky.

Since the sender_id constants aren't accessible at the place where the
histogram is recorded, they're duplicated. This should be okay since
existing sender_ids are not expected to change, and new ones are added
infrequently.

While I'm here, also add myself as an owner of other FCMInvalidations
histograms, since the original owner is OOO.

Bug: 1023813
Change-Id: I41d24f4d61c22641eb0e599176fcfff45f2740c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1972841
Commit-Queue: Marc Treib <treib@chromium.org>
Auto-Submit: Marc Treib <treib@chromium.org>
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726419}

components/invalidation/impl/fcm_network_handler.cc

@@ -98,6 +98,28 @@ InvalidationParsingStatus ParseIncomingMessage(
   return InvalidationParsingStatus::kSuccess;
 }
 
+void RecordFCMMessageStatus(InvalidationParsingStatus status,
+                            const std::string& sender_id) {
+  // These histograms are recorded quite frequently, so use the macros rather
+  // than the functions.
+  UMA_HISTOGRAM_ENUMERATION("FCMInvalidations.FCMMessageStatus", status);
+  // Also split the histogram by a few well-known senders. The actual constants
+  // aren't accessible here (they're defined in higher layers), so we simply
+  // duplicate them here, strictly only for the purpose of metrics.
+  constexpr char kInvalidationGCMSenderId[] = "8181035976";
+  constexpr char kDriveFcmSenderId[] = "947318989803";
+  constexpr char kPolicyFCMInvalidationSenderID[] = "1013309121859";
+  if (sender_id == kInvalidationGCMSenderId) {
+    UMA_HISTOGRAM_ENUMERATION("FCMInvalidations.FCMMessageStatus.Sync", status);
+  } else if (sender_id == kDriveFcmSenderId) {
+    UMA_HISTOGRAM_ENUMERATION("FCMInvalidations.FCMMessageStatus.Drive",
+                              status);
+  } else if (sender_id == kPolicyFCMInvalidationSenderID) {
+    UMA_HISTOGRAM_ENUMERATION("FCMInvalidations.FCMMessageStatus.Policy",
+                              status);
+  }
+}
+
 }  // namespace
 
 FCMNetworkHandler::FCMNetworkHandler(
@@ -239,16 +261,15 @@ void FCMNetworkHandler::OnStoreReset() {}
 void FCMNetworkHandler::OnMessage(const std::string& app_id,
                                   const gcm::IncomingMessage& message) {
   DCHECK_EQ(app_id, app_id_);
+
   std::string payload;
   std::string private_topic;
   std::string public_topic;
   int64_t version = 0;
-
   InvalidationParsingStatus status = ParseIncomingMessage(
       message, &payload, &private_topic, &public_topic, &version);
-  // This histogram is recorded quite frequently, so use the macro rather than
-  // the function.
-  UMA_HISTOGRAM_ENUMERATION("FCMInvalidations.FCMMessageStatus", status);
+
+  RecordFCMMessageStatus(status, sender_id_);
 
   if (status == InvalidationParsingStatus::kSuccess)
     DeliverIncomingMessage(payload, private_topic, public_topic, version);

tools/metrics/histograms/histograms.xml

@@ -51796,7 +51796,10 @@ uploading your change for review.
     enum="FCMInvalidationMessageStatus" expires_after="never">
 <!-- expires-never: For monitoring FCM based invalidations. -->
 
+<!-- Name completed by histogram_suffixes name="FCMInvalidationSenders" -->
+
   <owner>melandory@chromium.org</owner>
+  <owner>treib@chromium.org</owner>
   <summary>
     Status of the message from the FCM channel. Recorded upon receiving response
     from the FCM channel.
@@ -51808,6 +51811,7 @@ uploading your change for review.
 <!-- expires-never: For monitoring FCM based invalidations. -->
 
   <owner>melandory@chromium.org</owner>
+  <owner>treib@chromium.org</owner>
   <summary>
     Status of the initial attempt to retrieve the instance id token.
   </summary>
@@ -51828,6 +51832,7 @@ uploading your change for review.
 <!-- expires-never: For monitoring FCM based invalidations. -->
 
   <owner>melandory@chromium.org</owner>
+  <owner>treib@chromium.org</owner>
   <summary>
     Status of subscription request to the Per User Topic server. Recorded upon
     receiving response from server.
@@ -51839,6 +51844,7 @@ uploading your change for review.
 <!-- expires-never: For monitoring FCM based invalidations. -->
 
   <owner>melandory@chromium.org</owner>
+  <owner>treib@chromium.org</owner>
   <summary>
     For each subcription request to the FCM Per-User-Topic server, log the
     response received from the server.
@@ -51850,6 +51856,7 @@ uploading your change for review.
 <!-- expires-never: For monitoring FCM based invalidations. -->
 
   <owner>melandory@chromium.org</owner>
+  <owner>treib@chromium.org</owner>
   <summary>
     For each subcription request to the FCM Per-User-Topic server, log the
     response received from the server per topic. Note: This is only recorded
@@ -51890,6 +51897,7 @@ uploading your change for review.
 <!-- expires-never: For monitoring FCM based invalidations. -->
 
   <owner>melandory@chromium.org</owner>
+  <owner>treib@chromium.org</owner>
   <summary>
     Status of unsubscription request to the Per User Topic server. Recorded upon
     receiving response from server.
@@ -177548,6 +177556,13 @@ regressions. -->
 
 </histogram_suffixes>
 
+<histogram_suffixes name="FCMInvalidationSenders" separator=".">
+  <suffix name="Drive" label="The message was sent with the Drive sender ID"/>
+  <suffix name="Policy" label="The message was sent with the Policy sender ID"/>
+  <suffix name="Sync" label="The message was sent with the Sync sender ID"/>
+  <affected-histogram name="FCMInvalidations.FCMMessageStatus"/>
+</histogram_suffixes>
+
 <histogram_suffixes name="FeedElementType" separator=".">
   <suffix name="CardLargeImage" label="Card with a large image"/>
   <suffix name="CardSmallImage" label="Card with a small image"/>