Skip to content

Commit f19a245

Browse files
davidfowlCopilot
andauthored
Fix the logging pipeline so that logger provider filtering works (#12476)
* Fix the logging pipeline so that logger provider filtering works - Including stack traces in logs of steps is another flag - Step loggers are normal ILoggers and Logging to the current step is via an async local and PipelineLoggerProvider. * Update src/Aspire.Hosting/Pipelines/PipelineLoggerProvider.cs Co-authored-by: Copilot <[email protected]> * Update src/Aspire.Hosting/Pipelines/PipelineLoggerProvider.cs * Refactor pipeline activity reporting and command argument handling for improved clarity and functionality --------- Co-authored-by: Copilot <[email protected]>
1 parent 6a49433 commit f19a245

18 files changed

+191
-240
lines changed

src/Aspire.Cli/Commands/DeployCommand.cs

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ namespace Aspire.Cli.Commands;
1515
internal sealed class DeployCommand : PipelineCommandBase
1616
{
1717
private readonly Option<bool> _clearCacheOption;
18-
private readonly Option<string?> _stepOption;
1918

2019
public DeployCommand(IDotNetCliRunner runner, IInteractionService interactionService, IProjectLocator projectLocator, AspireCliTelemetry telemetry, IDotNetSdkInstaller sdkInstaller, IFeatures features, ICliUpdateNotifier updateNotifier, CliExecutionContext executionContext, ICliHostEnvironment hostEnvironment)
2120
: base("deploy", DeployCommandStrings.Description, runner, interactionService, projectLocator, telemetry, sdkInstaller, features, updateNotifier, executionContext, hostEnvironment)
@@ -25,12 +24,6 @@ public DeployCommand(IDotNetCliRunner runner, IInteractionService interactionSer
2524
Description = "Clear the deployment cache associated with the current environment and do not save deployment state"
2625
};
2726
Options.Add(_clearCacheOption);
28-
29-
_stepOption = new Option<string?>("--step")
30-
{
31-
Description = "Run a specific deployment step and its dependencies"
32-
};
33-
Options.Add(_stepOption);
3427
}
3528

3629
protected override string OperationCompletedPrefix => DeployCommandStrings.OperationCompletedPrefix;
@@ -60,21 +53,16 @@ protected override string[] GetRunArguments(string? fullyQualifiedOutputPath, st
6053
baseArgs.AddRange(["--log-level", logLevel!]);
6154
}
6255

63-
var environment = parseResult.GetValue(_environmentOption);
64-
if (!string.IsNullOrEmpty(environment))
56+
var includeExceptionDetails = parseResult.GetValue(_includeExceptionDetailsOption);
57+
if (includeExceptionDetails)
6558
{
66-
baseArgs.AddRange(["--environment", environment!]);
59+
baseArgs.AddRange(["--include-exception-details", "true"]);
6760
}
6861

69-
var step = parseResult.GetValue(_stepOption);
70-
if (step != null)
71-
{
72-
baseArgs.AddRange(["--step", step]);
73-
}
74-
else
62+
var environment = parseResult.GetValue(_environmentOption);
63+
if (!string.IsNullOrEmpty(environment))
7564
{
76-
// Default to the "deploy" step
77-
baseArgs.AddRange(["--step", "deploy"]);
65+
baseArgs.AddRange(["--environment", environment!]);
7866
}
7967

8068
baseArgs.AddRange(unmatchedTokens);
@@ -86,7 +74,6 @@ protected override string[] GetRunArguments(string? fullyQualifiedOutputPath, st
8674

8775
protected override string GetProgressMessage(ParseResult parseResult)
8876
{
89-
var step = parseResult.GetValue(_stepOption);
90-
return $"Executing step \"{step ?? "deploy"}\"";
77+
return "Executing step deploy";
9178
}
9279
}

src/Aspire.Cli/Commands/DoCommand.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@ protected override string[] GetRunArguments(string? fullyQualifiedOutputPath, st
5252
baseArgs.AddRange(["--log-level", logLevel!]);
5353
}
5454

55+
var includeExceptionDetails = parseResult.GetValue(_includeExceptionDetailsOption);
56+
if (includeExceptionDetails)
57+
{
58+
baseArgs.AddRange(["--include-exception-details", "true"]);
59+
}
60+
5561
var environment = parseResult.GetValue(_environmentOption);
5662
if (!string.IsNullOrEmpty(environment))
5763
{
@@ -68,6 +74,6 @@ protected override string[] GetRunArguments(string? fullyQualifiedOutputPath, st
6874
protected override string GetProgressMessage(ParseResult parseResult)
6975
{
7076
var step = parseResult.GetValue(_stepArgument);
71-
return $"Executing step \"{step}\"";
77+
return $"Executing step {step}";
7278
}
7379
}

src/Aspire.Cli/Commands/PipelineCommandBase.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ internal abstract class PipelineCommandBase : BaseCommand
3434
Description = "Set the minimum log level for pipeline logging (trace, debug, information, warning, error, critical). The default is 'information'."
3535
};
3636

37+
protected readonly Option<bool> _includeExceptionDetailsOption = new("--include-exception-details")
38+
{
39+
Description = "Include exception details (stack traces) in pipeline logs."
40+
};
41+
3742
protected readonly Option<string?> _environmentOption = new("--environment", "-e")
3843
{
3944
Description = "The environment to use for the operation. The default is 'Production'."
@@ -82,6 +87,7 @@ protected PipelineCommandBase(string name, string description, IDotNetCliRunner
8287

8388
Options.Add(_logLevelOption);
8489
Options.Add(_environmentOption);
90+
Options.Add(_includeExceptionDetailsOption);
8591

8692
// In the publish and deploy commands we forward all unrecognized tokens
8793
// through to the underlying tooling when we launch the app host.

src/Aspire.Cli/Commands/PublishCommand.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,12 @@ protected override string[] GetRunArguments(string? fullyQualifiedOutputPath, st
6363
baseArgs.AddRange(["--log-level", logLevel!]);
6464
}
6565

66+
var includeExceptionDetails = parseResult.GetValue(_includeExceptionDetailsOption);
67+
if (includeExceptionDetails)
68+
{
69+
baseArgs.AddRange(["--include-exception-details", "true"]);
70+
}
71+
6672
var environment = parseResult.GetValue(_environmentOption);
6773
if (!string.IsNullOrEmpty(environment))
6874
{
@@ -76,5 +82,5 @@ protected override string[] GetRunArguments(string? fullyQualifiedOutputPath, st
7682

7783
protected override string GetCanceledMessage() => InteractionServiceStrings.OperationCancelled;
7884

79-
protected override string GetProgressMessage(ParseResult parseResult) => "Executing step \"publish\"";
85+
protected override string GetProgressMessage(ParseResult parseResult) => "Executing step publish";
8086
}

src/Aspire.Cli/Utils/ConsoleActivityLogger.cs

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ public void Continuation(string message)
178178
}
179179
}
180180

181-
public void WriteSummary(string? dashboardUrl = null)
181+
public void WriteSummary()
182182
{
183183
lock (_lock)
184184
{
@@ -253,19 +253,6 @@ public void WriteSummary(string? dashboardUrl = null)
253253
{
254254
AnsiConsole.MarkupLine(_finalStatusHeader!);
255255
}
256-
if (!string.IsNullOrEmpty(dashboardUrl))
257-
{
258-
// Render dashboard URL as clickable link in interactive terminals, plain in non-interactive
259-
var url = dashboardUrl;
260-
if (!_hostEnvironment.SupportsInteractiveOutput || !_enableColor)
261-
{
262-
AnsiConsole.MarkupLine($"Dashboard: {url.EscapeMarkup()}");
263-
}
264-
else
265-
{
266-
AnsiConsole.MarkupLine($"Dashboard: [link={url}]{url}[/]");
267-
}
268-
}
269256
AnsiConsole.MarkupLine(line);
270257
AnsiConsole.WriteLine(); // Ensure final newline after deployment summary
271258
}

src/Aspire.Hosting/DistributedApplicationBuilder.cs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -465,9 +465,11 @@ public DistributedApplicationBuilder(DistributedApplicationOptions options)
465465
_innerBuilder.Services.AddSingleton(Pipeline);
466466

467467
// Configure pipeline logging options
468-
_innerBuilder.Services.Configure<PipelineLoggingOptions>(o =>
468+
_innerBuilder.Services.Configure<PipelineLoggingOptions>(options =>
469469
{
470-
o.MinimumLogLevel = _innerBuilder.Configuration["Pipeline:LogLevel"]?.ToLowerInvariant() switch
470+
var config = _innerBuilder.Configuration;
471+
472+
options.MinimumLogLevel = config["Pipeline:LogLevel"]?.ToLowerInvariant() switch
471473
{
472474
"trace" => LogLevel.Trace,
473475
"debug" => LogLevel.Debug,
@@ -477,6 +479,8 @@ public DistributedApplicationBuilder(DistributedApplicationOptions options)
477479
"crit" or "critical" => LogLevel.Critical,
478480
_ => LogLevel.Information
479481
};
482+
483+
options.IncludeExceptionDetails = config.GetBool("Pipeline:IncludeExceptionDetails") ?? false;
480484
});
481485

482486
_innerBuilder.Services.AddSingleton<ILoggerProvider, PipelineLoggerProvider>();
@@ -576,17 +580,26 @@ private void ConfigurePipelineOptions(DistributedApplicationOptions options)
576580
{
577581
var switchMappings = new Dictionary<string, string>()
578582
{
583+
// Legacy mappings for backward compatibility
579584
{ "--operation", "AppHost:Operation" },
580585
{ "--publisher", "Publishing:Publisher" },
586+
587+
// Pipeline options (valid for aspire do based commands)
588+
{ "--step", "Pipeline:Step" },
581589
{ "--output-path", "Pipeline:OutputPath" },
582590
{ "--log-level", "Pipeline:LogLevel" },
591+
{ "--include-exception-details", "Pipeline:IncludeExceptionDetails" },
592+
593+
// TODO: Rename this to something related to deployment state
583594
{ "--clear-cache", "Pipeline:ClearCache" },
584-
{ "--step", "Pipeline:Step" },
595+
596+
// DCP Publisher options, we should only process these in run mode
585597
{ "--dcp-cli-path", "DcpPublisher:CliPath" },
586598
{ "--dcp-container-runtime", "DcpPublisher:ContainerRuntime" },
587599
{ "--dcp-dependency-check-timeout", "DcpPublisher:DependencyCheckTimeout" },
588600
{ "--dcp-dashboard-path", "DcpPublisher:DashboardPath" }
589601
};
602+
590603
_innerBuilder.Configuration.AddCommandLine(options.Args ?? [], switchMappings);
591604

592605
// Configure PipelineOptions from the Pipeline section

src/Aspire.Hosting/Pipelines/DistributedApplicationPipeline.cs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
using Microsoft.Extensions.DependencyInjection;
1616
using Microsoft.Extensions.Hosting;
1717
using Microsoft.Extensions.Logging;
18-
using Microsoft.Extensions.Logging.Abstractions;
1918
using Microsoft.Extensions.Options;
2019

2120
namespace Aspire.Hosting.Pipelines;
@@ -538,19 +537,19 @@ async Task ExecuteStepWithDependencies(PipelineStep step)
538537
try
539538
{
540539
var activityReporter = context.Services.GetRequiredService<IPipelineActivityReporter>();
541-
var publishingStep = await activityReporter.CreateStepAsync(step.Name, context.CancellationToken).ConfigureAwait(false);
540+
var reportingStep = await activityReporter.CreateStepAsync(step.Name, context.CancellationToken).ConfigureAwait(false);
542541

543-
await using (publishingStep.ConfigureAwait(false))
542+
await using (reportingStep.ConfigureAwait(false))
544543
{
545544
var stepContext = new PipelineStepContext
546545
{
547546
PipelineContext = context,
548-
ReportingStep = publishingStep
547+
ReportingStep = reportingStep
549548
};
550549

551550
try
552551
{
553-
PipelineLoggerProvider.CurrentLogger = stepContext.Logger;
552+
PipelineLoggerProvider.CurrentStep = reportingStep;
554553

555554
await ExecuteStepAsync(step, stepContext).ConfigureAwait(false);
556555
}
@@ -559,12 +558,12 @@ async Task ExecuteStepWithDependencies(PipelineStep step)
559558
stepContext.Logger.LogError(ex, "Step '{StepName}' failed.", step.Name);
560559

561560
// Report the failure to the activity reporter before disposing
562-
await publishingStep.FailAsync(ex.Message, CancellationToken.None).ConfigureAwait(false);
561+
await reportingStep.FailAsync(ex.Message).ConfigureAwait(false);
563562
throw;
564563
}
565564
finally
566565
{
567-
PipelineLoggerProvider.CurrentLogger = NullLogger.Instance;
566+
PipelineLoggerProvider.CurrentStep = null;
568567
}
569568
}
570569

src/Aspire.Hosting/Pipelines/IPipelineActivityReporter.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ public interface IPipelineActivityReporter
2424
/// </summary>
2525
/// <param name="completionMessage">The completion message of the publishing process.</param>
2626
/// <param name="completionState">The completion state of the publishing process. When null, the state is automatically aggregated from all steps.</param>
27-
/// <param name="isDeploy">Whether this is a deployment operation rather than a publishing operation.</param>
2827
/// <param name="cancellationToken">The cancellation token.</param>
29-
Task CompletePublishAsync(string? completionMessage = null, CompletionState? completionState = null, bool isDeploy = false, CancellationToken cancellationToken = default);
28+
Task CompletePublishAsync(string? completionMessage = null, CompletionState? completionState = null, CancellationToken cancellationToken = default);
3029
}

src/Aspire.Hosting/Pipelines/NullPipelineActivityReporter.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public Task<IReportingStep> CreateStepAsync(string title, CancellationToken canc
2121
}
2222

2323
/// <inheritdoc />
24-
public Task CompletePublishAsync(string? completionMessage = null, CompletionState? completionState = null, bool isDeploy = false, CancellationToken cancellationToken = default)
24+
public Task CompletePublishAsync(string? completionMessage = null, CompletionState? completionState = null, CancellationToken cancellationToken = default)
2525
{
2626
return Task.CompletedTask;
2727
}

src/Aspire.Hosting/Pipelines/PipelineActivityReporter.cs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -228,12 +228,11 @@ public async Task CompleteTaskAsync(ReportingTask task, CompletionState completi
228228
await ActivityItemUpdated.Writer.WriteAsync(state, cancellationToken).ConfigureAwait(false);
229229
}
230230

231-
public async Task CompletePublishAsync(string? completionMessage = null, CompletionState? completionState = null, bool isDeploy = false, CancellationToken cancellationToken = default)
231+
public async Task CompletePublishAsync(string? completionMessage = null, CompletionState? completionState = null, CancellationToken cancellationToken = default)
232232
{
233233
// Use provided state or aggregate from all steps
234234
var finalState = completionState ?? CalculateOverallAggregatedState();
235235

236-
var operationName = "Pipeline";
237236
var state = new PublishingActivity
238237
{
239238
Type = PublishingActivityTypes.PublishComplete,
@@ -242,10 +241,10 @@ public async Task CompletePublishAsync(string? completionMessage = null, Complet
242241
Id = PublishingActivityTypes.PublishComplete,
243242
StatusText = completionMessage ?? finalState switch
244243
{
245-
CompletionState.Completed => $"{operationName} completed successfully",
246-
CompletionState.CompletedWithWarning => $"{operationName} completed with warnings",
247-
CompletionState.CompletedWithError => $"{operationName} completed with errors",
248-
_ => $"{operationName} completed"
244+
CompletionState.Completed => "Pipeline completed successfully",
245+
CompletionState.CompletedWithWarning => "Pipeline completed with warnings",
246+
CompletionState.CompletedWithError => "Pipeline completed with errors",
247+
_ => "Pipeline completed"
249248
},
250249
CompletionState = ToBackchannelCompletionState(finalState)
251250
}

0 commit comments

Comments
 (0)