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

Add setting to allow for custom log path #1345

Closed
SimonWahlin opened this issue Jun 3, 2018 · 22 comments
Closed

Add setting to allow for custom log path #1345

SimonWahlin opened this issue Jun 3, 2018 · 22 comments
Assignees
Labels
Area-Configuration Issue-Enhancement A feature request (enhancement). Resolution-Fixed Will close automatically.

Comments

@SimonWahlin
Copy link

Summary of the new feature

Running VSCode as a non-administrator I don't have write-access to the extensions folder.
This causes the PowerShell extension to fail to load when trying to write to the logs subfolder of the PowerShell extension folder.

To be able to load the extension without write-access to the extensions directory I would like to configure the extension to either turn of logging completely or configure the path to the logs folder to a folder where the user has write access.

Proposed technical implementation details (optional)
Any or all of the following:

  • Add setting to make the logs path configurable.
  • Add setting(s) to disable logging and anything else that will cause writes to the extensions folder.
@rkeithhill
Copy link
Contributor

rkeithhill commented Jun 3, 2018

Running VSCode as a non-administrator I don't have write-access to the extensions folder.

What OS? I've only used the PowerShell extension as a standard user on both Windows and Ubuntu and never had any problems with the extension writing to the logs dir.

VSCode extensions are typically installed under the user's home dir where the user, running as a standard user, should have write perms. In fact, if the extension couldn't write to the logs dir, it probably couldn't write to the sessions dir which means the extension would never be able to attach to language/debug server.

@SimonWahlin
Copy link
Author

The problem occurs when using application whitelisting.

Putting the extensions folder where a user can write makes for complex white-listing scenarios. Putting the extensions folder where user can not write works for most extensions and allows for a simple path whitelisting.

I see, then the sessions folder will be a problem as well. Will the extension only store data in the session or does it need to execute code from there? Would it be possible to alsp have the sessionpath configurable?

@rkeithhill
Copy link
Contributor

Rather than adding multiple settings that require user configuration to make work in this scenario, perhaps we should write logs/session info to the user's appdata/local folder? If we did this, I'd also like to look at periodically deleting old log folders. Right now, we create a new log folder for every new session and pretty much the only way those get cleaned up now is when you update to a newer version of the extension (and the old version is removed).

@SimonWahlin
Copy link
Author

That sounds like a great idea!

@rkeithhill
Copy link
Contributor

So who knows what the appropriate location for the session & log files would be for Linux and macOS? Or maybe we only make this change for Windows since app whitelisting is only an issue on Windows? Not sure I like having different ways of handling these files for different platforms. Thoughts?

@yume-chan
Copy link

yume-chan commented Jun 14, 2018

I'm using OneDrive to sync my Code profile, and today I was surprised that OneDrive is uploading a 2.2GB file and it belongs to the PowerShell extension.

I always expect the extension folder is kind-of read-only. Logs and temporary files should be in my temp folder and be cleaned periodically.

capture

@TylerLeonhardt
Copy link
Member

TylerLeonhardt commented Jun 14, 2018

We could always use something like:

[System.IO.Path]::GetTempPath()

which works cross platform.

But then we need a cleanup scenario.

@rkeithhill
Copy link
Contributor

Couple that temp path with a subdir like vscode-powershell\sessions and vscode-powershell\logs? OTOH I thought that on Linux/macOS there were prescribed locations for log files? /var/log on Linux, right?

@rjmholt
Copy link
Contributor

rjmholt commented Jun 14, 2018

@yume-chan Your issue there is probably orthogonal to controlling the log path.

I think there are two problems you might be hitting:

  1. Logs seem to get stuck in a loop sometimes and basically dump output as fast as they can push IO, which is a categorical bug.
  2. Logs should have a configurable lifetime, so they don't just massively build up over time.

The temp directory suggestion has a couple of issues:

  • The convention for application logs is to write them to an AppData-type location -- writing logs to the system temp directory would make us a bit unusual as an application. Also the temp directory seems to work differently on every platform, so implementing things that way will be more error-prone and less predictable for users.
  • We have no control over how often the temp directory gets cleaned. It makes sense to clean out the logs, but I feel like it should be up to you as a user, not an implementation detail of your host OS.

If you've got time, can you please open a new issue about the large output of the logfiles you're experiencing? And/or wanting to periodically delete logs?

@rkeithhill it feels like /var/log/ is for system daemon logs, like systemd, XOrg, cups. From what I looked into, there's no standard application data log location on linux. There was a suggestion that we use the systemd log tools but that uses a binary format and there are probably plenty of reasons not to go that way. For user-specific applications, the suggestion seems to be that we want a dotfile in $HOME - but going that path we should probably work out how to best separate that from the present implementation. For Linux' purposes, we might already be doing "the right thing" for logs.

@TylerLeonhardt
Copy link
Member

I tried to see if AppData was set in .NET on macOS and sadly, it's not.

System.Environment.GetEnvironmentVariables() gave me an APPDATA on Windows but no APPDATA on macOS.

TEMP was there on both.

So the options seem to be:

  • Use Temp - less complex but not standard
  • Conditionally use a different location based on OS - more complicated but uses the "right" folders

I think there's also a 3rd option... if we have access to the normal path we're using now, keep it. If we can't write to it, then maybe we have a back up (temp or app data). Similar to the 2nd bullet but then maybe we'll be a bit more consistent than just doing things for different OS's.

Any thoughts/opinions on these 3 points?

@rjmholt
Copy link
Contributor

rjmholt commented Jun 15, 2018

