push improved MSBuildLogger to a separate project in case we want to spin it off

[baronfel]

This is part of reducing the diff in #49556.

During that PR, I implemented a number of improvements to the simple Microsoft.Extensions.Logging.Abstractions.ILogger wrapper we had that ties that ILogger to MSBuild's ILogger interfaces.

The main feature is being able to use state/props from MEL.ILogger messages as inputs to MSbuild's various parameters on the LogX overloads - things like code, file, line, etc can now all be set by props. State is also persisted and attached to messages so ambient context is available in the binlog and other MSBuild Loggers even if not directly used in the Message.

[Copilot]

Pull Request Overview

This PR extracts an improved MSBuild logging component into a separate project to potentially spin it off as a standalone package. The main enhancement is the ability to use properties from Microsoft.Extensions.Logging.ILogger messages as inputs to MSBuild's logging parameters (code, file, line, etc.), while preserving state information for binlog and other MSBuild loggers.

• Moves MSBuild logging functionality from Microsoft.NET.Build.Containers to a new Microsoft.Extensions.Logging.MSBuild project
• Enhances the logger to support extracting MSBuild-specific parameters from logging state/scope
• Updates namespace and visibility from internal to public for the extracted components

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/Microsoft.Extensions.Logging.MSBuild/Microsoft.Extensions.Logging.MSBuild.csproj	New project file defining the extracted MSBuild logging library
src/Microsoft.Extensions.Logging.MSBuild/MSBuildLoggerProvider.cs	Updated provider with scope support and public visibility
src/Microsoft.Extensions.Logging.MSBuild/MSBuildLogger.cs	Enhanced logger with parameter extraction from state/scope
src/Containers/Microsoft.NET.Build.Containers/Tasks/CreateNewImage.cs	Updated namespace import to use new project
src/Containers/Microsoft.NET.Build.Containers/Tasks/CreateImageIndex.cs	Updated namespace import to use new project
src/Containers/Microsoft.NET.Build.Containers/Microsoft.NET.Build.Containers.csproj	Added project reference to new logging library
src/Containers/Microsoft.NET.Build.Containers/Logging/MSBuildLogger.cs	Removed old implementation

Comments suppressed due to low confidence (1)

[Copilot]

Potential null reference exception when originalFormat is null. The variable is declared as 'null!' but can remain null if no {OriginalFormat} key is found, causing a null reference when calling Contains().

Suggested change

	if (originalFormat.Contains(wrappedKey))
	if (originalFormat != null && originalFormat.Contains(wrappedKey))

[KalleOlaviNiemitalo]

originalFormat is also null if "{OriginalFormat}" is a key in the list but the foreach (var kvp in stateItems) loop has not reached it yet. There is no documented requirement that "{OriginalFormat}" be in the first element of the list.

[Copilot]

Potential null reference exception when originalFormat is null. This is the same issue as line 140 - originalFormat can be null when no {OriginalFormat} key is found in the state items.

Suggested change

	if (originalFormat.Contains(wrappedKey))
	if (originalFormat != null && originalFormat.Contains(wrappedKey))

[KalleOlaviNiemitalo]

If the message template includes formatting like {CreationDate:O}, this won't match.

[baronfel]

Totally agree - I should possibly look at interpreting this via CompositeFormat.

[KalleOlaviNiemitalo]

The CompositeFormat.Parse method does not support named parameter placeholders like log message templates have. The CompositeFormat class does not have public members for examining the structure of the format string. I don't think CompositeFormat is useful here.

[baronfel]

I was hoping it would be slightly more capable. In the past I've used MessageTemplates for this kind of thing, but there would be source-build concerns with using an external library. Might have to punt for v1 of this - at least if it stays in the SDK repo directly.

[KalleOlaviNiemitalo]

originalFormat is also null if "{OriginalFormat}" is a key in the list but the foreach (var kvp in stateItems) loop has not reached it yet. There is no documented requirement that "{OriginalFormat}" be in the first element of the list.

[KalleOlaviNiemitalo]

How do you create the state objects that are passed to ILogger.Log<TState> and then analysed by this logger?

Methods like LoggerExtensions.LogInformation and LoggerMessage.Define parse the log message template to learn the names of attributes, so if you use those and want to give e.g. "File" to MSBuild, then you have to include a "{File}" placeholder in the template. But then the formatted log message also includes the file name, and MSBuild adds the file name again, so the user sees it twice.

Is source-generated logging different in this respect?

[baronfel]

How do you create the state objects that are passed to ILogger.Log and then analysed by this logger?

Methods like LoggerExtensions.LogInformation and LoggerMessage.Define parse the log message template to learn the names of attributes, so if you use those and want to give e.g. "File" to MSBuild, then you have to include a "{File}" placeholder in the template. But then the formatted log message also includes the file name, and MSBuild adds the file name again, so the user sees it twice.

Is source-generated logging different in this respect?

The state objects are created via ILogger.BeginScope<TState>(TState state) - here's a specific example from the linked PR that I'm extracting this from. Serilog has good docs here, but basically it seems to be up to the specific logger how to handle various kinds of state - single primitive structures like strings you could add as some kind of tag, but collections/maps of key/value pairs are a natural fit for adding to a property bag of some kind, etc.

[KalleOlaviNiemitalo]

The state objects are created via ILogger.BeginScope<TState>(TState state)

Oh so if you don't want to include an attribute in the formatted log message, then you place it in the state of the logger scope, not in the state of the log entry.

- here's a specific example from the linked PR that I'm extracting this from.

That one doesn't seem to have any of the attributes that MSBuildLogger would recognise.

[baronfel]

That one doesn't seem to have any of the attributes that MSBuildLogger would recognise.

Correct - I didn't really make use of the MSBuild-specific attributes in that PR. They just kind of emerged from the fact that I could now walk state/scope in a structured way and do something with the attributes I found.

We have this problem in the SDK codebase more generally when we log in our Tasks where we have to call special "LogXFromFormat" methods because those know how to extract specific kinds of data from the shapes of our localized strings - "wouldn't it be nice if that was done in a more 'structured' way" was basically the genesis of this entire PR.