From caec043085990710070108f375cd0aeab45e1017 Mon Sep 17 00:00:00 2001 From: Yang Cao Date: Wed, 28 Dec 2022 11:56:53 -0500 Subject: [PATCH] Always upload to avoid issues (#2334) * Remove unnecessary timelineId and timelineRecordId and use Guid stepId * Log upload error to kusto * Remove try-catch * Using a well known telmetry record to avoid replacing issues * fix Guid format --- src/Runner.Common/Constants.cs | 2 ++ src/Runner.Common/JobServerQueue.cs | 28 +++++++++++++--------- src/Runner.Worker/ExecutionContext.cs | 6 ++--- src/Runner.Worker/FileCommandManager.cs | 13 +++++----- src/Sdk/WebApi/WebApi/ResultsHttpClient.cs | 4 ++++ 5 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/Runner.Common/Constants.cs b/src/Runner.Common/Constants.cs index 57f8e8e4d..0caee0378 100644 --- a/src/Runner.Common/Constants.cs +++ b/src/Runner.Common/Constants.cs @@ -158,9 +158,11 @@ namespace GitHub.Runner.Common } public static readonly string InternalTelemetryIssueDataKey = "_internal_telemetry"; + public static readonly Guid TelemetryRecordId = new Guid("11111111-1111-1111-1111-111111111111"); public static readonly string WorkerCrash = "WORKER_CRASH"; public static readonly string LowDiskSpace = "LOW_DISK_SPACE"; public static readonly string UnsupportedCommand = "UNSUPPORTED_COMMAND"; + public static readonly string ResultsUploadFailure = "RESULTS_UPLOAD_FAILURE"; public static readonly string UnsupportedCommandMessage = "The `{0}` command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/"; public static readonly string UnsupportedCommandMessageDisabled = "The `{0}` command is disabled. Please upgrade to using Environment Files or opt into unsecure command execution by setting the `ACTIONS_ALLOW_UNSECURE_COMMANDS` environment variable to `true`. For more information see: https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/"; public static readonly string UnsupportedStopCommandTokenDisabled = "You cannot use a endToken that is an empty string, the string 'pause-logging', or another workflow command. For more information see: https://docs.github.com/actions/learn-github-actions/workflow-commands-for-github-actions#example-stopping-and-starting-workflow-commands or opt into insecure command execution by setting the `ACTIONS_ALLOW_UNSECURE_STOPCOMMAND_TOKENS` environment variable to `true`."; diff --git a/src/Runner.Common/JobServerQueue.cs b/src/Runner.Common/JobServerQueue.cs index fb97ec732..6440da736 100644 --- a/src/Runner.Common/JobServerQueue.cs +++ b/src/Runner.Common/JobServerQueue.cs @@ -20,7 +20,7 @@ namespace GitHub.Runner.Common void Start(Pipelines.AgentJobRequestMessage jobRequest); void QueueWebConsoleLine(Guid stepRecordId, string line, long? lineNumber = null); void QueueFileUpload(Guid timelineId, Guid timelineRecordId, string type, string name, string path, bool deleteSource); - void QueueSummaryUpload(Guid timelineId, Guid timelineRecordId, string stepId, string name, string path, bool deleteSource); + void QueueSummaryUpload(Guid stepRecordId, string name, string path, bool deleteSource); void QueueTimelineRecordUpdate(Guid timelineId, TimelineRecord timelineRecord); } @@ -230,25 +230,20 @@ namespace GitHub.Runner.Common _fileUploadQueue.Enqueue(newFile); } - public void QueueSummaryUpload(Guid timelineId, Guid timelineRecordId, string stepId, string name, string path, bool deleteSource) + public void QueueSummaryUpload(Guid stepRecordId, string name, string path, bool deleteSource) { - ArgUtil.NotEmpty(timelineId, nameof(timelineId)); - ArgUtil.NotEmpty(timelineRecordId, nameof(timelineRecordId)); - // all parameter not null, file path exist. var newFile = new SummaryUploadFileInfo() { - TimelineId = timelineId, - TimelineRecordId = timelineRecordId, Name = name, Path = path, PlanId = _planId.ToString(), JobId = _jobTimelineRecordId.ToString(), - StepId = stepId, + StepId = stepRecordId.ToString(), DeleteSource = deleteSource }; - Trace.Verbose("Enqueue results file upload queue: file '{0}' attach to record {1}", newFile.Path, timelineRecordId); + Trace.Verbose("Enqueue results file upload queue: file '{0}' attach to job {1} step {2}", newFile.Path, _jobTimelineRecordId, stepRecordId); _summaryFileUploadQueue.Enqueue(newFile); } @@ -476,8 +471,21 @@ namespace GitHub.Runner.Common } catch (Exception ex) { + var issue = new Issue() { Type = IssueType.Warning, Message = $"Caught exception during summary file upload to results. {ex.Message}" }; + issue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = Constants.Runner.ResultsUploadFailure; + + var telemetryRecord = new TimelineRecord() + { + Id = Constants.Runner.TelemetryRecordId, + }; + telemetryRecord.Issues.Add(issue); + QueueTimelineRecordUpdate(_jobTimelineId, telemetryRecord); + Trace.Info("Catch exception during summary file upload to results, keep going since the process is best effort."); Trace.Error(ex); + } + finally + { errorCount++; } } @@ -816,8 +824,6 @@ namespace GitHub.Runner.Common internal class SummaryUploadFileInfo { - public Guid TimelineId { get; set; } - public Guid TimelineRecordId { get; set; } public string Name { get; set; } public string Path { get; set; } public string PlanId { get; set; } diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index e716e0a5a..c4a7db6fe 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -80,7 +80,7 @@ namespace GitHub.Runner.Worker // logging long Write(string tag, string message); void QueueAttachFile(string type, string name, string filePath); - void QueueSummaryFile(string name, string filePath, string stepId); + void QueueSummaryFile(string name, string filePath, Guid stepRecordId); // timeline record update methods void Start(string currentOperation = null); @@ -847,7 +847,7 @@ namespace GitHub.Runner.Worker _jobServerQueue.QueueFileUpload(_mainTimelineId, _record.Id, type, name, filePath, deleteSource: false); } - public void QueueSummaryFile(string name, string filePath, string stepId) + public void QueueSummaryFile(string name, string filePath, Guid stepRecordId) { ArgUtil.NotNullOrEmpty(name, nameof(name)); ArgUtil.NotNullOrEmpty(filePath, nameof(filePath)); @@ -857,7 +857,7 @@ namespace GitHub.Runner.Worker throw new FileNotFoundException($"Can't upload (name:{name}) file: {filePath}. File does not exist."); } - _jobServerQueue.QueueSummaryUpload(_mainTimelineId, _record.Id, stepId, name, filePath, deleteSource: false); + _jobServerQueue.QueueSummaryUpload(stepRecordId, name, filePath, deleteSource: false); } // Add OnMatcherChanged diff --git a/src/Runner.Worker/FileCommandManager.cs b/src/Runner.Worker/FileCommandManager.cs index a4eb2e676..646e56548 100644 --- a/src/Runner.Worker/FileCommandManager.cs +++ b/src/Runner.Worker/FileCommandManager.cs @@ -208,20 +208,19 @@ namespace GitHub.Runner.Worker ? context.Id.ToString() : context.EmbeddedId.ToString(); + Trace.Info($"Queueing file ({filePath}) for attachment upload ({attachmentName})"); + // Attachments must be added to the parent context (job), not the current context (step) + context.Root.QueueAttachFile(ChecksAttachmentType.StepSummary, attachmentName, scrubbedFilePath); + + // Dual upload the same files to Results Service context.Global.Variables.TryGetValue("system.github.results_endpoint", out string resultsReceiverEndpoint); if (resultsReceiverEndpoint != null) { Trace.Info($"Queueing results file ({filePath}) for attachment upload ({attachmentName})"); - var stepId = context.Id.ToString(); + var stepId = context.Id; // Attachments must be added to the parent context (job), not the current context (step) context.Root.QueueSummaryFile(attachmentName, scrubbedFilePath, stepId); } - else - { - Trace.Info($"Queueing file ({filePath}) for attachment upload ({attachmentName})"); - // Attachments must be added to the parent context (job), not the current context (step) - context.Root.QueueAttachFile(ChecksAttachmentType.StepSummary, attachmentName, scrubbedFilePath); - } } catch (Exception e) { diff --git a/src/Sdk/WebApi/WebApi/ResultsHttpClient.cs b/src/Sdk/WebApi/WebApi/ResultsHttpClient.cs index 674abb20d..77a733eae 100644 --- a/src/Sdk/WebApi/WebApi/ResultsHttpClient.cs +++ b/src/Sdk/WebApi/WebApi/ResultsHttpClient.cs @@ -113,6 +113,10 @@ namespace GitHub.Services.Results.Client { // Get the upload url var uploadUrlResponse = await GetStepSummaryUploadUrlAsync(planId, jobId, stepId, cancellationToken); + if (uploadUrlResponse == null) + { + throw new Exception("Failed to get step summary upload url"); + } // Do we want to throw an exception here or should we just be uploading/truncating the data var fileSize = new FileInfo(file).Length;