Well my thinking is:

  • We should try to behave as a subordinate to VSCode and try to live within its established areas. We're a plugin, not a standalone application, and we're only going to annoy users if we start carving out new parts of the hard drive for ourselves.
  • But, clearly doing everything in the VSCode extensions folder doesn't work due to permissions. Cleaning that folder out periodically would also address the root problem here maybe, but really users should have full access to the logs of their sessions, especially because they're mainly needed to send to us when things go wrong. Quietly erasing the logs every interval could make issues much harder to diagnose.
  • A platform-specific solution is more complex, but VSCode and PowerShell already do a lot of platform-specific things in the interest of doing the right thing on the platform while keeping the user an abstraction layer above. That's the ecosystem we live in.
  • It might be that using a temp directory might be more fraught than it first appears: Provide a platform-agnostic way to determine the temp. directory, e.g., via an automatic variable. PowerShell#4216, /tmp vs /var/tmp, TMPDIR vs other options on macOS. Basically there are a lot of loose rules on temporary directories, and they are more platform-specific, not less.

So my personal preference would be to use:

  • A common, VSCode-subordinate log path, OR
  • A conventional application log file:
    • %APPDATA% on Windows;
    • /Users/<user>/Library/Application Support/ on macOS;
    • $HOME/.vscode or similar on Linux

@yume-chan
Copy link

yume-chan commented Jun 19, 2018

@rjmholt

writing logs to the system temp directory would make us a bit unusual as an application.

At least I think writing logs to your applications' "installation location" (for extensions, isn't the vscode extension folder their home?) is more unusual.

It makes sense to clean out the logs, but I feel like it should be up to you as a user, not an implementation detail of your host OS.

I personally feel comfortable when the OS can clean my unneeded files automatically. Applications should be more consistent with the OS, even if it means they will work differently on different OSs. I know it's cross-platform, but I don't think it should re-invent the wheel to create another self-styled behavior.

One more merit to use the temp folder:

Code is adding support for portable mode, ideally all extensions should follow and don't leave any file on the host machine.

Code will write the temp folder path into TEMP(win32) or TMPDIR(other) environment variable, so the easiest way should be always reading from these environment variables.

(I assume this environment variable will also be set for the extension host process, but I haven't try it. So if I'm wrong, please point me out)

EDIT: I know, and agree that temp folder is not a suitable path for logs, but at least it's easy to use. If it's not easily accessible for the user, the extension can create their own log folder in the temp folder, then add a command to quickly open it for the user.

@rjmholt
Copy link
Contributor

rjmholt commented Jun 19, 2018

At least I think writing logs to your applications' "installation location" (for extensions, isn't the vscode extension folder their home?) is more unusual.

Agreed there - I'm suggesting we should consider writing logs to conventional application data folders instead. Not saying I'm against writing to a temp folder, just want to make it clear that I wasn't saying we should write to the install location -- in fact I think we shouldn't do that.

@rjmholt
Copy link
Contributor

rjmholt commented Jun 19, 2018

A couple of notes on the temp folder situation:

  • %TEMP%/$env:TEMP works well on Windows (and also happens to be under %APPDATA%).
  • $TMPDIR exists on macOS, but is randomly generated. The one on my machine right now is at /var/folders/jx/qs_zdfg95_1gkf70pn7nrb7h0000gn/T/, and I think it changes on startup.
  • None of $TMP, $TEMP or $TMPDIR is defined on Ubuntu 16.04 (the machine I've got with me), and it looks like those variables can't be relied on in general on Linux. So there we'd want to use something like /tmp or /var/tmp.

@TylerLeonhardt
Copy link
Member

@rjmholt can you check [System.IO.Path]::GetTempPath() on ubuntu (my VM is messed up lol)

on CentOS 7, [System.IO.Path]::GetTempPath() returns /tmp/

@rjmholt
Copy link
Contributor

rjmholt commented Jun 19, 2018

Ubuntu 16.04 also returns that result

@SeeminglyScience
Copy link
Collaborator

Looks like VSCode added something that tells extensions where they can store logs. From the release notes:

Extension logging

The ExtensionContext that comes as an argument of the activate function has a new property logPath. This is the absolute file path of a directory where extensions can store log files. The path is unique for an extension and not reused by other extensions.

@TylerLeonhardt
Copy link
Member

That's really cool!

@rjmholt
Copy link
Contributor

rjmholt commented Sep 10, 2018

Looks like the default settings for the log path API are here: https://github.com/Microsoft/vscode/blob/f3b191ab38fb9f1717ce5ce3396bb41204ffb399/src/paths.js#L18-L25

Those should all be deleteable for users I imagine

@andyleejordan
Copy link
Member

Hey @SimonWahlin, I know this issue is from 2018 but I was looking through old, highly-commented issues and stumbled upon it. I wanted to let you know that in recent versions of the extension with #4240, the log path is now set to use VS Code's globalStorageUri.

A global storage URI pointing to a local directory where your extension has read/write access. This is a good option if you need to store large files that are accessible from all workspaces.

So this should no longer be a problem. If it is you can now also disable all logging via:

"powershell.developer.editorServicesLogLevel": "None"

Thanks!

@andyleejordan andyleejordan added Issue-Enhancement A feature request (enhancement). Resolution-Fixed Will close automatically. labels Apr 27, 2023
@andyleejordan andyleejordan self-assigned this Apr 27, 2023
@andyleejordan andyleejordan moved this to Done in Flying Fox Apr 27, 2023
@ghost ghost closed this as completed Apr 27, 2023
@ghost
Copy link

ghost commented Apr 27, 2023

This issue has been marked as fixed. It has been automatically closed for housekeeping purposes.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Configuration Issue-Enhancement A feature request (enhancement). Resolution-Fixed Will close automatically.
Projects
Status: Done
Development

No branches or pull requests

8 participants