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

Reduce BlueZ.ChipLost metrics pollution

Project Member Reported by sonnysasaka@chromium.org, Aug 15

Issue description

A bug in the metrics collection causes false positives when adapter is disabled and suspend/resume happens.
 
Cc: bhthompson@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 16

Labels: merge-merged-chromeos-5.44
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/bluez/+/a7a2f0011bcc577c5a93904087e4e7ff7169d0ba

commit a7a2f0011bcc577c5a93904087e4e7ff7169d0ba
Author: Miao-chen Chou <mcchou@chromium.org>
Date: Thu Aug 16 20:14:38 2018

CHROMIUM: Refine chip lost metrics

The current ChipLost metrics depends on the adapter flag,
discovery_suspended_by_system, to eliminate the chip lost caused by
system suspends/resumes. However, the flag is not set to true if the
adapter is not powered when the system is going to suspend, and this
will lead to false recording of ChipLost sample when the system comes
back up.

This patch introduces a separate flag, system_suspended, to avoid the
false recording and send the chip lost sample to the refined metrics,
BlueZ.ChipLost2.

The corresponding UMA histogram CL is at https://crrev.com/c/1176371.

BUG= chromium:874687 
TEST=(1) Turn off Bluetooth and suspend the system without seeing
         a sample of BlueZ.ChipLost2 emitted in chrome://histograms page.
     (2) Turn off Bluetooth, remove btusb module and see a sample of
         BlueZ.ChipLost2 gets emitted.
CQ-DEPEND=CL:1176657

Change-Id: Ib1ce84724992931dd4c3c3917978fdaf87d5bada
Reviewed-on: https://chromium-review.googlesource.com/1176398
Commit-Ready: Miao-chen Chou <mcchou@chromium.org>
Tested-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>

[modify] https://crrev.com/a7a2f0011bcc577c5a93904087e4e7ff7169d0ba/src/metrics.c
[modify] https://crrev.com/a7a2f0011bcc577c5a93904087e4e7ff7169d0ba/src/adapter.c
[modify] https://crrev.com/a7a2f0011bcc577c5a93904087e4e7ff7169d0ba/src/metrics.h

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/bluez/+/edf5e16ce5cb9cb1942480eea6ab2e4b9bf1c09b

commit edf5e16ce5cb9cb1942480eea6ab2e4b9bf1c09b
Author: Sonny Sasaka <sonnysasaka@chromium.org>
Date: Thu Aug 16 20:14:37 2018

CHROMIUM: Use uptime instead of wall clock for metrics timer

This applies for all current metrics.

BUG= chromium:874687 
TEST=rmmod btusb, sleep for minutes, resume, modprobe btusb. Check that
the metrics recorded is not minutes but seconds.
CQ-DEPEND=CL:1176398

Change-Id: I551753fca7e7ec9c933d0ab426b79324c7dd8ae5
Reviewed-on: https://chromium-review.googlesource.com/1176657
Commit-Ready: Miao-chen Chou <mcchou@chromium.org>
Tested-by: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>

[modify] https://crrev.com/edf5e16ce5cb9cb1942480eea6ab2e4b9bf1c09b/src/metrics.c

Another CL related to this bug: https://crrev.com/c/1176371
Labels: Merge-Request-68
Labels: -Merge-Request-68 Merge-Approved-68
Labels: Merge-Rejected-69
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 17

Labels: merge-merged-release-R68-10718.B-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/4dfef3905aba1a26badda760ec538540768b63e7

commit 4dfef3905aba1a26badda760ec538540768b63e7
Author: Miao-chen Chou <mcchou@chromium.org>
Date: Fri Aug 17 18:50:15 2018

CHROMIUM: Bluetooth: change the log level of Intel Hw error

Changed the log level of Intel hardware error to warning so we can get
the statistics via crash report.

BUG= chromium:874687 
TEST=build

Change-Id: Ic8c1865500e305f7ab7e7a15ce41bcec3eefa726
Signed-off-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1180183
Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>

