Create tool to automate reviews of metrics files (#3620)

This adds an automatic tool to review metrics files. It should run as
part of PRB and post back the results of computing the diff.

You can see a version of its sample output here:
https://github.com/FoundationDB/fdb-record-layer/actions/runs/17975684135?pr=3620
That particular version was modified so that it would use `4.5.10.0` as
the base branch. The extra queries there account for why there were so
many identified in the analysis.

The basic structure of the report is:

1. A basic summary of the number of queries added, dropped, and changed
2. The number of new queries (per file)
3. A list of dropped queries
4. Statistics for query metric changes for queries that have modified
their plans, along with a list of outlier queries with large regressions
5. The same for queries where only the metrics changed

Those two categories of queries are separated out as we expect some
amount of churn whenever a query plan changes, as it requires that there
is some larger kind of planner change. When only the metrics change
(particularly when there is a regression), that can be indicative of
some kind of problem, as the planner may have done more work without
seeing a better outcome.

One thing that I have not been able to test is the ability to post back
comments to the PR because `pull_request` events don't have write access
(even for comments). The current version uses `pull_request_target` to
do that, which should be able to post comments back to the PR (including
in-line comments), though I have not been able to test that. I did also
look at using GitHub annotations, which would be more forgiving, but
those have the downside of being limited to something like 10 in a
single step as well as not supporting markdown formatting. So comments
seem preferable if we can get them to work.

Author: alecgrieser

.github/workflows/metrics_analysis.yml

@@ -0,0 +1,147 @@
+name: Metrics Diff Analysis
+
+on:
+  pull_request_target:
+    paths:
+      - '**/*.metrics.yaml'
+
+jobs:
+  metrics-analysis:
+    runs-on: ubuntu-latest
+    permissions:
+      issues: write
+      pull-requests: write
+
+    steps:
+    - name: Checkout code
+      uses: actions/checkout@v4
+      with:
+        fetch-depth: 0  # Need full history for git diff
+
+    - name: Setup Base Environment
+      uses: ./actions/setup-base-env
+
+    - name: Run metrics diff analysis
+      id: metrics-diff
+      run: |
+        # Run the analysis. Compare against the base hash of this PR
+        ./gradlew :yaml-tests:analyzeMetrics \
+          -PmetricsAnalysis.baseRef="${{ github.sha }}" \
+          -PmetricsAnalysis.headRef="${{ github.event.pull_request.head.sha }}" \
+          -PmetricsAnalysis.urlBase="${{ github.server_url }}/${{ github.repository }}/blob" \
+          -PmetricsAnalysis.repositoryRoot="${{ github.workspace }}" \
+          -PmetricsAnalysis.output="${{ github.workspace }}/metrics-analysis-output.txt" \
+          -PmetricsAnalysis.outlierQueries="${{ github.workspace }}/outlier-queries.txt"
+
+    - name: Add Report To Summary
+      run: cat metrics-analysis-output.txt > $GITHUB_STEP_SUMMARY
+
+    - name: Check for outliers
+      id: check-changes
+      run: |
+        if [[ -f outlier-queries.txt ]] ; then
+          echo "SIGNIFICANT_CHANGES=true" >> $GITHUB_OUTPUT
+        else
+          echo "SIGNIFICANT_CHANGES=false" >> $GITHUB_OUTPUT
+        fi
+
+    - name: Comment on PR
+      uses: actions/github-script@v7
+      continue-on-error: true
+      with:
+        github-token: ${{ secrets.GITHUB_TOKEN }}
+        script: |
+          const fs = require('fs');
+          const report = fs.readFileSync("${{ github.workspace }}/metrics-analysis-output.txt", { encoding: 'utf8', flag: 'r'});
+
+          // Find existing comment
+          const { data: comments } = await github.rest.issues.listComments({
+            owner: context.repo.owner,
+            repo: context.repo.repo,
+            issue_number: context.issue.number,
+          });
+
+          const existingComment = comments.find(comment =>
+            comment.user.type === 'Bot' && comment.body.includes('📊 Metrics Diff Analysis')
+          );
+
+          if (existingComment) {
+            // Delete previous comment
+            await github.rest.issues.deleteComment({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              comment_id: existingComment.id,
+            });
+          }
+
+          // Create new comment
+          await github.rest.issues.createComment({
+            owner: context.repo.owner,
+            repo: context.repo.repo,
+            issue_number: context.issue.number,
+            body: report
+          });
+
+    - name: Add inline comments for outliers
+      uses: actions/github-script@v7
+      if: steps.check-changes.outputs.SIGNIFICANT_CHANGES == 'true'
+      continue-on-error: true
+      with:
+        github-token: ${{ secrets.GITHUB_TOKEN }}
+        script: |
+          // First, delete any existing comments from previous runs
+          const { data: reviewComments } = await github.rest.pulls.listReviewComments({
+            owner: context.repo.owner,
+            repo: context.repo.repo,
+            pull_number: context.issue.number,
+          });
+
+          const metricsComments = reviewComments.filter(comment =>
+            comment.user.type === 'Bot' && comment.body.includes('**Significant Metrics Change**')
+          );
+
+          for (const comment of metricsComments) {
+            try {
+              await github.rest.pulls.deleteReviewComment({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                comment_id: comment.id,
+              });
+              console.log(`Deleted previous metrics comment ${comment.id}`);
+            } catch (error) {
+              console.log(`Could not delete comment ${comment.id}: ${error.message}`);
+            }
+          }
+
+          // Parse the outliers report analysis output to find files with notable queries
+          const fs = require('fs');
+          const output = fs.readFileSync("${{ github.workspace }}/outlier-queries.txt", { encoding: 'utf8', flag: 'r' });
+
+          // Each query appears separated by a blank line
+          const queries = output.split('\n\n');
+
+          for (const query of queries) {
+            var newl = query.indexOf('\n');
+            if (newl < 0) {
+              continue;
+            }
+            const info = query.substring(0, newl);
+            const match = info.match(/^(.+\.metrics\.yaml):(\d+): (.+)$/);
+            if (match) {
+              const [, filePath, lineNumber, query] = match;
+              const data = query.substring(newl, query.length);
+              try {
+                await github.rest.pulls.createReviewComment({
+                  owner: context.repo.owner,
+                  repo: context.repo.repo,
+                  pull_number: context.issue.number,
+                  body: "**Significant Metrics Change**\n\nThis query's metrics have changed significantly.\n\n" + data + "\n",
+                  path: filePath,
+                  line: lineNumber,
+                  side: 'RIGHT'
+                });
+              } catch (error) {
+                console.log(`Could not add comment to ${filePath}: ${error.message}`);
+              }
+            }
+          }

yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/GitMetricsFileFinder.java

@@ -0,0 +1,183 @@
+/*
+ * GitMetricsFileFinder.java
+ *
+ * This source file is part of the FoundationDB open source project
+ *
+ * Copyright 2015-2025 Apple Inc. and the FoundationDB project authors
+ *
+ * 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.apple.foundationdb.relational.yamltests;
+
+import com.apple.foundationdb.record.logging.KeyValueLogMessage;
+import com.apple.foundationdb.relational.api.exceptions.ErrorCode;
+import com.apple.foundationdb.relational.api.exceptions.RelationalException;
+import com.google.common.collect.ImmutableSet;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+
+/**
+ * Utility class for finding changed metrics files using git integration.
+ * This class provides methods to identify metrics files that have changed between different git references.
+ */
+public final class GitMetricsFileFinder {
+    private static final Logger logger = LoggerFactory.getLogger(GitMetricsFileFinder.class);
+
+    private GitMetricsFileFinder() {
+        // Utility class
+    }
+
+    /**
+     * Finds all metrics YAML files that have changed between two git references.
+     *
+     * @param baseRef the base git reference (e.g., "main", "HEAD~1", commit SHA)
+     * @param headRef the target git reference (e.g., "main", "HEAD~1", commit SHA)
+     * @param repositoryRoot the root directory of the git repository
+     * @return set of paths to changed metrics YAML files
+     * @throws RelationalException if git command fails or repository is not valid
+     */
+    @Nonnull
+    public static Set<Path> findChangedMetricsYamlFiles(@Nonnull final String baseRef,
+                                                        @Nonnull final String headRef,
+                                                        @Nonnull final Path repositoryRoot) throws RelationalException {
+        final List<String> changedFiles = getChangedFiles(baseRef, headRef, repositoryRoot);
+        final ImmutableSet.Builder<Path> metricsFiles = ImmutableSet.builder();
+
+        for (final var filePath : changedFiles) {
+            if (isMetricsYamlFile(filePath)) {
+                metricsFiles.add(repositoryRoot.resolve(filePath));
+            }
+        }
+
+        return metricsFiles.build();
+    }
+
+    /**
+     * Gets a list of all files changed between two git references.
+     *
+     * @param baseRef the base reference
+     * @param headRef the target reference
+     * @param repositoryRoot the repository root directory
+     * @return list of relative file paths that have changed
+     * @throws RelationalException if git command fails
+     */
+    @Nonnull
+    private static List<String> getChangedFiles(@Nonnull final String baseRef,
+                                                @Nonnull final String headRef,
+                                                @Nonnull final Path repositoryRoot) throws RelationalException {
+        final var command = List.of("git", "diff", "--name-only", baseRef + "..." + headRef);
+        try {
+            if (logger.isDebugEnabled()) {
+                logger.debug(KeyValueLogMessage.of("Executing git command",
+                        "root", repositoryRoot,
+                        "command", String.join(" ", command)));
+            }
+            final var processBuilder = new ProcessBuilder(command)
+                    .directory(repositoryRoot.toFile())
+                    .redirectErrorStream(true);
+
+            final var process = processBuilder.start();
+            final List<String> result = new ArrayList<>();
+
+            try (BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream(), StandardCharsets.UTF_8))) {
+                for (String line = reader.readLine(); line != null; line = reader.readLine()) {
+                    final String stripped = line.strip();
+                    if (!stripped.isEmpty()) {
+                        result.add(stripped);
+                    }
+                }
+            }
+
+            final var exitCode = process.waitFor();
+            if (exitCode != 0) {
+                throw new RelationalException(
+                        "Git command failed with exit code " + exitCode + ": " + String.join(" ", command),
+                        ErrorCode.INTERNAL_ERROR);
+            }
+
+            return result;
+        } catch (final IOException | InterruptedException e) {
+            throw new RelationalException("Failed to execute git command: " + String.join(" ", command), ErrorCode.INTERNAL_ERROR, e);
+        }
+    }
+
+    /**
+     * Checks if a file path represents a metrics YAML file.
+     *
+     * @param filePath the file path to check
+     * @return true if the file is a metrics file
+     */
+    private static boolean isMetricsYamlFile(@Nonnull final String filePath) {
+        return filePath.endsWith(".metrics.yaml");
+    }
+
+    /**
+     * Gets the file contents for a specific git reference (commit).
+     * This method checks out the file content at the specified reference and
+     * saves it into a temporary file located at the returned {@link Path}.
+     * If the file does not exist at the given reference, it will return a
+     * {@code null} path.
+     *
+     * @param filePath the relative file path
+     * @param gitRef the git reference
+     * @param repositoryRoot the repository root
+     * @return path to a temporary file containing the content at the specified reference
+     *    or {@code null} if it didn't exist at the given ref
+     * @throws RelationalException if git command fails
+     */
+    @Nullable
+    public static Path getFileAtReference(@Nonnull final String filePath,
+                                          @Nonnull final String gitRef,
+                                          @Nonnull final Path repositoryRoot) throws RelationalException {
+        try {
+            final var tempFile = Files.createTempFile("metrics-diff-", gitRef + ".metrics.yaml");
+            final var command = List.of("git", "show", gitRef + ":" + filePath);
+            final var processBuilder = new ProcessBuilder(command)
+                    .directory(repositoryRoot.toFile())
+                    .redirectOutput(tempFile.toFile())
+                    .redirectErrorStream(false);
+
+            final var process = processBuilder.start();
+            final var exitCode = process.waitFor();
+
+            if (exitCode != 0) {
+                // File could not be loaded. It was probably not present at that version, so return null
+                if (logger.isDebugEnabled()) {
+                    logger.debug(KeyValueLogMessage.of("Unable to get file contents at reference",
+                            "path", filePath,
+                            "ref", gitRef));
+                }
+                Files.deleteIfExists(tempFile);
+                return null;
+            }
+
+            return tempFile;
+        } catch (final IOException | InterruptedException e) {
+            throw new RelationalException(
+                    "Failed to get file content at reference " + gitRef + ":" + filePath, ErrorCode.INTERNAL_ERROR, e);
+        }
+    }
+}