From 6e4c40114663f7f9055145a4d8fea122dfda48dc Mon Sep 17 00:00:00 2001
From: Terry Kong <terryk@nvidia.com>
Date: Tue, 30 Jan 2024 22:03:37 -0800
Subject: [PATCH] Updates upstream mgmn metric table creation and ensures no
 collision of metrics (#503)

Follow up of #496

## changes:
1. upstream MGMN metrics for pax and t5x were being combined so that if
t5x finishes first, then paxml will print both t5x and paxml metrics.
This is fixed by changing the artifact prefix so that t5x only prints
t5x metrics and pax only prints pax metrics.
2. Updates the metric printing to the same as #496 to show everything in
a markdown table instead of a bunch of json files which was difficult to
visually inspect
---
 .github/workflows/_sitrep_mgmn.yaml | 26 ++++++++++++++++++--------
 .github/workflows/_test_pax.yaml    |  2 +-
 .github/workflows/_test_t5x.yaml    |  2 +-
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/.github/workflows/_sitrep_mgmn.yaml b/.github/workflows/_sitrep_mgmn.yaml
index eea405ace..f3b8f2aaf 100644
--- a/.github/workflows/_sitrep_mgmn.yaml
+++ b/.github/workflows/_sitrep_mgmn.yaml
@@ -60,13 +60,23 @@ jobs:
         shell: bash -x -e {0}
         run: |
           METRICS_SUMMARY_FILE="metrics_summary.json"
-          echo -e "\n\n## ${{ inputs.FW_NAME }} MGMN Test Metrics" >> $METRICS_SUMMARY_FILE
-          for i in metrics-test-log/*_metrics.json; do
-            echo $i | cut -d'.' -f1
-            echo '```json'
-            jq . $i
-            echo '```'
-          done | tee -a $METRICS_SUMMARY_FILE
+
+          echo '## Upstream ${{ inputs.FW_NAME }} MGMN Test Metrics' | tee -a $METRICS_SUMMARY_FILE
+          python <<EOF | tee -a $METRICS_SUMMARY_FILE
+          import json
+          files = "$(echo upstream-${{ inputs.FW_NAME }}-metrics-test-log/*_metrics.json)".split()
+          header = None
+          print_row = lambda lst: print('| ' + ' | '.join(str(el) for el in lst) + ' |')
+          for path in files:
+            with open(path) as f:
+              obj = json.loads(f.read())
+              if not header:
+                header = list(obj.keys())
+                print_row(["Job Name"] + header)
+                print_row(["---"] * (1+len(header)))
+              job_name = path[:-len('_metrics.json')]
+              print_row([job_name] + [obj[h] for h in header])
+          EOF
 
           cat $METRICS_SUMMARY_FILE >> $GITHUB_STEP_SUMMARY
           echo "METRICS_SUMMARY_FILE=$METRICS_SUMMARY_FILE" >> ${GITHUB_OUTPUT}
@@ -83,7 +93,7 @@ jobs:
           failed_tests=$(jq -r '. | select ((.state != "COMPLETED") or (.exitcode != "0")) | .state' $EXIT_STATUSES | wc -l)
           total_tests=$(ls $EXIT_STATUSES | wc -l)
 
-          METRICS_LOG=metrics-test-log/report.jsonl
+          METRICS_LOG=upstream-${{ inputs.FW_NAME }}-metrics-test-log/report.jsonl
           all_outcomes() {
             cat $METRICS_LOG | jq -r '. | select((.["$report_type"] == "TestReport") and (.when == "call")) | .outcome'
           }
diff --git a/.github/workflows/_test_pax.yaml b/.github/workflows/_test_pax.yaml
index 5c04ac30c..b952793df 100644
--- a/.github/workflows/_test_pax.yaml
+++ b/.github/workflows/_test_pax.yaml
@@ -474,7 +474,7 @@ jobs:
       - name: Upload metrics test json logs
         uses: actions/upload-artifact@v3
         with:
-          name: metrics-test-log
+          name: upstream-${{ inputs.FW_NAME }}-metrics-test-log
           path: |
             report.jsonl
             *_metrics.json
diff --git a/.github/workflows/_test_t5x.yaml b/.github/workflows/_test_t5x.yaml
index 86a3944cd..810c66f91 100644
--- a/.github/workflows/_test_t5x.yaml
+++ b/.github/workflows/_test_t5x.yaml
@@ -320,7 +320,7 @@ jobs:
       - name: Upload metrics test json logs
         uses: actions/upload-artifact@v3
         with:
-          name: metrics-test-log
+          name: upstream-${{ inputs.FW_NAME }}-metrics-test-log
           path: |
             report.jsonl
             *_metrics.json