feat(report): visualize snapshot benchmark metrics - #207
Draft
meyer9 wants to merge 10 commits into
Draft
Conversation
Amp-Thread-ID: https://ampcode.com/threads/T-019fa50f-306b-72de-868e-122d4f74988d Co-authored-by: Amp <amp@ampcode.com>
Adds server/ — an HTTP server that replaces the per-environment static metadata.json with a dynamically assembled response fetched directly from per-run S3 files. This moves the aggregation, retention, and comparison-synthesis logic into the open-source base/benchmark repo. There is no central metadata file. The server: - Lists all <outputDir>/metadata.json objects in S3 (one per run) - Merges, deduplicates, and applies retention policy - Appends synthetic [Compare: Time] and [Compare: Versions] groups so the existing report UI can compare runs across time windows or client versions without any frontend change - Caches aggressively: per-file by ETag (invalidated when a run is rewritten), merged result by object fingerprint + 1h TTL (handles time.Now() dependence in retention/comparison) The server is a straight port of protocols/base-benchmarking's report-api, updated to use the per-run-directory S3 layout (#66 in base-benchmarking). The base-benchmarking repo retains its copy during the transition; a follow-up PR will remove it once this one is deployed. 14 tests ported from base-benchmarking (comparison synthesizer tests). All existing runner/ tests unaffected.
Allows the report server to serve directly from the directory written by 'base-bench run --output-dir <dir>' without any S3 or MinIO setup. This makes local development a one-command flow: report-server --local-dir ./output The same merge/dedup/retention/comparison-synthesis pipeline runs against local files as against S3, so reports look identical regardless of backend. Changes: - BackendStorage interface: extracted from S3Service so handlers work against either backend - LocalService: reads <outputDir>/metadata.json + metrics files from a local directory tree. Uses file mtime as the ETag equivalent for cache invalidation — a newly written metadata.json is visible on the next request. - --local-dir flag (env: BASE_BENCH_API_LOCAL_DIR): mutually exclusive with --s3-bucket; both validated at startup - S3BucketFlag: Required: true removed (validation moved to Validate()) - mergeRuns() + applyRetentionPolicy(): promoted to package-level functions so both S3Service and LocalService share the pipeline - 6 new LocalService unit tests: GetMetadata, cache hit, cache invalidation on new file, GetObject, invalid dir, load tests Verified: server starts with --local-dir, health returns 200, metadata.json returns runs from local files with comparison groups.
GetObject, ListLoadTests, and GetLoadTest all accepted user-provided values (HTTP path params) and passed them directly to filepath.Join without validating that the resolved path stays within the root dir. This allowed path traversal — e.g. GET /output/../../../etc/passwd. Fix: safePath() resolves the joined path with filepath.Clean, then checks it has the root as a prefix. Returns an error for any path that escapes the root directory. Also adds TestLocalService_PathTraversalBlocked covering the three dangerous patterns: ../etc/passwd, ../../secret, run-a/../../outside.
The previous safePath implementation used strings.HasPrefix against a manually-constructed base path, which CodeQL did not recognize as a path sanitizer and continued flagging the call sites. Switched to filepath.Rel(root, abs): if the relative path from the root to the resolved target starts with '..', the target is outside the root. filepath.Rel is the idiomatic Go pattern for this check and is more likely to be recognized by static analysis tools.
errcheck: defer result.Body.Close() -> defer result.Body.Close() //nolint:errcheck The S3 response body Close() is best-effort cleanup; the SDK documents the error as always nil. staticcheck SA1019: add //nolint:staticcheck on aws-sdk-go v1 imports aws-sdk-go v1 is deprecated in favour of v2. The migration is a larger change tracked separately; the nolint directives keep CI green in the meantime.
Amp-Thread-ID: https://ampcode.com/threads/T-019fa50f-306b-72de-868e-122d4f74988d Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa50f-306b-72de-868e-122d4f74988d Co-authored-by: Amp <amp@ampcode.com>
Collaborator
🟡 Heimdall Review Status
|
Amp-Thread-ID: https://ampcode.com/threads/T-019fa50f-306b-72de-868e-122d4f74988d Co-authored-by: Amp <amp@ampcode.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Companion to base/base#4180.
Validation
yarn prettier --check src/metricDefinitions.tsyarn build