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

Issue 832145 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 16
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

create metrics for time spent on WiFi and LTE

Project Member Reported by semenzato@chromium.org, Apr 12 2018

Issue description

There's demand for finding out absolute and relative times spent on WiFi and LTE.  We would prefer histograms reporting per-device cumulative data, rather than uncorrelated usage samples.  To clarify:

We could easily produce a sample every minute (or so) reporting whether we're in one of these states:

-- connected on WiFi
-- connected on LTE
-- otherwise connected
-- not connected

However this would not answer some questions about devices that have both WiFi and LTE---for instance, on those devices, what's the distribution of time spent on LTE vs. WiFi, and what's the distribution of daily usage of LTE across devices.

To get these stats we need to accumulate usage metrics across sessions, and report them daily, similarly to Platform.DailyUseTime.

All these questions apply to "data usage" in addition to "connection time".
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/751f14997f1d1660edf671a69e3a59116bedd755

commit 751f14997f1d1660edf671a69e3a59116bedd755
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Tue May 08 11:57:52 2018

metrics: add CumulativeMetrics class

CumulativeMetrics facilitates the accumulation and reporting
of quantities on a device across periods of time.  For
instance, how long a device has been playing music in
one day.

CQ-DEPEND=CL:1012199

BUG= chromium:832145 
TEST=FEATURES=test emerge-cyan metrics

Change-Id: I656604e78601965af7bbe7c970219181c20d2e6d
Reviewed-on: https://chromium-review.googlesource.com/1012196
Commit-Ready: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>

[add] https://crrev.com/751f14997f1d1660edf671a69e3a59116bedd755/metrics/persistent_integer_test_base.cc
[modify] https://crrev.com/751f14997f1d1660edf671a69e3a59116bedd755/metrics/metrics_daemon_test.cc
[add] https://crrev.com/751f14997f1d1660edf671a69e3a59116bedd755/metrics/cumulative_metrics_test.cc
[add] https://crrev.com/751f14997f1d1660edf671a69e3a59116bedd755/metrics/cumulative_metrics.h
[modify] https://crrev.com/751f14997f1d1660edf671a69e3a59116bedd755/metrics/uploader/upload_service_test.cc
[add] https://crrev.com/751f14997f1d1660edf671a69e3a59116bedd755/metrics/cumulative_metrics.cc
[add] https://crrev.com/751f14997f1d1660edf671a69e3a59116bedd755/metrics/persistent_integer_test_base.h
[modify] https://crrev.com/751f14997f1d1660edf671a69e3a59116bedd755/metrics/metrics.gyp
[modify] https://crrev.com/751f14997f1d1660edf671a69e3a59116bedd755/metrics/persistent_integer_test.cc
[modify] https://crrev.com/751f14997f1d1660edf671a69e3a59116bedd755/metrics/libmetrics.gypi
[modify] https://crrev.com/751f14997f1d1660edf671a69e3a59116bedd755/metrics/README.md
[modify] https://crrev.com/751f14997f1d1660edf671a69e3a59116bedd755/metrics/persistent_integer.h
[modify] https://crrev.com/751f14997f1d1660edf671a69e3a59116bedd755/metrics/persistent_integer.cc

Project Member

Comment 2 by bugdroid1@chromium.org, May 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/40f41292e2d965b5f623681319a3f1d30df52032

commit 40f41292e2d965b5f623681319a3f1d30df52032
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Tue May 08 11:57:53 2018

chromeos-base/metrics: add cumulative metrics class

With this CL the metrics library exports the cumulative metrics
and persistent integer functionality.

This makes it easier for clients to implement cumulative metrics,
such as <something>DailyUse.

The CL installs two header files and adds one test.

CQ-DEPEND=CL:1012196

BUG= chromium:832145 
TEST=emerge-cyan metrics

