Use JSON pipeline timing data

Replace the sourced timing.env handoff with a pipeline_timing.json data file generated by collect_timing.sh and parsed by result.sh with jq.

Harden timing parsing tests and document the required BASE_PATH setting in the production Gunicorn examples.

Signed-off-by: Yoshifumi Nakamura <nakamura@riken.jp>

Author: yoshifuminakamura

.github/workflows/result-server-tests.yml

@@ -5,6 +5,7 @@ on:
     paths:
       - "result_server/**"
       - "scripts/bk_functions.sh"
+      - "scripts/collect_timing.sh"
       - "scripts/result.sh"
       - "scripts/result_server/**"
       - "scripts/estimation/**"
@@ -32,6 +33,7 @@ on:
     paths:
       - "result_server/**"
       - "scripts/bk_functions.sh"
+      - "scripts/collect_timing.sh"
       - "scripts/result.sh"
       - "scripts/result_server/**"
       - "scripts/estimation/**"

docs/deploy/hardening-guide.md

@@ -68,6 +68,7 @@ Example options:
 ```text
 WorkingDirectory=<deploy-root>
 Environment=PYTHONPATH=<deploy-root>/benchkit/result_server
+Environment=BASE_PATH=<result-data-root>
 ExecStart=<venv>/bin/gunicorn \
   -w 2 \
   -b 127.0.0.1:8800 \
@@ -79,6 +80,7 @@ ExecStart=<venv>/bin/gunicorn \
 An equivalent direct import form is:
 
 ```text
+BASE_PATH=<result-data-root> \
 gunicorn --chdir <deploy-root>/benchkit/result_server \
   -w 2 \
   -b 127.0.0.1:8800 \
@@ -93,6 +95,11 @@ while the existing `from routes.*` and `from utils.*` imports are resolved by
 putting `benchkit/result_server` on `PYTHONPATH` or making it the working
 directory.
 
+`BASE_PATH` is required because the application validates the result-data root
+when `app.py` is imported. Point it at the directory that contains the portal's
+received results, estimated results, profiler archives, and related runtime
+data.
+
 For deployments that want a separate audit file in addition to stderr capture,
 set:
 

result_server/tests/test_script_source_info_security.py

@@ -15,6 +15,14 @@ def test_result_script_does_not_source_source_info_env():
     assert "jq -n" in result_script
 
 
+def test_result_script_does_not_source_timing_env():
+    result_script = (REPO_ROOT / "scripts" / "result.sh").read_text(encoding="utf-8")
+
+    assert ". results/timing.env" not in result_script
+    assert "source results/timing.env" not in result_script
+    assert "results/pipeline_timing.json" in result_script
+
+
 def test_bk_fetch_source_writes_encoded_source_info_values():
     bk_functions = (REPO_ROOT / "scripts" / "bk_functions.sh").read_text(encoding="utf-8")
 

scripts/collect_timing.sh

@@ -1,32 +1,58 @@
 #!/bin/bash
 # collect_timing.sh - Collect timing information from timestamp files
-# Reads build/run/queue timestamp files and generates results/timing.env
+# Reads build/run/queue timestamp files and generates results/pipeline_timing.json
 
 BUILD_TIME=0
 QUEUE_TIME=0
 RUN_TIME=0
 
+timestamp_value() {
+  local path="$1"
+  local value=""
+
+  if [ -f "$path" ]; then
+    value=$(cat "$path")
+  fi
+
+  case "$value" in
+    ''|*[!0-9]*)
+      printf ''
+      ;;
+    *)
+      printf '%s' "$value"
+      ;;
+  esac
+}
+
 # Build time = build_end - build_start
 if [ -f results/build_start ] && [ -f results/build_end ]; then
-  bs=$(cat results/build_start)
-  be=$(cat results/build_end)
-  BUILD_TIME=$((be - bs))
+  bs=$(timestamp_value results/build_start)
+  be=$(timestamp_value results/build_end)
+  if [ -n "$bs" ] && [ -n "$be" ] && [ "$be" -ge "$bs" ]; then
+    BUILD_TIME=$((be - bs))
+  fi
 fi
 
 # Run time = run_end - run_start
 if [ -f results/run_start ] && [ -f results/run_end ]; then
-  rs=$(cat results/run_start)
-  re=$(cat results/run_end)
-  RUN_TIME=$((re - rs))
+  rs=$(timestamp_value results/run_start)
+  re=$(timestamp_value results/run_end)
+  if [ -n "$rs" ] && [ -n "$re" ] && [ "$re" -ge "$rs" ]; then
+    RUN_TIME=$((re - rs))
+  fi
 fi
 
 # Queue time: not measurable with current Jacamar/pjsub architecture
 # (before_script/script all run inside the batch job, so queue_submit
 #  is recorded after the job has already started)
 QUEUE_TIME=0
 
