Implement support in the LSIF generator for logging to stderr #70067
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
Previously we were just passing around a TextWriter which wasn't great.
jasonmalinowski
force-pushed
the
implement-lsif-logging-format
branch
from
7ee0a5b to
1586fab
Compare
|
IIRC @noellelc had a wish-list for "guard-rail" metrics returned in telemetry. I don't recall the specifics... maybe it was "DocumentCount" and "ProjectCount".
This is intentionally open ended in hopes you have ideas for what would be most useful for optimizing the generator. Some ideas that come to mind for me are:
|
|
|
||
| var solution = await openAsync(msbuildWorkspace, cancellationToken); | ||
|
|
||
| var options = GeneratorOptions.Default; | ||
|
|
||
| await logFile.WriteLineAsync($"Load completed in {solutionLoadStopwatch.Elapsed.ToDisplayString()}."); | ||
| var lsifGenerator = Generator.CreateAndWriteCapabilitiesVertex(lsifWriter, logFile); | ||
| logger.LogInformation($"Load completed in {solutionLoadStopwatch.Elapsed.ToDisplayString()}."); |
There was a problem hiding this comment.
Are any of these stopwatch times things that we want to be aggregated? If so, they could be passed as telemetry also/instead.
Perhaps a future improvement.
There was a problem hiding this comment.
Yes, we will, but for this was just step 1.
jasonmalinowski
force-pushed
the
implement-lsif-logging-format
branch
from
1586fab to
4bc3379
Compare
LSIF tools can implement an optional logging format which is written to stderr; this implements that format.
jasonmalinowski
force-pushed
the
implement-lsif-logging-format
branch
from
4bc3379 to
3f27bc1
Compare
| } | ||
| } | ||
|
|
||
| private sealed record CommandWithParameters(string Command, object Parameters); |
There was a problem hiding this comment.
Is this a standard format from somewhere? Or are we creating our own logging protocol?
There was a problem hiding this comment.
It's a new format for LSIF generators specifically; @gundermanc can comment more on that. There's a part I haven't implemented yet here where it also can contain data points like "number of documents processed" which we can also use to monitor system health.
|
|
||
| ILogger logger; | ||
| if (logFile is not null) | ||
| logger = new PlainTextLogger(logFile); |
There was a problem hiding this comment.
do we never want to write the lsif format log to a file?
There was a problem hiding this comment.
@gundermanc do you want that?
There was a problem hiding this comment.
If the standard error stream works, I don't intend on using the log file.
ghost
added this to the 
LSIF tools can implement an optional logging format which is written to stderr; this implements that format.
Addresses part of #69610; there isn't telemetry yet.