From 7e5f7f54c7cefe44f5d3a805b2a2cbbe4e458dca Mon Sep 17 00:00:00 2001 From: Yoshifumi Nakamura Date: Mon, 29 Jun 2026 10:28:44 +0200 Subject: [PATCH] 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 --- .github/workflows/result-server-tests.yml | 2 + docs/deploy/hardening-guide.md | 7 +++ .../tests/test_script_source_info_security.py | 8 ++++ scripts/collect_timing.sh | 46 +++++++++++++++---- scripts/result.sh | 24 ++++++---- scripts/tests/test_result_profile_data.sh | 35 ++++++++++++++ 6 files changed, 104 insertions(+), 18 deletions(-) diff --git a/.github/workflows/result-server-tests.yml b/.github/workflows/result-server-tests.yml index 63eef40..95244b3 100644 --- a/.github/workflows/result-server-tests.yml +++ b/.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/**" diff --git a/docs/deploy/hardening-guide.md b/docs/deploy/hardening-guide.md index 2110bbd..9d38309 100644 --- a/docs/deploy/hardening-guide.md +++ b/docs/deploy/hardening-guide.md @@ -68,6 +68,7 @@ Example options: ```text WorkingDirectory= Environment=PYTHONPATH=/benchkit/result_server +Environment=BASE_PATH= ExecStart=/bin/gunicorn \ -w 2 \ -b 127.0.0.1:8800 \ @@ -79,6 +80,7 @@ ExecStart=/bin/gunicorn \ An equivalent direct import form is: ```text +BASE_PATH= \ gunicorn --chdir /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: diff --git a/result_server/tests/test_script_source_info_security.py b/result_server/tests/test_script_source_info_security.py index a7d25c0..27cea3a 100644 --- a/result_server/tests/test_script_source_info_security.py +++ b/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") diff --git a/scripts/collect_timing.sh b/scripts/collect_timing.sh index d8d99b2..9a0c2de 100644 --- a/scripts/collect_timing.sh +++ b/scripts/collect_timing.sh @@ -1,23 +1,45 @@ #!/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 @@ -25,8 +47,12 @@ fi # 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 </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 diff --git a/scripts/tests/test_result_profile_data.sh b/scripts/tests/test_result_profile_data.sh index 2aa611f..19f7490 100644 --- a/scripts/tests/test_result_profile_data.sh +++ b/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"