Change-Id: I6564c20eb871b3b7e52e64431902e3135377f629
Reviewed-on: https://chromium-review.googlesource.com/1012199
Commit-Ready: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/40f41292e2d965b5f623681319a3f1d30df52032/chromeos-base/metrics/metrics-9999.ebuild

Project Member

Comment 3 by bugdroid1@chromium.org, May 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/501139b2d954844702909db9405444bf2ef2b3ae

commit 501139b2d954844702909db9405444bf2ef2b3ae
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Wed May 09 21:29:26 2018

Revert "chromeos-base/metrics: add cumulative metrics class"

This reverts commit 40f41292e2d965b5f623681319a3f1d30df52032.

Reason for revert: unit test is flaky and breaks canaries

Original change's description:
> chromeos-base/metrics: add cumulative metrics class
> 
> With this CL the metrics library exports the cumulative metrics
> and persistent integer functionality.
> 
> This makes it easier for clients to implement cumulative metrics,
> such as <something>DailyUse.
> 
> The CL installs two header files and adds one test.
> 
> CQ-DEPEND=CL:1012196
> 
> BUG= chromium:832145 
> TEST=emerge-cyan metrics
> 
> Change-Id: I6564c20eb871b3b7e52e64431902e3135377f629
> Reviewed-on: https://chromium-review.googlesource.com/1012199
> Commit-Ready: Luigi Semenzato <semenzato@chromium.org>
> Tested-by: Luigi Semenzato <semenzato@chromium.org>
> Reviewed-by: Mike Frysinger <vapier@chromium.org>

Bug:  chromium:832145 
Change-Id: Ie9bb5285994805d3cf6db1b7a45fe1841c7a8b6e
Reviewed-on: https://chromium-review.googlesource.com/1052968
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Tested-by: Philip Chen <philipchen@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Commit-Queue: Luigi Semenzato <semenzato@chromium.org>
Trybot-Ready: Luigi Semenzato <semenzato@chromium.org>

[modify] https://crrev.com/501139b2d954844702909db9405444bf2ef2b3ae/chromeos-base/metrics/metrics-9999.ebuild

Project Member

Comment 4 by bugdroid1@chromium.org, May 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/ac388102a723da9ff2f9f600b9b51313fd81d666

commit ac388102a723da9ff2f9f600b9b51313fd81d666
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Wed May 09 21:29:49 2018

Revert "metrics: add CumulativeMetrics class"

This reverts commit 751f14997f1d1660edf671a69e3a59116bedd755.

Reason for revert: unit test is flaky and breaks canaries

Original change's description:
> metrics: add CumulativeMetrics class
> 
> CumulativeMetrics facilitates the accumulation and reporting
> of quantities on a device across periods of time.  For
> instance, how long a device has been playing music in
> one day.
> 
> CQ-DEPEND=CL:1012199
> 
> BUG= chromium:832145 
> TEST=FEATURES=test emerge-cyan metrics
> 
> Change-Id: I656604e78601965af7bbe7c970219181c20d2e6d
> Reviewed-on: https://chromium-review.googlesource.com/1012196
> Commit-Ready: Luigi Semenzato <semenzato@chromium.org>
> Tested-by: Luigi Semenzato <semenzato@chromium.org>
> Reviewed-by: Luigi Semenzato <semenzato@chromium.org>

Bug:  chromium:832145 
Change-Id: I1efee618fbd42aa94705aff4384ce92bf3f9b710
Reviewed-on: https://chromium-review.googlesource.com/1052829
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Commit-Queue: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Trybot-Ready: Luigi Semenzato <semenzato@chromium.org>

[delete] https://crrev.com/41b3f4cb8ad715f538e0cc227c0dec32ee1682f0/metrics/persistent_integer_test_base.cc
[modify] https://crrev.com/ac388102a723da9ff2f9f600b9b51313fd81d666/metrics/metrics_daemon_test.cc
[delete] https://crrev.com/41b3f4cb8ad715f538e0cc227c0dec32ee1682f0/metrics/cumulative_metrics_test.cc
[delete] https://crrev.com/41b3f4cb8ad715f538e0cc227c0dec32ee1682f0/metrics/cumulative_metrics.h
[modify] https://crrev.com/ac388102a723da9ff2f9f600b9b51313fd81d666/metrics/uploader/upload_service_test.cc
[delete] https://crrev.com/41b3f4cb8ad715f538e0cc227c0dec32ee1682f0/metrics/cumulative_metrics.cc
[delete] https://crrev.com/41b3f4cb8ad715f538e0cc227c0dec32ee1682f0/metrics/persistent_integer_test_base.h
[modify] https://crrev.com/ac388102a723da9ff2f9f600b9b51313fd81d666/metrics/metrics.gyp
[modify] https://crrev.com/ac388102a723da9ff2f9f600b9b51313fd81d666/metrics/persistent_integer_test.cc
[modify] https://crrev.com/ac388102a723da9ff2f9f600b9b51313fd81d666/metrics/libmetrics.gypi
[modify] https://crrev.com/ac388102a723da9ff2f9f600b9b51313fd81d666/metrics/README.md
[modify] https://crrev.com/ac388102a723da9ff2f9f600b9b51313fd81d666/metrics/persistent_integer.h
[modify] https://crrev.com/ac388102a723da9ff2f9f600b9b51313fd81d666/metrics/persistent_integer.cc

Project Member

Comment 5 by bugdroid1@chromium.org, May 10 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/62cdff24dbe985b45b39f7c556311f0bc87a24c8

commit 62cdff24dbe985b45b39f7c556311f0bc87a24c8
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Thu May 10 22:09:33 2018

chromeos-base/metrics: add cumulative metrics class

With this CL the metrics library exports the cumulative metrics
and persistent integer functionality.

This makes it easier for clients to implement cumulative metrics,
such as <something>DailyUse.

The CL installs two header files and adds one test.

CQ-DEPEND=CL:1053306

BUG= chromium:832145 
TEST=emerge-cyan metrics

Change-Id: I8d767aca0340cca6bdb3df65aaab6206e24997b4
Reviewed-on: https://chromium-review.googlesource.com/1012199
Commit-Ready: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1053408
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>

[modify] https://crrev.com/62cdff24dbe985b45b39f7c556311f0bc87a24c8/chromeos-base/metrics/metrics-9999.ebuild

Project Member

Comment 6 by bugdroid1@chromium.org, May 10 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/1abb3cbbb32d19ec08069c45f886a1dc8d0cead6

commit 1abb3cbbb32d19ec08069c45f886a1dc8d0cead6
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Thu May 10 22:09:33 2018

metrics: add CumulativeMetrics class

CumulativeMetrics facilitates the accumulation and reporting
of quantities on a device across periods of time.  For
instance, how long a device has been playing music in
one day.

CQ-DEPEND=CL:1053408

BUG= chromium:832145 
TEST=FEATURES=test emerge-cyan metrics

Change-Id: Ie0d2c6a33679a9a8c656f1b32e4f9231cc7c2258
Reviewed-on: https://chromium-review.googlesource.com/1053306
Commit-Ready: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[add] https://crrev.com/1abb3cbbb32d19ec08069c45f886a1dc8d0cead6/metrics/persistent_integer_test_base.cc
[modify] https://crrev.com/1abb3cbbb32d19ec08069c45f886a1dc8d0cead6/metrics/metrics_daemon_test.cc
[add] https://crrev.com/1abb3cbbb32d19ec08069c45f886a1dc8d0cead6/metrics/cumulative_metrics_test.cc
[add] https://crrev.com/1abb3cbbb32d19ec08069c45f886a1dc8d0cead6/metrics/cumulative_metrics.h
[modify] https://crrev.com/1abb3cbbb32d19ec08069c45f886a1dc8d0cead6/metrics/uploader/upload_service_test.cc
[add] https://crrev.com/1abb3cbbb32d19ec08069c45f886a1dc8d0cead6/metrics/cumulative_metrics.cc
[add] https://crrev.com/1abb3cbbb32d19ec08069c45f886a1dc8d0cead6/metrics/persistent_integer_test_base.h
[modify] https://crrev.com/1abb3cbbb32d19ec08069c45f886a1dc8d0cead6/metrics/metrics.gyp
[modify] https://crrev.com/1abb3cbbb32d19ec08069c45f886a1dc8d0cead6/metrics/persistent_integer_test.cc
[modify] https://crrev.com/1abb3cbbb32d19ec08069c45f886a1dc8d0cead6/metrics/libmetrics.gypi
[modify] https://crrev.com/1abb3cbbb32d19ec08069c45f886a1dc8d0cead6/metrics/README.md
[modify] https://crrev.com/1abb3cbbb32d19ec08069c45f886a1dc8d0cead6/metrics/persistent_integer.h
[modify] https://crrev.com/1abb3cbbb32d19ec08069c45f886a1dc8d0cead6/metrics/persistent_integer.cc

