New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 672067 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
no longer working on chrome
Closed: Dec 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

GCM lacks UMA for DELETED_MESSAGES and collapse_key

Project Member Reported by joh...@chromium.org, Dec 7 2016

Issue description

We can't tell how often messages are getting deleted on the server due to exceeding the 100 queued non-collapsible messages limit (which seems to be the only case where DELETED_MESSAGES is sent). And we can't tell how often non-collapsible messages are being sent, in order to have a baseline for this.
 
Also no UMA is logged at all for received messages on Android (other than client specific UMA like push messaging).
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/409dc538bb1495c458a72f7e93eb9e1dab102036

commit 409dc538bb1495c458a72f7e93eb9e1dab102036
Author: johnme <johnme@chromium.org>
Date: Tue Dec 13 23:43:42 2016

GCM: Add Android UMA, and UMA for deleted/collapsed messages

- Adds GCM.DeletedMessagesReceived UMA for incoming DELETED_MESSAGES,
  which happens when the 100 non-collapsible messages limit is exceeded
  on the GCM server.

- Adds GCM.DataMessageReceivedHasCollapseKey UMA for incoming messages
  with collapse_key, giving a baseline to compare the previous UMA with.

- Makes the existing GCM.DataMessageReceivedHasRegisteredApp and
  GCM.DataMessageReceived UMAs only count DATA_MESSAGE rather than
  DELETED_MESSAGES, to better match their names (and be more useful).

- Also adds all of the above UMAs to Android, which used to have no UMA.

BUG= 672067 

Review-Url: https://codereview.chromium.org/2558553002
Cr-Commit-Position: refs/heads/master@{#438339}

[modify] https://crrev.com/409dc538bb1495c458a72f7e93eb9e1dab102036/chrome/android/java/src/org/chromium/chrome/browser/services/gcm/ChromeGcmListenerService.java
[modify] https://crrev.com/409dc538bb1495c458a72f7e93eb9e1dab102036/chrome/android/java/src/org/chromium/chrome/browser/services/gcm/GcmUma.java
[modify] https://crrev.com/409dc538bb1495c458a72f7e93eb9e1dab102036/components/gcm_driver/gcm_client_impl.cc
[modify] https://crrev.com/409dc538bb1495c458a72f7e93eb9e1dab102036/components/gcm_driver/gcm_client_impl.h
[modify] https://crrev.com/409dc538bb1495c458a72f7e93eb9e1dab102036/components/gcm_driver/gcm_stats_recorder_impl.cc
[modify] https://crrev.com/409dc538bb1495c458a72f7e93eb9e1dab102036/google_apis/gcm/monitoring/gcm_stats_recorder.h
[modify] https://crrev.com/409dc538bb1495c458a72f7e93eb9e1dab102036/tools/metrics/histograms/histograms.xml

Comment 3 by joh...@chromium.org, Dec 14 2016

Labels: Merge-Request-56
Requesting merge to Chrome 56. Safe patch that only changes UMA logging, and will help us diagnose some concerning issues.

Comment 4 by dimu@chromium.org, Dec 14 2016

Labels: -Merge-Request-56 Merge-Approved-56 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M56 (branch: 2924)
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 15 2016

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5b6db9c7dfd03651ccbad98eb27a7daf4b8de8e2

commit 5b6db9c7dfd03651ccbad98eb27a7daf4b8de8e2
Author: John Mellor <johnme@chromium.org>
Date: Thu Dec 15 00:21:41 2016

GCM: Add Android UMA, and UMA for deleted/collapsed messages

- Adds GCM.DeletedMessagesReceived UMA for incoming DELETED_MESSAGES,
  which happens when the 100 non-collapsible messages limit is exceeded
  on the GCM server.

- Adds GCM.DataMessageReceivedHasCollapseKey UMA for incoming messages
  with collapse_key, giving a baseline to compare the previous UMA with.

- Makes the existing GCM.DataMessageReceivedHasRegisteredApp and
  GCM.DataMessageReceived UMAs only count DATA_MESSAGE rather than
  DELETED_MESSAGES, to better match their names (and be more useful).

- Also adds all of the above UMAs to Android, which used to have no UMA.

BUG= 672067 

Review-Url: https://codereview.chromium.org/2558553002
Cr-Commit-Position: refs/heads/master@{#438339}
(cherry picked from commit 409dc538bb1495c458a72f7e93eb9e1dab102036)

Review URL: https://codereview.chromium.org/2573333002 .

Cr-Commit-Position: refs/branch-heads/2924@{#502}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/5b6db9c7dfd03651ccbad98eb27a7daf4b8de8e2/chrome/android/java/src/org/chromium/chrome/browser/services/gcm/ChromeGcmListenerService.java
[modify] https://crrev.com/5b6db9c7dfd03651ccbad98eb27a7daf4b8de8e2/chrome/android/java/src/org/chromium/chrome/browser/services/gcm/GcmUma.java
[modify] https://crrev.com/5b6db9c7dfd03651ccbad98eb27a7daf4b8de8e2/components/gcm_driver/gcm_client_impl.cc
[modify] https://crrev.com/5b6db9c7dfd03651ccbad98eb27a7daf4b8de8e2/components/gcm_driver/gcm_client_impl.h
[modify] https://crrev.com/5b6db9c7dfd03651ccbad98eb27a7daf4b8de8e2/components/gcm_driver/gcm_stats_recorder_impl.cc
[modify] https://crrev.com/5b6db9c7dfd03651ccbad98eb27a7daf4b8de8e2/google_apis/gcm/monitoring/gcm_stats_recorder.h
[modify] https://crrev.com/5b6db9c7dfd03651ccbad98eb27a7daf4b8de8e2/tools/metrics/histograms/histograms.xml

Comment 6 by joh...@chromium.org, Dec 15 2016

Labels: M-56
Status: Fixed (was: Started)

Sign in to add a comment