[SuggestionProvider] Highlight locally executed actions when remote execution is used

analyzer/java/com/engflow/bazel/invocation/analyzer/bazelprofile/BUILD

@@ -1,4 +1,5 @@
 TYPES = [
+    "BazelProfileConstants.java",
     "BazelProfilePhase.java",
     "ThreadId.java",
 ]
@@ -25,4 +26,7 @@ java_library(
     name = "types",
     srcs = TYPES,
     visibility = ["//visibility:public"],
+    deps = [
+        "//third_party/guava",
+    ],
 )

analyzer/java/com/engflow/bazel/invocation/analyzer/bazelprofile/BazelProfileConstants.java

@@ -66,6 +66,8 @@ public class BazelProfileConstants {
   public static final String CAT_CRITICAL_PATH_COMPONENT = "critical path component";
   public static final String CAT_GARBAGE_COLLECTION = "gc notification";
   public static final String CAT_GENERAL_INFORMATION = "general information";
+
+  public static final String CAT_LOCAL_ACTION_EXECUTION = "local action execution";
   public static final String CAT_REMOTE_ACTION_CACHE_CHECK = "remote action cache check";
   public static final String CAT_REMOTE_ACTION_EXECUTION = "remote action execution";
   public static final String CAT_REMOTE_OUTPUT_DOWNLOAD = "remote output download";
@@ -82,6 +84,10 @@ public class BazelProfileConstants {
   // CompleteEvent names
   public static final String COMPLETE_MAJOR_GARBAGE_COLLECTION = "major GC";
   public static final String COMPLETE_MINOR_GARBAGE_COLLECTION = "minor GC";
+
+  // One of the events written when an action is executed locally, see
+  // https://github.com/bazelbuild/bazel/blob/36614cf29867c7ca86ae44b60ac2d92dcf03f204/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java#L147
+  public static final String COMPLETE_SUBPROCESS_RUN = "subprocess.run";
   public static final String COMPLETE_EXECUTE_REMOTELY = "execute remotely";
 
   // InstantEvent names

analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/BUILD

@@ -9,6 +9,7 @@ TYPES = [
     "EstimatedCoresUsed.java",
     "EstimatedJobsFlagValue.java",
     "GarbageCollectionStats.java",
+    "LocalActions.java",
     "MergedEventsPresent.java",
     "TotalDuration.java",
 ]

analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/LocalActions.java

@@ -1,8 +1,13 @@
 package com.engflow.bazel.invocation.analyzer.dataproviders;
 
+import static com.engflow.bazel.invocation.analyzer.bazelprofile.BazelProfileConstants.CAT_GENERAL_INFORMATION;
+import static com.engflow.bazel.invocation.analyzer.bazelprofile.BazelProfileConstants.CAT_LOCAL_ACTION_EXECUTION;
+import static com.engflow.bazel.invocation.analyzer.bazelprofile.BazelProfileConstants.COMPLETE_SUBPROCESS_RUN;
+
 import com.engflow.bazel.invocation.analyzer.core.Datum;
 import com.engflow.bazel.invocation.analyzer.dataproviders.LocalActions.LocalAction;
 import com.engflow.bazel.invocation.analyzer.traceeventformat.CompleteEvent;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Objects;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
@@ -14,14 +19,15 @@
 
 /** Organizes events into {@link LocalAction} by category and time period */
 public class LocalActions implements Datum, Iterable<LocalAction> {
-
   private final ImmutableList<LocalAction> actions;
 
   private LocalActions(List<LocalAction> actions) {
+    Preconditions.checkNotNull(actions);
     this.actions = actions.stream().sorted().collect(ImmutableList.toImmutableList());
   }
 
-  static LocalActions create(List<LocalAction> actions) {
+  @VisibleForTesting
+  public static LocalActions create(List<LocalAction> actions) {
     var duplicated = Lists.newArrayList();
     var actionEvents = Sets.newHashSet();
     for (LocalAction action : actions) {
@@ -97,18 +103,29 @@ public Stream<LocalAction> parallelStream() {
   /** An event and the events related by thread and time period. */
   public static class LocalAction implements Comparable<LocalAction> {
 
-    public final CompleteEvent action;
-    public final ImmutableList<CompleteEvent> relatedEvents;
+    private final CompleteEvent action;
+    private final ImmutableList<CompleteEvent> relatedEvents;
+    private final boolean executedLocally;
 
-    LocalAction(CompleteEvent action, List<CompleteEvent> relatedEvents) {
+    @VisibleForTesting
+    public LocalAction(CompleteEvent action, List<CompleteEvent> relatedEvents) {
       this.action = action;
       this.relatedEvents = ImmutableList.copyOf(relatedEvents);
+      this.executedLocally = relatedEvents.stream().anyMatch(e -> indicatesLocalExecution(e));
     }
 
     public CompleteEvent getAction() {
       return action;
     }
 
+    public List<CompleteEvent> getRelatedEvents() {
+      return relatedEvents;
+    }
+
+    public boolean isExecutedLocally() {
+      return executedLocally;
+    }
+
     @Override
     public String toString() {
       return "LocalAction{action=" + action + ", relatedEvents=" + relatedEvents + '}';
@@ -149,5 +166,11 @@ public int compareTo(LocalAction o) {
       if (i != 0) return i;
       return action.start.compareTo(o.action.start);
     }
+
+    public static boolean indicatesLocalExecution(CompleteEvent event) {
+      return CAT_LOCAL_ACTION_EXECUTION.equals(event.category)
+          || CAT_GENERAL_INFORMATION.equals(event.category)
+              && COMPLETE_SUBPROCESS_RUN.equals(event.name);
+    }
   }
 }

analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/LocalActionsDataProvider.java

@@ -22,10 +22,16 @@
 import com.google.common.collect.Iterators;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 public class LocalActionsDataProvider extends DataProvider {
+  private static final Set<String> RELATED_EVENTS_CATEGORIES =
+      Set.of(
+          CAT_REMOTE_ACTION_CACHE_CHECK,
+          CAT_REMOTE_OUTPUT_DOWNLOAD,
+          CAT_REMOTE_EXECUTION_UPLOAD_TIME);
 
   @Override
   public List<DatumSupplierSpecification<?>> getSuppliers() {
@@ -44,16 +50,17 @@ LocalActions derive() throws InvalidProfileException, MissingInputException, Nul
             .collect(Collectors.toList()));
   }
 
+  private static boolean isRelatedEvent(CompleteEvent event) {
+    return RELATED_EVENTS_CATEGORIES.contains(event.category)
+        || LocalActions.LocalAction.indicatesLocalExecution(event);
+  }
+
   Stream<LocalAction> coalesce(ProfileThread thread) {
     var localActionEvents =
         ofCategoryTypes(thread.getCompleteEvents(), BazelProfileConstants.CAT_ACTION_PROCESSING);
     var relatedEvents =
         Iterators.peekingIterator(
-            ofCategoryTypes(
-                thread.getCompleteEvents(),
-                CAT_REMOTE_ACTION_CACHE_CHECK,
-                CAT_REMOTE_OUTPUT_DOWNLOAD,
-                CAT_REMOTE_EXECUTION_UPLOAD_TIME));
+            thread.getCompleteEvents().stream().filter(event -> isRelatedEvent(event)).iterator());
 
     var out = Stream.<LocalAction>builder();
 

analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/RemoteCacheMetricsDataProvider.java

@@ -43,7 +43,7 @@ RemoteCacheMetrics derive()
   }
 
   RemoteCacheData coalesce(LocalAction action) {
-    return action.relatedEvents.stream()
+    return action.getRelatedEvents().stream()
         .reduce(RemoteCacheData.EMPTY, RemoteCacheData::plus, RemoteCacheData::plus)
         .calculateCacheState();
   }

analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/remoteexecution/BUILD

@@ -17,6 +17,7 @@ java_library(
     deps = [
         ":types",
         "//analyzer/java/com/engflow/bazel/invocation/analyzer/bazelprofile",
+        "//analyzer/java/com/engflow/bazel/invocation/analyzer/bazelprofile:types",
         "//analyzer/java/com/engflow/bazel/invocation/analyzer/core",
         "//analyzer/java/com/engflow/bazel/invocation/analyzer/time",
         "//analyzer/java/com/engflow/bazel/invocation/analyzer/traceeventformat",

analyzer/java/com/engflow/bazel/invocation/analyzer/suggestionproviders/LocalActionsWithRemoteExecutionSuggestionProvider.java

@@ -0,0 +1,114 @@
+/*
+ * Copyright 2023 EngFlow Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package com.engflow.bazel.invocation.analyzer.suggestionproviders;
+
+import com.engflow.bazel.invocation.analyzer.Caveat;
+import com.engflow.bazel.invocation.analyzer.Suggestion;
+import com.engflow.bazel.invocation.analyzer.SuggestionCategory;
+import com.engflow.bazel.invocation.analyzer.SuggestionOutput;
+import com.engflow.bazel.invocation.analyzer.core.DataManager;
+import com.engflow.bazel.invocation.analyzer.core.MissingInputException;
+import com.engflow.bazel.invocation.analyzer.core.SuggestionProvider;
+import com.engflow.bazel.invocation.analyzer.dataproviders.LocalActions;
+import com.engflow.bazel.invocation.analyzer.dataproviders.remoteexecution.RemoteExecutionUsed;
+import com.engflow.bazel.invocation.analyzer.time.DurationUtil;
+import com.google.common.annotations.VisibleForTesting;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * A {@link SuggestionProvider} that suggests migrating locally executed events to remote execution
+ * if remote execution is already being used.
+ */
+public class LocalActionsWithRemoteExecutionSuggestionProvider extends SuggestionProviderBase {
+  private static final String ANALYZER_CLASSNAME =
+      LocalActionsWithRemoteExecutionSuggestionProvider.class.getName();
+  private static final String SUGGESTION_ID_LOCAL_ACTIONS_WITH_REMOTE_EXECUTION =
+      "LocalActionsWithRemoteExecution";
+
+  public static LocalActionsWithRemoteExecutionSuggestionProvider createDefault() {
+    return new LocalActionsWithRemoteExecutionSuggestionProvider(5);
+  }
+
+  public static LocalActionsWithRemoteExecutionSuggestionProvider createVerbose() {
+    return new LocalActionsWithRemoteExecutionSuggestionProvider(Integer.MAX_VALUE);
+  }
+
+  private final int maxActions;
+
+  @VisibleForTesting
+  LocalActionsWithRemoteExecutionSuggestionProvider(int maxActions) {
+    this.maxActions = maxActions;
+  }
+
+  @Override
+  public SuggestionOutput getSuggestions(DataManager dataManager) {
+    try {
+      boolean remoteExecutionUsed =
+          dataManager.getDatum(RemoteExecutionUsed.class).isRemoteExecutionUsed();
+      List<Suggestion> suggestions = new ArrayList<>();
+      List<Caveat> caveats = new ArrayList<>();
+      if (remoteExecutionUsed) {
+        var localActions = dataManager.getDatum(LocalActions.class);
+        if (!localActions.isEmpty()) {
+          var locallyExecuted =
+              localActions.stream()
+                  .filter(action -> action.isExecutedLocally())
+                  .sorted((a, b) -> b.getAction().duration.compareTo(a.getAction().duration))
+                  .collect(Collectors.toList());
+          if (locallyExecuted.size() > maxActions) {
+            caveats.add(
+                SuggestionProviderUtil.createCaveat(
+                    String.format(
+                        "Only the %d longest, locally executed actions of the %d found were"
+                            + " listed.",
+                        maxActions, locallyExecuted.size()),
+                    true));
+          }
+          StringBuilder recommendation = new StringBuilder();
+          recommendation.append(
+              "Although remote execution was used for this invocation, some actions were still"
+                  + " executed locally. Investigate whether you can migrate these actions to remote"
+                  + " execution to speed up future builds and improve hermeticity:\n");
+          locallyExecuted.stream()
+              .limit(maxActions)
+              .forEachOrdered(
+                  action -> {
+                    recommendation.append("\t- ");
+                    recommendation.append(action.getAction().name);
+                    recommendation.append(" (");
+                    recommendation.append(DurationUtil.formatDuration(action.getAction().duration));
+                    recommendation.append(")\n");
+                  });
+          suggestions.add(
+              SuggestionProviderUtil.createSuggestion(
+                  SuggestionCategory.OTHER,
+                  createSuggestionId(SUGGESTION_ID_LOCAL_ACTIONS_WITH_REMOTE_EXECUTION),
+                  "Migrate locally executed actions to remote execution",
+                  recommendation.toString(),
+                  null,
+                  null,
+                  caveats));
+        }
+      }
+      return SuggestionProviderUtil.createSuggestionOutput(ANALYZER_CLASSNAME, suggestions, null);
+    } catch (MissingInputException e) {
+      return SuggestionProviderUtil.createSuggestionOutputForMissingInput(ANALYZER_CLASSNAME, e);
+    } catch (Throwable t) {
+      return SuggestionProviderUtil.createSuggestionOutputForFailure(ANALYZER_CLASSNAME, t);
+    }
+  }
+}

analyzer/java/com/engflow/bazel/invocation/analyzer/suggestionproviders/SuggestionProviderUtil.java

@@ -45,6 +45,9 @@ public static List<SuggestionProvider> getAllSuggestionProviders(boolean verbose
         verbose
             ? BottleneckSuggestionProvider.createVerbose()
             : BottleneckSuggestionProvider.createDefault(),
+        verbose
+            ? LocalActionsWithRemoteExecutionSuggestionProvider.createVerbose()
+            : LocalActionsWithRemoteExecutionSuggestionProvider.createDefault(),
         new BuildWithoutTheBytesSuggestionProvider(),
         new CriticalPathNotDominantSuggestionProvider(),
         new GarbageCollectionSuggestionProvider(),

analyzer/javatests/com/engflow/bazel/invocation/analyzer/dataproviders/EventThreadBuilder.java → analyzer/javatests/com/engflow/bazel/invocation/analyzer/EventThreadBuilder.java

@@ -1,4 +1,4 @@
-package com.engflow.bazel.invocation.analyzer.dataproviders;
+package com.engflow.bazel.invocation.analyzer;
 
 import static com.engflow.bazel.invocation.analyzer.WriteBazelProfile.complete;
 import static com.engflow.bazel.invocation.analyzer.WriteBazelProfile.mnemonic;
@@ -15,7 +15,7 @@
 import java.util.ArrayList;
 import java.util.List;
 
-class EventThreadBuilder {
+public class EventThreadBuilder {
 
   private final int id;
   private final int pid;
@@ -26,13 +26,14 @@ class EventThreadBuilder {
   private int nextActionStart = 0;
   private int nextRelatedStart = 0;
 
-  EventThreadBuilder(int id, int index) {
+  public EventThreadBuilder(int id, int index) {
     this.id = id;
     this.pid = 1; // hard coded by bazel
     this.index = index;
   }
 
-  public CompleteEvent action(String name, String mnemonic, int start, int duration) {
+  public CompleteEvent actionProcessingAction(
+      String name, String mnemonic, int start, int duration) {
     CompleteEvent event =
         new CompleteEvent(
             name,
@@ -48,14 +49,14 @@ public CompleteEvent action(String name, String mnemonic, int start, int duratio
     return event;
   }
 
-  public CompleteEvent action(String name, String mnemonic, int duration) {
-    return action(name, mnemonic, nextActionStart, duration);
+  public CompleteEvent actionProcessingAction(String name, String mnemonic, int duration) {
+    return actionProcessingAction(name, mnemonic, nextActionStart, duration);
   }
 
-  public CompleteEvent related(int start, int duration, String category) {
+  public CompleteEvent related(int start, int duration, String category, String name) {
     var event =
         new CompleteEvent(
-            category,
+            name,
             category,
             Timestamp.ofSeconds(start),
             Duration.ofSeconds(duration),
@@ -67,11 +68,15 @@ public CompleteEvent related(int start, int duration, String category) {
     return event;
   }
 
+  public CompleteEvent related(int start, int duration, String category) {
+    return related(start, duration, category, category);
+  }
+
   public CompleteEvent related(int duration, String category) {
     return related(nextRelatedStart, duration, category);
   }
 
-  TraceEvent asEvent() {
+  public TraceEvent asEvent() {
     return thread(
         id,
         index,