Owner: semenzato@chromium.org
Status: Fixed (was: Untriaged)
This is all done.
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 10

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/dc0f260466c76c2ece177e5c67c0d3b02956bee1

commit dc0f260466c76c2ece177e5c67c0d3b02956bee1
Author: Kirtika Ruchandani <kirtika@google.com>
Date: Fri Aug 10 08:44:32 2018

shill: Cleanup metrics logic tracking online state

1. was_online_ is meant to leave notes for future invocations
of NotifyDefaultServiceChanged, and meant to be used along with
last_default_technology_. Change it to was_last_online to make the
connection more obvious.
2. "Ignore changes that are not online/offline transitions" is
hard to read. This should be "only consider transitions b/w
online and offline". Make that snippet more readable.

BUG= chromium:832145 
TEST=shill unit-tests pass

Change-Id: Iaeef47e12a2188563d0fe799436304993631a37e
Signed-off-by: Kirtika Ruchandani <kirtika@google.com>
Reviewed-on: https://chromium-review.googlesource.com/1167888
Commit-Ready: Kirtika Ruchandani <kirtika@chromium.org>
Tested-by: Kirtika Ruchandani <kirtika@chromium.org>
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>

[modify] https://crrev.com/dc0f260466c76c2ece177e5c67c0d3b02956bee1/metrics.h
[modify] https://crrev.com/dc0f260466c76c2ece177e5c67c0d3b02956bee1/metrics.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/f5540a3a2546fce5d536420a1026edae5dfeba63

commit f5540a3a2546fce5d536420a1026edae5dfeba63
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Thu Aug 23 14:37:20 2018

metrics: allow persistent integer files to be in subdirectories

Motivation:
Because daemons run as separate users, for proper isolation their
persistent integer files must reside in directories with different
owners and permissions, rather than all in one place.

Details:
PersistentIntegers now take a path for their backing store, instead
of a name, from which a path was constructed into /var/lib/metrics.
Additionally, the CumulativeMetrics constructor now takes as argument
a FilePath naming the backing directory where their persistent integers
are stored.

BUG= chromium:832145 
TEST=unit tests pass

Change-Id: Ie942e7c8c5c84e566333aedada24f814efb05ea2
Reviewed-on: https://chromium-review.googlesource.com/1171256
Commit-Ready: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>