[modify] https://crrev.com/4dfef3905aba1a26badda760ec538540768b63e7/net/bluetooth/hci_event.c

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 17

Labels: merge-merged-release-R69-10895.B-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/05cd1f3ee050416d27eeefb938e087b4cb742961

commit 05cd1f3ee050416d27eeefb938e087b4cb742961
Author: Miao-chen Chou <mcchou@chromium.org>
Date: Fri Aug 17 18:50:17 2018

CHROMIUM: Bluetooth: change the log level of Intel Hw error

Changed the log level of Intel hardware error to warning so we can get
the statistics via crash report.

BUG= chromium:874687 
TEST=build

Change-Id: Ic8c1865500e305f7ab7e7a15ce41bcec3eefa726
Signed-off-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1180184
Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>

[modify] https://crrev.com/05cd1f3ee050416d27eeefb938e087b4cb742961/net/bluetooth/hci_event.c

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 17

Labels: merge-merged-release-R69-10895.B-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/7c4ed3d198470225c711f117d04b38f884e50978

commit 7c4ed3d198470225c711f117d04b38f884e50978
Author: Miao-chen Chou <mcchou@chromium.org>
Date: Fri Aug 17 18:50:18 2018

CHROMIUM: Bluetooth: change the log level of Intel Hw error

Changed the log level of Intel hardware error to warning so we can get
the statistics via crash report.

BUG= chromium:874687 
TEST=build

Change-Id: Ic8c1865500e305f7ab7e7a15ce41bcec3eefa726
Signed-off-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1180186
Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>

[modify] https://crrev.com/7c4ed3d198470225c711f117d04b38f884e50978/net/bluetooth/hci_event.c

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 17

Labels: merge-merged-release-R68-10718.B-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/f0bafd65338db55909c62789745ffc1bd89e98c4

commit f0bafd65338db55909c62789745ffc1bd89e98c4
Author: Miao-chen Chou <mcchou@chromium.org>
Date: Fri Aug 17 18:50:20 2018

CHROMIUM: Bluetooth: change the log level of Intel Hw error

Changed the log level of Intel hardware error to warning so we can get
the statistics via crash report.

BUG= chromium:874687 
TEST=build

Change-Id: Ic8c1865500e305f7ab7e7a15ce41bcec3eefa726
Signed-off-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1180185
Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>

[modify] https://crrev.com/f0bafd65338db55909c62789745ffc1bd89e98c4/net/bluetooth/hci_event.c

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 17

Labels: merge-merged-release-R68-10718.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/bluez/+/882cc6c90dde1154b3ead2994837cfa0f3f041bd

commit 882cc6c90dde1154b3ead2994837cfa0f3f041bd
Author: Sonny Sasaka <sonnysasaka@chromium.org>
Date: Fri Aug 17 18:58:09 2018

CHROMIUM: Use uptime instead of wall clock for metrics timer

This applies for all current metrics.

BUG= chromium:874687 
TEST=rmmod btusb, sleep for minutes, resume, modprobe btusb. Check that
the metrics recorded is not minutes but seconds.
CQ-DEPEND=CL:1176398

Change-Id: I551753fca7e7ec9c933d0ab426b79324c7dd8ae5
Reviewed-on: https://chromium-review.googlesource.com/1176657
Commit-Ready: Miao-chen Chou <mcchou@chromium.org>
Tested-by: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
(cherry picked from commit edf5e16ce5cb9cb1942480eea6ab2e4b9bf1c09b)
Reviewed-on: https://chromium-review.googlesource.com/1180221
Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
Commit-Queue: Miao-chen Chou <mcchou@chromium.org>
Tested-by: Miao-chen Chou <mcchou@chromium.org>

[modify] https://crrev.com/882cc6c90dde1154b3ead2994837cfa0f3f041bd/src/metrics.c

Project Member

Comment 13 by bugdroid1@chromium.org, Aug 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/bluez/+/cad142718ac21af9c0fd50f1ca1dad1b0b9a4448

commit cad142718ac21af9c0fd50f1ca1dad1b0b9a4448
Author: Miao-chen Chou <mcchou@chromium.org>
Date: Fri Aug 17 18:58:10 2018

