Skip to content

Refactor command logging a tad #1547

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.

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,9 @@ csharp_style_namespace_declarations = file_scoped
dotnet_analyzer_diagnostic.severity = warning
dotnet_analyzer_diagnostic.category-Style.severity = warning

# can be made static
dotnet_diagnostic.CA1822.severity = suggestion

# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1848
dotnet_diagnostic.CA1848.severity = suggestion
# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca2201
Expand Down
4 changes: 2 additions & 2 deletions src/Elastic.ApiExplorer/OpenApiGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ public record ApiEndpoint(List<ApiOperation> Operations, string? Name) : IApiGro
public Task RenderAsync(FileSystemStream stream, ApiRenderContext context, CancellationToken ctx = default) => Task.CompletedTask;
}

public class OpenApiGenerator(BuildContext context, IMarkdownStringRenderer markdownStringRenderer, ILoggerFactory logger)
public class OpenApiGenerator(ILoggerFactory logFactory, BuildContext context, IMarkdownStringRenderer markdownStringRenderer)
{
private readonly ILogger _logger = logger.CreateLogger<OpenApiGenerator>();
private readonly ILogger _logger = logFactory.CreateLogger<OpenApiGenerator>();
private readonly IFileSystem _writeFileSystem = context.WriteFileSystem;
private readonly StaticFileContentHashProvider _contentHashProvider = new(new EmbeddedOrPhysicalFileProvider(context));

Expand Down
4 changes: 2 additions & 2 deletions src/Elastic.Markdown/DocumentationGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public class DocumentationGenerator

public DocumentationGenerator(
DocumentationSet docSet,
ILoggerFactory logger,
ILoggerFactory logFactory,
INavigationHtmlWriter? navigationHtmlWriter = null,
IDocumentationFileOutputProvider? documentationFileOutputProvider = null,
IMarkdownExporter[]? markdownExporters = null,
Expand All @@ -67,7 +67,7 @@ public DocumentationGenerator(
_documentationFileOutputProvider = documentationFileOutputProvider;
_conversionCollector = conversionCollector;
_writeFileSystem = docSet.Context.WriteFileSystem;
_logger = logger.CreateLogger(nameof(DocumentationGenerator));
_logger = logFactory.CreateLogger(nameof(DocumentationGenerator));

DocumentationSet = docSet;
Context = docSet.Context;
Expand Down
4 changes: 2 additions & 2 deletions src/Elastic.Markdown/IO/DocumentationSet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public class DocumentationSet : INavigationLookups, IPositionalNavigation

public DocumentationSet(
BuildContext context,
ILoggerFactory logger,
ILoggerFactory logFactory,
ICrossLinkResolver? linkResolver = null,
TableOfContentsTreeCollector? treeCollector = null
)
Expand All @@ -144,7 +144,7 @@ public DocumentationSet(
SourceDirectory = context.DocumentationSourceDirectory;
OutputDirectory = context.DocumentationOutputDirectory;
LinkResolver =
linkResolver ?? new CrossLinkResolver(new ConfigurationCrossLinkFetcher(context.Configuration, Aws3LinkIndexReader.CreateAnonymous(), logger));
linkResolver ?? new CrossLinkResolver(new ConfigurationCrossLinkFetcher(logFactory, context.Configuration, Aws3LinkIndexReader.CreateAnonymous()));
Configuration = context.Configuration;
EnabledExtensions = InstantiateExtensions();
treeCollector ??= new TableOfContentsTreeCollector();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

namespace Elastic.Markdown.Links.CrossLinks;

public class ConfigurationCrossLinkFetcher(ConfigurationFile configuration, ILinkIndexReader linkIndexProvider, ILoggerFactory logger) : CrossLinkFetcher(linkIndexProvider, logger)
public class ConfigurationCrossLinkFetcher(ILoggerFactory logFactory, ConfigurationFile configuration, ILinkIndexReader linkIndexProvider) : CrossLinkFetcher(logFactory, linkIndexProvider)
{
public override async Task<FetchedCrossLinks> Fetch(Cancel ctx)
{
Expand Down
6 changes: 3 additions & 3 deletions src/Elastic.Markdown/Links/CrossLinks/CrossLinkFetcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ public record FetchedCrossLinks
};
}

public abstract class CrossLinkFetcher(ILinkIndexReader linkIndexProvider, ILoggerFactory logger) : IDisposable
public abstract class CrossLinkFetcher(ILoggerFactory logFactory, ILinkIndexReader linkIndexProvider) : IDisposable
{
private readonly ILogger _logger = logger.CreateLogger(nameof(CrossLinkFetcher));
private readonly ILogger _logger = logFactory.CreateLogger(nameof(CrossLinkFetcher));
private readonly HttpClient _client = new();
private LinkRegistry? _linkIndex;

Expand Down Expand Up @@ -151,7 +151,7 @@ private void WriteLinksJsonCachedFile(string repository, LinkRegistryEntry linkR
public void Dispose()
{
_client.Dispose();
logger.Dispose();
logFactory.Dispose();
GC.SuppressFinalize(this);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

namespace Elastic.Markdown.Links.InboundLinks;

public class LinksIndexCrossLinkFetcher(ILinkIndexReader linkIndexProvider, ILoggerFactory logger) : CrossLinkFetcher(linkIndexProvider, logger)
public class LinksIndexCrossLinkFetcher(ILoggerFactory logFactory, ILinkIndexReader linkIndexProvider) : CrossLinkFetcher(logFactory, linkIndexProvider)
{
public override async Task<FetchedCrossLinks> Fetch(Cancel ctx)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@

namespace Elastic.Markdown.Links.InboundLinks;

public class LinkIndexLinkChecker(ILoggerFactory logger)
public class LinkIndexLinkChecker(ILoggerFactory logFactory)
{
private readonly ILogger _logger = logger.CreateLogger<LinkIndexLinkChecker>();
private readonly ILogger _logger = logFactory.CreateLogger<LinkIndexLinkChecker>();
private readonly ILinkIndexReader _linkIndexProvider = Aws3LinkIndexReader.CreateAnonymous();
private sealed record RepositoryFilter
{
Expand All @@ -25,7 +25,7 @@ private sealed record RepositoryFilter

public async Task CheckAll(IDiagnosticsCollector collector, Cancel ctx)
{
var fetcher = new LinksIndexCrossLinkFetcher(_linkIndexProvider, logger);
var fetcher = new LinksIndexCrossLinkFetcher(logFactory, _linkIndexProvider);
var resolver = new CrossLinkResolver(fetcher);
var crossLinks = await resolver.FetchLinks(ctx);

Expand All @@ -34,7 +34,7 @@ public async Task CheckAll(IDiagnosticsCollector collector, Cancel ctx)

public async Task CheckRepository(IDiagnosticsCollector collector, string? toRepository, string? fromRepository, Cancel ctx)
{
var fetcher = new LinksIndexCrossLinkFetcher(_linkIndexProvider, logger);
var fetcher = new LinksIndexCrossLinkFetcher(logFactory, _linkIndexProvider);
var resolver = new CrossLinkResolver(fetcher);
var crossLinks = await resolver.FetchLinks(ctx);
var filter = new RepositoryFilter
Expand All @@ -48,7 +48,7 @@ public async Task CheckRepository(IDiagnosticsCollector collector, string? toRep

public async Task CheckWithLocalLinksJson(IDiagnosticsCollector collector, string repository, string localLinksJson, Cancel ctx)
{
var fetcher = new LinksIndexCrossLinkFetcher(_linkIndexProvider, logger);
var fetcher = new LinksIndexCrossLinkFetcher(logFactory, _linkIndexProvider);
var resolver = new CrossLinkResolver(fetcher);
// ReSharper disable once RedundantAssignment
var crossLinks = await resolver.FetchLinks(ctx);
Expand Down
4 changes: 2 additions & 2 deletions src/authoring/Elastic.Documentation.Refactor/Move.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ public record ChangeSet(IFileInfo From, IFileInfo To);
public record Change(IFileInfo Source, string OriginalContent, string NewContent);
public record LinkModification(string OldLink, string NewLink, string SourceFile, int LineNumber, int ColumnNumber);

public partial class Move(IFileSystem readFileSystem, IFileSystem writeFileSystem, DocumentationSet documentationSet, ILoggerFactory loggerFactory)
public partial class Move(ILoggerFactory logFactory, IFileSystem readFileSystem, IFileSystem writeFileSystem, DocumentationSet documentationSet)
{

private readonly ILogger _logger = loggerFactory.CreateLogger<Move>();
private readonly ILogger _logger = logFactory.CreateLogger<Move>();
private readonly Dictionary<ChangeSet, List<Change>> _changes = [];
private readonly Dictionary<ChangeSet, List<LinkModification>> _linkModifications = [];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@

namespace Elastic.Documentation.Tooling.Diagnostics.Console;

public class ConsoleDiagnosticsCollector(ILoggerFactory loggerFactory, ICoreService? githubActions = null)
: DiagnosticsCollector([new Log(loggerFactory.CreateLogger<Log>()), new GithubAnnotationOutput(githubActions)]
public class ConsoleDiagnosticsCollector(ILoggerFactory logFactory, ICoreService? githubActions = null)
: DiagnosticsCollector([new Log(logFactory.CreateLogger<Log>()), new GithubAnnotationOutput(githubActions)]
)
{
private readonly List<Diagnostic> _errors = [];
Expand Down
10 changes: 5 additions & 5 deletions src/tooling/docs-assembler/AssembleSources.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,26 +50,26 @@ public class AssembleSources

public PublishEnvironmentUriResolver UriResolver { get; }

public static async Task<AssembleSources> AssembleAsync(ILoggerFactory logger, AssembleContext context, Checkout[] checkouts, VersionsConfiguration versionsConfiguration, Cancel ctx)
public static async Task<AssembleSources> AssembleAsync(ILoggerFactory logFactory, AssembleContext context, Checkout[] checkouts, VersionsConfiguration versionsConfiguration, Cancel ctx)
{
var sources = new AssembleSources(logger, context, checkouts, versionsConfiguration);
var sources = new AssembleSources(logFactory, context, checkouts, versionsConfiguration);
foreach (var (_, set) in sources.AssembleSets)
await set.DocumentationSet.ResolveDirectoryTree(ctx);
return sources;
}

private AssembleSources(ILoggerFactory logger, AssembleContext assembleContext, Checkout[] checkouts, VersionsConfiguration versionsConfiguration)
private AssembleSources(ILoggerFactory logFactory, AssembleContext assembleContext, Checkout[] checkouts, VersionsConfiguration versionsConfiguration)
{
AssembleContext = assembleContext;
TocTopLevelMappings = GetConfiguredSources(assembleContext);
HistoryMappings = GetHistoryMapping(assembleContext);
var linkIndexProvider = Aws3LinkIndexReader.CreateAnonymous();
var crossLinkFetcher = new AssemblerCrossLinkFetcher(logger, assembleContext.Configuration, assembleContext.Environment, linkIndexProvider);
var crossLinkFetcher = new AssemblerCrossLinkFetcher(logFactory, assembleContext.Configuration, assembleContext.Environment, linkIndexProvider);
UriResolver = new PublishEnvironmentUriResolver(TocTopLevelMappings, assembleContext.Environment);
var crossLinkResolver = new CrossLinkResolver(crossLinkFetcher, UriResolver);
AssembleSets = checkouts
.Where(c => c.Repository is { Skip: false })
.Select(c => new AssemblerDocumentationSet(logger, assembleContext, c, crossLinkResolver, TreeCollector, versionsConfiguration))
.Select(c => new AssemblerDocumentationSet(logFactory, assembleContext, c, crossLinkResolver, TreeCollector, versionsConfiguration))
.ToDictionary(s => s.Checkout.Repository.Name, s => s)
.ToFrozenDictionary();

Expand Down
8 changes: 4 additions & 4 deletions src/tooling/docs-assembler/Building/AssemblerBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ public enum ExportOption
}

public class AssemblerBuilder(
ILoggerFactory logger,
ILoggerFactory logFactory,
AssembleContext context,
GlobalNavigation navigation,
GlobalNavigationHtmlWriter writer,
GlobalNavigationPathProvider pathProvider,
ILegacyUrlMapper? legacyUrlMapper
)
{
private readonly ILogger<AssemblerBuilder> _logger = logger.CreateLogger<AssemblerBuilder>();
private readonly ILogger<AssemblerBuilder> _logger = logFactory.CreateLogger<AssemblerBuilder>();

private GlobalNavigationHtmlWriter HtmlWriter { get; } = writer;

Expand All @@ -49,7 +49,7 @@ public async Task BuildAllAsync(FrozenDictionary<string, AssemblerDocumentationS
var esExporter =
Environment.GetEnvironmentVariable("ELASTIC_API_KEY") is { } apiKey &&
Environment.GetEnvironmentVariable("ELASTIC_URL") is { } url
? new ElasticsearchMarkdownExporter(logger, context.Collector, url, apiKey)
? new ElasticsearchMarkdownExporter(logFactory, context.Collector, url, apiKey)
: null;

var markdownExporters = new List<IMarkdownExporter>(3);
Expand Down Expand Up @@ -146,7 +146,7 @@ private async Task<GenerationResult> BuildAsync(AssemblerDocumentationSet set, b
SetFeatureFlags(set);
var generator = new DocumentationGenerator(
set.DocumentationSet,
logger, HtmlWriter,
logFactory, HtmlWriter,
pathProvider,
legacyUrlMapper: LegacyUrlMapper,
positionalNavigation: navigation,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@

namespace Documentation.Assembler.Building;

public class AssemblerCrossLinkFetcher(ILoggerFactory logger, AssemblyConfiguration configuration, PublishEnvironment publishEnvironment, ILinkIndexReader linkIndexProvider)
: CrossLinkFetcher(linkIndexProvider, logger)
public class AssemblerCrossLinkFetcher(ILoggerFactory logFactory, AssemblyConfiguration configuration, PublishEnvironment publishEnvironment, ILinkIndexReader linkIndexProvider)
: CrossLinkFetcher(logFactory, linkIndexProvider)
{
public override async Task<FetchedCrossLinks> Fetch(Cancel ctx)
{
Expand Down
11 changes: 0 additions & 11 deletions src/tooling/docs-assembler/Cli/ContentSourceCommands.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
// See the LICENSE file in the project root for more information

using System.Diagnostics.CodeAnalysis;
using System.IO.Abstractions;
using Actions.Core.Services;
using ConsoleAppFramework;
Expand All @@ -16,18 +15,9 @@ namespace Documentation.Assembler.Cli;

internal sealed class ContentSourceCommands(ICoreService githubActionsService, ILoggerFactory logFactory)
{
[SuppressMessage("Usage", "CA2254:Template should be a static expression")]
private void AssignOutputLogger()
{
var log = logFactory.CreateLogger<Program>();
ConsoleApp.Log = msg => log.LogInformation(msg);
ConsoleApp.LogError = msg => log.LogError(msg);
}

[Command("validate")]
public async Task<int> Validate(Cancel ctx = default)
{
AssignOutputLogger();
await using var collector = new ConsoleDiagnosticsCollector(logFactory, githubActionsService)
{
NoHints = true
Expand Down Expand Up @@ -80,7 +70,6 @@ public async Task<int> Validate(Cancel ctx = default)
[Command("match")]
public async Task<int> Match([Argument] string? repository = null, [Argument] string? branchOrTag = null, Cancel ctx = default)
{
AssignOutputLogger();
var logger = logFactory.CreateLogger<ContentSourceCommands>();

var repo = repository ?? githubActionsService.GetInput("repository");
Expand Down
16 changes: 7 additions & 9 deletions src/tooling/docs-assembler/Cli/DeployCommands.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@

namespace Documentation.Assembler.Cli;

internal sealed class DeployCommands(ILoggerFactory logger, ICoreService githubActionsService)
internal sealed class DeployCommands(ILoggerFactory logFactory, ICoreService githubActionsService)
{
[SuppressMessage("Usage", "CA2254:Template should be a static expression")]
private void AssignOutputLogger()
{
var log = logger.CreateLogger<Program>();
var log = logFactory.CreateLogger<Program>();
ConsoleApp.Log = msg => log.LogInformation(msg);
ConsoleApp.LogError = msg => log.LogError(msg);
}
Expand All @@ -36,13 +36,13 @@ public async Task<int> Plan(
string environment, string s3BucketName, string @out = "", Cancel ctx = default)
{
AssignOutputLogger();
await using var collector = new ConsoleDiagnosticsCollector(logger, githubActionsService)
await using var collector = new ConsoleDiagnosticsCollector(logFactory, githubActionsService)
{
NoHints = true
}.StartAsync(ctx);
var assembleContext = new AssembleContext(environment, collector, new FileSystem(), new FileSystem(), null, null);
var s3Client = new AmazonS3Client();
IDocsSyncPlanStrategy planner = new AwsS3SyncPlanStrategy(s3Client, s3BucketName, assembleContext, logger);
IDocsSyncPlanStrategy planner = new AwsS3SyncPlanStrategy(logFactory, s3Client, s3BucketName, assembleContext);
var plan = await planner.Plan(ctx);
ConsoleApp.Log("Total files to sync: " + plan.Count);
ConsoleApp.Log("Total files to delete: " + plan.DeleteRequests.Count);
Expand Down Expand Up @@ -73,7 +73,7 @@ public async Task<int> Apply(
Cancel ctx = default)
{
AssignOutputLogger();
await using var collector = new ConsoleDiagnosticsCollector(logger, githubActionsService)
await using var collector = new ConsoleDiagnosticsCollector(logFactory, githubActionsService)
{
NoHints = true
}.StartAsync(ctx);
Expand All @@ -84,7 +84,7 @@ public async Task<int> Apply(
ConcurrentServiceRequests = Environment.ProcessorCount * 2,
MinSizeBeforePartUpload = AwsS3SyncPlanStrategy.PartSize
});
IDocsSyncApplyStrategy applier = new AwsS3SyncApplyStrategy(s3Client, transferUtility, s3BucketName, assembleContext, logger, collector);
IDocsSyncApplyStrategy applier = new AwsS3SyncApplyStrategy(logFactory, s3Client, transferUtility, s3BucketName, assembleContext, collector);
if (!File.Exists(planFile))
{
collector.EmitError(planFile, "Plan file does not exist.");
Expand All @@ -103,15 +103,13 @@ public async Task<int> Apply(
/// <param name="redirectsFile">Path to the redirects mapping pre-generated by docs-assembler</param>
/// <param name="ctx"></param>
[Command("update-redirects")]
[ConsoleAppFilter<StopwatchFilter>]
[ConsoleAppFilter<CatchExceptionFilter>]
public async Task<int> UpdateRedirects(
string environment,
string redirectsFile = ".artifacts/assembly/redirects.json",
Cancel ctx = default)
{
AssignOutputLogger();
await using var collector = new ConsoleDiagnosticsCollector(logger, githubActionsService)
await using var collector = new ConsoleDiagnosticsCollector(logFactory, githubActionsService)
{
NoHints = true
}.StartAsync(ctx);
Expand Down
Loading
Loading