[modify] https://crrev.com/f5540a3a2546fce5d536420a1026edae5dfeba63/metrics/metrics_daemon.cc
[delete] https://crrev.com/3fed6d64e70f243cde4de980e582272394fe3b00/metrics/persistent_integer_test_base.cc
[modify] https://crrev.com/f5540a3a2546fce5d536420a1026edae5dfeba63/metrics/metrics_daemon_test.cc
[modify] https://crrev.com/f5540a3a2546fce5d536420a1026edae5dfeba63/metrics/metrics_daemon_main.cc
[modify] https://crrev.com/f5540a3a2546fce5d536420a1026edae5dfeba63/metrics/cumulative_metrics_test.cc
[modify] https://crrev.com/f5540a3a2546fce5d536420a1026edae5dfeba63/metrics/cumulative_metrics.h
[modify] https://crrev.com/f5540a3a2546fce5d536420a1026edae5dfeba63/metrics/uploader/upload_service_test.cc
[modify] https://crrev.com/f5540a3a2546fce5d536420a1026edae5dfeba63/metrics/cumulative_metrics.cc
[delete] https://crrev.com/3fed6d64e70f243cde4de980e582272394fe3b00/metrics/persistent_integer_test_base.h
[modify] https://crrev.com/f5540a3a2546fce5d536420a1026edae5dfeba63/metrics/metrics.gyp
[modify] https://crrev.com/f5540a3a2546fce5d536420a1026edae5dfeba63/metrics/persistent_integer_test.cc
[modify] https://crrev.com/f5540a3a2546fce5d536420a1026edae5dfeba63/metrics/README.md
[modify] https://crrev.com/f5540a3a2546fce5d536420a1026edae5dfeba63/metrics/persistent_integer_mock.h
[modify] https://crrev.com/f5540a3a2546fce5d536420a1026edae5dfeba63/metrics/metrics_daemon.h
[modify] https://crrev.com/f5540a3a2546fce5d536420a1026edae5dfeba63/metrics/uploader/system_profile_cache.cc
[modify] https://crrev.com/f5540a3a2546fce5d536420a1026edae5dfeba63/metrics/persistent_integer.h
[modify] https://crrev.com/f5540a3a2546fce5d536420a1026edae5dfeba63/metrics/persistent_integer.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/0de32f8e6ff63aef43837a2e635dd08c6a425212

commit 0de32f8e6ff63aef43837a2e635dd08c6a425212
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Thu Aug 30 21:41:58 2018

security_StatefulPermissions: add /var/lib/shill

This is for storing PersistentInteger backing files
for CumulativeMetrics, with the immediate motivation of
reporting time on wifi vs. cellular.

BUG= chromium:832145 
TEST=none

Change-Id: I4828b90ccc2ed8413f5714ff9aedfc16095adf3d
Reviewed-on: https://chromium-review.googlesource.com/1192408
Commit-Ready: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/0de32f8e6ff63aef43837a2e635dd08c6a425212/client/site_tests/security_StatefulPermissions/security_StatefulPermissions.py

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 31

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/0859267efafb319e4604b475210bad21c9539333

commit 0859267efafb319e4604b475210bad21c9539333
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Fri Aug 31 12:23:33 2018

shill: add cumulative metrics for time online

This uses the CumulativeMetrics class to accumulate
and report daily connectivity on a per-technology basis
(wifi, cellular, ...).  The new histograms report (at most)
one sample per day per device, and each sample indicates
how much (active) time the device spent connected via
various types of technology.

To test this, I ran it on a device for > 24 hours, switching
between WiFi and Ethernet, and verified that the histograms
samples were as expected.

CQ-DEPEND=CL:1171256,CL:1192408
BUG= chromium:832145 
TEST=manual (see above)

Change-Id: I69bee9e9157ae1dd7a78c702192032052e089d07
Reviewed-on: https://chromium-review.googlesource.com/1121419
Commit-Ready: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>

[modify] https://crrev.com/0859267efafb319e4604b475210bad21c9539333/metrics.h
[modify] https://crrev.com/0859267efafb319e4604b475210bad21c9539333/init/shill-pre-start.sh
[modify] https://crrev.com/0859267efafb319e4604b475210bad21c9539333/metrics.cc

Your CL missed M70 branch-point by one OS version, and I suspect you'll want it cp-ed to M70: 
https://chromium-review.googlesource.com/c/aosp/platform/system/connectivity/shill/+/1121419

(landed in 11022.0.0, branch cut at 11021.0.0). 