CHROMIUM: Refine chip lost metrics

The current ChipLost metrics depends on the adapter flag,
discovery_suspended_by_system, to eliminate the chip lost caused by
system suspends/resumes. However, the flag is not set to true if the
adapter is not powered when the system is going to suspend, and this
will lead to false recording of ChipLost sample when the system comes
back up.

This patch introduces a separate flag, system_suspended, to avoid the
false recording and send the chip lost sample to the refined metrics,
BlueZ.ChipLost2.

The corresponding UMA histogram CL is at https://crrev.com/c/1176371.

BUG= chromium:874687 
TEST=(1) Turn off Bluetooth and suspend the system without seeing
         a sample of BlueZ.ChipLost2 emitted in chrome://histograms page.
     (2) Turn off Bluetooth, remove btusb module and see a sample of
         BlueZ.ChipLost2 gets emitted.
CQ-DEPEND=CL:1176657

Change-Id: Ib1ce84724992931dd4c3c3917978fdaf87d5bada
Reviewed-on: https://chromium-review.googlesource.com/1176398
Commit-Ready: Miao-chen Chou <mcchou@chromium.org>
Tested-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
(cherry picked from commit a7a2f0011bcc577c5a93904087e4e7ff7169d0ba)
Reviewed-on: https://chromium-review.googlesource.com/1180223
Commit-Queue: Miao-chen Chou <mcchou@chromium.org>

[modify] https://crrev.com/cad142718ac21af9c0fd50f1ca1dad1b0b9a4448/src/metrics.c
[modify] https://crrev.com/cad142718ac21af9c0fd50f1ca1dad1b0b9a4448/src/adapter.c
[modify] https://crrev.com/cad142718ac21af9c0fd50f1ca1dad1b0b9a4448/src/metrics.h

Project Member

Comment 14 by bugdroid1@chromium.org, Aug 17

Labels: merge-merged-release-R69-10895.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/bluez/+/fd8d19abd1d11db5649c0a18e570c545a34a5cde

commit fd8d19abd1d11db5649c0a18e570c545a34a5cde
Author: Miao-chen Chou <mcchou@chromium.org>
Date: Fri Aug 17 18:58:10 2018

CHROMIUM: Refine chip lost metrics

The current ChipLost metrics depends on the adapter flag,
discovery_suspended_by_system, to eliminate the chip lost caused by
system suspends/resumes. However, the flag is not set to true if the
adapter is not powered when the system is going to suspend, and this
will lead to false recording of ChipLost sample when the system comes
back up.

This patch introduces a separate flag, system_suspended, to avoid the
false recording and send the chip lost sample to the refined metrics,
BlueZ.ChipLost2.

The corresponding UMA histogram CL is at https://crrev.com/c/1176371.

BUG= chromium:874687 
TEST=(1) Turn off Bluetooth and suspend the system without seeing
         a sample of BlueZ.ChipLost2 emitted in chrome://histograms page.
     (2) Turn off Bluetooth, remove btusb module and see a sample of
         BlueZ.ChipLost2 gets emitted.
CQ-DEPEND=CL:1176657

Change-Id: Ib1ce84724992931dd4c3c3917978fdaf87d5bada
Reviewed-on: https://chromium-review.googlesource.com/1176398
Commit-Ready: Miao-chen Chou <mcchou@chromium.org>
Tested-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
(cherry picked from commit a7a2f0011bcc577c5a93904087e4e7ff7169d0ba)
Reviewed-on: https://chromium-review.googlesource.com/1180224
Commit-Queue: Miao-chen Chou <mcchou@chromium.org>

[modify] https://crrev.com/fd8d19abd1d11db5649c0a18e570c545a34a5cde/src/metrics.c
[modify] https://crrev.com/fd8d19abd1d11db5649c0a18e570c545a34a5cde/src/adapter.c
[modify] https://crrev.com/fd8d19abd1d11db5649c0a18e570c545a34a5cde/src/metrics.h

Project Member

