Merged PR 655464: Target Error Field Review Changes

Added two fields ShortPipDescription and PipExecutionTimeMs to the TargetFailedEvent as per review suggestions.

Related work items: #1915342

Public/Src/App/Bxl/AzureDevOpsListener.cs

@@ -167,7 +167,7 @@ namespace BuildXL
                     if (actualEventId == (int)LogEventId.PipProcessError)
                     {
                         var pipProcessErrorEventFields = new PipProcessErrorEventFields(eventData.Payload, true);
-                        addPipErrors(pipProcessErrorEventFields, (string)eventData.Payload[15]); 
+                        addPipErrors(pipProcessErrorEventFields, (string)eventData.Payload[16]);
                     }
                 }
                 break;
@@ -175,7 +175,7 @@ namespace BuildXL
 
             void addPipErrors(PipProcessErrorEventFields pipProcessErrorEventFields, string workerId)
             {
-                var semiStableHash = Pip.FormatSemiStableHash(pipProcessErrorEventFields.PipSemiStableHash);
+                string semiStableHash = Pip.FormatSemiStableHash(pipProcessErrorEventFields.PipSemiStableHash);
                 m_buildViewModel.BuildSummary.AddPipError(new BuildSummaryPipDiagnostic
                 {
                     SemiStablePipId = semiStableHash,
@@ -193,6 +193,8 @@ namespace BuildXL
                     TargetId = semiStableHash,
                     StdOutputPath = pipProcessErrorEventFields.OutputToLog,
                     PipDescription = pipProcessErrorEventFields.PipDescription,
+                    ShortPipDescription = pipProcessErrorEventFields.ShortPipDescription,
+                    PipExecutionTimeMs = pipProcessErrorEventFields.PipExecutionTimeMs                    
                 });
             }
         }

Public/Src/App/UnitTests/Bxl/DevOpsListenerTests.cs

@@ -79,7 +79,7 @@ namespace Test.BuildXL
             var eventName = "PipProcessError";
             var text = "Pip process error message";
             var pipSemiStableHash = (long)24;
-
+            
             m_eventListener.RegisterEventSource(global::BuildXL.Engine.ETWLogger.Log);
             m_eventListener.NestedLoggerHandler += eventData =>
             {
@@ -119,6 +119,7 @@ namespace Test.BuildXL
             public static PipProcessErrorTestElement Create(BuildXLTestBase testBase)
             {
                 var result = new PipProcessErrorTestElement();
+                long totalElapsedTimeMs = Convert.ToInt64(TimeSpan.FromSeconds(15).TotalMilliseconds);
                 var pipProcessError = new PipProcessErrorEventFields(
                     (long)24,
                     "my cool pip",
@@ -130,7 +131,8 @@ namespace Test.BuildXL
                     @"specs\workingDir\out.txt",
                     -1,
                     "what does this do?",
-                    "my pip");
+                    "my pip",
+                    totalElapsedTimeMs);
 
                 var processedOutputToLog = "Failure message Line1%0D%0A##[error]Failure message Line2%0D##[error]Failure message Line3%0A##[error]";
                 result.ExpectingConsoleLog = @$"##vso[task.logIssue type=error;]DX0064 [Pip0000000000000018, {pipProcessError.ShortPipDescription}, {pipProcessError.PipSpecPath}] - failed with exit code {pipProcessError.ExitCode}, {pipProcessError.OptionalMessage}%0D%0A##[error]{processedOutputToLog}%0D%0A##[error]{pipProcessError.MessageAboutPathsToLog}%0D%0A##[error]{pipProcessError.PathsToLog}";
@@ -157,7 +159,8 @@ namespace Test.BuildXL
                     PipProcessError.PathsToLog,
                     PipProcessError.ExitCode,
                     PipProcessError.OptionalMessage,
-                    PipProcessError.ShortPipDescription);
+                    PipProcessError.ShortPipDescription,
+                    PipProcessError.PipExecutionTimeMs);
             }
 
             public void Dispose()