https://chromium-review.googlesource.com/c/aosp/platform/system/connectivity/shill/+/1176166
is going into M70 and it creates/uses /var/lib/shill too so we need to coordinate?



The CQ has been bad... OK, thanks.
Kirtika, Ketaki, do we want this in R70?
Cc: mortonm@chromium.org geo...@google.com
Labels: Merge-Request-70
Yes, we do. Please check that we don't end up with any conflicts with this R70 cherry-pick:
https://chromium-review.googlesource.com/c/aosp/platform/system/connectivity/shill/+/1211647

I think the conflict I am concerned about is here: 
https://chromium-review.googlesource.com/c/aosp/platform/system/connectivity/shill/+/1204652

but mortonm@ isn't cherry-picking that to R70  - why? 

Oh yeah if the metrics stuff that modified shill-pre-start.sh is going to be cherry picked back to M70 then CL:1204652 should be as well. I guess we should be able to avoid any merge conflicts by doing this one first: https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/0859267efafb319e4604b475210bad21c9539333 ?

There shouldn't be any conflicts between this metrics stuff and CL:1211647 AFAICT
Project Member

Comment 17 by bugdroid1@chromium.org, Sep 12

Labels: merge-merged-release-R70-11021.B
The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/523c52b279610c78c224cb9718578dc18327b5b0

commit 523c52b279610c78c224cb9718578dc18327b5b0
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Wed Sep 12 02:04:14 2018

shill: add cumulative metrics for time online

This uses the CumulativeMetrics class to accumulate
and report daily connectivity on a per-technology basis
(wifi, cellular, ...).  The new histograms report (at most)
one sample per day per device, and each sample indicates
how much (active) time the device spent connected via
various types of technology.

To test this, I ran it on a device for > 24 hours, switching
between WiFi and Ethernet, and verified that the histograms
samples were as expected.

CQ-DEPEND=CL:1171256,CL:1192408
BUG= chromium:832145 
TEST=manual (see above)

Reviewed-on: https://chromium-review.googlesource.com/1121419
Commit-Ready: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>

Change-Id: I69bee9e9157ae1dd7a78c702192032052e089d07

[modify] https://crrev.com/523c52b279610c78c224cb9718578dc18327b5b0/metrics.h
[modify] https://crrev.com/523c52b279610c78c224cb9718578dc18327b5b0/init/shill-pre-start.sh
[modify] https://crrev.com/523c52b279610c78c224cb9718578dc18327b5b0/metrics.cc

I think this was marked "fixed" by mistake, but it is really fixed now (minus bugs ;)

Project Member

Comment 19 by sheriffbot@chromium.org, Sep 12

