Conversation

JustinGrote

Eliminate Serilog dependencies and wire up HostLogger to MEL directly after initialization.

I confirmed the HostLogger logs are showing up on the other side (this is using the new logoutputwindow on the vscode side but should look fine with the old LSP trace as well)

image

This shows where the file based logging stops and then resumes inside LSP communication
image

As part of the resolutions, this also drops net6.0 support due to some library changes (which was going to happen anyways as net6.0 is EOL as of November)

Fixes PowerShell/vscode-powershell#5084

@JustinGrote

Test for the debug adapter is stuck on reading the stream for some reason...

int actualCount = _underlyingStream.Read(buffer, offset, count);
LogData("READ", buffer, offset, actualCount);

EDIT: StdioServerProcess used for testing has some loggerfactory references still. I'll investigate tomorrow. Should still be good for review, works otherwise for LSP and DAP as far as I can tell, just a faulty test.

@dkattan

This is awesome! Serilog version mismatches have caused lots of issues for us when launching from our parent project.

@JustinGrote

I'm having issues with this test even on main, and I suspect it has to do with this deadlock weirdness.
image

@JustinGrote

@andyleejordan this should be ready but it needs a package update to run tests.
Failed to retrieve information about 'Microsoft.Extensions.Logging.Debug' from remote source 'https://pkgs.dev.azure.com/powershell/2972bb5c-f20c-4a60-8bd9-00ffe9987edc/_packaging/e59e749b-417c-4664-9322-c190bb60de7f/nuget/v3/flat2/microsoft.extensions.logging.debug/index.json'.

I can remove MEL.Debug if you want but I hope to expand it more throughout the extension.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heck yeah.

@JustinGroteJustinGrote changed the title Remove Serilog and implement HostLogger integration Remove Serilog, implement HostLogger integration, and deprecate net6.0 Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks Justin!

@andyleejordanandyleejordan added the Issue-BugA bug to squash.label Nov 14, 2024
@andyleejordanandyleejordan added this pull request to the merge queue Nov 14, 2024
Merged via the queue into main with commit feabfd5 Nov 14, 2024
8 checks passed
@andyleejordanandyleejordan deleted the fix/RemoveSerilog branch November 14, 2024 23:37
Sign up for free to join this conversation on . Already have an account? Sign in to comment
Issue-BugA bug to squash.
Status: Done

Successfully merging this pull request may close these issues.

2024.4.0: Unable to start when using Windows PowerShell if older Serilog is already present