Skip to content

Use logger factory instead of console for codeowners library logging #12022

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

Merged
benbp merged 1 commit into from
Sep 10, 2025
Merged

Use logger factory instead of console for codeowners library logging #12022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 7 additions & 7 deletions tools/azsdk-cli/Azure.Sdk.Tools.Cli.Tests/Tools/CodeownersToolsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ public void Setup()
_mockLogger = new Mock<ILogger<CodeownersTools>>();
_mockCodeownersValidator = new Mock<ICodeownersValidatorHelper>();


_tool = new CodeownersTools(
_mockGithub,
_mockOutput.Object,
_mockLogger.Object,
null,
_mockCodeownersValidator.Object);
}

Expand Down Expand Up @@ -63,7 +63,7 @@ public async Task Update_LabelMissing_NotInReview_NoPath_Throws()
gh.Setup(s => s.GetContentsSingleAsync(Constants.AZURE_OWNER_PATH, "repo", Constants.AZURE_CODEOWNERS_PATH, It.IsAny<string>()))
.ReturnsAsync(new RepositoryContent("CODEOWNERS", ".github/CODEOWNERS", "shaCode", 0, ContentType.File, null, null, null, null, "utf-8", Convert.ToBase64String(System.Text.Encoding.UTF8.GetBytes("")), null, null));

var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, _mockCodeownersValidator.Object);
var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, null, _mockCodeownersValidator.Object);

var result = await tool.UpdateCodeowners("repo", false, "", "NonExistService", new List<string>(), new List<string>(), true);

Expand Down Expand Up @@ -92,7 +92,7 @@ public async Task Update_AddOwnersToExistingEntry_Success()
var validator = new Mock<ICodeownersValidatorHelper>();
validator.Setup(v => v.ValidateCodeOwnerAsync(It.IsAny<string>(), It.IsAny<bool>())).ReturnsAsync(new CodeownersValidationResult { Username = "user", IsValidCodeOwner = true });

var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, validator.Object);
var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, null, validator.Object);
var result = await tool.UpdateCodeowners("repoName", false, "/sdk/myservice/", "MyService", new List<string> { "@oldowner", "@newowner" }, new List<string>() { "@newowner", "@newowner2" }, true);
Assert.IsNotNull(result);
Assert.IsTrue(result.Contains("URL:") || result.Contains("Created"));
Expand All @@ -119,7 +119,7 @@ public async Task Update_RemoveOwnersFromExistingEntry_Success()
var validator = new Mock<ICodeownersValidatorHelper>();
validator.Setup(v => v.ValidateCodeOwnerAsync(It.IsAny<string>(), It.IsAny<bool>())).ReturnsAsync(new CodeownersValidationResult { Username = "user", IsValidCodeOwner = true });

var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, validator.Object);
var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, null, validator.Object);
// Remove @removeowner, keep @oldowner
var result = await tool.UpdateCodeowners("repoName", false, "/sdk/myservice/", "MyService", new List<string> { "@oldowner" }, new List<string>(), false);
Assert.IsNotNull(result);
Expand Down Expand Up @@ -160,7 +160,7 @@ public async Task Update_CreateNewEntry_PRSuccess()
validator.Setup(v => v.ValidateCodeOwnerAsync(It.IsAny<string>(), It.IsAny<bool>()))
.ReturnsAsync(new CodeownersValidationResult { Username = "user", IsValidCodeOwner = true });

var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, validator.Object);
var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, null, validator.Object);

var result = await tool.UpdateCodeowners("repoName", false, "/sdk/newsvc/", "NewSvc", new List<string> { "@a", "@b" }, new List<string> { "@s1", "@s2" }, true);
Assert.IsNotNull(result);
Expand Down Expand Up @@ -196,7 +196,7 @@ public async Task Update_CreateNewEntry_PRFailure_ReturnsPRFailureMessage()
validator.Setup(v => v.ValidateCodeOwnerAsync(It.IsAny<string>(), It.IsAny<bool>()))
.ReturnsAsync(new CodeownersValidationResult { Username = "user", IsValidCodeOwner = true });

