1136294 - (ioslogging) Integrate logging library, expose logs to user, and allow for sharing logs

Status: RESOLVED FIXED

(Firefox for iOS :: Build & Test, defect)

Description

[Nick Alexander :nalexander [he/him]]

One of the early decisions rnewman made that has really paid of for the Android Background Services team is to log everything (when desired).  We are careful to cache log levels so we don't destroy performance, but in general we log enough to re-create most flows in their entirety.  We've never exposed those logs in a more friendly way than by using |adb logcat| and filters, so we haven't got the full benefit.  I think this was a mistake: we should have made it easier to view and share logs for subsystems without needing |adb logcat|.

I would like to:
* integrate and use a logging library;
* expose logs to the user (perhaps only for some builds, or when some settings are toggled);
* and allow for sharing of logs.

The consensus favourite logging library for Swift appears to be https://github.com/DaveWoodCom/XCGLogger but I could be swayed.

Comment 1

[Nick Alexander :nalexander [he/him]]

Some considerations:

* we need to be very careful not to expose Personally Identifying Information in logs;
* we need to rotate logs, so that we don't consume too much device space.

Comment 2

[Nick Alexander :nalexander [he/him]]

rnewman, stefan, wesj, bnicholson, mfinkle: if you have an opinion, please chime in.  I want this for the FxA sign-in/sign-up flow and am volunteering to do something quick for my own purposes.

Comment 3

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

That library looks good. If we log to ~/Documents/Logs/ then it is simply to copy logs off the device with iTunes or Xcode.

My only wish is that the library also supports logging to the console, visible in the Xcode console pane when running the app.

Comment 4

[Brian Nicholson (:bnicholson)]

I'm glad you're taking this...adding a proper logging library has been on my backlog for awhile. +1 from me.

Comment 5

[Brian Nicholson (:bnicholson)]

Clearing info for [e-mail]. Wrong stefan ;)

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

+1 from me

Comment 7

[Wesley Johnston (:wesj)]

Looks good to me too. Lets try!

Comment 8

[James Hugman [:jhugman] [@jhugman]]

https://github.com/mozilla/firefox-ios/pull/192

Comment 9

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Attachment: PR: https://github.com/mozilla/firefox-ios/pull/192

Comment 10

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

(In reply to James Hugman [:jhugman] [@jhugman] from comment #8)
> https://github.com/mozilla/firefox-ios/pull/192

James, please add PRs as a patch to the bug and then set one or two reviewers. I've done that already for this one.

Just one quick comment that I added on Github and then we can merge this.

Comment 11

[Brian Nicholson (:bnicholson)]

Attachment: Pull request

Updating to point to the new PR.

Comment 12

[Richard Newman [:rnewman]]

Looks like a good starting point.

Comment 13

[James Hugman [:jhugman] [@jhugman]]

https://github.com/mozilla/firefox-ios/pull/192 Now has something which appears to be merging now. st3fan Please could you have a look?

Comment 14

[James Hugman [:jhugman] [@jhugman]]

(In reply to James Hugman [:jhugman] [@jhugman] from comment #13)
> https://github.com/mozilla/firefox-ios/pull/192 Now has something which
> appears to be merging now. st3fan Please could you have a look?

https://github.com/mozilla/firefox-ios/pull/201

Comment 15

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Merged this in.

Comment 16

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Comment on attachment 8574117 [details] [review]
Pull request

Thought I already +d this since I merged this patch yesterday.

Comment 17

[Brian Nicholson (:bnicholson)]

Comment on attachment 8574117 [details] [review]
Pull request

We never removed the explicit version from Cartfile, so I guess we can address that in a follow-up.