Make logging behavior optional in ExecutionContext::AddIssue

This commit is contained in:
John Wesley Walker III
2023-01-16 23:34:26 +00:00
parent 7036627d47
commit 3902257e9d
6 changed files with 61 additions and 79 deletions

View File

@@ -276,7 +276,7 @@ namespace GitHub.Runner.Worker
Message = $"Can't update {blocked} environment variable using ::set-env:: command." Message = $"Can't update {blocked} environment variable using ::set-env:: command."
}; };
issue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = $"{Constants.Runner.UnsupportedCommand}_{envName}"; issue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = $"{Constants.Runner.UnsupportedCommand}_{envName}";
context.AddIssue(issue); context.AddIssue(issue, writeToLog: true);
return; return;
} }
@@ -315,7 +315,7 @@ namespace GitHub.Runner.Worker
Message = String.Format(Constants.Runner.UnsupportedCommandMessage, this.Command) Message = String.Format(Constants.Runner.UnsupportedCommandMessage, this.Command)
}; };
issue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = Constants.Runner.UnsupportedCommand; 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)) 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) Message = String.Format(Constants.Runner.UnsupportedCommandMessage, this.Command)
}; };
issue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = Constants.Runner.UnsupportedCommand; 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)) 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) public static void ValidateLinesAndColumns(ActionCommand command, IExecutionContext context)

View File

