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.

Sign up for GitHub

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

Jump to bottom

Miscellaneous logging improvements #6326

Merged
merged 14 commits into from Apr 20, 2021
Merged

Miscellaneous logging improvements #6326

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c09a537
Don't log BuildEventArgs.Message where it can be reconstructed.
KirillOsenkov Apr 5, 2021
c9b5e2e
Log TaskParameterEventArgs for inputs/outputs of TargetSkipped
KirillOsenkov Apr 5, 2021
ace9956
Do not allocate a single string for Task Parameter
KirillOsenkov Apr 5, 2021
66acffd
Do not log property reassignment for MSBuildAllProjects
KirillOsenkov Apr 5, 2021
b6540bc
Cache resource strings in RAR logging.
KirillOsenkov Apr 5, 2021
95021b1
Cache resource strings in the Copy task.
KirillOsenkov Apr 5, 2021
c191e15
Use named arguments.
KirillOsenkov Apr 5, 2021
da331c9
Add NOPERFORMANCESUMMARY switch for console loggers.
KirillOsenkov Apr 5, 2021
e270fea
Add TestLoggingForSkippedTargetInputsAndOutputs
KirillOsenkov Apr 5, 2021
6fea565
Compare console logs after binary logging roundtrip.
KirillOsenkov Apr 5, 2021
1de22cd
Add more coverage for console logs from binlog playback.
KirillOsenkov Apr 5, 2021
785ffa1
Fix test to work on unix.
KirillOsenkov Apr 5, 2021
2410c41
Fix typo: HelpHeyword -> HelpKeyword.
KirillOsenkov Apr 8, 2021
ca456ad
Add GetResourceEightSpaces
KirillOsenkov Apr 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions ref/Microsoft.Build.Framework/net/Microsoft.Build.Framework.cs
View file
Open in desktop
Expand Up @@ -38,6 +38,7 @@ public abstract partial class BuildEventArgs : System.EventArgs
public Microsoft.Build.Framework.BuildEventContext BuildEventContext { get { throw null; } set { } }
public string HelpKeyword { get { throw null; } }
public virtual string Message { get { throw null; } protected set { } }
protected internal string RawMessage { get { throw null; } set { } }
protected internal System.DateTime RawTimestamp { get { throw null; } set { } }
public string SenderName { get { throw null; } }
public int ThreadId { get { throw null; } }
Expand Down Expand Up @@ -340,6 +341,8 @@ public partial interface ITaskItem2 : Microsoft.Build.Framework.ITaskItem
}
public partial class LazyFormattedBuildEventArgs : Microsoft.Build.Framework.BuildEventArgs
{
[System.NonSerializedAttribute]
protected object locker;
protected LazyFormattedBuildEventArgs() { }
public LazyFormattedBuildEventArgs(string message, string helpKeyword, string senderName) { }
public LazyFormattedBuildEventArgs(string message, string helpKeyword, string senderName, System.DateTime eventTimestamp, params object[] messageArgs) { }
Expand Down Expand Up @@ -408,6 +411,7 @@ public partial class ProjectFinishedEventArgs : Microsoft.Build.Framework.BuildS
protected ProjectFinishedEventArgs() { }
public ProjectFinishedEventArgs(string message, string helpKeyword, string projectFile, bool succeeded) { }
public ProjectFinishedEventArgs(string message, string helpKeyword, string projectFile, bool succeeded, System.DateTime eventTimestamp) { }
public override string Message { get { throw null; } }
public string ProjectFile { get { throw null; } }
public bool Succeeded { get { throw null; } }
}
Expand All @@ -431,6 +435,7 @@ public partial class ProjectStartedEventArgs : Microsoft.Build.Framework.BuildSt
public ProjectStartedEventArgs(string message, string helpKeyword, string projectFile, string targetNames, System.Collections.IEnumerable properties, System.Collections.IEnumerable items, System.DateTime eventTimestamp) { }
public System.Collections.Generic.IDictionary<string, string> GlobalProperties { get { throw null; } }
public System.Collections.IEnumerable Items { get { throw null; } }
public override string Message { get { throw null; } }
public Microsoft.Build.Framework.BuildEventContext ParentProjectBuildEventContext { get { throw null; } }
public string ProjectFile { get { throw null; } }
public int ProjectId { get { throw null; } }
Expand All @@ -452,6 +457,7 @@ public partial class PropertyReassignmentEventArgs : Microsoft.Build.Framework.B
public PropertyReassignmentEventArgs() { }
public PropertyReassignmentEventArgs(string propertyName, string previousValue, string newValue, string location, string message, string helpKeyword = null, string senderName = null, Microsoft.Build.Framework.MessageImportance importance = Microsoft.Build.Framework.MessageImportance.Low) { }
public string Location { get { throw null; } set { } }
public override string Message { get { throw null; } }
public string NewValue { get { throw null; } set { } }
public string PreviousValue { get { throw null; } set { } }
public string PropertyName { get { throw null; } set { } }
Expand Down Expand Up @@ -558,6 +564,7 @@ public partial class TargetFinishedEventArgs : Microsoft.Build.Framework.BuildSt
public TargetFinishedEventArgs(string message, string helpKeyword, string targetName, string projectFile, string targetFile, bool succeeded) { }
public TargetFinishedEventArgs(string message, string helpKeyword, string targetName, string projectFile, string targetFile, bool succeeded, System.Collections.IEnumerable targetOutputs) { }
public TargetFinishedEventArgs(string message, string helpKeyword, string targetName, string projectFile, string targetFile, bool succeeded, System.DateTime eventTimestamp, System.Collections.IEnumerable targetOutputs) { }
public override string Message { get { throw null; } }
public string ProjectFile { get { throw null; } }
public bool Succeeded { get { throw null; } }
public string TargetFile { get { throw null; } }
Expand All @@ -570,6 +577,10 @@ public partial class TargetSkippedEventArgs : Microsoft.Build.Framework.BuildMes
public TargetSkippedEventArgs() { }
public TargetSkippedEventArgs(string message, params object[] messageArgs) { }
public Microsoft.Build.Framework.TargetBuiltReason BuildReason { get { throw null; } set { } }
public string Condition { get { throw null; } set { } }
public string EvaluatedCondition { get { throw null; } set { } }
public override string Message { get { throw null; } }
public bool OriginallySucceeded { get { throw null; } set { } }
public string ParentTarget { get { throw null; } set { } }
public string TargetFile { get { throw null; } set { } }
public string TargetName { get { throw null; } set { } }
Expand All @@ -581,6 +592,7 @@ public partial class TargetStartedEventArgs : Microsoft.Build.Framework.BuildSta
public TargetStartedEventArgs(string message, string helpKeyword, string targetName, string projectFile, string targetFile, string parentTarget, Microsoft.Build.Framework.TargetBuiltReason buildReason, System.DateTime eventTimestamp) { }
public TargetStartedEventArgs(string message, string helpKeyword, string targetName, string projectFile, string targetFile, string parentTarget, System.DateTime eventTimestamp) { }
public Microsoft.Build.Framework.TargetBuiltReason BuildReason { get { throw null; } }
public override string Message { get { throw null; } }
public string ParentTarget { get { throw null; } }
public string ProjectFile { get { throw null; } }
public string TargetFile { get { throw null; } }
Expand All @@ -600,6 +612,7 @@ public partial class TaskFinishedEventArgs : Microsoft.Build.Framework.BuildStat
protected TaskFinishedEventArgs() { }
public TaskFinishedEventArgs(string message, string helpKeyword, string projectFile, string taskFile, string taskName, bool succeeded) { }
public TaskFinishedEventArgs(string message, string helpKeyword, string projectFile, string taskFile, string taskName, bool succeeded, System.DateTime eventTimestamp) { }
public override string Message { get { throw null; } }
public string ProjectFile { get { throw null; } }
public bool Succeeded { get { throw null; } }
public string TaskFile { get { throw null; } }
Expand All @@ -621,6 +634,8 @@ public enum TaskParameterMessageKind
TaskOutput = 1,
AddItem = 2,
RemoveItem = 3,
SkippedTargetInputs = 4,
SkippedTargetOutputs = 5,
}
public partial class TaskPropertyInfo
{
Expand All @@ -637,6 +652,7 @@ public partial class TaskStartedEventArgs : Microsoft.Build.Framework.BuildStatu
protected TaskStartedEventArgs() { }
public TaskStartedEventArgs(string message, string helpKeyword, string projectFile, string taskFile, string taskName) { }
public TaskStartedEventArgs(string message, string helpKeyword, string projectFile, string taskFile, string taskName, System.DateTime eventTimestamp) { }
public override string Message { get { throw null; } }
public string ProjectFile { get { throw null; } }
public string TaskFile { get { throw null; } }
public string TaskName { get { throw null; } }
Expand Down
16 changes: 16 additions & 0 deletions ref/Microsoft.Build.Framework/netstandard/Microsoft.Build.Framework.cs
View file
Open in desktop
Expand Up @@ -38,6 +38,7 @@ public abstract partial class BuildEventArgs : System.EventArgs
public Microsoft.Build.Framework.BuildEventContext BuildEventContext { get { throw null; } set { } }
public string HelpKeyword { get { throw null; } }
public virtual string Message { get { throw null; } protected set { } }
protected internal string RawMessage { get { throw null; } set { } }
protected internal System.DateTime RawTimestamp { get { throw null; } set { } }
public string SenderName { get { throw null; } }
public int ThreadId { get { throw null; } }
Expand Down Expand Up @@ -340,6 +341,8 @@ public partial interface ITaskItem2 : Microsoft.Build.Framework.ITaskItem
}
public partial class LazyFormattedBuildEventArgs : Microsoft.Build.Framework.BuildEventArgs
{
[System.NonSerializedAttribute]
protected object locker;
protected LazyFormattedBuildEventArgs() { }
public LazyFormattedBuildEventArgs(string message, string helpKeyword, string senderName) { }
public LazyFormattedBuildEventArgs(string message, string helpKeyword, string senderName, System.DateTime eventTimestamp, params object[] messageArgs) { }
Expand Down Expand Up @@ -407,6 +410,7 @@ public partial class ProjectFinishedEventArgs : Microsoft.Build.Framework.BuildS
protected ProjectFinishedEventArgs() { }
public ProjectFinishedEventArgs(string message, string helpKeyword, string projectFile, bool succeeded) { }
public ProjectFinishedEventArgs(string message, string helpKeyword, string projectFile, bool succeeded, System.DateTime eventTimestamp) { }
public override string Message { get { throw null; } }
public string ProjectFile { get { throw null; } }
public bool Succeeded { get { throw null; } }
}
Expand All @@ -430,6 +434,7 @@ public partial class ProjectStartedEventArgs : Microsoft.Build.Framework.BuildSt
public ProjectStartedEventArgs(string message, string helpKeyword, string projectFile, string targetNames, System.Collections.IEnumerable properties, System.Collections.IEnumerable items, System.DateTime eventTimestamp) { }
public System.Collections.Generic.IDictionary<string, string> GlobalProperties { get { throw null; } }
public System.Collections.IEnumerable Items { get { throw null; } }
public override string Message { get { throw null; } }
public Microsoft.Build.Framework.BuildEventContext ParentProjectBuildEventContext { get { throw null; } }
public string ProjectFile { get { throw null; } }
public int ProjectId { get { throw null; } }
Expand All @@ -451,6 +456,7 @@ public partial class PropertyReassignmentEventArgs : Microsoft.Build.Framework.B
public PropertyReassignmentEventArgs() { }
public PropertyReassignmentEventArgs(string propertyName, string previousValue, string newValue, string location, string message, string helpKeyword = null, string senderName = null, Microsoft.Build.Framework.MessageImportance importance = Microsoft.Build.Framework.MessageImportance.Low) { }
public string Location { get { throw null; } set { } }
public override string Message { get { throw null; } }
public string NewValue { get { throw null; } set { } }
public string PreviousValue { get { throw null; } set { } }
public string PropertyName { get { throw null; } set { } }
Expand Down Expand Up @@ -557,6 +563,7 @@ public partial class TargetFinishedEventArgs : Microsoft.Build.Framework.BuildSt
public TargetFinishedEventArgs(string message, string helpKeyword, string targetName, string projectFile, string targetFile, bool succeeded) { }
public TargetFinishedEventArgs(string message, string helpKeyword, string targetName, string projectFile, string targetFile, bool succeeded, System.Collections.IEnumerable targetOutputs) { }
public TargetFinishedEventArgs(string message, string helpKeyword, string targetName, string projectFile, string targetFile, bool succeeded, System.DateTime eventTimestamp, System.Collections.IEnumerable targetOutputs) { }
public override string Message { get { throw null; } }
public string ProjectFile { get { throw null; } }
public bool Succeeded { get { throw null; } }
public string TargetFile { get { throw null; } }
Expand All @@ -569,6 +576,10 @@ public partial class TargetSkippedEventArgs : Microsoft.Build.Framework.BuildMes
public TargetSkippedEventArgs() { }
public TargetSkippedEventArgs(string message, params object[] messageArgs) { }
public Microsoft.Build.Framework.TargetBuiltReason BuildReason { get { throw null; } set { } }
public string Condition { get { throw null; } set { } }
public string EvaluatedCondition { get { throw null; } set { } }
public override string Message { get { throw null; } }
public bool OriginallySucceeded { get { throw null; } set { } }
public string ParentTarget { get { throw null; } set { } }
public string TargetFile { get { throw null; } set { } }
public string TargetName { get { throw null; } set { } }
Expand All @@ -580,6 +591,7 @@ public partial class TargetStartedEventArgs : Microsoft.Build.Framework.BuildSta
public TargetStartedEventArgs(string message, string helpKeyword, string targetName, string projectFile, string targetFile, string parentTarget, Microsoft.Build.Framework.TargetBuiltReason buildReason, System.DateTime eventTimestamp) { }
public TargetStartedEventArgs(string message, string helpKeyword, string targetName, string projectFile, string targetFile, string parentTarget, System.DateTime eventTimestamp) { }
public Microsoft.Build.Framework.TargetBuiltReason BuildReason { get { throw null; } }
public override string Message { get { throw null; } }
public string ParentTarget { get { throw null; } }
public string ProjectFile { get { throw null; } }
public string TargetFile { get { throw null; } }
Expand All @@ -599,6 +611,7 @@ public partial class TaskFinishedEventArgs : Microsoft.Build.Framework.BuildStat
protected TaskFinishedEventArgs() { }
public TaskFinishedEventArgs(string message, string helpKeyword, string projectFile, string taskFile, string taskName, bool succeeded) { }
public TaskFinishedEventArgs(string message, string helpKeyword, string projectFile, string taskFile, string taskName, bool succeeded, System.DateTime eventTimestamp) { }
public override string Message { get { throw null; } }
public string ProjectFile { get { throw null; } }
public bool Succeeded { get { throw null; } }
public string TaskFile { get { throw null; } }
Expand All @@ -620,6 +633,8 @@ public enum TaskParameterMessageKind
TaskOutput = 1,
AddItem = 2,
RemoveItem = 3,
SkippedTargetInputs = 4,
SkippedTargetOutputs = 5,
}
public partial class TaskPropertyInfo
{
Expand All @@ -636,6 +651,7 @@ public partial class TaskStartedEventArgs : Microsoft.Build.Framework.BuildStatu
protected TaskStartedEventArgs() { }
public TaskStartedEventArgs(string message, string helpKeyword, string projectFile, string taskFile, string taskName) { }
public TaskStartedEventArgs(string message, string helpKeyword, string projectFile, string taskFile, string taskName, System.DateTime eventTimestamp) { }
public override string Message { get { throw null; } }
public string ProjectFile { get { throw null; } }
public string TaskFile { get { throw null; } }
public string TaskName { get { throw null; } }
Expand Down
11 changes: 11 additions & 0 deletions src/Build.UnitTests/BackEnd/MockLoggingService.cs
View file
Open in desktop
Expand Up @@ -300,6 +300,17 @@ public void LogCommentFromText(BuildEventContext buildEventContext, MessageImpor
_writer(message);
}

/// <inheritdoc />
public void LogCommentFromText(BuildEventContext buildEventContext, MessageImportance importance, string message, params object[] messageArgs)
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder what the gains of offering a few overloads for it would be:

public void LogCommentFromText(BuildEventContext buildEventContext, MessageImportance importance, string message, params object[] messageArgs)
public void LogCommentFromText(BuildEventContext buildEventContext, MessageImportance importance, string message, object arg1)
public void LogCommentFromText(BuildEventContext buildEventContext, MessageImportance importance, string message, object arg1, object arg2)

That would avoid most of the allocations for the string.Format calls, I guess.

Copy link
Member Author

Choose a reason for hiding this comment

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

We’ll need an array later on anyway, to store the arguments on LazyFormattedBuildEventArgs

{
if (messageArgs?.Length > 0)
{
message = string.Format(message, messageArgs);
}

_writer(message);
}

/// <summary>
/// Logs a pre-formed build event
/// </summary>
Expand Down
24 changes: 15 additions & 9 deletions src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs
View file
Open in desktop
Expand Up @@ -16,6 +16,12 @@ namespace Microsoft.Build.UnitTests
{
public class BuildEventArgsSerializationTests
{
public BuildEventArgsSerializationTests()
{
// touch the type so that static constructor runs
_ = ItemGroupLoggingHelper.ItemGroupIncludeLogMessagePrefix;
}

[Fact]
public void RoundtripBuildStartedEventArgs()
{
Expand Down Expand Up @@ -62,7 +68,7 @@ public void RoundtripProjectStartedEventArgs()
{
var args = new ProjectStartedEventArgs(
projectId: 42,
message: "Project started message",
message: "Project \"test.proj\" (Build target(s)):",
helpKeyword: "help",
projectFile: "C:\\test.proj",
targetNames: "Build",
Expand Down Expand Up @@ -431,7 +437,7 @@ public void RoundtripProjectImportedEventArgs()
public void RoundtripTargetSkippedEventArgs()
{
var args = new TargetSkippedEventArgs(
"Message")
"Target \"target\" skipped. Previously built unsuccessfully.")
{
BuildEventContext = BuildEventContext.Invalid,
ProjectFile = "foo.csproj",
Expand Down Expand Up @@ -473,13 +479,13 @@ public void RoundTripEnvironmentVariableReadEventArgs()
public void RoundTripPropertyReassignmentEventArgs()
{
var args = new PropertyReassignmentEventArgs(
propertyName: Guid.NewGuid().ToString(),
previousValue: Guid.NewGuid().ToString(),
newValue: Guid.NewGuid().ToString(),
location: Guid.NewGuid().ToString(),
message: Guid.NewGuid().ToString(),
helpKeyword: Guid.NewGuid().ToString(),
senderName: Guid.NewGuid().ToString());
propertyName: "a",
previousValue: "b",
newValue: "c",
location: "d",
message: "Property reassignment: $(a)=\"c\" (previous value: \"b\") at d",
helpKeyword: "e",
senderName: "f");

Roundtrip(args,
e => e.PropertyName,
Expand Down
12 changes: 12 additions & 0 deletions src/Build/BackEnd/Components/Logging/ILoggingService.cs
View file
Open in desktop
Expand Up @@ -297,6 +297,18 @@ bool IncludeTaskInputs
/// <param name="importance">Importance level of the message</param>
/// <param name="message">message to log</param>
void LogCommentFromText(BuildEventContext buildEventContext, MessageImportance importance, string message);

/// <summary>
/// Log a comment from a format string and arguments
/// </summary>
/// <param name="buildEventContext">Event context information which describes who is logging the event</param>
/// <param name="importance">How important is the message, this will determine which verbosities the message will show up on.
/// The higher the importance the lower the verbosity needs to be for the message to be seen</param>
/// <param name="message">Message to log</param>
/// <param name="messageArgs">Message formatting arguments</param>
/// <exception cref="InternalErrorException">BuildEventContext is null</exception>
/// <exception cref="InternalErrorException">Message is null</exception>
void LogCommentFromText(BuildEventContext buildEventContext, MessageImportance importance, string message, params object[] messageArgs);
#endregion

#region Log events
Expand Down
12 changes: 12 additions & 0 deletions src/Build/BackEnd/Components/Logging/LoggingContext.cs
View file
Open in desktop
Expand Up @@ -134,6 +134,18 @@ internal void LogCommentFromText(MessageImportance importance, string message)
_loggingService.LogCommentFromText(_eventContext, importance, message);
}

/// <summary>
/// Helper method to create a message build event from a string
/// </summary>
/// <param name="importance">Importance level of the message</param>
/// <param name="message">Message to log</param>
/// <param name="messageArgs">Format string arguments</param>
internal void LogCommentFromText(MessageImportance importance, string message, params object[] messageArgs)
{
ErrorUtilities.VerifyThrow(_isValid, "must be valid");
_loggingService.LogCommentFromText(_eventContext, importance, message, messageArgs);
}

/// <summary>
/// Log an error
/// </summary>
Expand Down