Create tool to automate reviews of metrics files

.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);
+        }
+    }
+}