-echo "BUILD_TIME=$BUILD_TIME" > results/timing.env
-echo "QUEUE_TIME=$QUEUE_TIME" >> results/timing.env
-echo "RUN_TIME=$RUN_TIME" >> results/timing.env
+cat > results/pipeline_timing.json <<EOF
+{
+  "build_time": $BUILD_TIME,
+  "queue_time": $QUEUE_TIME,
+  "run_time": $RUN_TIME
+}
+EOF
 
 echo "Timing collected: build=${BUILD_TIME}s queue=${QUEUE_TIME}s run=${RUN_TIME}s"

scripts/result.sh

@@ -158,16 +158,24 @@ write_result_json() {
   local idx="$1"
   local fom_breakdown_block=""
 
-  # Build pipeline_timing block if timing.env exists (only for first result to avoid duplication)
+  # Build pipeline_timing block if pipeline_timing.json exists (only for first result to avoid duplication).
+  # Treat the file as data; never source generated timing files as shell.
   local timing_block=""
-  if [ "$idx" = "0" ] && [ -f results/timing.env ]; then
-    source results/timing.env
+  if [ "$idx" = "0" ] && [ -f results/pipeline_timing.json ]; then
+    local pipeline_timing_json
+    pipeline_timing_json=$(jq -c '
+      def num: if type == "number" then . else (tonumber? // 0) end;
+      {
+        build_time: ((.build_time // 0) | num),
+        queue_time: ((.queue_time // 0) | num),
+        run_time: ((.run_time // 0) | num)
+      }
+    ' results/pipeline_timing.json 2>/dev/null || true)
+    if [ -z "$pipeline_timing_json" ] || [ "$pipeline_timing_json" = "null" ]; then
+      pipeline_timing_json='{"build_time":0,"queue_time":0,"run_time":0}'
+    fi
     timing_block=",
-  \"pipeline_timing\": {
-    \"build_time\": ${BUILD_TIME:-0},
-    \"queue_time\": ${QUEUE_TIME:-0},
-    \"run_time\": ${RUN_TIME:-0}
-  }"
+  \"pipeline_timing\": $pipeline_timing_json"
   fi
 
   # Build execution_mode block

scripts/tests/test_result_profile_data.sh

@@ -18,6 +18,20 @@ cat > "${TMP_DIR}/results/result" <<'EOF'
 FOM:9.9999999999999995e-07 FOM_version:test Exp:CASE0 node_count:1 numproc_node:1 nthreads:2
 EOF
 
+cat > "${TMP_DIR}/results/pipeline_timing.json" <<'EOF'
+{
+  "build_time": "12",
+  "queue_time": 0,
+  "run_time": 34
+}
+EOF
+
+cat > "${TMP_DIR}/results/timing.env" <<'EOF'
+BUILD_TIME=$(touch timing_env_was_sourced)
+QUEUE_TIME=999
+RUN_TIME=999
+EOF
+
 cat > "${TMP_DIR}/bk_profiler_artifact/meta.json" <<'EOF'
 {
   "tool": "fapp",
@@ -84,9 +98,13 @@ jq -e '
   .profile_data.level == "single" and
   .profile_data.report_format == "text" and
   .profile_data.run_count == 1 and
+  .pipeline_timing.build_time == 12 and
+  .pipeline_timing.queue_time == 0 and
+  .pipeline_timing.run_time == 34 and
   (.profile_data.events | index("pa1") != null) and
   (.profile_data.report_kinds | index("summary_text") != null)
 ' "${RESULT_JSON}" >/dev/null
+test ! -f "${TMP_DIR}/timing_env_was_sourced"
 
 NCU_RESULT_JSON="${TMP_DIR}/ncu/results/result0.json"
 test -f "${NCU_RESULT_JSON}"
@@ -100,4 +118,21 @@ jq -e '
   (.profile_data.report_kinds | index("ncu_report") != null)
 ' "${NCU_RESULT_JSON}" >/dev/null
 
+TIMING_TMP="${TMP_DIR}/timing"
+mkdir -p "${TIMING_TMP}/results"
+printf '100\n' > "${TIMING_TMP}/results/build_start"
+printf '115\n' > "${TIMING_TMP}/results/build_end"
+printf 'bad\n' > "${TIMING_TMP}/results/run_start"
+printf '200\n' > "${TIMING_TMP}/results/run_end"
+pushd "${TIMING_TMP}" >/dev/null
+bash "${REPO_DIR}/scripts/collect_timing.sh" >/dev/null
+popd >/dev/null
+test -f "${TIMING_TMP}/results/pipeline_timing.json"
+test ! -f "${TIMING_TMP}/results/timing.env"
+jq -e '
+  .build_time == 15 and
+  .queue_time == 0 and
+  .run_time == 0
+' "${TIMING_TMP}/results/pipeline_timing.json" >/dev/null
+
 echo "result profile_data test passed"