@@ -16,6 +16,7 @@ using GitHub.Runner.Common.Util;
using GitHub.Runner.Sdk; using GitHub.Runner.Sdk;
using GitHub.Runner.Worker.Container; using GitHub.Runner.Worker.Container;
using GitHub.Runner.Worker.Handlers; using GitHub.Runner.Worker.Handlers;
using GitHub.Services.Common;
using Newtonsoft.Json; using Newtonsoft.Json;
using ObjectTemplating = GitHub.DistributedTask.ObjectTemplating; using ObjectTemplating = GitHub.DistributedTask.ObjectTemplating;
using Pipelines = GitHub.DistributedTask.Pipelines; using Pipelines = GitHub.DistributedTask.Pipelines;
@@ -90,7 +91,7 @@ namespace GitHub.Runner.Worker
void SetGitHubContext(string name, string value); void SetGitHubContext(string name, string value);
void SetOutput(string name, string value, out string reference); void SetOutput(string name, string value, out string reference);
void SetTimeout(TimeSpan? timeout); 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 Progress(int percentage, string currentOperation = null);
void UpdateDetailTimelineRecord(TimelineRecord record); void UpdateDetailTimelineRecord(TimelineRecord record);
@@ -446,7 +447,7 @@ namespace GitHub.Runner.Worker
{ {
foreach (var issue in _embeddedIssueCollector) foreach (var issue in _embeddedIssueCollector)
{ {
AddIssue(issue); AddIssue(issue, writeToLog: false);
} }
} }
@@ -577,73 +578,50 @@ namespace GitHub.Runner.Worker
_jobServerQueue.QueueTimelineRecordUpdate(_mainTimelineId, _record); _jobServerQueue.QueueTimelineRecordUpdate(_mainTimelineId, _record);
} }
// This is not thread safe, the caller need to take lock before calling issue() // This is not thread safe, the caller needs to take lock before calling issue()
public void AddIssue(Issue issue, string logMessage = null) public void AddIssue(Issue issue, bool writeToLog)
{ {
ArgUtil.NotNull(issue, nameof(issue)); ArgUtil.NotNull(issue, nameof(issue));
if (string.IsNullOrEmpty(logMessage)) string refinedMessage = PrimitiveExtensions.TrimExcess(HostContext.SecretMasker.MaskSecrets(issue.Message), _maxIssueMessageLength);
{ issue.Message = refinedMessage;
logMessage = issue.Message;
}
issue.Message = HostContext.SecretMasker.MaskSecrets(issue.Message); // It's important to keep track of the step number (key:stepNumber) and the line number (key:logFileLineNumber) of every issue that gets logged.
if (issue.Message.Length > _maxIssueMessageLength) // Actions UI from the run summary page use both values to easily link to an exact locations in logs where annotations originate from.
{
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
if (_record.Order != null) if (_record.Order != null)
{ {
issue.Data["stepNumber"] = _record.Order.ToString(); issue.Data["stepNumber"] = _record.Order.ToString();
} }
if (issue.Type == IssueType.Error) string wellKnownTag = null;
Int32? previousCountForIssueType = null;
switch (issue.Type)
{ {
if (!string.IsNullOrEmpty(logMessage)) case IssueType.Error:
{ wellKnownTag = WellKnownTags.Error;
long logLineNumber = Write(WellKnownTags.Error, logMessage); previousCountForIssueType = _record.ErrorCount++;
issue.Data["logFileLineNumber"] = logLineNumber.ToString(); break;
} case IssueType.Warning:
wellKnownTag = WellKnownTags.Warning;
if (_record.ErrorCount < _maxIssueCount) previousCountForIssueType = _record.WarningCount++;
{ break;
_record.Issues.Add(issue); case IssueType.Notice:
} wellKnownTag = WellKnownTags.Notice;
previousCountForIssueType = _record.NoticeCount++;
_record.ErrorCount++; 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(); issue.Data["logFileLineNumber"] = logLineNumber.ToString();
} }
if (previousCountForIssueType.GetValueOrDefault(0) < _maxIssueCount)
if (_record.WarningCount < _maxIssueCount)
{ {
_record.Issues.Add(issue); _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. // 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) && if ((issue.Type == IssueType.Error || issue.Type == IssueType.Warning) &&
!string.IsNullOrEmpty(issue.Message)) !string.IsNullOrEmpty(issue.Message))
{ {
string issueTelemetry; string issueTelemetry = PrimitiveExtensions.TrimExcess(issue.Message, _maxIssueMessageLengthInTelemetry);
if (issue.Message.Length > _maxIssueMessageLengthInTelemetry)
{
issueTelemetry = $"{issue.Message[.._maxIssueMessageLengthInTelemetry]}";
}
else
{
issueTelemetry = issue.Message;
}
StepTelemetry.ErrorMessages.Add(issueTelemetry); StepTelemetry.ErrorMessages.Add(issueTelemetry);
// Only send over the first 3 issues to avoid sending too much data. // 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(). // Do not add a format string overload. See comment on ExecutionContext.Write().
public static void Error(this IExecutionContext context, string message) 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(). // Do not add a format string overload. See comment on ExecutionContext.Write().
public static void InfrastructureError(this IExecutionContext context, string message) 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(). // Do not add a format string overload. See comment on ExecutionContext.Write().
public static void Warning(this IExecutionContext context, string message) 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(). // Do not add a format string overload. See comment on ExecutionContext.Write().

View File

@@ -108,7 +108,6 @@ namespace GitHub.Runner.Worker.Handlers
try try
{ {
match = matcher.Match(stripped); match = matcher.Match(stripped);
break; break;
} }
catch (RegexMatchTimeoutException ex) catch (RegexMatchTimeoutException ex)
@@ -116,7 +115,7 @@ namespace GitHub.Runner.Worker.Handlers
if (attempt < _maxAttempts) if (attempt < _maxAttempts)
{ {
// Debug // 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 else
{ {
@@ -139,12 +138,10 @@ namespace GitHub.Runner.Worker.Handlers
// Convert to issue // Convert to issue
var issue = ConvertToIssue(match); var issue = ConvertToIssue(match);
if (issue != null) if (issue != null)
{ {
// Log issue // Log issue
_executionContext.AddIssue(issue, stripped); _executionContext.AddIssue(issue, writeToLog: true);
return; return;
} }
} }

View File

@@ -325,10 +325,10 @@ namespace GitHub.Runner.Worker
if (message.ContextData.TryGetValue("inputs", out var pipelineContextData)) if (message.ContextData.TryGetValue("inputs", out var pipelineContextData))
{ {
var inputs = pipelineContextData.AssertDictionary("inputs"); var inputs = pipelineContextData.AssertDictionary("inputs");
if (inputs.Any()) if (inputs.Any())
{ {
context.Output($"##[group] Inputs"); context.Output($"##[group] Inputs");
foreach (var input in inputs) foreach (var input in inputs)
{ {
context.Output($" {input.Key}: {input.Value}"); 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}"); 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" }; 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; issue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = Constants.Runner.LowDiskSpace;
context.AddIssue(issue); context.AddIssue(issue, writeToLog: true);
return; return;
} }

View File

@@ -84,7 +84,8 @@ namespace GitHub.Runner.Worker
default: default:
throw new ArgumentException(HostContext.RunnerShutdownReason.ToString(), nameof(HostContext.RunnerShutdownReason)); 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. // Validate directory permissions.

View File

@@ -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) public static string ToBase64StringNoPaddingFromString(string utf8String)
{ {
return ToBase64StringNoPadding(Encoding.UTF8.GetBytes(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 //These methods convert To and From base64 strings without padding
//for JWT scenarios //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 //http://tools.ietf.org/html/draft-ietf-jose-json-web-signature-08#appendix-C
public static String ToBase64StringNoPadding(this byte[] bytes) public static String ToBase64StringNoPadding(this byte[] bytes)
{ {