var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, validator.Object);
var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, null, validator.Object);

var result = await tool.UpdateCodeowners("repoName", false, "/sdk/newsvc/", "NewSvc", new List<string> { "@a", "@b" }, new List<string> { "@s1", "@s2" }, true);
Assert.IsNotNull(result);
Expand Down Expand Up @@ -239,7 +239,7 @@ public async Task Validate_CODEOWNERS_Missing_ReturnsError()
var output = new Mock<IOutputHelper>();
var validator = new Mock<ICodeownersValidatorHelper>();

var tool = new CodeownersTools(gh.Object, output.Object, _mockLogger.Object, validator.Object);
var tool = new CodeownersTools(gh.Object, output.Object, _mockLogger.Object, null, validator.Object);

var result = await tool.ValidateCodeownersEntryForService("test-repo", "Any", null);
Assert.IsNotNull(result);
Expand Down
5 changes: 4 additions & 1 deletion tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/EngSys/CodeownersTools/CodeownersTools.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
using Azure.Sdk.Tools.Cli.Helpers;
using Azure.Sdk.Tools.Cli.Services;
using Azure.Sdk.Tools.Cli.Contract;
using Azure.Sdk.Tools.CodeownersUtils.Parsing;
using Azure.Sdk.Tools.CodeownersUtils.Editing;
using Azure.Sdk.Tools.CodeownersUtils.Parsing;
using Azure.Sdk.Tools.Cli.Configuration;
using Azure.Sdk.Tools.Cli.Models.Responses;
using Azure.Sdk.Tools.Cli.Commands;
Expand Down Expand Up @@ -47,13 +47,16 @@ public CodeownersTools(
IGitHubService githubService,
IOutputHelper output,
ILogger<CodeownersTools> logger,
ILoggerFactory? loggerFactory,
ICodeownersValidatorHelper codeownersValidator) : base()
{
this.githubService = githubService;
this.output = output;
this.logger = logger;
this.codeownersValidator = codeownersValidator;

CodeownersUtils.Utils.Log.Configure(loggerFactory);

CommandHierarchy =
[
SharedCommandGroups.EngSys
Expand Down
15 changes: 8 additions & 7 deletions tools/codeowners-utils/Azure.Sdk.Tools.CodeownersLinter.Tests/TestHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Azure.Sdk.Tools.CodeownersUtils.Caches;
using Azure.Sdk.Tools.CodeownersUtils.Parsing;
using Azure.Sdk.Tools.CodeownersUtils.Utils;
using Microsoft.Extensions.Logging;

namespace Azure.Sdk.Tools.CodeownersUtils.Tests
{
Expand All @@ -26,7 +27,7 @@ public static class TestHelpers
/// The user/org visibility data will consist of 5 users, TestOwner0..TestOwner4, with only even users
/// being visible.
/// The team/user data will consist of 5 teams with the number of users in each team equal to the
/// team number, TestTeam0...TestTeam4.The users in each team will consist of the same users in
/// team number, TestTeam0...TestTeam4.The users in each team will consist of the same users in
/// the user/org data.
/// </summary>
/// <returns>Populated OwnerDataUtils</returns>
Expand All @@ -38,7 +39,7 @@ public static OwnerDataUtils SetupOwnerData()
Dictionary<string, List<string>> teamUserDict = new Dictionary<string, List<string>>(StringComparer.InvariantCultureIgnoreCase);

// Create 5 teams
for (int i = 0;i < _maxItems; i++)
for (int i = 0; i < _maxItems; i++)
{
string team = $"{TestTeamNamePartial}{i}";
List<string> users = new List<string>();
Expand All @@ -58,7 +59,7 @@ public static OwnerDataUtils SetupOwnerData()
UserOrgVisibilityCache userOrgVisibilityCache = new UserOrgVisibilityCache(null);
Dictionary<string, bool> userOrgVisDict = new Dictionary<string, bool>(StringComparer.InvariantCultureIgnoreCase);
// Make every even user visible
for (int i = 0;i < 5;i++)
for (int i = 0; i < 5; i++)
{
string user = $"{TestOwnerNamePartial}{i}";
if (i % 2 == 0)
Expand All @@ -82,7 +83,7 @@ public static RepoLabelDataUtils SetupRepoLabelData()
{
Dictionary<string, HashSet<string>> repoLabelDict = new Dictionary<string, HashSet<string>>(StringComparer.InvariantCultureIgnoreCase);
HashSet<string> repoLabels = new HashSet<string>(StringComparer.InvariantCultureIgnoreCase);
for (int i = 0;i < 5;i++)
for (int i = 0; i < 5; i++)
{
string label = $"{TestLabelNamePartial}{i}";
repoLabels.Add(label);
Expand Down Expand Up @@ -112,7 +113,7 @@ public static bool StringListsAreEqual(List<string> actuaList, List<string> expe
/// <param name="actualCodeownerEntries">The actual, or parsed, list of Codeowners entries.</param>
/// <param name="expectedCodeownerEntries">The expected list of Codeowners entries</param>
/// <returns>True if they're equal, false otherwise</returns>
public static bool CodeownersEntryListsAreEqual(List<CodeownersEntry> actualCodeownerEntries,
public static bool CodeownersEntryListsAreEqual(List<CodeownersEntry> actualCodeownerEntries,
List<CodeownersEntry> expectedCodeownerEntries)
{
// SequenceEqual determines whether two sequences are equal by comparing the length and
Expand Down Expand Up @@ -190,8 +191,8 @@ public static void TempGetSerializedString(List<CodeownersEntry> codeownersEntri
WriteIndented = true
};
string jsonString = JsonSerializer.Serialize(codeownersEntries, jsonSerializerOptions);
Console.WriteLine(jsonString);
Console.WriteLine("something to break on");
Log.Logger.LogInformation("{Json}", jsonString);
Log.Logger.LogInformation("something to break on");
}
}
}
9 changes: 5 additions & 4 deletions tools/codeowners-utils/Azure.Sdk.Tools.CodeownersLinter.Tests/Utils/BaselineUtilsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Azure.Sdk.Tools.CodeownersUtils.Utils;
using Azure.Sdk.Tools.CodeownersUtils.Verification;
using NUnit.Framework;
using Microsoft.Extensions.Logging;

namespace Azure.Sdk.Tools.CodeownersUtils.Tests.Utils
{
Expand Down Expand Up @@ -43,7 +44,7 @@ public void CleanupTestData()
// Cleanup any temporary files created by the tests
if (_tempFiles.Count > 0)
{
foreach(string tempFile in _tempFiles)
foreach (string tempFile in _tempFiles)
{
int tryNumber = 0;
while (tryNumber < maxTries)
Expand All @@ -55,7 +56,7 @@ public void CleanupTestData()
}
catch (Exception ex)
{
Console.WriteLine($"Exception trying to delete {tempFile}. Ex={ex}");
Log.Logger.LogError(ex, "Exception trying to delete {TempFile}.", tempFile);
// sleep 100ms and try again
System.Threading.Thread.Sleep(100);
}
Expand All @@ -64,7 +65,7 @@ public void CleanupTestData()
// Report that the test wasn't able to delete the file after successive tries
if (tryNumber == maxTries)
{
Console.WriteLine($"Unable to delete {tempFile} after {maxTries}, see above for exception details.");
Log.Logger.LogWarning("Unable to delete {TempFile} after {MaxTries}, see above for exception details.", tempFile, maxTries);
}
}
}
Expand All @@ -89,7 +90,7 @@ public void TestBaselineGenerationAndFiltering(string testCodeownerFile,
// Load the expected baseline file and, for sanity's sake, ensure that all errors are filtered
BaselineUtils expectedBaselineUtils = new BaselineUtils(expectedBaselineFile);
var filteredWithExpected = expectedBaselineUtils.FilterErrorsUsingBaseline(actualErrors);

// This piece is for sanity, to verify that the the filter only filters out SingleLineErrors and leaves
// the BlockFormattingErrors.
if (expectedNumberOfBlockErrors > 0)
Expand Down
1 change: 1 addition & 0 deletions ...codeowners-utils/Azure.Sdk.Tools.CodeownersLinter/Azure.Sdk.Tools.CodeownersLinter.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

<ItemGroup>
<PackageReference Include="System.CommandLine" Version="2.0.0-beta4.22272.1" />
<PackageReference Include="Microsoft.Extensions.Logging.Console" Version="8.0.0" />
</ItemGroup>

<ItemGroup>
Expand Down
47 changes: 28 additions & 19 deletions tools/codeowners-utils/Azure.Sdk.Tools.CodeownersLinter/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.IO;
using System.Collections.Generic;
using System.Linq;
using Microsoft.Extensions.Logging;

namespace Azure.Sdk.Tools.CodeownersLinter
{
Expand All @@ -20,6 +21,14 @@ static void Main(string[] args)
Stopwatch stopWatch = new Stopwatch();
stopWatch.Start();

// Configure logging for the application and pass the logger to the library helper.
var loggerFactory = LoggerFactory.Create(builder =>
{
builder.AddSimpleConsole();
builder.SetMinimumLevel(LogLevel.Information);
});
Log.Configure(loggerFactory.CreateLogger(typeof(Program).Namespace));

// The storage URIs are in the azure-sdk-write-teams-container-blobs pipeline variable group.
var teamUserBlobStorageUriOption = new Option<string>
(name: "--teamUserBlobStorageURI",
Expand Down Expand Up @@ -123,17 +132,17 @@ static void Main(string[] args)
string elapsedTime = String.Format("{0:00}:{1:00}:{2:00}.{3:00}",
ts.Hours, ts.Minutes, ts.Seconds,
ts.Milliseconds / 10);
Console.WriteLine($"Total run time: {elapsedTime}");
Log.Logger.LogInformation("Total run time: {ElapsedTime}", elapsedTime);

Console.WriteLine($"Exiting with return code {returnCode}");
Log.Logger.LogInformation("Exiting with return code {ReturnCode}", returnCode);
Environment.Exit(returnCode);
}

/// <summary>
/// Verify the arguments and call to process the CODEOWNERS file. If errors are being filtered with a
/// Verify the arguments and call to process the CODEOWNERS file. If errors are being filtered with a
/// baseline, or used to regenerate the baseline, that's done in here. Note that filtering errors and
/// regenerating the baseline cannot both be done in the same run.
///
///
/// The baseBranchBaselineFile
/// This file will be primarily used in PR validation where two calls will be made. be made. The first
/// call will use the -gbl option and generate the secondary file to a different location. It can't use
Expand All @@ -156,13 +165,13 @@ static void Main(string[] args)
/// <param name="baseBranchBaselineFile">The name of the base branch baseline file to generate or use.</param>
/// <returns>integer, used to set the return code</returns>
/// <exception cref="ArgumentException">Thrown if any arguments, or argument combinations, are invalid.</exception>
static int LintCodeownersFile(string teamUserBlobStorageUri,
string userOrgVisibilityBlobStorageUri,
string repoLabelBlobStorageUri,
string repoRoot,
static int LintCodeownersFile(string teamUserBlobStorageUri,
string userOrgVisibilityBlobStorageUri,
string repoLabelBlobStorageUri,
string repoRoot,
string repoName,
bool filterBaselineErrors,
bool generateBaseline,
bool filterBaselineErrors,
bool generateBaseline,
string baseBranchBaselineFile)
{
// Don't allow someone to create and use a baseline in the same run
Expand Down Expand Up @@ -213,15 +222,15 @@ static int LintCodeownersFile(string teamUserBlobStorageUri,
}
else
{
Console.WriteLine($"The CODEOWNERS baseline error file, {codeownersBaselineFile}, file for {repoName} does not exist. No filtering will be done for errors.");
Log.Logger.LogWarning("The CODEOWNERS baseline error file, {BaselineFile}, file for {RepoName} does not exist. No filtering will be done for errors.", codeownersBaselineFile, repoName);
}
}

DirectoryUtils directoryUtils = new DirectoryUtils(repoRoot);
OwnerDataUtils ownerData = new OwnerDataUtils(teamUserBlobStorageUri, userOrgVisibilityBlobStorageUri);

var errors = CodeownersUtils.Verification.CodeownersLinter.LintCodeownersFile(directoryUtils,
ownerData,
var errors = CodeownersUtils.Verification.CodeownersLinter.LintCodeownersFile(directoryUtils,
ownerData,
repoLabelData,
codeownersFileFullPath);

Expand Down Expand Up @@ -251,7 +260,7 @@ static int LintCodeownersFile(string teamUserBlobStorageUri,
{
if (errors.Count == 0)
{
Console.WriteLine($"##vso[task.LogIssue type=warning;]There were no CODEOWNERS parsing errors but there is a baseline file {codeownersBaselineFile} for filtering. If the file is empty, or all errors have been fixed, then it should be deleted.");
Log.Logger.LogWarning("##vso[task.LogIssue type=warning;]There were no CODEOWNERS parsing errors but there is a baseline file {BaselineFile} for filtering. If the file is empty, or all errors have been fixed, then it should be deleted.", codeownersBaselineFile);
}
else
{
Expand Down Expand Up @@ -279,11 +288,11 @@ static int LintCodeownersFile(string teamUserBlobStorageUri,
// DevOps only adds the first 4 errors to the github checks list so lets always add the generic one first and then as many of the individual ones as can be found afterwards
if (loggingInDevOps)
{
Console.WriteLine($"##vso[task.logissue type=error;]There are linter errors. Please visit {linterErrorsHelpLink} for guidance on how to handle them.");
Log.Logger.LogError("##vso[task.logissue type=error;]There are linter errors. Please visit {HelpLink} for guidance on how to handle them.", linterErrorsHelpLink);
}
else
{
Console.WriteLine($"There are linter errors. Please visit {linterErrorsHelpLink} for guidance on how to handle them.");
Log.Logger.LogError("There are linter errors. Please visit {HelpLink} for guidance on how to handle them.", linterErrorsHelpLink);
}

// Output the errors sorted ascending by line number and by type. If there's a block
Expand All @@ -296,11 +305,11 @@ static int LintCodeownersFile(string teamUserBlobStorageUri,
if (loggingInDevOps)
{
// Environment.NewLine needs to be replaced by an encoded NewLine "%0D%0A" in order to display on GitHub and DevOps checks
Console.WriteLine($"##vso[task.logissue type=error;sourcepath={codeownersFileFullPath};linenumber={error.LineNumber};columnnumber=1;]{error.ToString().Replace(Environment.NewLine,"%0D%0A")}");
Log.Logger.LogError("##vso[task.logissue type=error;sourcepath={SourcePath};linenumber={LineNumber};columnnumber=1;]{Error}", codeownersFileFullPath, error.LineNumber, error.ToString().Replace(Environment.NewLine, "%0D%0A"));
}
else
else
{
Console.WriteLine(error + Environment.NewLine);
Log.Logger.LogError("{Error}{NewLine}", error, Environment.NewLine);
}
}
}
Expand Down
Loading
Loading