Comment 15 by bugdroid1@chromium.org, Aug 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/bluez/+/ae6b7850b87916f64993acdb4f38f71e1731a579

commit ae6b7850b87916f64993acdb4f38f71e1731a579
Author: Sonny Sasaka <sonnysasaka@chromium.org>
Date: Fri Aug 17 18:58:11 2018

CHROMIUM: Use uptime instead of wall clock for metrics timer

This applies for all current metrics.

BUG= chromium:874687 
TEST=rmmod btusb, sleep for minutes, resume, modprobe btusb. Check that
the metrics recorded is not minutes but seconds.
CQ-DEPEND=CL:1176398

Change-Id: I551753fca7e7ec9c933d0ab426b79324c7dd8ae5
Reviewed-on: https://chromium-review.googlesource.com/1176657
Commit-Ready: Miao-chen Chou <mcchou@chromium.org>
Tested-by: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
(cherry picked from commit edf5e16ce5cb9cb1942480eea6ab2e4b9bf1c09b)
Reviewed-on: https://chromium-review.googlesource.com/1180222
Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
Commit-Queue: Miao-chen Chou <mcchou@chromium.org>
Tested-by: Miao-chen Chou <mcchou@chromium.org>

[modify] https://crrev.com/ae6b7850b87916f64993acdb4f38f71e1731a579/src/metrics.c

Labels: -Merge-Rejected-69 Merge-Request-69
Project Member

Comment 17 by sheriffbot@chromium.org, Aug 17

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -ReleaseBlock-Stable
Since this is merged into 68, dropping RBS. 
Project Member

Comment 19 by sheriffbot@chromium.org, Aug 20

Cc: bhthompson@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 20 by bugdroid1@chromium.org, Aug 23

Labels: merge-merged-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/a79819c082693001b9a31798e57001e691785381

commit a79819c082693001b9a31798e57001e691785381
Author: Miao-chen Chou <mcchou@chromium.org>
Date: Thu Aug 23 14:37:04 2018

CHROMIUM: Bluetooth: change the log level of Intel Hw error

Changed the log level of Intel hardware error to warning so we can get
the statistics via crash report.

BUG= chromium:874687 
TEST=build
CQ-DEPEND=CL:1179340

Change-Id: Ic8c1865500e305f7ab7e7a15ce41bcec3eefa726
Signed-off-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1180241
Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>

[modify] https://crrev.com/a79819c082693001b9a31798e57001e691785381/net/bluetooth/hci_event.c

Project Member

Comment 21 by bugdroid1@chromium.org, Aug 24

Labels: merge-merged-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/9f2c000df175c46000bddf69ef5f08733e4b48de

commit 9f2c000df175c46000bddf69ef5f08733e4b48de
Author: Miao-chen Chou <mcchou@chromium.org>
Date: Fri Aug 24 06:27:52 2018

CHROMIUM: Bluetooth: change the log level of Intel Hw error

Changed the log level of Intel hardware error to warning so we can get
the statistics via crash report.

BUG= chromium:874687 
TEST=build

Change-Id: Ic8c1865500e305f7ab7e7a15ce41bcec3eefa726
Signed-off-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1178478
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Sameer Nanda <snanda@chromium.org>
Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>

[modify] https://crrev.com/9f2c000df175c46000bddf69ef5f08733e4b48de/net/bluetooth/hci_event.c

Project Member

Comment 22 by sheriffbot@chromium.org, Aug 24

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 23 by sheriffbot@chromium.org, Oct 8

Labels: -Merge-Approved-68
This issue hasn't been updated in the last 6 weeks, so removing its merge approval label. Please re-request a merge if needed.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: shijinabraham@chromium.org
Owner: snanda@chromium.org
Status: Assigned (was: Untriaged)
Can we close this? It's an open P0 - but appears to be merged everywhere necessary.
Cc: adlr@chromium.org
Owner: sonnysasaka@chromium.org
Status: Fixed (was: Assigned)
I believe it can be closed.

Sonny, feel free to reopen and adjust priority as needed.

Sign in to add a comment