Skip to content

Commit b01d2c4

Browse files
Copilotbaronfel
Copilot
and
baronfel
committed
Fix telemetry logger to use distributed logging pattern with central and forwarding loggers
Co-authored-by: baronfel <[email protected]>
1 parent 63b3896 commit b01d2c4

File tree

4 files changed

+165
-46
lines changed

4 files changed

+165
-46
lines changed

‎src/Cli/dotnet/Commands/Run/RunCommand.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,9 @@ static ProjectInstance EvaluateProject(string? projectFilePath, Func<ProjectColl
416416

417417
var globalProperties = CommonRunHelpers.GetGlobalPropertiesFromArgs(msbuildArgs);
418418

419-
var collection = new ProjectCollection(globalProperties: globalProperties, loggers: binaryLogger is null ? null : [binaryLogger], toolsetDefinitionLocations: ToolsetDefinitionLocations.Default);
419+
// Include telemetry logger for evaluation
420+
var loggers = ProjectInstanceExtensions.CreateLoggersWithTelemetry(binaryLogger is null ? null : [binaryLogger]);
421+
var collection = new ProjectCollection(globalProperties: globalProperties, loggers: loggers, toolsetDefinitionLocations: ToolsetDefinitionLocations.Default);
420422

421423
if (projectFilePath is not null)
422424
{

‎src/Cli/dotnet/Commands/Test/MTP/MSBuildUtility.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ public static (IEnumerable<ParallelizableTestModuleGroupWithSequentialInnerModul
3737

3838
var msbuildArgs = MSBuildArgs.AnalyzeMSBuildArguments(buildOptions.MSBuildArgs, CommonOptions.PropertiesOption, CommonOptions.RestorePropertiesOption, CommonOptions.MSBuildTargetOption(), CommonOptions.VerbosityOption());
3939

40-
using var collection = new ProjectCollection(globalProperties: CommonRunHelpers.GetGlobalPropertiesFromArgs(msbuildArgs), loggers: logger is null ? null : [logger], toolsetDefinitionLocations: ToolsetDefinitionLocations.Default);
40+
// Include telemetry logger for evaluation
41+
var loggers = ProjectInstanceExtensions.CreateLoggersWithTelemetry(logger is null ? null : [logger]);
42+
using var collection = new ProjectCollection(globalProperties: CommonRunHelpers.GetGlobalPropertiesFromArgs(msbuildArgs), loggers: loggers, toolsetDefinitionLocations: ToolsetDefinitionLocations.Default);
4143
var evaluationContext = EvaluationContext.Create(EvaluationContext.SharingPolicy.Shared);
4244
ConcurrentBag<ParallelizableTestModuleGroupWithSequentialInnerModules> projects = GetProjectsProperties(collection, evaluationContext, solutionModel.SolutionProjects.Select(p => Path.Combine(rootDirectory, p.FilePath)), buildOptions);
4345
logger?.ReallyShutdown();
@@ -59,7 +61,9 @@ public static (IEnumerable<ParallelizableTestModuleGroupWithSequentialInnerModul
5961

6062
var msbuildArgs = MSBuildArgs.AnalyzeMSBuildArguments(buildOptions.MSBuildArgs, CommonOptions.PropertiesOption, CommonOptions.RestorePropertiesOption, CommonOptions.MSBuildTargetOption(), CommonOptions.VerbosityOption());
6163

62-
using var collection = new ProjectCollection(globalProperties: CommonRunHelpers.GetGlobalPropertiesFromArgs(msbuildArgs), logger is null ? null : [logger], toolsetDefinitionLocations: ToolsetDefinitionLocations.Default);
64+
// Include telemetry logger for evaluation
65+
var loggers = ProjectInstanceExtensions.CreateLoggersWithTelemetry(logger is null ? null : [logger]);
66+
using var collection = new ProjectCollection(globalProperties: CommonRunHelpers.GetGlobalPropertiesFromArgs(msbuildArgs), loggers, toolsetDefinitionLocations: ToolsetDefinitionLocations.Default);
6367
var evaluationContext = EvaluationContext.Create(EvaluationContext.SharingPolicy.Shared);
6468
IEnumerable<ParallelizableTestModuleGroupWithSequentialInnerModules> projects = SolutionAndProjectUtility.GetProjectProperties(projectFilePath, collection, evaluationContext, buildOptions);
6569
logger?.ReallyShutdown();

‎src/Cli/dotnet/Extensions/ProjectInstanceExtensions.cs‎

Lines changed: 114 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using Microsoft.Build.Execution;
55
using Microsoft.Build.Framework;
6+
using Microsoft.Build.Logging;
67
using Microsoft.DotNet.Cli.Commands.MSBuild;
78

89
namespace Microsoft.DotNet.Cli.Extensions;
@@ -51,17 +52,18 @@ public static IEnumerable<string> GetConfigurations(this ProjectInstance project
5152
}
5253

5354
/// <summary>
54-
/// Creates telemetry loggers for API-based MSBuild usage if telemetry is enabled.
55-
/// Returns null if telemetry is not enabled or if there's an error creating the loggers.
55+
/// Creates the central telemetry logger for API-based MSBuild usage if telemetry is enabled.
56+
/// This logger should be used for evaluation (ProjectCollection) and as a central logger for builds.
57+
/// Returns null if telemetry is not enabled or if there's an error creating the logger.
5658
/// </summary>
57-
/// <returns>A list of loggers to use with ProjectInstance.Build, or null if telemetry is disabled.</returns>
58-
public static ILogger[]? CreateTelemetryLoggers()
59+
/// <returns>The central telemetry logger, or null if telemetry is disabled.</returns>
60+
public static ILogger? CreateTelemetryCentralLogger()
5961
{
6062
if (Telemetry.Telemetry.CurrentSessionId != null)
6163
{
6264
try
6365
{
64-
return [new MSBuildLogger()];
66+
return new MSBuildLogger();
6567
}
6668
catch (Exception)
6769
{
@@ -72,32 +74,72 @@ public static IEnumerable<string> GetConfigurations(this ProjectInstance project
7274
}
7375

7476
/// <summary>
75-
/// Builds the project with the specified targets, automatically including telemetry loggers.
77+
/// Creates the forwarding logger record for distributed builds if telemetry is enabled.
78+
/// This should be used with the remoteLoggers parameter of ProjectInstance.Build.
79+
/// Returns an empty collection if telemetry is not enabled or if there's an error creating the logger.
80+
/// </summary>
81+
/// <returns>An array containing the forwarding logger record, or empty array if telemetry is disabled.</returns>
82+
public static ForwardingLoggerRecord[] CreateTelemetryForwardingLoggerRecords()
83+
{
84+
if (Telemetry.Telemetry.CurrentSessionId != null)
85+
{
86+
try
87+
{
88+
var forwardingLogger = new MSBuildForwardingLogger();
89+
var loggerRecord = new ForwardingLoggerRecord(forwardingLogger, new Microsoft.Build.Logging.LoggerDescription(
90+
loggerClassName: typeof(MSBuildLogger).FullName!,
91+
loggerAssemblyName: typeof(MSBuildLogger).Assembly.Location,
92+
loggerAssemblyFile: null,
93+
loggerSwitchParameters: null,
94+
verbosity: LoggerVerbosity.Normal));
95+
return [loggerRecord];
96+
}
97+
catch (Exception)
98+
{
99+
// Exceptions during telemetry shouldn't cause anything else to fail
100+
}
101+
}
102+
return [];
103+
}
104+
105+
/// <summary>
106+
/// Builds the project with the specified targets, automatically including telemetry loggers
107+
/// as a distributed logger (central logger + forwarding logger).
76108
/// </summary>
77109
public static bool BuildWithTelemetry(
78110
this ProjectInstance projectInstance,
79111
string[] targets,
80112
IEnumerable<ILogger>? additionalLoggers = null)
81113
{
82114
var loggers = new List<ILogger>();
115+
var forwardingLoggers = new List<ForwardingLoggerRecord>();
83116

84-
var telemetryLoggers = CreateTelemetryLoggers();
85-
if (telemetryLoggers != null)
117+
// Add central telemetry logger
118+
var telemetryCentralLogger = CreateTelemetryCentralLogger();
119+
if (telemetryCentralLogger != null)
86120
{
87-
loggers.AddRange(telemetryLoggers);
121+
loggers.Add(telemetryCentralLogger);
122+
123+
// Add forwarding logger for distributed builds
124+
forwardingLoggers.AddRange(CreateTelemetryForwardingLoggerRecords());
88125
}
89126

90127
if (additionalLoggers != null)
91128
{
92129
loggers.AddRange(additionalLoggers);
93130
}
94131

95-
return projectInstance.Build(targets, loggers.Count > 0 ? loggers : null);
132+
// Use the overload that accepts forwarding loggers for proper distributed logging
133+
return projectInstance.Build(
134+
targets,
135+
loggers.Count > 0 ? loggers : null,
136+
forwardingLoggers.Count > 0 ? forwardingLoggers : null,
137+
out _);
96138
}
97139

98140
/// <summary>
99-
/// Builds the project with the specified targets, automatically including telemetry loggers.
100-
/// Overload for Build with targetOutputs parameter.
141+
/// Builds the project with the specified targets, automatically including telemetry loggers
142+
/// as a distributed logger (central logger + forwarding logger).
101143
/// </summary>
102144
public static bool BuildWithTelemetry(
103145
this ProjectInstance projectInstance,
@@ -106,45 +148,94 @@ public static bool BuildWithTelemetry(
106148
out IDictionary<string, TargetResult> targetOutputs)
107149
{
108150
var allLoggers = new List<ILogger>();
151+
var forwardingLoggers = new List<ForwardingLoggerRecord>();
109152

110-
var telemetryLoggers = CreateTelemetryLoggers();
111-
if (telemetryLoggers != null)
153+
// Add central telemetry logger
154+
var telemetryCentralLogger = CreateTelemetryCentralLogger();
155+
if (telemetryCentralLogger != null)
112156
{
113-
allLoggers.AddRange(telemetryLoggers);
157+
allLoggers.Add(telemetryCentralLogger);
158+
159+
// Add forwarding logger for distributed builds
160+
forwardingLoggers.AddRange(CreateTelemetryForwardingLoggerRecords());
114161
}
115162

116163
if (loggers != null)
117164
{
118165
allLoggers.AddRange(loggers);
119166
}
120167

121-
return projectInstance.Build(targets, allLoggers.Count > 0 ? allLoggers : null, out targetOutputs);
168+
// Use the overload that accepts forwarding loggers for proper distributed logging
169+
return projectInstance.Build(
170+
targets,
171+
allLoggers.Count > 0 ? allLoggers : null,
172+
forwardingLoggers.Count > 0 ? forwardingLoggers : null,
173+
out targetOutputs);
122174
}
123175

124176
/// <summary>
125-
/// Builds the project with the specified targets, automatically including telemetry loggers.
126-
/// Overload for Build with loggers, remoteLoggers, and targetOutputs parameters.
177+
/// Builds the project with the specified targets, automatically including telemetry loggers
178+
/// as a distributed logger (central logger + forwarding logger).
127179
/// </summary>
128180
public static bool BuildWithTelemetry(
129181
this ProjectInstance projectInstance,
130182
string[] targets,
131183
IEnumerable<ILogger>? loggers,
132-
IEnumerable<Microsoft.Build.Logging.ForwardingLoggerRecord>? remoteLoggers,
184+
IEnumerable<ForwardingLoggerRecord>? remoteLoggers,
133185
out IDictionary<string, TargetResult> targetOutputs)
134186
{
135187
var allLoggers = new List<ILogger>();
188+
var allForwardingLoggers = new List<ForwardingLoggerRecord>();
136189

137-
var telemetryLoggers = CreateTelemetryLoggers();
138-
if (telemetryLoggers != null)
190+
// Add central telemetry logger
191+
var telemetryCentralLogger = CreateTelemetryCentralLogger();
192+
if (telemetryCentralLogger != null)
139193
{
140-
allLoggers.AddRange(telemetryLoggers);
194+
allLoggers.Add(telemetryCentralLogger);
195+
196+
// Add forwarding logger for distributed builds
197+
allForwardingLoggers.AddRange(CreateTelemetryForwardingLoggerRecords());
141198
}
142199

143200
if (loggers != null)
144201
{
145202
allLoggers.AddRange(loggers);
146203
}
147204

148-
return projectInstance.Build(targets, allLoggers.Count > 0 ? allLoggers : null, remoteLoggers, out targetOutputs);
205+
if (remoteLoggers != null)
206+
{
207+
allForwardingLoggers.AddRange(remoteLoggers);
208+
}
209+
210+
return projectInstance.Build(
211+
targets,
212+
allLoggers.Count > 0 ? allLoggers : null,
213+
allForwardingLoggers.Count > 0 ? allForwardingLoggers : null,
214+
out targetOutputs);
215+
}
216+
217+
/// <summary>
218+
/// Creates a logger collection that includes the telemetry central logger.
219+
/// This is useful for ProjectCollection scenarios where evaluation needs telemetry.
220+
/// </summary>
221+
/// <param name="additionalLoggers">Additional loggers to include in the collection.</param>
222+
/// <returns>An array of loggers including telemetry logger if enabled, or null if no loggers.</returns>
223+
public static ILogger[]? CreateLoggersWithTelemetry(IEnumerable<ILogger>? additionalLoggers = null)
224+
{
225+
var loggers = new List<ILogger>();
226+
227+
// Add central telemetry logger for evaluation
228+
var telemetryCentralLogger = CreateTelemetryCentralLogger();
229+
if (telemetryCentralLogger != null)
230+
{
231+
loggers.Add(telemetryCentralLogger);
232+
}
233+
234+
if (additionalLoggers != null)
235+
{
236+
loggers.AddRange(additionalLoggers);
237+
}
238+
239+
return loggers.Count > 0 ? loggers.ToArray() : null;
149240
}
150241
}

‎test/dotnet.Tests/CommandTests/MSBuild/GivenProjectInstanceExtensions.cs‎

Lines changed: 42 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,30 +12,29 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests;
1212
public class GivenProjectInstanceExtensions
1313
{
1414
[Fact]
15-
public void CreateTelemetryLoggers_WhenTelemetryDisabled_ReturnsNull()
15+
public void CreateTelemetryCentralLogger_WhenTelemetryDisabled_ReturnsNull()
1616
{
1717
// Ensure telemetry is disabled
1818
Telemetry.Telemetry.CurrentSessionId = null;
1919

20-
var loggers = ProjectInstanceExtensions.CreateTelemetryLoggers();
20+
var logger = ProjectInstanceExtensions.CreateTelemetryCentralLogger();
2121

22-
loggers.Should().BeNull();
22+
logger.Should().BeNull();
2323
}
2424

2525
[Fact]
26-
public void CreateTelemetryLoggers_WhenTelemetryEnabled_ReturnsLoggers()
26+
public void CreateTelemetryCentralLogger_WhenTelemetryEnabled_ReturnsLogger()
2727
{
2828
// Enable telemetry with a session ID
2929
var originalSessionId = Telemetry.Telemetry.CurrentSessionId;
3030
try
3131
{
3232
Telemetry.Telemetry.CurrentSessionId = Guid.NewGuid().ToString();
3333

34-
var loggers = ProjectInstanceExtensions.CreateTelemetryLoggers();
34+
var logger = ProjectInstanceExtensions.CreateTelemetryCentralLogger();
3535

36-
loggers.Should().NotBeNull();
37-
loggers.Should().HaveCount(1);
38-
loggers[0].Should().BeOfType<Commands.MSBuild.MSBuildLogger>();
36+
logger.Should().NotBeNull();
37+
logger.Should().BeOfType<Commands.MSBuild.MSBuildLogger>();
3938
}
4039
finally
4140
{
@@ -45,32 +44,55 @@ public void CreateTelemetryLoggers_WhenTelemetryEnabled_ReturnsLoggers()
4544
}
4645

4746
[Fact]
48-
public void BuildWithTelemetry_WhenTelemetryDisabled_CallsBuildWithoutTelemetryLogger()
47+
public void CreateTelemetryForwardingLoggerRecords_WhenTelemetryDisabled_ReturnsEmpty()
4948
{
50-
// This is a basic smoke test to ensure the extension method doesn't throw
51-
// We can't easily test the actual build without setting up a full project
52-
5349
// Ensure telemetry is disabled
5450
Telemetry.Telemetry.CurrentSessionId = null;
5551

56-
// CreateTelemetryLoggers should return null when telemetry is disabled
57-
var loggers = ProjectInstanceExtensions.CreateTelemetryLoggers();
58-
loggers.Should().BeNull();
52+
var loggerRecords = ProjectInstanceExtensions.CreateTelemetryForwardingLoggerRecords();
53+
54+
loggerRecords.Should().BeEmpty();
5955
}
6056

6157
[Fact]
62-
public void BuildWithTelemetry_WhenTelemetryEnabled_CreatesTelemetryLogger()
58+
public void CreateTelemetryForwardingLoggerRecords_WhenTelemetryEnabled_ReturnsLoggerRecords()
6359
{
6460
// Enable telemetry with a session ID
6561
var originalSessionId = Telemetry.Telemetry.CurrentSessionId;
6662
try
6763
{
6864
Telemetry.Telemetry.CurrentSessionId = Guid.NewGuid().ToString();
6965

70-
// CreateTelemetryLoggers should return logger when telemetry is enabled
71-
var loggers = ProjectInstanceExtensions.CreateTelemetryLoggers();
72-
loggers.Should().NotBeNull();
73-
loggers.Should().HaveCount(1);
66+
var loggerRecords = ProjectInstanceExtensions.CreateTelemetryForwardingLoggerRecords();
67+
68+
loggerRecords.Should().NotBeEmpty();
69+
loggerRecords.Should().HaveCount(1);
70+
// ForwardingLoggerRecord contains the ForwardingLogger and LoggerDescription
71+
loggerRecords[0].Should().NotBeNull();
72+
}
73+
finally
74+
{
75+
// Restore original session ID
76+
Telemetry.Telemetry.CurrentSessionId = originalSessionId;
77+
}
78+
}
79+
80+
[Fact]
81+
public void BuildWithTelemetry_WhenTelemetryEnabled_CreatesDistributedLogger()
82+
{
83+
// Enable telemetry with a session ID
84+
var originalSessionId = Telemetry.Telemetry.CurrentSessionId;
85+
try
86+
{
87+
Telemetry.Telemetry.CurrentSessionId = Guid.NewGuid().ToString();
88+
89+
// CreateTelemetryCentralLogger should return logger when telemetry is enabled
90+
var centralLogger = ProjectInstanceExtensions.CreateTelemetryCentralLogger();
91+
centralLogger.Should().NotBeNull();
92+
93+
// CreateTelemetryForwardingLoggerRecords should return forwarding logger when telemetry is enabled
94+
var forwardingLoggers = ProjectInstanceExtensions.CreateTelemetryForwardingLoggerRecords();
95+
forwardingLoggers.Should().NotBeEmpty();
7496
}
7597
finally
7698
{

0 commit comments

Comments
 (0)