-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Reduce logging in production builds #20694
Conversation
21c7a20
to
40533c5
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #20694 +/- ##
===========================================
- Coverage 68.82% 68.82% -0.01%
===========================================
Files 997 997
Lines 38931 38934 +3
Branches 10447 10454 +7
===========================================
Hits 26794 26794
- Misses 12137 12140 +3
☔ View full report in Codecov by Sentry. |
Builds ready [40533c5]
Page Load Metrics (1512 ± 27 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
67d7cfa
to
1645c41
Compare
Builds ready [1645c41]
Page Load Metrics (1590 ± 49 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Our logger middleware can be quite noisy in production, logging all RPC requests. It has been updated to have more condensed logs in production builds, but preserving the existing logging for development builds.
1645c41
to
e046338
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
Builds ready [e046338]
Page Load Metrics (1755 ± 51 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Explanation
Our logger middleware can be quite noisy in production, logging all RPC requests. It has been updated to have more condensed logs in production builds, but preserving the existing logging for development builds.
A few error logs in the method handling middleware have been suppressed in production as well. They were redundant anyway.
This fixes https://github.com/MetaMask/MetaMask-planning/issues/1281
Manual Testing Steps
yarn start
oryarn start:test
). But on non-development builds they have less detail (and there is no longer a separate log entry for errors).Pre-merge author checklist
Pre-merge reviewer checklist
If further QA is required (e.g. new feature, complex testing steps, large refactor), add the
Extension QA Board
label.In this case, a QA Engineer approval will be be required.