From efc0a92cc73aa97b7c014d3f1184223401443ae7 Mon Sep 17 00:00:00 2001 From: John Wesley Walker III <81404201+jww3@users.noreply.github.com> Date: Tue, 17 Jan 2023 13:43:04 +0000 Subject: [PATCH] Let the `IssueMetadata` class be the mechanism for specifying log message overrides. --- src/Runner.Worker/ActionCommandManager.cs | 2 +- src/Runner.Worker/ExecutionContext.cs | 13 ++++-- src/Runner.Worker/Handlers/OutputManager.cs | 8 ++-- src/Runner.Worker/IssueMatcher.cs | 35 +++++++++++++--- src/Sdk/DTWebApi/WebApi/Issue.cs | 44 +++++---------------- src/Test/L0/Worker/OutputManagerL0.cs | 4 +- 6 files changed, 55 insertions(+), 51 deletions(-) diff --git a/src/Runner.Worker/ActionCommandManager.cs b/src/Runner.Worker/ActionCommandManager.cs index 42230588b..1691d0eb0 100644 --- a/src/Runner.Worker/ActionCommandManager.cs +++ b/src/Runner.Worker/ActionCommandManager.cs @@ -648,7 +648,7 @@ namespace GitHub.Runner.Worker var filteredDictionaryEntries = command.Properties .Where(kvp => !string.Equals(kvp.Key, keyToExclude, StringComparison.OrdinalIgnoreCase)); - var metadata = new IssueMetadata(issueCategory, false, filteredDictionaryEntries); + var metadata = new IssueMetadata(issueCategory, false, null, filteredDictionaryEntries); var issue = context.CreateIssue(this.Type, command.Data, metadata, true); context.AddIssue(issue); } diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index ec59b029d..1f7c6e620 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -620,10 +620,15 @@ namespace GitHub.Runner.Worker if (!string.IsNullOrEmpty(wellKnownTag)) { - if (writeToLog && !string.IsNullOrEmpty(refinedMessage)) + if (writeToLog) { - long logLineNumber = Write(wellKnownTag, refinedMessage); - result.Data["logFileLineNumber"] = logLineNumber.ToString(); + //Note that ::Write() has it's own secret masking logic + string logText = metadata?.LogMessageOverride ?? result.Message; + if (!string.IsNullOrEmpty(logText)) + { + long logLineNumber = Write(wellKnownTag, logText); + result.Data["logFileLineNumber"] = logLineNumber.ToString(); + } } if (previousCountForIssueType.GetValueOrDefault(0) < _maxIssueCount) { @@ -1192,7 +1197,7 @@ namespace GitHub.Runner.Worker // Do not add a format string overload. See comment on ExecutionContext.Write(). public static void InfrastructureError(this IExecutionContext context, string message) { - var metadata = new IssueMetadata(null, true, Enumerable.Empty>()); + var metadata = new IssueMetadata(null, true, null, Enumerable.Empty>()); var issue = context.CreateIssue(IssueType.Error, message, metadata, true); context.AddIssue(issue); } diff --git a/src/Runner.Worker/Handlers/OutputManager.cs b/src/Runner.Worker/Handlers/OutputManager.cs index 3f5301e15..6ab611b5d 100644 --- a/src/Runner.Worker/Handlers/OutputManager.cs +++ b/src/Runner.Worker/Handlers/OutputManager.cs @@ -98,7 +98,7 @@ namespace GitHub.Runner.Worker.Handlers var matchers = _matchers; // Strip color codes - var stripped = line.Contains(_colorCodePrefix) ? _colorCodeRegex.Replace(line, string.Empty) : line; + var refinedLine = line.Contains(_colorCodePrefix) ? _colorCodeRegex.Replace(line, string.Empty) : line; foreach (var matcher in matchers) { @@ -108,7 +108,7 @@ namespace GitHub.Runner.Worker.Handlers // Match try { - match = matcher.Match(stripped); + match = matcher.Match(refinedLine); break; } catch (RegexMatchTimeoutException ex) @@ -116,7 +116,7 @@ namespace GitHub.Runner.Worker.Handlers if (attempt < _maxAttempts) { // Debug - _executionContext.Debug($"Timeout processing issue matcher '{matcher.Owner}' against line '{stripped}'. Exception: {ex}"); + _executionContext.Debug($"Timeout processing issue matcher '{matcher.Owner}' against line '{refinedLine}'. Exception: {ex}"); } else { @@ -324,7 +324,7 @@ namespace GitHub.Runner.Worker.Handlers _executionContext.Debug($"Dropping file value '{match.File}' and fromPath value '{match.FromPath}'. Exception during validation: {ex}"); } - var metadata = new IssueMetadata(null, false, issueData); + var metadata = new IssueMetadata(null, false, match.SourceText, issueData); var issue = _executionContext.CreateIssue(issueType, match.Message, metadata, true); return issue; } diff --git a/src/Runner.Worker/IssueMatcher.cs b/src/Runner.Worker/IssueMatcher.cs index 0a6eb0fc4..a1e85fed5 100644 --- a/src/Runner.Worker/IssueMatcher.cs +++ b/src/Runner.Worker/IssueMatcher.cs @@ -8,6 +8,28 @@ namespace GitHub.Runner.Worker { public delegate void OnMatcherChanged(object sender, MatcherChangedEventArgs e); + + public sealed class IssueMetadata + { + public IssueMetadata(string key, string value) + : this(null, false, null, new []{ KeyValuePair.Create(key, value) }) + { + } + + public IssueMetadata(string category, bool infrastructureIssue, string logMessageOverride, IEnumerable> data) + { + this.Category = category; + this.IsInfrastructureIssue = infrastructureIssue; + this.LogMessageOverride = logMessageOverride; + this.Data = data; + } + + public readonly string Category; + public readonly bool IsInfrastructureIssue; + public readonly string LogMessageOverride; + public readonly IEnumerable> Data; + } + public sealed class MatcherChangedEventArgs : EventArgs { public MatcherChangedEventArgs(IssueMatcherConfig config) @@ -69,7 +91,7 @@ namespace GitHub.Runner.Worker if (regexMatch.Success) { - return new IssueMatch(null, pattern, regexMatch.Groups, DefaultSeverity); + return new IssueMatch(line, null, pattern, regexMatch.Groups, DefaultSeverity); } return null; @@ -110,13 +132,13 @@ namespace GitHub.Runner.Worker } // Return - return new IssueMatch(runningMatch, pattern, regexMatch.Groups, DefaultSeverity); + return new IssueMatch(line, runningMatch, pattern, regexMatch.Groups, DefaultSeverity); } // Not the last pattern else { // Store the match - _state[i] = new IssueMatch(runningMatch, pattern, regexMatch.Groups); + _state[i] = new IssueMatch(line, runningMatch, pattern, regexMatch.Groups); } } // Not matched @@ -184,8 +206,9 @@ namespace GitHub.Runner.Worker public sealed class IssueMatch { - public IssueMatch(IssueMatch runningMatch, IssuePattern pattern, GroupCollection groups, string defaultSeverity = null) + public IssueMatch(string sourceText, IssueMatch runningMatch, IssuePattern pattern, GroupCollection groups, string defaultSeverity = null) { + SourceText = sourceText; File = runningMatch?.File ?? GetValue(groups, pattern.File); Line = runningMatch?.Line ?? GetValue(groups, pattern.Line); Column = runningMatch?.Column ?? GetValue(groups, pattern.Column); @@ -200,6 +223,8 @@ namespace GitHub.Runner.Worker } } + public string SourceText { get; } + public string File { get; } public string Line { get; } @@ -455,7 +480,7 @@ namespace GitHub.Runner.Worker if (Loop && Message == null) { throw new ArgumentException($"The {_loopPropertyName} pattern must set '{_messagePropertyName}'"); - } + } var regex = new Regex(Pattern ?? string.Empty, RegexOptions); var groupCount = regex.GetGroupNumbers().Length; diff --git a/src/Sdk/DTWebApi/WebApi/Issue.cs b/src/Sdk/DTWebApi/WebApi/Issue.cs index d68815402..c9dbbd789 100644 --- a/src/Sdk/DTWebApi/WebApi/Issue.cs +++ b/src/Sdk/DTWebApi/WebApi/Issue.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Runtime.Serialization; +using GitHub.Services.Common; namespace GitHub.DistributedTask.WebApi { @@ -14,48 +15,25 @@ namespace GitHub.DistributedTask.WebApi string this[string key] { get; } } - public class IssueMetadata - { - - public IssueMetadata(string key, string value) - : this(null, false, new []{ KeyValuePair.Create(key, value) }) - { - } - - public IssueMetadata(string category, bool infrastructureIssue, IEnumerable> data) - { - this.Category = category; - this.IsInfrastructureIssue = infrastructureIssue; - this.Data = data; - } - - - public readonly string Category; - public readonly bool IsInfrastructureIssue; - public readonly IEnumerable> Data; - } - [DataContract] public class Issue : IReadOnlyIssue { public Issue() + : this(null) { } private Issue(Issue original) { - this.Type = original.Type; - this.Category = original.Category; - this.Message = original.Message; - this.IsInfrastructureIssue = original.IsInfrastructureIssue; - - if (original.m_data != null) + m_data = new Dictionary(StringComparer.OrdinalIgnoreCase); + if (original != null) { - foreach (var item in original.m_data) - { - this.Data.Add(item); - } + this.Type = original.Type; + this.Category = original.Category; + this.Message = original.Message; + this.IsInfrastructureIssue = original.IsInfrastructureIssue; + this.Data.AddRange(original.Data); } } @@ -91,10 +69,6 @@ namespace GitHub.DistributedTask.WebApi { get { - if (m_data == null) - { - m_data = new Dictionary(StringComparer.OrdinalIgnoreCase); - } return m_data; } } diff --git a/src/Test/L0/Worker/OutputManagerL0.cs b/src/Test/L0/Worker/OutputManagerL0.cs index e511bd73f..09d98d965 100644 --- a/src/Test/L0/Worker/OutputManagerL0.cs +++ b/src/Test/L0/Worker/OutputManagerL0.cs @@ -983,8 +983,8 @@ namespace GitHub.Runner.Common.Tests.Worker { _onMatcherChanged = handler; }); - _executionContext.Setup(x => x.CreateIssue(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - .Returns((DTWebApi.IssueType type, string message, DTWebApi.IssueMetadata metadata, bool writeToLog) => + _executionContext.Setup(x => x.CreateIssue(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((DTWebApi.IssueType type, string message, IssueMetadata metadata, bool writeToLog) => { var result = new DTWebApi.Issue() {