chore: merge develop branch into main#19
Merged
Conversation
…na integration details
…improved setup instructions
…rom-snapshot script
…sage instructions
… script for new clients
… for threshold calculation
…ate validation logic
…trieval in common queries
…tgreSQL settings in docker-compose.yml
* fix: Grafana Startup and Warnings * fix: Copy configs into final stage docker container * fix: Use a mount for runner configs * fix: Update README and prometheusRWEndpoint * fix: Restore endpoint configs * fix: formatting * Update README.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Brings develop to security parity with main (commits 53997ff, a9bd670, c860d7f): - Add runner/api/inputvalidation.go with ValidateID, ValidateSeverity, ValidateComparisonMode, SanitizeLogValue (+ tests, verbatim from main). - Wire 34 validation/sanitization call sites across handlers.go, grafana_api.go, websocket.go. - Disable HTML branch in HandleGetReport (return JSON for any format query) to close CodeQL go/reflected-xss. - Add artifact-metadata: write to build_docker_image.yml. - Bump dashboard deps: vitest ^4.1.0, @typescript-eslint/* ^8.60.0, axios ^1.16.1. Regenerate package-lock.json.
Replaces the flag-modal `runner` entry point with explicit subcommands: - `runner benchmark` — run a benchmark (k6 → Prometheus → reports) - `runner api` — start the HTTP API server - `runner historic` — generate a historic-analysis report from PostgreSQL `runner` with no subcommand prints usage and exits 2. Global `--output` and `--log-level` flags live on the root command. The old single-dash flag-modal invocations (`runner -api`, `runner -historic-mode`, `-config`, ...) are no longer recognised and will fail at flag-parse time with a clear error — preferable to silently doing nothing. - Add `github.com/spf13/cobra` as a direct dependency. - Extract the existing flows from `runner/main.go` into a new `runner/cmd/` package (root, benchmark, api, historic, common). - Shrink `runner/main.go` to a thin `cmd.Execute()` entry point. - Update `runner/Dockerfile` to build the full package (`./runner` instead of `./runner/main.go`). - Update `docker-compose.yml`'s runner service to invoke `runner api --storage-config=...`. - Update README invocations + add a "Migrating from the pre-refactor CLI" table.
…rison
Adds the `compare` Cobra subcommand for one-shot cross-client JSON-RPC
response comparison. Flags-only, no YAML — uses the existing
`clients.yaml` registry for client definitions:
runner compare \
--clients ./config/clients.yaml \
--client-refs geth,nethermind \
--methods eth_blockNumber,eth_chainId,eth_gasPrice \
--output ./out
Always writes both `comparison-results.json` and `comparison-report.html`
to `--output`. Filesystem-only; does not touch the historic-runs DB.
Cleans up the dead `ValidateResponses` field from the benchmark config
(removed from `config.Config`, the backward-compat loader struct, and
the four YAML files under `config/` that still set it). Also drops the
orphaned `runner/validator/` package and the dead
`runner/comparator/utils.go` bridge — neither had any importers after
the Cobra refactor.
Adds OpenRPC-spec-driven cross-client response comparison as the fifth
Cobra subcommand. Flags-only, reuses the `clients.yaml` registry from
the `compare` subcommand, and produces the same filesystem artefacts
(`comparison-results.json` and `comparison-report.html` under
`--output`).
runner compare-openrpc \
--spec ./openrpc.json \
--variations ./config/param_variations.yaml \
--clients ./config/clients.yaml \
--client-refs geth,nethermind \
--filter eth_blockNumber,eth_getBlockByNumber \
--output ./openrpc-results
Implements the historical debug aids from main's old
`cmd/compare-openrpc/main.go`:
- `--filter` accepts both base method names and their full variant
names; matching on the base name keeps every `_variantN` sibling
generated from the variations file.
- `--curl` enables the comparator's verbose mode, which logs a
curl-equivalent command for each JSON-RPC request.
Filesystem-only; does not touch the historic-runs DB, matching the
behaviour of `compare`.
Make benchmark metrics resilient when Prometheus loses data for a
subset of (client, method) pairs by gap-filling from the k6
--summary-export JSON. Per-pair, not all-or-nothing: a pair with even
one Prometheus sample stays on Prometheus.
- Add runner/metrics/summary_fallback.go with the k6 summary parser,
per-(client,method) extractor, and per-pair walker. The summary file
is loaded lazily, once per CollectClientsMetrics call, and the load
error (if any) is cached so we don't re-attempt per pair.
- Modify metrics.CollectClientsMetrics to accept a *logrus.Logger and
emit one Warn per (client, method) pair where Prometheus returned
zero samples:
"Prometheus had no data for %s.%s, falling back to summary.json"
- Add TODO(post-merge) markers on ClientMetrics.ErrorTypes and
ClientMetrics.StatusCodes — they stay declared and empty pending the
follow-up that wires them from k6 Prometheus labels.
Lifts the field-population logic from main's runner/generator/
metrics_parser.go but explicitly drops ValidatePercentiles,
extractClientNames, the percentile-ordering sanity checks, the
"p99=0 for traffic" per-method warn, and the p99-coverage logging:
user-defined k6 thresholds per Call are the replacement for static
percentile checks.
Tests cover the happy path, the fallback path against a synthetic
summary fixture, and the no-summary-file path (Warn + leave-at-zero,
no panic).
Final pass before the develop->main merge. - Add --html-report flag to `runner benchmark` (default false). JSON and CSV exports remain on by default; the cascading HTML report block is now opt-in. `compare` and `compare-openrpc` are unaffected and continue to always generate their HTML report. - Trim the dead `runner/comparator` rendering branch from runner/generator/html_report.go and delete runner/generator/response_utils.go, both of which had no callers after the Stage 2 cmd/ split. - Drop metrics/dashboards/jsonrpc-benchmark.json. Its Prometheus queries target custom counters (method_calls_*, rpc_errors_*, rpc_calls_*) that develop's k6 driver no longer emits. The three remaining dashboards stay in place. - Strip non-ASCII characters from all Go source under runner/: arrows and em-dashes in comments and Cobra `Short:` descriptions, plus emoji glyphs in HTML report titles, recommendation strings, and a handful of debug-binary `fmt.Println` calls. - README: document --html-report, refresh the migration callout, and swap em-dashes for plain ASCII.
…ment GetHistoricSummary
Three pre-existing bugs surfaced during the live pre-merge smoke. All
of them blocked end-to-end runs of subcommands the refactor stream
otherwise wired up correctly.
- benchmark could not query Prometheus after the run because
--prometheus-rw http://localhost:9090/api/v1/write was used both as
k6's remote-write target and as the Prometheus query client's base
address. The query client then issued GET <base>/api/v1/query, which
resolved to http://localhost:9090/api/v1/write/api/v1/query and
404ed. Split the single flag into a base URL plus a path-only
override:
* --prometheus (default http://localhost:9090): base URL. Used
directly by the query client; --prometheus-rw-path is appended
to form the remote-write target.
* --prometheus-rw-path (default /api/v1/write): path appended for
remote-write. Override only when your deployment uses a
non-standard path.
* --prometheus-rw (the old full-URL flag) is removed. Old
invocations migrate by passing --prometheus http://host:port and
optionally --prometheus-rw-path <path>.
- runner historic required a resolvable client registry even though
it only consumes cfg.TestName from the loaded config. Add a
--clients flag mirroring runner benchmark so configs using
clients: [name, name] references resolve correctly. Also guard
against an empty GetHistoricSummary result so the report generator
is never called with a nil BestRun.
- storage.HistoricStorage.GetHistoricSummary was a "not implemented"
stub. Implement it by aggregating over ListHistoricRuns: TotalRuns,
FirstRun/LastRun timestamps, BestRun and WorstRun by lowest/highest
avg_latency, and a RecentRuns slice (top 10). Also drop the error
from GetHistoricTrends so the warn-log path doesn't fire while real
trend aggregation remains a post-merge follow-up.
cbermudez97
changed the title
CI's Trivy scan reports 19 vulnerabilities (HIGH: 17, CRITICAL: 2) in the grafana/k6:1.4.0 base image, all in libcrypto3 / libssl3 (OpenSSL 3.5.4-r0). Fixed versions exist in Alpine 3.22's update channel; the base image pin just hasn't picked them up. Add an `apk --no-cache upgrade` step before the package install so the runtime stage refreshes against Alpine 3.22's current security patches without changing the pinned k6 release.
CI's Trivy scan was flagging 15 stdlib CVEs (CRITICAL: 1, HIGH: 14) in the runner binary built against Go 1.25.4, plus 19 stdlib CVEs in the k6 binary embedded in grafana/k6:1.4.0 (Go 1.25.3). Bump both: - builder: golang:1.25.4-alpine -> golang:1.25.11-alpine (matches main; covers all listed crypto/tls, crypto/x509, net/http, net/url, encoding/asn1 CVE fixes for our binary). - runtime: grafana/k6:1.4.0 -> grafana/k6:1.8.0 (latest published k6 release; brings the embedded Go runtime forward).
Switch the build_docker_image workflow from the Nethermind-shared docker-build-push-dockerhub workflow to a self-contained GHCR pipeline. The shared workflows repo doesn't ship a GHCR equivalent. - Registry: docker.io -> ghcr.io. - Image name: nethermindeth/jsonrpc-bench-runner -> ghcr.io/<repo>, which resolves to ghcr.io/nethermindeth/json-bench to match the repository name. - Auth: drop the DOCKER_HUB_USERNAME / DOCKER_HUB_PASSWORD secrets; use the built-in GITHUB_TOKEN with `packages: write` instead. - Trivy: keep the CRITICAL/HIGH gate inline. Add `ignore-unfixed: true` so upstream CVEs without a published fix do not block CI (they still appear in the report). - Build provenance: attest the pushed image via actions/attest-build-provenance@v2.
`${{ github.repository }}` resolves to `NethermindEth/json-bench`
with the org's camelcase. Docker reference grammar requires repo
paths to be lowercase, so Trivy was aborting at parse time with
"could not parse reference: NethermindEth/json-bench:scan-...".
The push tags coming from docker/metadata-action are already
lowercased, so only the locally-loaded scan tag is affected.
Decouple that local-only handle from the registry image name by
tagging it `local/runner:scan-<sha>`.
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.
Merges
chore/app-refactorintomain. The branch carries siximplementation stages, each landed as its own commit so the history
stays bisectable.
What changed
runner/api/inputvalidation.go+ tests verbatimfrom main, wired ~32 validator/sanitizer call sites across the API
handlers, disabled the XSS-prone HTML branch in
HandleGetReport,added
artifact-metadata: writeto the docker workflow, and bumpeddashboard deps.
runnerentry point withexplicit Cobra subcommands (
benchmark,api,historic).runnerwith no arguments prints usage and exits 2.
comparesubcommand for one-shot cross-clientJSON-RPC response comparison; swept out the dead
Config.ValidateResponsesfield, thevalidate_responses:YAML keys,the orphan
runner/validator/package, andrunner/comparator/utils.go.compare-openrpcsubcommand with--variations, variant-aware--filter, and the--curldebug aid.loss by gap-filling from the k6
--summary-exportJSON on aper-(client, method) basis, with a per-pair Warn log.
--html-report(default off), trimmed the dead comparator-render branch from the
generators, dropped the broken
jsonrpc-benchmark.jsonGrafanadashboard, and stripped non-ASCII characters from Go source.
Follow-up work is tracked in
POST_MERGE_FOLLOWUPS.md.Breaking changes
runner -api,runner -historic-modeand other pre-refactor flag-modal invocations no longer parse; the
mapping is documented under "Migrating from the pre-refactor CLI" in
README.md.
runner benchmarkno longer writesoutputs/report.htmlby default.Pass
--html-reportto restore it. JSON and CSV exports remain on.endpoints + frequencyYAML schema is no longeraccepted; configs must be migrated by hand to the
calls:schema.metrics/dashboards/jsonrpc-benchmark.jsonhas been removed; itsqueries targeted custom counters the current k6 driver no longer
emits.
Test plan
runner benchmark --config config/mixed.yaml --clients config/clients.yaml --html-reportagainst a live docker stack producesoutputs/results.{json,csv}andoutputs/report.html.--html-reportproducesoutputs/results.{json,csv}and noreport.html.runner compareandrunner compare-openrpcagainst local clients each produce bothcomparison-results.jsonandcomparison-report.html.runner api --storage-config config/storage-example.yamlboots and serves/api/health.runner historic --storage-config config/storage-example.yaml --config config/mixed.yamlruns to completion against a populated database.