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

Issue 757600 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Some Current Session Metrics Should Be Also Reported in ProvidePreviousSessionData()

Project Member Reported by mpear...@chromium.org, Aug 21 2017

Issue description


The histogram MemoryAndroid.LowRamDevice should be emitted by AndroidMetricsProvider::ProvidePreviousSessionData().  It's unlikely to change between Chrome restarts.  This is a useful step to allow attributing things that caused the previous log not to recorded/uploaded, e.g., for identifying browser crashes.

 

Comment 1 by holte@chromium.org, Aug 22 2017

Summary: MemoryAndroid.LowRamDevice Should Be Reported in ProvidePreviousSessionData() (was: MemoryAndroid.LowRamDevice Should Be Reported in ProvideCurrentSessionData())
Owner: mpear...@chromium.org
Status: Started (was: Available)
I audited the calls to ProvideCurrentSessionData().  Most are not appropriate to emit for the previous session.  I found two places where it's likely appropriate though.  I'll put together separate changes for each.

https://cs.chromium.org/chromium/src/components/metrics/metrics_state_manager.cc?q=ProvideCurrentSessionData+file:cc$+-file:test&sq=package:chromium&l=84&dr=CSs

https://cs.chromium.org/chromium/src/chrome/browser/metrics/chromeos_metrics_provider.cc?q=ProvideCurrentSessionData+file:cc$+-file:test&sq=package:chromium&l=233&dr=CSs
probably only enrollment status:
  RecordEnrollmentStatus();

Summary: Some Current Session Metrics Should Be Also Reported in ProvidePreviousSessionData() (was: MemoryAndroid.LowRamDevice Should Be Reported in ProvidePreviousSessionData())
broadening scope of bug
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 28 2017

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

commit 0ad80996ae08477966bc8b6fe061ba3769d8e3c1
Author: Mark Pearson <mpearson@chromium.org>
Date: Mon Aug 28 19:25:03 2017

Metrics - Report MemoryAndroid.LowRamDevice on Previous Sessions Too

Bug:  757600 
Change-Id: I116a87d80e426dbc91dc0840825d0adcf627c924
Reviewed-on: https://chromium-review.googlesource.com/634408
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Commit-Queue: Mark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497833}
[modify] https://crrev.com/0ad80996ae08477966bc8b6fe061ba3769d8e3c1/chrome/browser/metrics/android_metrics_provider.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 1 2017

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

commit 59856116ce6e38042eaa42ade7f99f888e04cd8d
Author: Mark Pearson <mpearson@chromium.org>
Date: Fri Sep 01 19:04:32 2017

Low Ram Histogram - Forgot to Change to Stability History

In
https://chromium-review.googlesource.com/c/chromium/src/+/634408
I forgot change the histogram to a stability histogram, otherwise
it won't get picked up in ProvidePreviousSessionData().

Bug:  757600 
Change-Id: Ieab926d4df7e26d930142bce7775703aa099a524
Reviewed-on: https://chromium-review.googlesource.com/646967
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Commit-Queue: Mark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499248}
[modify] https://crrev.com/59856116ce6e38042eaa42ade7f99f888e04cd8d/chrome/browser/metrics/android_metrics_provider.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 14 2017

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

commit 25a36bd497b5aa9c5ddf26fe221ff073861aa56c
Author: Mark Pearson <mpearson@chromium.org>
Date: Thu Sep 14 22:39:15 2017

Record Enrollment Status and ARC State in PreviousSession UMA Logs Too

These are unlikely to change between Chrome restarts.

This is part of a general audit to make sure all relatively-stable
things emitted by ProvideCurrentSessionData() are also emitted in
ProvidePreviousSessionData().  Otherwise missing data can leave
holes in metrics when some logs are inappropriately excluded from
analysis.

Bug:  757600 
Change-Id: I4e9b8214db4a0d83c97d9854b2689e7f01aad55f
Reviewed-on: https://chromium-review.googlesource.com/639010
Commit-Queue: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Elijah Taylor <elijahtaylor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502048}
[modify] https://crrev.com/25a36bd497b5aa9c5ddf26fe221ff073861aa56c/chrome/browser/chromeos/arc/arc_optin_uma.cc
[modify] https://crrev.com/25a36bd497b5aa9c5ddf26fe221ff073861aa56c/chrome/browser/metrics/chromeos_metrics_provider.cc

Status: Fixed (was: Started)
And with the submission of the below change, I think this bug is fixed.  (I fixed all issues my previous audit found.)
---
https://chromium.googlesource.com/chromium/src.git/+/d4f91d118274c0f0583e7a6f196be6c48da4acc9
Make PreviousSession UMA Logs Use Previous Client Id

Also make them report IsClonedInstall too, if the previous session
was detected as being a cloned install.

Do so by remembering the previous client id seen upon startup
and remembering whether IsClonedInstall was reported in the last
session and using those values when responding to
ProvidePreviousSessionData().

The change in histogram macros was to make this a stability-tagged
histogram, otherwise even if we record it in
ProvidePreviousSessionData(), it would not be emitted for the
previous session.

Bug:  754758 
Change-Id: I9901c4e4de7efe1bb26bdda053a6ee734461db1a
Reviewed-on: https://chromium-review.googlesource.com/638841
Commit-Queue: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514703}

Sign in to add a comment