Labels: -Merge-Request-70 Hotlist-Merge-Approved Merge-Approved-70
Your change meets the bar and is auto-approved for M70. Please go ahead and merge the CL to branch 3538 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
(sorry I didn't wait for the merge-approved)
Project Member

Comment 21 by sheriffbot@chromium.org, Sep 17

Cc: kirtika@google.com
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 22 by sheriffbot@chromium.org, Sep 21

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Approved-70 Merge-Merged-70
Merged in #17 IIUC.
#17 yes, thank you Brian.
Project Member

Comment 25 by bugdroid1@chromium.org, Oct 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/72c95056c0a71fc55c714f3fa156c83c309bd23a

commit 72c95056c0a71fc55c714f3fa156c83c309bd23a
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Thu Oct 18 21:53:34 2018

shill: revise wifi/cellular metrics

Here we collect histograms for daily/monthly online times
on wifi/cellular, as well as fractions of wifi and cellular
over total connection time, limited to samples collected
when both technologies are present and a specific one is
chosen.

These histograms do not distinguish the case of a built-in
cellular modem vs. a USB dongle.  Unfortunately it is not
easy to tell them apart because they both show up as USB
devices.

This adds a total of 10 histograms:
[any / cellular / wifi / cellular fraction / wifi fraction] x
[daily / monthly]

BUG= chromium:832145 
TEST=ran on device

Change-Id: I92dd76e2f303ee49e24d499acc1e0738f66ba6a0
Reviewed-on: https://chromium-review.googlesource.com/1237139
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>

[modify] https://crrev.com/72c95056c0a71fc55c714f3fa156c83c309bd23a/shill/metrics.h
[modify] https://crrev.com/72c95056c0a71fc55c714f3fa156c83c309bd23a/shill/metrics.cc

Labels: -Hotlist-Merge-Approved -merge-merged-release-R70-11021.B -Merge-Merged-70 Merge-Request-70
Another merge request.  I don't know if it is feasible at this point, but it would be nice.
Project Member

Comment 27 by sheriffbot@chromium.org, Oct 18

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Is this critical for M70? We're going to prep for Stable next week. I'd prefer we get this into M71 instead.
Labels: -Merge-Review-70 Merge-Approved-70
Per offline chat. Metrics will be needed for features in M70. Approved for M70.
Labels: Merge-Request-71
Also needs to be in 71 and would prefer sooner rather than later.  Thanks!
Project Member

Comment 31 by sheriffbot@chromium.org, Oct 19

Labels: -Merge-Request-71 Hotlist-Merge-Approved Merge-Approved-71
Your change meets the bar and is auto-approved for M71. Please go ahead and merge the CL to branch 3578 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 32 by bugdroid1@chromium.org, Oct 20

Labels: merge-merged-release-R70-11021.B
The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/1f86bc7456824cf77d1d301dfd4cf5f643c6006b

commit 1f86bc7456824cf77d1d301dfd4cf5f643c6006b
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Fri Oct 19 08:44:45 2018

shill: revise wifi/cellular metrics

Here we collect histograms for daily/monthly online times
on wifi/cellular, as well as fractions of wifi and cellular
over total connection time, limited to samples collected
when both technologies are present and a specific one is
chosen.

These histograms do not distinguish the case of a built-in
cellular modem vs. a USB dongle.  Unfortunately it is not
easy to tell them apart because they both show up as USB
devices.

This adds a total of 10 histograms:
[any / cellular / wifi / cellular fraction / wifi fraction] x
[daily / monthly]

BUG= chromium:832145 
TEST=ran on device

Change-Id: I2d61260c210d92e8b529beeea6d4d0e7121f6fc4

[modify] https://crrev.com/1f86bc7456824cf77d1d301dfd4cf5f643c6006b/metrics.h
[modify] https://crrev.com/1f86bc7456824cf77d1d301dfd4cf5f643c6006b/metrics.cc

Project Member

Comment 33 by bugdroid1@chromium.org, Nov 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/fa976c81cdf8f2ba161e11c2c51593b70d6eed96

commit fa976c81cdf8f2ba161e11c2c51593b70d6eed96
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Wed Nov 21 13:53:02 2018

shill: add test for wifi/cell cumulative metrics

The test checks that the update and report callbacks
for cumulative wifi an cellular times are behaving
as expected.  It does *not* check that the callbacks
are automatically invoked as expected: that is tested
in the cumulative metrics unit test.

(Thanks to Kirtika for showing how to deal with mocks and
inheritance and other help.)

BUG= chromium:832145 
TEST=test completes correctly

Change-Id: I1e2d5c47640e82f7b5b8790b5e9f1bf085a8450c
Reviewed-on: https://chromium-review.googlesource.com/1286228
Commit-Ready: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/fa976c81cdf8f2ba161e11c2c51593b70d6eed96/shill/metrics_test.cc
[modify] https://crrev.com/fa976c81cdf8f2ba161e11c2c51593b70d6eed96/shill/metrics.h
[modify] https://crrev.com/fa976c81cdf8f2ba161e11c2c51593b70d6eed96/shill/metrics.cc
[modify] https://crrev.com/fa976c81cdf8f2ba161e11c2c51593b70d6eed96/metrics/cumulative_metrics.h

Sign in to add a comment