Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement (optional) logging for all users #4828

Open
bengtan opened this issue Feb 24, 2020 · 8 comments
Open

Implement (optional) logging for all users #4828

bengtan opened this issue Feb 24, 2020 · 8 comments
Assignees

Comments

@bengtan
Copy link
Contributor

bengtan commented Feb 24, 2020

Some location uploading issues have been occurring on Prod recently. It's unknown whether they are the same as recent (solved) issues on Staging or are completely new. In order to find out, we need to enable and retrieve the debug log from Prod apps.

At the same time, it would be useful for some external/beta users to send us debug logs. To facilitate this, we should implement a nicer, more stream-lined interface for retrieving debug logs.

Here's a list of suggested changes:

  • Implement a flag in the user object (server side and client side) called debug (or enable_debug or something) which the app can set or clear.

    • On the server side, if this flag is enabled, the server should turn on all audit (traffic,push,location,geo-console) logging for that user.
  • On the client side, in the 'Edit Profile' screen, add a checkbox/toggle/slider control which allows the user to turn debugging on or off. For now, the control can be called 'Enable debugging'.

    • When the control is on, it sets the user's debug flag, enables the debug log, and exposes another button 'Send debug log'.
    • When 'Send debug log' is pressed, it uploads the debug log to the server side.
  • Implement a facility (ie. S3 bucket) at the server side where the client can upload debug logs. The client needs to be able to specify the filename of the uploaded file.

  • Implement an API for the client to ask the server for a URL which it can upload debug logs to. This could be a time-limited S3 url or any other sort of URL.

@bengtan
Copy link
Contributor Author

bengtan commented Mar 9, 2020

Change of plans.

Uploading directly to S3 turned out to be too much trouble. Let's just stick with getting RNBGL to email logs.

Here's the revised to do list:

  • Add a new debug section/screen that is available to all users. (This is separate from the existing debug screen that is accessed by long-pressing on the main/hamburger menu.)

I suggest that this section/screen be accessed from the 'Edit Profile' screen. It can be a separate section that appears near the bottom of the existing 'Edit Profile' form or there can be a new link ('Debug options') to leads to the new screen.

I suggest keeping this new debug section/screen separate from the existing one because the existing one isn't really suitable for public use. Well ... unless we modify the existing one extensively. Open for discussion.

  • In the new debug section/screen, add a new slider/checkbox/control 'Enable logging'.
    • When the control is enabled:
      • Increase RNBGL logging level to Verbose.
      • Call mutation userFullAudit(true)
    • When the control is disabled:
      • Decrease RNBGL logging level to Off.
      • Call mutation userFullAudit(false)

There is a graphql user field fullAudit which can be used to query the result of the mutation userFullAudit(). This field can only be modified by calling userFullAudit(). It might be a suitable proxy for the state of the control.

  • When 'Enable logging' is on, then show another button 'Email log'.

When this button is pressed, invoke LocationStore.emailLog('[email protected]')

@bengtan
Copy link
Contributor Author

bengtan commented Mar 9, 2020

@irfirl:

Do you want to create mock-ups for this ticket? (UI is open for discussion.)

If not, the developer will just proceed as they see best.

@southerneer
Copy link
Contributor

southerneer commented Mar 9, 2020

If the user turns verbose logging on do we just assume that it's on in perpetuity until they turn it off? Or do we turn it off when they logout? Or is the setting limited by device+user? I think ideally it would be unique for device+user, but I'm jumping in a little late on the conversation so maybe that die is already cast.

@irfirl
Copy link
Contributor

irfirl commented Mar 9, 2020

I think it should look something like this?
Add a bit of margin in between the text buttons on the bottom and add "Debug Options":
https://zpl.io/b6ERLmR
After tapping into it with logging off:
https://zpl.io/bepdQJz
Logging enabled:
https://zpl.io/a7Nz18p

@bengtan
Copy link
Contributor Author

bengtan commented Mar 10, 2020

If the user turns verbose logging on do we just assume that it's on in perpetuity until they turn it off?

It can be permanently on Verbose because RNBGL limits the log to the last 3 days (by default).

Or do we turn it off when they logout?

How about this ...

  • On every app startup or user login, it should check the status of the fullAudit flag and set loglevel accordingly.
  • On user logout, don't alter the loglevel.

The implication here is that, once the loglevel has been set to Verbose, the only way to set it to Off is to

  • Log in and turn it off (which sets fullAudit to false),
  • Log in as a user which has fullAudit=false.
  • Uninstall and reinstall

This is not ideal but is a pragmatic tradeoff.

Or is the setting limited by device+user? I think ideally it would be unique for device+user, but I'm jumping in a little late on the conversation so maybe that die is already cast.

It's complicated.

The server side logging is per-user.

The client side logging is per-device.

But I'd like to make it just one slider/control that combines the two loggings instead of have two controls (one each for server side and client side).

So it's sort-of per-device+user but the abstraction is leaky in some corner cases ... and I'm hoping we don't have to care about those corner cases.


Alternatively, I'm open to ideas (... such as making it two sliders/controls).

@southerneer
Copy link
Contributor

I guess if this is persisted on the backend then the setting will survive uninstall/reinstall.

@bengtan bengtan removed the Epic label Mar 11, 2020
@bengtan
Copy link
Contributor Author

bengtan commented Mar 11, 2020

I guess if this is persisted on the backend then the setting will survive uninstall/reinstall.

The fullAudit setting is, yes.

fullAudit instructs the server side to log that user's location,traffic, and push notifications to loggly.

@bengtan bengtan self-assigned this Mar 16, 2020
@bengtan bengtan changed the title Improve debug log capability Implement optional debug log for all users Mar 16, 2020
@bengtan bengtan changed the title Implement optional debug log for all users Implement (optional) logging for all users Mar 17, 2020
@bengtan
Copy link
Contributor Author

bengtan commented Mar 18, 2020

QA Notes:

The 'Email Log' button has been moved to the new Debug Options screen.

The destination email address is crashreports@hippware...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants