Skip to content

[feat] Custom logging location #4466

Closed

Description

@EvanCarroll

Before Opening Please...

  • Search for an existing/duplicate RRFC which might be relevant to your RRFC

There is one RFC about logging: using STDERR for errors instead of STDOUT. This is unrelated.

Motivation ("The Why")

Currently some CI providers, specifically GitLab, restrict job artifacts to being relative to the repository. Presumably, this stops you from uploading runner files that may have dirty-state left from previous runs in non-containerized workflows. Specifically the GitLab documents say,

The paths keyword determines which files to add to the job artifacts. All paths to files and directories are relative to the repository where the job was created.

This effectively means NPM logs on error can not be captured as job artifacts.

Example

How

Current Behaviour

What I would like to do is something like this (ignore the actual error)

$ npx ng e2e
npm ERR! could not determine executable to run
npm ERR! A complete log of this run can be found in:
npm ERR!     /root/.npm/_logs/2021-11-22T17_24_50_085Z-debug.lo

Then I want to upload /root/.npm/_logs/2021-11-22T17_24_50_085Z-debug.log as a job artifact.

This is not currently possible

Desired Behaviour

In light of GitLab not permitting global files to be uploaded as job artifacts I would like an option to set the log file,

npx [--log <LOCATION>] ng e2e

Such that LOCATION could be relative to the cwd.

Then I could do,

$ npx --log "./.npm/_logs/" ng e2e

And set ./.npm/_logs/* as my artifacts to upload to GitLab.

References

Activity

  1. ljharb commented on Dec 2, 2021

    @ljharb
    Contributor

    What difficulties are there with copying the log to the desired location prior to creating the artifact?

  2. EvanCarroll commented on Dec 2, 2021

    @EvanCarroll
    Author

    Does the RFC process require proving difficulty, or mere inconvenience and improvement?

    Nothing is difficult or impossible with an open operating system.

  3. ljharb commented on Dec 2, 2021

    @ljharb
    Contributor

    I don't know about a requirement, but any addition to anything should be weighed against the alternatives without it. In other words, every convenience/improvement for one group is an inconvenience/detriment for another, so subjective decisions always need to be made.

    If the delta between:

    npm foo --log=$logdir/$filename bar
    # or
    NPM_CONFIG_LOG=$logdir/$filenamenpm foo bar

    and

    npm foo bar
    cp $HOME/.npm/_logs $logdir

    doesn't feel like it outweighs the downside of the added complexity to npm, then that would be a very important consideration for the RFC.

  4. whschultz commented on Dec 10, 2021

    @whschultz

    If we have multiple npm commands running at the same time, being able to specify a custom log location for each one individually would help ensure that we know which log goes with which command.

  5. lukekarrys commented on Dec 15, 2021

    @lukekarrys
    Contributor

    I would be in favor of this. I would use it in the cases described above but also when debugging as I often do npm <COMMAND> --cache=. when all I really want is to have the logs in a separate place to compare across runs without messing with any of my "real" logs.

    Some questions I have about expected behavior @EvanCarroll:

    • We currently clean out the logs directory with the logs-max config option (which defaults to 10). If supplying your own logdir would you expect that to ever be cleaned out for you? I would lean towards no.
    • npm expects to own whatever cache directory it uses (which is where logs are currently stored). So something like this is run when creating a logfile:
    const dir = path.dirname(logfile)
    mkdirpInferOwner.sync(dir)
    fs.openSync(logfile, 'a')
    const st = fs.lstatSync(dir)
    fs.chownSync(dir, st.uid, st.gid)
    fs.chownSync(file, st.uid, st.gid)

    If supplying your own logdir, would you expect anything like that to happen? Or to error if npm can't access the directory? I would lean towards not doing anything special to the directory or logfile and erroring if npm can't write to it.

  6. lukekarrys commented on Dec 15, 2021

    @lukekarrys
    Contributor

    If we have multiple npm commands running at the same time, being able to specify a custom log location for each one individually would help ensure that we know which log goes with which command.

    I agree with this use case being another beneficial reason to add this option, but I don't think the filename should be made configurable, only the directory. npm has some logic to prevent runaway logfile writing in the cases of infinite loops that involves changing the log file name, so passing a complete filename wouldn't actually give you the file being written.

  7. EvanCarroll commented on Dec 29, 2021

    @EvanCarroll
    Author

    We currently clean out the logs directory with the logs-max config option (which defaults to 10). If supplying your own logdir would you expect that to ever be cleaned out for you? I would lean towards no.

    I have no strong preference on this as ultimately I'm just logging for the CI run. However I would treat them the same only so in the future this may be totally configurable in some future .npmrc where you can specify your log directory, and other options like rotation/cleanup. In the traditional Linux FHS scheme, it seems likely that some users will want these logs on different partitions like /var/log or following something like XDG's $XDG_STATE_HOME. Perhaps this is something to note npm/rfcs#427

    If supplying your own logdir, would you expect anything like that to happen? Or to error if npm can't access the directory? I would lean towards not doing anything special to the directory or logfile and erroring if npm can't write to it.

    I would not chown things I didn't create. I would error if I couldn't access them or if I had reason to believe they were egregiously misconfigured or otherwise likely to cause a security problem (this is what SSH does if you chmod your private key improperly ~/.ssh/id_rsa).

  8. lukekarrys commented on Jan 12, 2022

    @lukekarrys
    Contributor

    Thanks @EvanCarroll, I agree with both those points.

    I can make an actual RFC for this based on the current discussion if it gets accepted, but I'm a +1 on it. To summarize the current implementation being discussed:

    • Add a config item that will write log files to that directory instead of the default <NPM_CACHE>/_logs
    • Setting this config will not change the behavior of the logs-max behavior or rotating of logfiles
    • Setting this config will not create or chown the directory but instead error if its not accessible or doesnt exist
  9. self-assigned this
    on Jan 20, 2022
  10. transferred this issue fromnpm/rfcson Feb 23, 2022
  11. changed the title [-][RRFC] Custom logging location[/-] [+][feat] Custom logging location[/+] on Feb 23, 2022
  12. lukekarrys commented on Feb 23, 2022

    @lukekarrys
    Contributor

    This ended up not needing an explicit RFC so I transferred to the cli repo as an enhancement

  13. added 4 commits that reference this issue on Mar 22, 2022
    4a6a890
    152b941
    6d49f56
    42d2b62
  14. added a commit that references this issue on Mar 24, 2022
    cc6c094
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Enhancementnew feature or improvement

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions