Repository: bazelbuild/bazel
Issue: Add JSON output format to aquery
Labels: P3
team-Performance


### Description of the problem / feature request:

Add --output=json to `bazel aquery`

### Feature requests: what underlying problem are you trying to solve with this feature?

I recently encountered the need to use aquery to find all the outputs being generated by a particular set of rules. The results needed post-processing to extract the exec_path. We ended up using a bunch of grep/sed. Having a more common text format like JSON would have allowed us to avoid regex-based processing of structured data.

### What operating system are you running Bazel on?

MacOS Mojave 10.14

### What's the output of `bazel info release`?

release 0.26.1

###  Have you found anything relevant by searching the web?

No

### Any other information, logs, or outputs that you want to share?

No


Issue comments:
Why aren't you parsing the proto message (instead of using grep/sed)?

If that isn't an option for you, perhaps you can just use proto2json step in between, e.g. using https://developers.google.com/protocol-buffers/docs/reference/java/com/google/protobuf/util/JsonFormat.html?
@meisterT 

> Why aren't you parsing the proto message (instead of using grep/sed)?

Avoiding grep/sed is exactly what I'd like to do, hence why I'm asking for JSON output. `jq` or similar would support doing what I want in a much cleaner fashion if I had JSON.

> If that isn't an option for you, perhaps you can just use proto2json step in between, e.g. using https://developers.google.com/protocol-buffers/docs/reference/java/com/google/protobuf/util/JsonFormat.html?

As JSON is a first-class format for proto3, I was hoping that the format could also become a first-class format for aquery instead of needing an extra step to be supported. This would also make it easy to integrate aquery with the large ecosystem of JSON-based tooling that already exists without having to add extra proto-to-json conversion steps into existing tools.
Do you perhaps want to send us a PR?

also cc @joeleba 
If the FR sounds reasonable, I can give it a try. Do you have any pointers on where to start? I'm not familiar with the codebase.
Check how we do text proto format in https://source.bazel.build/bazel/+/master:src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoOutputFormatterCallback.java
@meisterT PTAL @ #10426 

Pull request review comments:
nit: Indentation.
We can probably pass this in as an argument in the constructor? Makes it easier to test.
nit: Indentation.
Same for the rest of the instances of `jsonPrinter`.
Makes what easier to test?
For the textproto case we call `TextFormat.print()`, a static method that does not permit any injection. Why do we need something else in jsonproto case?
Fair enough.

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 875cf2473a2ba9..08cff8593d537c 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -685,6 +685,7 @@ java_library(
         "//third_party:guava",
         "//third_party:jsr305",
         "//third_party/protobuf:protobuf_java",
+        "//third_party/protobuf:protobuf_java_util",
         "@remoteapis//:build_bazel_remote_execution_v2_remote_execution_java_proto",
     ],
 )
diff --git a/src/main/java/com/google/devtools/build/lib/query2/BUILD b/src/main/java/com/google/devtools/build/lib/query2/BUILD
index 63d5a913431ec0..3a43ae314f3119 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/query2/BUILD
@@ -53,6 +53,7 @@ java_library(
         "//third_party:guava",
         "//third_party:jsr305",
         "//third_party/protobuf:protobuf_java",
+        "//third_party/protobuf:protobuf_java_util",
     ],
 )
 
diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoOutputFormatterCallback.java
index ee4158bbbb66cf..9283a24a07eb76 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoOutputFormatterCallback.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoOutputFormatterCallback.java
@@ -25,6 +25,7 @@
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
 import com.google.devtools.build.lib.skyframe.actiongraph.ActionGraphDump;
 import com.google.protobuf.TextFormat;
+import com.google.protobuf.util.JsonFormat;
 import java.io.IOException;
 import java.io.OutputStream;
 
@@ -34,7 +35,8 @@ public class ActionGraphProtoOutputFormatterCallback extends AqueryThreadsafeCal
   /** Defines the types of proto output this class can handle. */
   public enum OutputType {
     BINARY("proto"),
-    TEXT("textproto");
+    TEXT("textproto"),
+    JSON("jsonproto");
 
     private final String formatName;
 
@@ -50,6 +52,7 @@ public String formatName() {
   private final OutputType outputType;
   private final ActionGraphDump actionGraphDump;
   private final AqueryActionFilter actionFilters;
+  private final JsonFormat.Printer jsonPrinter = JsonFormat.printer();
 
   ActionGraphProtoOutputFormatterCallback(
       ExtendedEventHandler eventHandler,
@@ -110,6 +113,10 @@ public void close(boolean failFast) throws IOException {
         case TEXT:
           TextFormat.print(actionGraphContainer, printStream);
           break;
+        case JSON:
+          jsonPrinter.appendTo(actionGraphContainer, printStream);
+          printStream.println();
+          break;
         default:
           throw new IllegalStateException("Unknown outputType " + outputType.formatName());
       }
diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphQueryEnvironment.java
index 5e0e91ed8cb207..00d777241b0e0e 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphQueryEnvironment.java
@@ -178,6 +178,14 @@ public ConfiguredTargetValueAccessor getAccessor() {
                 accessor,
                 StreamedOutputHandler.OutputType.TEXT,
                 actionFilters),
+            new ActionGraphProtoV2OutputFormatterCallback(
+                eventHandler,
+                aqueryOptions,
+                out,
+                skyframeExecutor,
+                accessor,
+                StreamedOutputHandler.OutputType.JSON,
+                actionFilters),
             new ActionGraphTextOutputFormatterCallback(
                 eventHandler, aqueryOptions, out, skyframeExecutor, accessor, actionFilters))
         : ImmutableList.of(
@@ -197,6 +205,14 @@ public ConfiguredTargetValueAccessor getAccessor() {
                 accessor,
                 OutputType.TEXT,
                 actionFilters),
+            new ActionGraphProtoOutputFormatterCallback(
+                eventHandler,
+                aqueryOptions,
+                out,
+                skyframeExecutor,
+                accessor,
+                OutputType.JSON,
+                actionFilters),
             new ActionGraphTextOutputFormatterCallback(
                 eventHandler, aqueryOptions, out, skyframeExecutor, accessor, actionFilters));
   }
diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/AqueryOptions.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/AqueryOptions.java
index da45a892b91bd7..d0ed6178db18d6 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/aquery/AqueryOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/aquery/AqueryOptions.java
@@ -28,7 +28,7 @@ public class AqueryOptions extends CommonQueryOptions {
       effectTags = {OptionEffectTag.TERMINAL_OUTPUT},
       help =
           "The format in which the aquery results should be printed. Allowed values for aquery "
-              + "are: text, textproto, proto.")
+              + "are: text, textproto, proto, jsonproto.")
   public String outputFormat;
 
   @Option(
diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java
index e6d82f7bde0422..5b05015b50b510 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java
@@ -193,6 +193,14 @@ private static ImmutableList<QueryFunction> getCqueryFunctions() {
             accessor,
             aspectResolver,
             OutputType.TEXT),
+        new ProtoOutputFormatterCallback(
+            eventHandler,
+            cqueryOptions,
+            out,
+            skyframeExecutor,
+            accessor,
+            aspectResolver,
+            OutputType.JSON),
         new BuildOutputFormatterCallback(
             eventHandler, cqueryOptions, out, skyframeExecutor, accessor));
   }
diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java
index 11884c29c245fb..2de9e3ab8ab48e 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java
@@ -45,8 +45,8 @@ public enum Transitions {
       effectTags = {OptionEffectTag.TERMINAL_OUTPUT},
       help =
           "The format in which the cquery results should be printed. Allowed values for cquery "
-              + "are: label, textproto, transitions, proto. If you select 'transitions', you also "
-              + "have to specify the --transitions=(lite|full) option.")
+              + "are: label, textproto, transitions, proto, jsonproto. If you select "
+              + "'transitions', you also have to specify the --transitions=(lite|full) option.")
   public String outputFormat;
 
   @Option(
diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/ProtoOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/ProtoOutputFormatterCallback.java
index 10eaf03b92e842..cc79c8b1b3e25f 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/cquery/ProtoOutputFormatterCallback.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/ProtoOutputFormatterCallback.java
@@ -35,6 +35,7 @@
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
 import com.google.protobuf.Message;
 import com.google.protobuf.TextFormat;
+import com.google.protobuf.util.JsonFormat;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.util.Map;
@@ -45,7 +46,8 @@ class ProtoOutputFormatterCallback extends CqueryThreadsafeCallback {
   /** Defines the types of proto output this class can handle. */
   public enum OutputType {
     BINARY("proto"),
-    TEXT("textproto");
+    TEXT("textproto"),
+    JSON("jsonproto");
 
     private final String formatName;
 
@@ -60,6 +62,7 @@ public String formatName() {
 
   private final OutputType outputType;
   private final AspectResolver resolver;
+  private final JsonFormat.Printer jsonPrinter = JsonFormat.printer();
 
   private AnalysisProtos.CqueryResult.Builder protoResult;
 
@@ -108,6 +111,10 @@ private void writeData(Message message) throws IOException {
       case TEXT:
         TextFormat.print(message, printStream);
         break;
+      case JSON:
+        jsonPrinter.appendTo(message, printStream);
+        printStream.append('\n');
+        break;
       default:
         throw new IllegalStateException("Unknown outputType " + outputType.formatName());
     }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/StreamedOutputHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/StreamedOutputHandler.java
index b857b6eed6e60b..6412f0394be549 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/StreamedOutputHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/StreamedOutputHandler.java
@@ -16,6 +16,7 @@
 import com.google.devtools.build.lib.analysis.AnalysisProtosV2.ActionGraphComponent;
 import com.google.devtools.build.lib.analysis.AnalysisProtosV2.ActionGraphContainer;
 import com.google.protobuf.CodedOutputStream;
+import com.google.protobuf.util.JsonFormat;
 import java.io.IOException;
 import java.io.PrintStream;
 
@@ -24,7 +25,8 @@ public class StreamedOutputHandler {
   /** Defines the types of proto output this class can handle. */
   public enum OutputType {
     BINARY("proto"),
-    TEXT("textproto");
+    TEXT("textproto"),
+    JSON("jsonproto");
 
     private final String formatName;
 
@@ -40,6 +42,7 @@ public String formatName() {
   private final OutputType outputType;
   private final CodedOutputStream outputStream;
   private final PrintStream printStream;
+  private final JsonFormat.Printer jsonPrinter = JsonFormat.printer();
 
   public StreamedOutputHandler(
       OutputType outputType, CodedOutputStream outputStream, PrintStream printStream) {
@@ -62,6 +65,10 @@ public void printActionGraphComponent(ActionGraphComponent message) throws IOExc
       case TEXT:
         printStream.print(wrapperActionGraphComponent(message));
         break;
+      case JSON:
+        jsonPrinter.appendTo(message, printStream);
+        printStream.println();
+        break;
     }
   }
 
@@ -76,6 +83,7 @@ public void close() throws IOException {
         outputStream.flush();
         break;
       case TEXT:
+      case JSON:
         printStream.flush();
         printStream.close();
         break;


Test patch:
diff --git a/src/test/shell/integration/aquery_test.sh b/src/test/shell/integration/aquery_test.sh
index 30425c154a8b8e..e57d63e6f79288 100755
--- a/src/test/shell/integration/aquery_test.sh
+++ b/src/test/shell/integration/aquery_test.sh
@@ -132,7 +132,6 @@ EOF
   assert_not_contains "echo unused" output
 }
 
-
 function test_aquery_include_artifacts() {
   local pkg="${FUNCNAME[0]}"
   mkdir -p "$pkg" || fail "mkdir -p $pkg"
@@ -187,6 +186,32 @@ EOF
   assert_not_contains "echo unused" output
 }
 
+function test_aquery_jsonproto() {
+  local pkg="${FUNCNAME[0]}"
+  mkdir -p "$pkg" || fail "mkdir -p $pkg"
+  cat > "$pkg/BUILD" <<'EOF'
+genrule(
+    name = "bar",
+    srcs = ["dummy.txt"],
+    outs = ["bar_out.txt"],
+    cmd = "echo unused > $(OUTS)",
+)
+EOF
+  echo "hello aquery" > "$pkg/in.txt"
+
+  bazel aquery --output=jsonproto "//$pkg:bar" > output 2> "$TEST_log" \
+    || fail "Expected success"
+  cat output >> "$TEST_log"
+  assert_contains "\"execPath\": \"$pkg/dummy.txt\"" output
+  assert_contains "\"mnemonic\": \"Genrule\"" output
+  assert_contains "\"mnemonic\": \".*-fastbuild\"" output
+  assert_contains "echo unused" output
+
+  bazel aquery --output=jsonproto --noinclude_commandline "//$pkg:bar" > output \
+    2> "$TEST_log" || fail "Expected success"
+  assert_not_contains "echo unused" output
+}
+
 function test_aquery_skylark_env() {
   local pkg="${FUNCNAME[0]}"
   mkdir -p "$pkg" || fail "mkdir -p $pkg"
@@ -1113,4 +1138,33 @@ EOF
   assert_contains "mnemonic: \".*-fastbuild\"" output
   assert_contains "echo unused" output
 }
+
+function test_basic_aquery_jsonproto_v2() {
+  local pkg="${FUNCNAME[0]}"
+  mkdir -p "$pkg" || fail "mkdir -p $pkg"
+  cat > "$pkg/BUILD" <<'EOF'
+genrule(
+    name = "bar",
+    srcs = ["dummy.txt"],
+    outs = ["bar_out.txt"],
+    cmd = "echo unused > $(OUTS)",
+)
+EOF
+  bazel aquery --incompatible_proto_output_v2 --output=jsonproto "//$pkg:bar" > output 2> "$TEST_log" \
+    || fail "Expected success"
+  cat output >> "$TEST_log"
+
+  # Verify than ids come in integers instead of strings.
+  assert_contains "\"id\": 1," output
+  assert_not_contains "\"id\": \"1\"" output
+
+  # Verify that paths are broken down to path fragments.
+  assert_contains "\"pathFragment\": {" output
+
+  # Verify that the appropriate action was included.
+  assert_contains "\"label\": \"dummy.txt\"" output
+  assert_contains "\"mnemonic\": \"Genrule\"" output
+  assert_contains "\"mnemonic\": \".*-fastbuild\"" output
+  assert_contains "echo unused" output
+}
 run_suite "${PRODUCT_NAME} action graph query tests"