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

Epic: Debug Log File Redesign #367

Closed
3 tasks done
isaacs opened this issue Jun 11, 2021 · 2 comments
Closed
3 tasks done

Epic: Debug Log File Redesign #367

isaacs opened this issue Jun 11, 2021 · 2 comments
Assignees
Labels

Comments

@isaacs
Copy link

isaacs commented Jun 11, 2021

Make our handling of debug logs more efficient.

Related to: #366

  • A new module listens on all process log events.
  • Write all log messages to an append-only file in a known location, with a filename based on the ISO time stamp.
  • At the end of the npm cli process, remove any log files in the log folder older than the npm.config.get('logs-max') most recent.

This should not be a part of npmlog. In fact, it can be just a pretty short module in the npm lib/utils folder, once everything is using proc-log instead of calling npmlog directly.

@isaacs isaacs added the Epic label Jun 11, 2021
@isaacs isaacs mentioned this issue Jun 11, 2021
9 tasks
@wraithgar
Copy link
Member

From a slack discussion today:

for everything that's not arborist moving to proc log today is all benefit w/ no drawback. We could also consolidate onto proclog in everything except for arborist today for the same reasons. The last mile is of course the hard one cause we have to solve our progress bar/spinner issue.

@darcyclarke darcyclarke added this to the OSS - Sprint 34 milestone Jul 26, 2021
@darcyclarke darcyclarke modified the milestones: OSS - Sprint 34, OSS - Sprint 35 Aug 9, 2021
@settings settings bot removed the Epic label Aug 31, 2021
@darcyclarke darcyclarke changed the title npmlog is dead, long live npmlog! Debug Log File Redesign Epic Epic: Debug Log File Redesign Nov 2, 2021
@darcyclarke
Copy link
Contributor

@lukekarrys can we close this issue out?

lukekarrys added a commit to npm/cli that referenced this issue Nov 30, 2021
This decouples the log file writing from the terminal logging. We now
open an append only file at the start of the process and stream logs to
it. We still only display the log file message in timing mode or if
there is an error, but the file is still written regardless.

All logging now goes through `proc-log` and his is the first step to
removing `npmlog`. For now `npmlog` is still used for the terminal
logging but with a shim in front of it to make it easier to test and
use in conjunction with `proc-log`. Ref: npm/statusboard#366

This also refactors many of the tests to always use an explicit
`t.testdir` for their cache since the file is opened on each `new Npm()`.
Tests are also refactored to use more of `MockNpm` with behavior to
add config items and load `npm` if necessary. A new fixture `mockGlobals`
was also added to make much of this for ergonomic. Ref: npm/statusboard#410

Closes npm/statusboard#411
Closes npm/statusboard#367
lukekarrys added a commit to npm/cli that referenced this issue Dec 1, 2021
This decouples the log file writing from the terminal logging. We now
open an append only file at the start of the process and stream logs to
it. We still only display the log file message in timing mode or if
there is an error, but the file is still written regardless.

All logging now goes through `proc-log` and this is the first step to
removing `npmlog`. For now `npmlog` is still used for the terminal
logging but with a shim in front of it to make it easier to test and
use in conjunction with `proc-log`. Ref: npm/statusboard#366

This also refactors many of the tests to always use an explicit
`t.testdir` for their cache since the file is opened on each `new Npm()`.
Tests are also refactored to use more of `MockNpm` with behavior to
add config items and load `npm` if necessary. A new fixture `mockGlobals`
was also added to make much of this more ergonomic. Ref: npm/statusboard#410

Closes npm/statusboard#411
Closes npm/statusboard#367
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants