Sending telemetry about actions usage. #1688
Conversation
| var jobCompletedEvent = new JobCompletedEvent(message.RequestId, message.JobId, result, jobContext.JobOutputs, jobContext.ActionsEnvironment, jobContext.Global.StepsTelemetry, jobContext.Global.JobTelemetry); | ||
| Trace.Info($"Raising job completed event: {StringUtil.ConvertToJson(jobCompletedEvent)}"); |
There was a problem hiding this comment.
I forgot to revert this change from my previous PR.
| @@ -1,4 +1,6 @@ | |||
| using System.Runtime.Serialization; | |||
| using System; | |||
There was a problem hiding this comment.
changes are copied from server-side code.
| @@ -56,6 +56,8 @@ public async Task<List<IStep>> InitializeJob(IExecutionContext jobContext, Pipel | |||
|
|
|||
| // Create a new timeline record for 'Set up job' | |||
| IExecutionContext context = jobContext.CreateChild(Guid.NewGuid(), "Set up job", $"{nameof(JobExtension)}_Init", null, null, ActionRunStage.Pre); | |||
| context.StepTelemetry.Type = "runner"; | |||
There was a problem hiding this comment.
I use this to track all the "runner" internal steps.
| // Add Telemetry to JobContext to send with JobCompleteMessage | ||
| if (stage == ActionRunStage.Main) | ||
| { | ||
| ExecutionContext.StepTelemetry.IsEmbedded = ExecutionContext.IsEmbedded; |
There was a problem hiding this comment.
these will get set during ExecutionContext.CreateChild() and Handler.PrepareExecution()
| @@ -218,6 +211,11 @@ public async Task RunAsync(ActionRunStage stage) | |||
| } | |||
| } | |||
|
|
|||
| if (!string.IsNullOrEmpty(shellCommand)) | |||
| { | |||
| ExecutionContext.StepTelemetry.Action = shellCommand; | |||
There was a problem hiding this comment.
log out what shell we are using. bash, pwsh,cmd, etc
| @@ -35,6 +35,8 @@ public async Task RunAsync(ActionRunStage stage) | |||
| } | |||
|
|
|||
| ArgUtil.NotNullOrEmpty(plugin, nameof(plugin)); | |||
| // Set extra telemetry base on the current context. | |||
| ExecutionContext.StepTelemetry.Type = plugin; | |||
There was a problem hiding this comment.
only checkout/upload/download@v1 should have this.
| if (stage == ActionRunStage.Main) | ||
| { | ||
| ExecutionContext.StepTelemetry.Ref = GetActionRef(); |
There was a problem hiding this comment.
set via handler.PrepareExecution()
| ExecutionContext.StepTelemetry.HasPreStep = Data.HasPre; | ||
| ExecutionContext.StepTelemetry.HasPostStep = Data.HasPost; | ||
| ExecutionContext.StepTelemetry.IsEmbedded = ExecutionContext.IsEmbedded; |
There was a problem hiding this comment.
set via ExecutionContext.CreateChild()
| @@ -118,40 +166,6 @@ public override void Initialize(IHostContext hostContext) | |||
| ActionCommandManager = hostContext.CreateService<IActionCommandManager>(); | |||
| } | |||
|
|
|||
| protected string GetActionRef() | |||
There was a problem hiding this comment.
merged into PopulateActionTelemetry()
| if (stage == ActionRunStage.Main) | ||
| { | ||
| ExecutionContext.StepTelemetry.Ref = GetActionRef(); |
There was a problem hiding this comment.
set via handler.PrepareExecution()
| ExecutionContext.StepTelemetry.HasPreStep = Data.HasPre; | ||
| ExecutionContext.StepTelemetry.HasPostStep = Data.HasPost; | ||
| ExecutionContext.StepTelemetry.IsEmbedded = ExecutionContext.IsEmbedded; |
There was a problem hiding this comment.
set via ExecutionContext.CreateChild()
| @@ -84,15 +84,14 @@ public async Task RunAsync(ActionRunStage stage) | |||
| } | |||
| } | |||
|
|
|||
| ExecutionContext.StepTelemetry.Ref = GetActionRef(); | |||
There was a problem hiding this comment.
set via handler.PrepareExecution()
| ExecutionContext.StepTelemetry.HasPreStep = Data.HasPre; | ||
| ExecutionContext.StepTelemetry.HasPostStep = Data.HasPost; | ||
| ExecutionContext.StepTelemetry.IsEmbedded = ExecutionContext.IsEmbedded; |
There was a problem hiding this comment.
set via ExecutionContext.CreateChild()
| @@ -539,6 +542,10 @@ public void AddIssue(Issue issue, string logMessage = null) | |||
| } | |||
|
|
|||
| issue.Message = HostContext.SecretMasker.MaskSecrets(issue.Message); | |||
| if (issue.Message.Length > 4096) | |||
There was a problem hiding this comment.
the change is unrelated to this PR...
We do trim issue messages from the service side to 4096 chars, so there is no reason we have to send out messages with more than that.
| @@ -924,6 +931,47 @@ public void PublishStepTelemetry() | |||
| // Add to the global steps telemetry only if we have something to log. | |||
| if (!string.IsNullOrEmpty(StepTelemetry?.Type)) | |||
| { | |||
| if (!IsEmbedded) | |||
| { | |||
| StepTelemetry.Result = _record.Result; | |||
There was a problem hiding this comment.
only set for not embedded step, since the whole composite action will have a single result.
| _record.FinishTime != null && | ||
| _record.StartTime != null) | ||
| { | ||
| StepTelemetry.ExecutionTimeInSeconds = (int)Math.Ceiling((_record.FinishTime - _record.StartTime)?.TotalSeconds ?? 0); |
There was a problem hiding this comment.
same here, since we only trace execution for the entire composite action.
| } | ||
|
|
||
| if (!IsEmbedded && | ||
| _record.Issues.Count > 0) |
There was a problem hiding this comment.
same here, issues are for the entire composite action.
| !string.IsNullOrEmpty(issue.Message)) | ||
| { | ||
| string issueTelemetry; | ||
| if (issue.Message.Length > 256) |
There was a problem hiding this comment.
trim to the first 256 chars to reduce the telemetry size.
| issueTelemetry = issue.Message; | ||
| } | ||
|
|
||
| StepTelemetry.ErrorMessages.Add(issueTelemetry); |
There was a problem hiding this comment.
we don't need to rerun secret masker since all the messages in side issues are already masked.
| StepTelemetry.ErrorMessages.Add(issueTelemetry); | ||
|
|
||
| // Only send over the first 3 issues to avoid sending too much data. | ||
| if (StepTelemetry.ErrorMessages.Count > 2) |
There was a problem hiding this comment.
should we add a constant for this?
https://github.com/github/c2c-actions-runtime/issues/1674
We are basically blind about how actions are running today.
Adding telemetry about how actions are failing with error, slowing down on execution, etc.
This should help us better support our customers, especially for actions author.