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

Logging service #200

Open
3 of 4 tasks
mhluongo opened this issue Sep 3, 2021 · 9 comments
Open
3 of 4 tasks

Logging service #200

mhluongo opened this issue Sep 3, 2021 · 9 comments
Labels
Priority: Low Low Monetary / Growth impact, not time-sensitive. Status: Pending Needs further attention and categorization Type: Plumbing Internal product improvements

Comments

@mhluongo
Copy link
Contributor

mhluongo commented Sep 3, 2021

We need a single service that...

  • Wraps basic logging
  • Persists logs for export
  • Can disable console printing for production builds
  • Can act as a single point for future opt-in telemetry
@mhluongo mhluongo added the Type: Plumbing Internal product improvements label Sep 3, 2021
@mhluongo mhluongo self-assigned this Sep 3, 2021
@itsrachelfish
Copy link
Contributor

@mhluongo Since we are already using IndexDB for storing account and transaction information, I think it would make sense to persist log files in IndexDB as well. Any objections?

@mhluongo
Copy link
Contributor Author

mhluongo commented Sep 8, 2021

None, but let me open the "pre-issue" to this.. I have a feeling you'll want to do this in two pieces.

EDIT:

Here it is

#210

@Shadowfiend
Copy link
Contributor

Want to flag a concern that persistent rather than ephemeral logs may require much more careful usage of logging functions to avoid increasing the danger if the extension storage is compromised somehow.

@mhluongo mhluongo mentioned this issue Oct 7, 2021
@greg-nagy
Copy link
Contributor

greg-nagy commented Oct 13, 2021

I think I would go with session storage usage because logs can accumulate to terrible sizes and we are not really interested in logs from ages ago

I was not sure if it's available from the background script but it seems so based on the mdn js api list for bg scripts
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API

@0xDaedalus 0xDaedalus added Status: Pending Needs further attention and categorization Priority: Low Low Monetary / Growth impact, not time-sensitive. labels Feb 10, 2023
@0xDaedalus
Copy link
Contributor

We have pieces of this in logger.ts as well as in analytics service now. The only thing completely missing is Can disable console printing for production builds which could be a nice thing to have but might make troubleshooting user issues more difficult if we don't do it carefully.

@beemeeupnow
Copy link
Contributor

Would it be possible to have a toggle in the settings page for console logs?

If eventually disabling them by default is still a goal that would make the change extremely simple when the time comes.

@hyphenized
Copy link
Collaborator

Would it be possible to have a toggle in the settings page for console logs?

If eventually disabling them by default is still a goal that would make the change extremely simple when the time comes.

True but I'm not sure we'd want to allow disabling them completely. It's much simpler for a user who just ran into an issue to go to settings and export logs, rather than enable logs and try to reproduce the issue again.

That said, logging definitely needs some work. There seems to be a lot of unnecessary noise so the log level for certain things might not be entirely accurate 🤔

@Shadowfiend
Copy link
Contributor

To be clear, that bullet refers to not calling console.* in production, not disabling logging for export purposes. So folks would still be able to export logs, but if eg there's an extension observing logs in dev tools it wouldn't see production logs. Mostly just a way to reduce attack surface.

@hyphenized
Copy link
Collaborator

Ah, makes sense, thanks for making that clear :shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Low Low Monetary / Growth impact, not time-sensitive. Status: Pending Needs further attention and categorization Type: Plumbing Internal product improvements
Projects
None yet
Development

No branches or pull requests

7 participants