diff --git a/src/Runner.Worker/ActionCommandManager.cs b/src/Runner.Worker/ActionCommandManager.cs index 33fab1f21..5f0a7438c 100644 --- a/src/Runner.Worker/ActionCommandManager.cs +++ b/src/Runner.Worker/ActionCommandManager.cs @@ -276,7 +276,7 @@ namespace GitHub.Runner.Worker Message = $"Can't update {blocked} environment variable using ::set-env:: command." }; issue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = $"{Constants.Runner.UnsupportedCommand}_{envName}"; - context.AddIssue(issue); + context.AddIssue(issue, writeToLog: true); return; } @@ -315,7 +315,7 @@ namespace GitHub.Runner.Worker Message = String.Format(Constants.Runner.UnsupportedCommandMessage, this.Command) }; issue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = Constants.Runner.UnsupportedCommand; - context.AddIssue(issue); + context.AddIssue(issue, writeToLog: true); } if (!command.Properties.TryGetValue(SetOutputCommandProperties.Name, out string outputName) || string.IsNullOrEmpty(outputName)) @@ -350,7 +350,7 @@ namespace GitHub.Runner.Worker Message = String.Format(Constants.Runner.UnsupportedCommandMessage, this.Command) }; issue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = Constants.Runner.UnsupportedCommand; - context.AddIssue(issue); + context.AddIssue(issue, writeToLog: true); } if (!command.Properties.TryGetValue(SaveStateCommandProperties.Name, out string stateName) || string.IsNullOrEmpty(stateName)) @@ -666,7 +666,7 @@ namespace GitHub.Runner.Worker } } - context.AddIssue(issue); + context.AddIssue(issue, writeToLog: true); } public static void ValidateLinesAndColumns(ActionCommand command, IExecutionContext context) diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index 9034a9b87..e9e7dbf4f 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -16,6 +16,7 @@ using GitHub.Runner.Common.Util; using GitHub.Runner.Sdk; using GitHub.Runner.Worker.Container; using GitHub.Runner.Worker.Handlers; +using GitHub.Services.Common; using Newtonsoft.Json; using ObjectTemplating = GitHub.DistributedTask.ObjectTemplating; using Pipelines = GitHub.DistributedTask.Pipelines; @@ -90,7 +91,7 @@ namespace GitHub.Runner.Worker void SetGitHubContext(string name, string value); void SetOutput(string name, string value, out string reference); void SetTimeout(TimeSpan? timeout); - void AddIssue(Issue issue, string message = null); + void AddIssue(Issue issue, bool writeToLog); void Progress(int percentage, string currentOperation = null); void UpdateDetailTimelineRecord(TimelineRecord record); @@ -446,7 +447,7 @@ namespace GitHub.Runner.Worker { foreach (var issue in _embeddedIssueCollector) { - AddIssue(issue); + AddIssue(issue, writeToLog: false); } } @@ -577,73 +578,50 @@ namespace GitHub.Runner.Worker _jobServerQueue.QueueTimelineRecordUpdate(_mainTimelineId, _record); } - // This is not thread safe, the caller need to take lock before calling issue() - public void AddIssue(Issue issue, string logMessage = null) + // This is not thread safe, the caller needs to take lock before calling issue() + public void AddIssue(Issue issue, bool writeToLog) { ArgUtil.NotNull(issue, nameof(issue)); - if (string.IsNullOrEmpty(logMessage)) - { - logMessage = issue.Message; - } + string refinedMessage = PrimitiveExtensions.TrimExcess(HostContext.SecretMasker.MaskSecrets(issue.Message), _maxIssueMessageLength); + issue.Message = refinedMessage; - issue.Message = HostContext.SecretMasker.MaskSecrets(issue.Message); - if (issue.Message.Length > _maxIssueMessageLength) - { - issue.Message = issue.Message[.._maxIssueMessageLength]; - } - - // Tracking the line number (logFileLineNumber) and step number (stepNumber) for each issue that gets created - // Actions UI from the run summary page use both values to easily link to an exact locations in logs where annotations originate from + // It's important to keep track of the step number (key:stepNumber) and the line number (key:logFileLineNumber) of every issue that gets logged. + // Actions UI from the run summary page use both values to easily link to an exact locations in logs where annotations originate from. if (_record.Order != null) { issue.Data["stepNumber"] = _record.Order.ToString(); } - if (issue.Type == IssueType.Error) + string wellKnownTag = null; + Int32? previousCountForIssueType = null; + switch (issue.Type) { - if (!string.IsNullOrEmpty(logMessage)) - { - long logLineNumber = Write(WellKnownTags.Error, logMessage); - issue.Data["logFileLineNumber"] = logLineNumber.ToString(); - } - - if (_record.ErrorCount < _maxIssueCount) - { - _record.Issues.Add(issue); - } - - _record.ErrorCount++; + case IssueType.Error: + wellKnownTag = WellKnownTags.Error; + previousCountForIssueType = _record.ErrorCount++; + break; + case IssueType.Warning: + wellKnownTag = WellKnownTags.Warning; + previousCountForIssueType = _record.WarningCount++; + break; + case IssueType.Notice: + wellKnownTag = WellKnownTags.Notice; + previousCountForIssueType = _record.NoticeCount++; + break; } - else if (issue.Type == IssueType.Warning) + + if (!string.IsNullOrEmpty(wellKnownTag)) { - if (!string.IsNullOrEmpty(logMessage)) + if (writeToLog && !string.IsNullOrEmpty(refinedMessage)) { - long logLineNumber = Write(WellKnownTags.Warning, logMessage); + long logLineNumber = Write(wellKnownTag, refinedMessage); issue.Data["logFileLineNumber"] = logLineNumber.ToString(); } - - if (_record.WarningCount < _maxIssueCount) + if (previousCountForIssueType.GetValueOrDefault(0) < _maxIssueCount) { _record.Issues.Add(issue); } - - _record.WarningCount++; - } - else if (issue.Type == IssueType.Notice) - { - if (!string.IsNullOrEmpty(logMessage)) - { - long logLineNumber = Write(WellKnownTags.Notice, logMessage); - issue.Data["logFileLineNumber"] = logLineNumber.ToString(); - } - - if (_record.NoticeCount < _maxIssueCount) - { - _record.Issues.Add(issue); - } - - _record.NoticeCount++; } // Embedded ExecutionContexts (a.k.a. Composite actions) should never upload a timeline record to the server. @@ -1017,16 +995,7 @@ namespace GitHub.Runner.Worker if ((issue.Type == IssueType.Error || issue.Type == IssueType.Warning) && !string.IsNullOrEmpty(issue.Message)) { - string issueTelemetry; - if (issue.Message.Length > _maxIssueMessageLengthInTelemetry) - { - issueTelemetry = $"{issue.Message[.._maxIssueMessageLengthInTelemetry]}"; - } - else - { - issueTelemetry = issue.Message; - } - + string issueTelemetry = PrimitiveExtensions.TrimExcess(issue.Message, _maxIssueMessageLengthInTelemetry); StepTelemetry.ErrorMessages.Add(issueTelemetry); // Only send over the first 3 issues to avoid sending too much data. @@ -1200,19 +1169,22 @@ namespace GitHub.Runner.Worker // Do not add a format string overload. See comment on ExecutionContext.Write(). public static void Error(this IExecutionContext context, string message) { - context.AddIssue(new Issue() { Type = IssueType.Error, Message = message }); + var issue = new Issue() { Type = IssueType.Error, Message = message }; + context.AddIssue(issue, writeToLog: true); } // Do not add a format string overload. See comment on ExecutionContext.Write(). public static void InfrastructureError(this IExecutionContext context, string message) { - context.AddIssue(new Issue() { Type = IssueType.Error, Message = message, IsInfrastructureIssue = true }); + var issue = new Issue() { Type = IssueType.Error, Message = message, IsInfrastructureIssue = true }; + context.AddIssue(issue, writeToLog: true); } // Do not add a format string overload. See comment on ExecutionContext.Write(). public static void Warning(this IExecutionContext context, string message) { - context.AddIssue(new Issue() { Type = IssueType.Warning, Message = message }); + var issue = new Issue() { Type = IssueType.Warning, Message = message }; + context.AddIssue(issue, writeToLog: true); } // Do not add a format string overload. See comment on ExecutionContext.Write(). diff --git a/src/Runner.Worker/Handlers/OutputManager.cs b/src/Runner.Worker/Handlers/OutputManager.cs index 8b97c867d..dc683422f 100644 --- a/src/Runner.Worker/Handlers/OutputManager.cs +++ b/src/Runner.Worker/Handlers/OutputManager.cs @@ -108,7 +108,6 @@ namespace GitHub.Runner.Worker.Handlers try { match = matcher.Match(stripped); - break; } catch (RegexMatchTimeoutException ex) @@ -116,7 +115,7 @@ namespace GitHub.Runner.Worker.Handlers if (attempt < _maxAttempts) { // Debug - _executionContext.Debug($"Timeout processing issue matcher '{matcher.Owner}' against line '{stripped}'. Exception: {ex.ToString()}"); + _executionContext.Debug($"Timeout processing issue matcher '{matcher.Owner}' against line '{stripped}'. Exception: {ex}"); } else { @@ -139,12 +138,10 @@ namespace GitHub.Runner.Worker.Handlers // Convert to issue var issue = ConvertToIssue(match); - if (issue != null) { // Log issue - _executionContext.AddIssue(issue, stripped); - + _executionContext.AddIssue(issue, writeToLog: true); return; } } diff --git a/src/Runner.Worker/JobExtension.cs b/src/Runner.Worker/JobExtension.cs index 246274734..ec91510e1 100644 --- a/src/Runner.Worker/JobExtension.cs +++ b/src/Runner.Worker/JobExtension.cs @@ -325,10 +325,10 @@ namespace GitHub.Runner.Worker if (message.ContextData.TryGetValue("inputs", out var pipelineContextData)) { var inputs = pipelineContextData.AssertDictionary("inputs"); - if (inputs.Any()) + if (inputs.Any()) { context.Output($"##[group] Inputs"); - foreach (var input in inputs) + foreach (var input in inputs) { context.Output($" {input.Key}: {input.Value}"); } @@ -336,7 +336,7 @@ namespace GitHub.Runner.Worker } } - if (!string.IsNullOrWhiteSpace(message.JobDisplayName)) + if (!string.IsNullOrWhiteSpace(message.JobDisplayName)) { context.Output($"Complete job name: {message.JobDisplayName}"); } @@ -662,7 +662,7 @@ namespace GitHub.Runner.Worker { var issue = new Issue() { Type = IssueType.Warning, Message = $"You are running out of disk space. The runner will stop working when the machine runs out of disk space. Free space left: {freeSpaceInMB} MB" }; issue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = Constants.Runner.LowDiskSpace; - context.AddIssue(issue); + context.AddIssue(issue, writeToLog: true); return; } diff --git a/src/Runner.Worker/JobRunner.cs b/src/Runner.Worker/JobRunner.cs index 562ef9068..fca6c72e9 100644 --- a/src/Runner.Worker/JobRunner.cs +++ b/src/Runner.Worker/JobRunner.cs @@ -84,7 +84,8 @@ namespace GitHub.Runner.Worker default: throw new ArgumentException(HostContext.RunnerShutdownReason.ToString(), nameof(HostContext.RunnerShutdownReason)); } - jobContext.AddIssue(new Issue() { Type = IssueType.Error, Message = errorMessage }); + var issue = new Issue() { Type = IssueType.Error, Message = errorMessage }; + jobContext.AddIssue(issue, writeToLog: true); }); // Validate directory permissions. diff --git a/src/Sdk/Common/Common/Utility/PrimitiveExtensions.cs b/src/Sdk/Common/Common/Utility/PrimitiveExtensions.cs index f15bf502b..aae382489 100644 --- a/src/Sdk/Common/Common/Utility/PrimitiveExtensions.cs +++ b/src/Sdk/Common/Common/Utility/PrimitiveExtensions.cs @@ -27,6 +27,18 @@ namespace GitHub.Services.Common } } + public static string TrimExcess(string text, int maxLength) + { + string result = text; + + if (!string.IsNullOrEmpty(text) && text.Length > maxLength) + { + result = text.Substring(0, maxLength); + } + + return result; + } + public static string ToBase64StringNoPaddingFromString(string utf8String) { return ToBase64StringNoPadding(Encoding.UTF8.GetBytes(utf8String)); @@ -46,7 +58,7 @@ namespace GitHub.Services.Common //These methods convert To and From base64 strings without padding //for JWT scenarios - //code taken from the JWS spec here: + //code taken from the JWS spec here: //http://tools.ietf.org/html/draft-ietf-jose-json-web-signature-08#appendix-C public static String ToBase64StringNoPadding(this byte[] bytes) {