834551 - [Bluetooth] Add a debug flag for runtime switching on/off logging

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Eric Chou [:ericchou] [:echou]]

Recently we've got many Bluetooth bugs reported by users who use release build. They usually would try to provide information for us as much as possible. However, we can hardly figure out what the root cause is from adb logcat because we didn't leave enough debugging information in our code. On the other hand, if not necessary, we prefer not printing out logs too often. Otherwise Bluetooth logs may occupy the whole logcat.

So I added a debug flag which could be switched by modifying a key of mozSettings "bluetooth.debugging.enabled" and added a BT_LOG() macro which would check the debug flag to decide if the log should be printed out. In this case, with adding a checkbox in Gaia (settings/Device Information/More information/Developer), we will be able to switch the runtime logging on/off easily.

Comment 1

[Eric Chou [:ericchou] [:echou]]

Attachment: patch 1: v1: add Bluetooth debug flag and BT_LOG() macro

Comment 2

[Eric Chou [:ericchou] [:echou]]

Attachment: patch 1: v1: add Bluetooth debug flag and BT_LOG() macro

* fix a typo

Comment 3

[Eric Chou [:ericchou] [:echou]]

Attachment: patch 2: v1: Replace 'LOG()' with 'BT_WARNING()'

* Replace 'LOG()' with 'BT_WARNING()' defined in BluetoothCommon.h for consistency

Comment 4

[Eric Chou [:ericchou] [:echou]]

Attachment: patch 3: v1: Inserted logs at some important check points

Comment 5

[Eric Chou [:ericchou] [:echou]]

Attachment: patch 3: v2: Inserted logs at some important check points

* removed code for test

Comment 6

[Eric Chou [:ericchou] [:echou]]

Attachment: patch 3: v3: Inserted logs at some important check points

* added more check points according to Gina's suggestion

Comment 7

[Gina Yeh [:gyeh] [:ginayeh]]

Comment on attachment 706997 [details] [diff] [review]
patch 3: v3: Inserted logs at some important check points

Review of attachment 706997 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, r=me.

Comment 8

[Eric Chou [:ericchou] [:echou]]

try: https://tbpl.mozilla.org/?tree=Try&rev=673a45df69b3

Comment 9

[Eric Chou [:ericchou] [:echou]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e2108ea96ae5
https://hg.mozilla.org/integration/mozilla-inbound/rev/f7ead00b7654
https://hg.mozilla.org/integration/mozilla-inbound/rev/55ab9f026dc4

Comment 10

[Ed Morley [:emorley]]

Backed out for compile failures:
https://hg.mozilla.org/integration/mozilla-inbound/rev/436d46763d62

Comment 11

[Eric Chou [:ericchou] [:echou]]

oops, sorry, my bad.

Comment 12

[Eric Chou [:ericchou] [:echou]]

try again:

https://tbpl.mozilla.org/?tree=Try&rev=0318e79e4df7
https://tbpl.mozilla.org/?tree=Try&rev=0c90708979bb

Comment 13

[Eric Chou [:ericchou] [:echou]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/901abfff0d34
https://hg.mozilla.org/integration/mozilla-inbound/rev/07fb7c433d4a
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e652017d2f0

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/901abfff0d34
https://hg.mozilla.org/mozilla-central/rev/07fb7c433d4a
https://hg.mozilla.org/mozilla-central/rev/7e652017d2f0

Comment 15

[Eric Chou [:ericchou] [:echou]]

We usually got issues which are reported from partners without useful log. That's because we didn't print any log for bluetooth in release mode. This bug resolved the problem of lack of debugging info. Since it should not be a blocker of any projects, nominate as tracking-b2g18+ to see if we could have this landed on b2g18.

Comment 16

[Eric Chou [:ericchou] [:echou]]

Comment on attachment 706216 [details] [diff] [review]
patch 1: v1: add Bluetooth debug flag and BT_LOG() macro

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): no
User impact if declined: Lack of debugging info of Bluetooth
Testing completed: mozilla-central
Risk to taking this patch (and alternatives if risky): no
String or UUID changes made by this patch: no

Please notice that it may not be able to merged into b2g18 directly, so I'll provide b2g18-specific patch once we get the approval.

Comment 17

[Eric Chou [:ericchou] [:echou]]

Comment on attachment 706223 [details] [diff] [review]
patch 2: v1: Replace 'LOG()' with 'BT_WARNING()'

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): no
User impact if declined: Lack of debugging info of Bluetooth
Testing completed: mozilla-central
Risk to taking this patch (and alternatives if risky): no
String or UUID changes made by this patch: no

Please notice that it may not be able to merged into b2g18 directly, so I'll provide b2g18-specific patch once we get the approval.

Comment 18

[Eric Chou [:ericchou] [:echou]]

Comment on attachment 706997 [details] [diff] [review]
patch 3: v3: Inserted logs at some important check points

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): no
User impact if declined: Lack of debugging info of Bluetooth
Testing completed: mozilla-central
Risk to taking this patch (and alternatives if risky): no
String or UUID changes made by this patch: no

Please notice that it may not be able to merged into b2g18 directly, so I'll provide b2g18-specific patch once we get the approval.

Comment 19

[Lukas Blakk [:lsblakk] use ?needinfo]

So is the plan here to ask partners to enable this flag in their builds?  It's not going to be on by default for any release builds, right? Will someone be doing outreach with partners to let them know about this and to get them to enable it?

Comment 20

[Eric Chou [:ericchou] [:echou]]

(In reply to [e-mail] from comment #19)
> So is the plan here to ask partners to enable this flag in their builds? 
> It's not going to be on by default for any release builds, right? 
> Will someone be doing outreach with partners to let them know about this
> and to get them to enable it?

The original idea came after bug 806611, we want to have the same strategy as WiFi. I don't think we should enable this flag by default for release builds. As for partners' builds, I'll talk to PM team, making sure our partners will test Bluetooth with this flag enabled.

Comment 21

[Eric Chou [:ericchou] [:echou]]

ni? Joe Cheng, PM of Firefox OS

Hi Joe,

Could you help with telling our partners about the change after this lands on b2g18? Thanks.

Comment 22

[Joe Cheng [:jcheng] (please needinfo)]

sure, added other OEM facing PMs to this bug to be aware of this.

Comment 23

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 706216 [details] [diff] [review]
patch 1: v1: add Bluetooth debug flag and BT_LOG() macro

Approving since this has been on m-c for a long time and will be useful to partners' builds.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/0b3e103b7b52
https://hg.mozilla.org/releases/mozilla-b2g18/rev/671735ea0363
https://hg.mozilla.org/releases/mozilla-b2g18/rev/8ca8149ce236