Public/Src/Engine/Cache/Fingerprints/OpenBondDistribution.bond

@@ -130,6 +130,7 @@ struct PipProcessErrorEvent
     9: int32 ExitCode;
     10: string OptionalMessage;
     11: string ShortPipDescription;
+    12: int64 PipExecutionTimeMs;
 }
 
 /// Events that have been logged on workers and are transited to the orchestrator. Any changes here may require changes

Public/Src/Engine/Distribution.Grpc/Interfaces.proto

@@ -309,4 +309,5 @@ message PipProcessErrorEvent {
     int32 ExitCode = 9;
     string OptionalMessage = 10;
     string ShortPipDescription = 11;
+    int64 PipExecutionTimeMs = 12;
 }

Public/Src/Engine/Dll/Distribution/Grpc/OpenBondConversionUtils.cs

@@ -97,7 +97,8 @@ namespace BuildXL.Engine.Distribution.Grpc
                         PathsToLog = i.PipProcessErrorEvent.PathsToLog,
                         ExitCode = i.PipProcessErrorEvent.ExitCode,
                         OptionalMessage = i.PipProcessErrorEvent.OptionalMessage,
-                        ShortPipDescription = i.PipProcessErrorEvent.ShortPipDescription
+                        ShortPipDescription = i.PipProcessErrorEvent.ShortPipDescription,
+                        PipExecutionTimeMs = i.PipProcessErrorEvent.PipExecutionTimeMs
                     };
                 }
 
@@ -147,7 +148,8 @@ namespace BuildXL.Engine.Distribution.Grpc
                         PathsToLog = i.PipProcessErrorEvent.PathsToLog,
                         ExitCode = i.PipProcessErrorEvent.ExitCode,
                         OptionalMessage = i.PipProcessErrorEvent.OptionalMessage,
-                        ShortPipDescription = i.PipProcessErrorEvent.ShortPipDescription
+                        ShortPipDescription = i.PipProcessErrorEvent.ShortPipDescription,
+                        PipExecutionTimeMs = i.PipProcessErrorEvent.PipExecutionTimeMs
                     } : null,
                 });
             }

Public/Src/Engine/Dll/Distribution/OrchestratorService.cs

@@ -194,7 +194,9 @@ namespace BuildXL.Engine.Distribution
                                 forwardedEvent.PipProcessErrorEvent.PathsToLog,
                                 forwardedEvent.PipProcessErrorEvent.ExitCode,
                                 forwardedEvent.PipProcessErrorEvent.OptionalMessage,
-                                forwardedEvent.PipProcessErrorEvent.ShortPipDescription);
+                                forwardedEvent.PipProcessErrorEvent.ShortPipDescription,
+                                forwardedEvent.PipProcessErrorEvent.PipExecutionTimeMs
+                                );
 
                             logForwardedError(
                                 m_loggingContext,

Public/Src/Engine/Dll/Distribution/WorkerNotificationManager.EventListener.cs

@@ -74,6 +74,7 @@ namespace BuildXL.Engine.Distribution
                             ExitCode = pipProcessErrorEventFields.ExitCode,
                             OptionalMessage = pipProcessErrorEventFields.OptionalMessage,
                             ShortPipDescription = pipProcessErrorEventFields.ShortPipDescription,
+                            PipExecutionTimeMs = pipProcessErrorEventFields.PipExecutionTimeMs
                         };
                     }
 

Public/Src/Engine/Processes/SandboxedProcessPipExecutor.cs

@@ -4985,6 +4985,8 @@ namespace BuildXL.Processes
                     EventConstants.PipProcessErrorWroteToStandardError :
                     string.Empty;
 
+            long totalElapsedTimeMS = Convert.ToInt64(result.PrimaryProcessTimes.TotalWallClockTime.TotalMilliseconds);
+
             Tracing.Logger.Log.PipProcessError(
                 m_loggingContext,
                 m_pip.SemiStableHash,
@@ -4996,8 +4998,10 @@ namespace BuildXL.Processes
                 messageAboutPathsToLog,
                 AddTrailingNewLineIfNeeded(outputPathsToLog),
                 result.ExitCode,
-                optionalMessage,
-                m_pip.GetShortDescription(m_context));
+                optionalMessage,                
+                m_pip.GetShortDescription(m_context),
+                totalElapsedTimeMS);          
+
         }
 
         private void HandleErrorsFromTool(string error)

Public/Src/Engine/Processes/Tracing/Log.cs

@@ -798,8 +798,8 @@ namespace BuildXL.Processes.Tracing
             LoggingContext context,
 
             // CAUTION!!!
-            // ConsoleEventListener opens up the payload array to pluck off various members. It must be updated
-            // if the order or type of these parameters change
+            // Refer PipProcessErrorEventFields.cs if any of these fields or the order is changed.
+            // A reference to a field still remains in ConsoleEventListener.cs please refer to that when the fields or the order is changed.
             long pipSemiStableHash,
             string pipDescription,
             string pipSpecPath,
@@ -809,8 +809,9 @@ namespace BuildXL.Processes.Tracing
             string messageAboutPathsToLog,
             string pathsToLog,
             int exitCode,
-            string optionalMessage,
-            string shortPipDescription);
+            string optionalMessage,            
+            string shortPipDescription,
+            long pipExecutionTimeMs);
 
         [GeneratedEvent(
             (int)LogEventId.PipProcessWarning,

Public/Src/Utilities/Instrumentation/Common/PipProcessErrorEventFields.cs

@@ -45,6 +45,9 @@ namespace BuildXL.Utilities.Instrumentation.Common
         /// <nodoc />
         public string ShortPipDescription { get; }
 
+        /// <nodoc />
+        public long PipExecutionTimeMs { get; }
+
         /// <summary>
         /// Construct PipProcessErrorEventFields from eventPayload
         /// </summary>
@@ -66,6 +69,7 @@ namespace BuildXL.Utilities.Instrumentation.Common
 
             OptionalMessage = (string)eventPayload[9 + startIndex];
             ShortPipDescription = (string)eventPayload[10 + startIndex];
+            PipExecutionTimeMs = (long)eventPayload[11 + startIndex];
 #pragma warning restore CS8600
 #pragma warning restore CS8601
 
@@ -89,7 +93,8 @@ namespace BuildXL.Utilities.Instrumentation.Common
             string pathsToLog,
             int exitCode,
             string optionalMessage,
-            string shortPipDescription
+            string shortPipDescription,
+            long pipExecutionTimeMs
             )
         {
             PipSemiStableHash = pipSemiStableHash;
@@ -103,6 +108,7 @@ namespace BuildXL.Utilities.Instrumentation.Common
             ExitCode = exitCode;
             OptionalMessage = optionalMessage;
             ShortPipDescription = shortPipDescription;
+            PipExecutionTimeMs = pipExecutionTimeMs;
         }
     }
 }

Public/Src/Utilities/Instrumentation/Tracing/Events/TargetFailedEvent.cs

@@ -24,7 +24,7 @@ namespace BuildXL.Tracing.CloudBuild
         /// <remarks>
         /// WARNING: INCREMENT IF YOU UPDATE THE PRIMITIVE MEMBERS!
         /// </remarks>
-        public override int Version { get; set; } = 2;
+        public override int Version { get; set; } = 3;
 
         /// <inheritdoc />
         public override EventKind Kind { get; set; } = EventKind.TargetFailed;
@@ -59,5 +59,15 @@ namespace BuildXL.Tracing.CloudBuild
         /// </summary>
         public string? StdOutputPath { get; set; }
 
+        /// <summary>
+        /// Short Pip Description for Target Error Path
+        /// </summary>
+        public string? ShortPipDescription { get; set; }
+
+        /// <summary>
+        /// PipExecutionTime
+        /// </summary>
+        public long PipExecutionTimeMs { get; set; }
+
     }
 }