From a3c99852af91cb7de5eee6eb385a29ee9b7826c8 Mon Sep 17 00:00:00 2001 From: Joanna Krzek-Lubowiecka Date: Tue, 13 Jun 2023 09:13:43 +0000 Subject: [PATCH] Add flag to control trace writer logging behaviour --- src/Runner.Common/Constants.cs | 2 +- src/Runner.Worker/ActionManifestManager.cs | 15 ++++++++++++--- src/Runner.Worker/ExecutionContext.cs | 14 +------------- .../ObjectTemplating/TemplateContext.cs | 16 ++++++++++++++-- 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/Runner.Common/Constants.cs b/src/Runner.Common/Constants.cs index b6d728c48..021bb5e67 100644 --- a/src/Runner.Common/Constants.cs +++ b/src/Runner.Common/Constants.cs @@ -155,7 +155,7 @@ namespace GitHub.Runner.Common { public static readonly string DiskSpaceWarning = "runner.diskspace.warning"; public static readonly string Node12Warning = "DistributedTask.AddWarningToNode12Action"; - public static readonly string LogTemplateErrorsAsDebugMessages = "DistributedTask.LogTemplateErrorsAsDebugMessages"; + public static readonly string LogTemplateErrorsToTraceWriter = "DistributedTask.LogTemplateErrorsToTraceWriter"; public static readonly string UseContainerPathForTemplate = "DistributedTask.UseContainerPathForTemplate"; public static readonly string AllowRunnerContainerHooks = "DistributedTask.AllowRunnerContainerHooks"; } diff --git a/src/Runner.Worker/ActionManifestManager.cs b/src/Runner.Worker/ActionManifestManager.cs index 1b3f562a7..6ec2b8057 100644 --- a/src/Runner.Worker/ActionManifestManager.cs +++ b/src/Runner.Worker/ActionManifestManager.cs @@ -52,7 +52,7 @@ namespace GitHub.Runner.Worker public ActionDefinitionData Load(IExecutionContext executionContext, string manifestFile) { - var templateContext = CreateTemplateContext(executionContext); + var templateContext = CreateTemplateContext(executionContext, null, LogTemplateErrorsToTraceWriter(executionContext)); ActionDefinitionData actionDefinition = new(); // Clean up file name real quick @@ -303,7 +303,8 @@ namespace GitHub.Runner.Worker private TemplateContext CreateTemplateContext( IExecutionContext executionContext, - IDictionary extraExpressionValues = null) + IDictionary extraExpressionValues = null, + bool addErrorsToTraceWriter = true) { var result = new TemplateContext { @@ -315,6 +316,7 @@ namespace GitHub.Runner.Worker maxBytes: 10 * 1024 * 1024), Schema = _actionManifestSchema, TraceWriter = executionContext.ToTemplateTraceWriter(), + LogErrorsToTraceWriter = addErrorsToTraceWriter }; // Expression values from execution context @@ -448,7 +450,7 @@ namespace GitHub.Runner.Worker }; } } - else if (string.Equals(usingToken.Value, "node12", StringComparison.OrdinalIgnoreCase)|| + else if (string.Equals(usingToken.Value, "node12", StringComparison.OrdinalIgnoreCase) || string.Equals(usingToken.Value, "node16", StringComparison.OrdinalIgnoreCase)) { if (string.IsNullOrEmpty(mainToken?.Value)) @@ -539,6 +541,13 @@ namespace GitHub.Runner.Worker } } } + + private bool LogTemplateErrorsToTraceWriter(IExecutionContext executionContext) + { + if (executionContext == null || executionContext.Global == null || executionContext.Global.EnvironmentVariables == null) return true; + executionContext.Global.EnvironmentVariables.TryGetValue(Constants.Runner.Features.LogTemplateErrorsToTraceWriter, out var logErrorsAsDebug); + return StringUtil.ConvertToBoolean(logErrorsAsDebug, defaultValue: true); + } } } diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index 68e369eac..192d03925 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -1402,12 +1402,7 @@ namespace GitHub.Runner.Worker public void Error(string format, params Object[] args) { - - if (logTemplateErrorsAsDebugMessages()) - { - _executionContext.Debug(string.Format(CultureInfo.CurrentCulture, format, args)); - } - else _executionContext.Error(string.Format(CultureInfo.CurrentCulture, format, args)); + _executionContext.Error(string.Format(CultureInfo.CurrentCulture, format, args)); } public void Info(string format, params Object[] args) @@ -1420,13 +1415,6 @@ namespace GitHub.Runner.Worker // todo: switch to verbose? _executionContext.Debug(string.Format(CultureInfo.CurrentCulture, $"{format}", args)); } - - private bool logTemplateErrorsAsDebugMessages() - { - _executionContext.Global.EnvironmentVariables.TryGetValue(Constants.Runner.Features.LogTemplateErrorsAsDebugMessages, out var logErrorsAsDebug); - return StringUtil.ConvertToBoolean(logErrorsAsDebug, defaultValue: false); - } - } public static class WellKnownTags diff --git a/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateContext.cs b/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateContext.cs index 94c803ddb..279d87c2d 100644 --- a/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateContext.cs +++ b/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateContext.cs @@ -34,6 +34,11 @@ namespace GitHub.DistributedTask.ObjectTemplating m_errors = value; } } + /// + /// Because TraceWriter has access to the ExecutionContext and is logging template errors, duplicated issues are reported. + /// By setting LogErrorsToTraceWriter = false TemplateContext will add errors only to TemplateValidationErrors + /// + internal bool LogErrorsToTraceWriter = true; /// /// Available functions within expression contexts @@ -135,7 +140,11 @@ namespace GitHub.DistributedTask.ObjectTemplating { var prefix = GetErrorPrefix(fileId, line, column); Errors.Add(prefix, ex); - TraceWriter.Error(prefix, ex); + + if (LogErrorsToTraceWriter) + { + TraceWriter.Error(prefix, ex); + } } internal void Error( @@ -155,7 +164,10 @@ namespace GitHub.DistributedTask.ObjectTemplating var fullMessage = !String.IsNullOrEmpty(prefix) ? $"{prefix} {message}" : message; Errors.Add(fullMessage); - TraceWriter.Error(fullMessage); + if (LogErrorsToTraceWriter) + { + TraceWriter.Error(fullMessage); + } } internal INamedValueInfo[] GetExpressionNamedValues()