feat(benchmarks): replace wrk with k6, add APISIX comparative baseline#82
Merged
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @OliverWolffGIP, your pull request is larger than the review limit of 150000 diff characters
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository: cuioss/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (68)
📝 WalkthroughWalkthroughThe pull request adds JWKS egress and TLS trust-profile configuration, eagerly validates token configuration at startup, and replaces WRK integration benchmarks with k6. It also adds k6 result processing, comparative APISIX benchmarking, matching integration topology, tests, documentation, and CI updates. ChangesSecurity configuration and token validation
k6 benchmark migration
Comparison integration topology
Documentation and edge contracts
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the benchmark toolchain from wrk to k6, introducing a target-neutral harness and a comparative baseline against Apache APISIX. It adds support for logical trust profiles (tls_profile) and SSRF egress allowlists (allowed_egress_hosts) in gateway.yaml to securely handle JWKS validation. The wrk-based post-processing is replaced with K6ResultPostProcessor and K6BenchmarkConverter, and a ComparisonSummaryWriter is added to generate side-by-side markdown summaries. The review feedback focuses on improving the robustness of JSON parsing across these new post-processing utilities by catching syntax exceptions and verifying that parsed JSON elements are primitives before extracting them as strings.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
benchmarks/README.adoc (1)
243-301: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify the working directory for executing the comparison.
The documentation instructs the user to run the comparison from the
benchmarks/directory (scripts/run-comparison.sh), but later provides a Maven command (./mvnw -pl benchmarks ...) that must be executed from the repository root. Executing./mvnwfrom inside a submodule typically fails, as the wrapper is usually located at the project root.Consider updating the instructions to use a consistent execution context (e.g., executing everything from the repository root, or adjusting the Maven command to
../mvnwif the user is expected to remain in thebenchmarks/directory).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/README.adoc` around lines 243 - 301, Clarify the working directory in the comparison instructions so both scripts/run-comparison.sh and the ComparisonSummaryWriter Maven command are executable from the same location. Update the commands and surrounding text consistently, ensuring the Maven wrapper path and -pl benchmarks invocation resolve correctly from that directory.benchmarks/pom.xml (1)
231-451: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEight near-identical
docker compose run k6 run /scripts/*.jsexecutions — consider a driver script.Each block only varies by script path and (once) VU override. A single shell script (in the same spirit as
pre-benchmark-health-check.sh) that loops over an aspect list and invokesdocker compose runper entry would collapse ~220 lines of repeated XML into one execution plus a short script, and adding a new aspect would become a one-line change instead of a new 26-line block.♻️ Sketch of the consolidation
# benchmarks/src/main/resources/k6-scripts/run-benchmark-aspects.sh aspects="health_live gateway_health proxied_static bearer_proxied http2 graphql upload_small upload_large" for aspect in $aspects; do vus="$BENCHMARK_VUS" [ "$aspect" = "upload_large" ] && vus="$BENCHMARK_VUS_UPLOAD_LARGE" BENCHMARK_VUS="$vus" docker compose run --rm k6 run "/scripts/${aspect}.js" done🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/pom.xml` around lines 231 - 451, Consolidate the repeated k6 executions in the Maven configuration into one driver execution that invokes a new run-benchmark-aspects.sh script. Have the script iterate over health_live, gateway_health, proxied_static, bearer_proxied, http2, graphql, upload_small, and upload_large, passing the standard VU value except using the upload-large override for upload_large, while preserving the existing compose working directory, duration, output directory, and timeout behavior.benchmarks/scripts/run-comparison.sh (1)
110-117: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNormalize
OUTPUT_DIRto an absolute path before using it as a bind-mount source.The default is absolute, but a relative
--output(e.g../results) flows into-v "${target_dir}:/results"at Line 147. Docker treats a non-absolute bind source inconsistently, so a relative--outputcan silently fail to mount the results directory. Resolve it once after arg parsing.♻️ Suggested normalization
mkdir -p "${OUTPUT_DIR}" +OUTPUT_DIR="$(cd "${OUTPUT_DIR}" && pwd)" echo "Comparison run -> ${OUTPUT_DIR}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/scripts/run-comparison.sh` around lines 110 - 117, Normalize OUTPUT_DIR to an absolute path immediately after argument parsing and before creating directories or constructing bind-mount paths. Ensure relative --output values such as ./results are resolved once, then reuse the normalized value when computing target_dir and passing it to Docker.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/auth/TokenValidatorProducer.java`:
- Around line 81-83: Update onStartup in TokenValidatorProducer to explicitly
invoke the injected TokenValidator, forcing gatewayTokenValidator() and its
tls_profile validation during application startup rather than merely resolving
the normal-scoped proxy. Add a `@QuarkusTest` that boots with an unknown
tls_profile and verifies startup fails.
In `@benchmarks/Dockerfile.k6-dashboard`:
- Around line 8-40: Remove the custom builder stage and all
xk6-dashboard-related arguments, package installation, and build commands from
the Dockerfile. Use the stock digest-pinned grafana/k6:2.1.0 image directly, and
configure the dashboard through K6_WEB_DASHBOARD=true and
K6_WEB_DASHBOARD_EXPORT instead of linking an extension.
---
Nitpick comments:
In `@benchmarks/pom.xml`:
- Around line 231-451: Consolidate the repeated k6 executions in the Maven
configuration into one driver execution that invokes a new
run-benchmark-aspects.sh script. Have the script iterate over health_live,
gateway_health, proxied_static, bearer_proxied, http2, graphql, upload_small,
and upload_large, passing the standard VU value except using the upload-large
override for upload_large, while preserving the existing compose working
directory, duration, output directory, and timeout behavior.
In `@benchmarks/README.adoc`:
- Around line 243-301: Clarify the working directory in the comparison
instructions so both scripts/run-comparison.sh and the ComparisonSummaryWriter
Maven command are executable from the same location. Update the commands and
surrounding text consistently, ensuring the Maven wrapper path and -pl
benchmarks invocation resolve correctly from that directory.
In `@benchmarks/scripts/run-comparison.sh`:
- Around line 110-117: Normalize OUTPUT_DIR to an absolute path immediately
after argument parsing and before creating directories or constructing
bind-mount paths. Ensure relative --output values such as ./results are resolved
once, then reuse the normalized value when computing target_dir and passing it
to Docker.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3928edde-f2fc-4571-9172-a92abd74c209
📒 Files selected for processing (66)
.github/workflows/benchmark.ymlapi-sheriff/src/main/java/de/cuioss/sheriff/api/auth/JwksTrustProfileResolver.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/auth/TokenValidatorProducer.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/IssuerConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/DispatchStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GatewayEdgeRoute.javaapi-sheriff/src/main/resources/application.propertiesapi-sheriff/src/main/resources/schema/gateway.schema.jsonapi-sheriff/src/test/java/de/cuioss/sheriff/api/auth/JwksTrustProfileResolverTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/auth/TestTlsConfigurationRegistry.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/auth/TokenValidatorProducerTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/load/ConfigLoaderTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/model/ConfigModelContractTest.javabenchmarks/Dockerfile.k6benchmarks/Dockerfile.k6-dashboardbenchmarks/Dockerfile.wrkbenchmarks/README.adocbenchmarks/pom.xmlbenchmarks/scripts/run-comparison.shbenchmarks/src/main/java/de/cuioss/sheriff/api/k6/benchmark/ComparisonSummaryWriter.javabenchmarks/src/main/java/de/cuioss/sheriff/api/k6/benchmark/K6BenchmarkConverter.javabenchmarks/src/main/java/de/cuioss/sheriff/api/k6/benchmark/K6BenchmarkLogMessages.javabenchmarks/src/main/java/de/cuioss/sheriff/api/k6/benchmark/K6ResultPostProcessor.javabenchmarks/src/main/java/de/cuioss/sheriff/api/k6/benchmark/package-info.javabenchmarks/src/main/resources/k6-scripts/bearer_proxied.jsbenchmarks/src/main/resources/k6-scripts/gateway_health.jsbenchmarks/src/main/resources/k6-scripts/graphql.jsbenchmarks/src/main/resources/k6-scripts/health_live.jsbenchmarks/src/main/resources/k6-scripts/http2.jsbenchmarks/src/main/resources/k6-scripts/lib/summary.jsbenchmarks/src/main/resources/k6-scripts/lib/target.jsbenchmarks/src/main/resources/k6-scripts/pre-benchmark-health-check.shbenchmarks/src/main/resources/k6-scripts/proxied_static.jsbenchmarks/src/main/resources/k6-scripts/upload_large.jsbenchmarks/src/main/resources/k6-scripts/upload_small.jsbenchmarks/src/main/resources/wrk-scripts/api_health_benchmark.shbenchmarks/src/main/resources/wrk-scripts/api_health_check.luabenchmarks/src/main/resources/wrk-scripts/bearer_proxied_benchmark.shbenchmarks/src/main/resources/wrk-scripts/bearer_proxied_check.luabenchmarks/src/main/resources/wrk-scripts/health_live_benchmark.shbenchmarks/src/main/resources/wrk-scripts/health_live_check.luabenchmarks/src/main/resources/wrk-scripts/proxied_static_benchmark.shbenchmarks/src/main/resources/wrk-scripts/proxied_static_check.luabenchmarks/src/test/java/de/cuioss/sheriff/api/k6/benchmark/ComparisonSummaryWriterTest.javabenchmarks/src/test/java/de/cuioss/sheriff/api/k6/benchmark/K6BenchmarkConverterTest.javabenchmarks/src/test/java/de/cuioss/sheriff/api/wrk/benchmark/BearerProxiedRouteBenchmarkTest.javabenchmarks/src/test/java/de/cuioss/sheriff/api/wrk/benchmark/GatewayEndpointBenchmarkTest.javabenchmarks/src/test/resources/k6-summary-gateway-health.jsonbenchmarks/src/test/resources/k6-summary-health-live.jsonbenchmarks/src/test/resources/wrk-api-health-results.txtbenchmarks/src/test/resources/wrk-health-results.txtdoc/LogMessages.adocdoc/adr/0011-gatewayyaml_exposes_JWKS_trust_and_egress_as_neutral_names_bound_by_the_deployment.adocdoc/configuration.adocdoc/plan/04b-comparative-benchmark.adocdoc/plan/README.adocdoc/security-threat-model.adocintegration-tests/docker-compose.apisix.ymlintegration-tests/docker-compose.ymlintegration-tests/src/main/docker/apisix/apisix.yamlintegration-tests/src/main/docker/apisix/config.yamlintegration-tests/src/main/docker/apisix/render-tls-and-start.shintegration-tests/src/main/docker/keycloak/benchmark-realm.jsonintegration-tests/src/main/docker/sheriff-config/endpoints/httpbin.yamlintegration-tests/src/main/docker/sheriff-config/gateway.yamlpom.xml
💤 Files with no reviewable changes (13)
- benchmarks/src/main/resources/wrk-scripts/health_live_benchmark.sh
- benchmarks/src/main/resources/wrk-scripts/api_health_benchmark.sh
- benchmarks/Dockerfile.wrk
- benchmarks/src/test/resources/wrk-health-results.txt
- benchmarks/src/test/resources/wrk-api-health-results.txt
- benchmarks/src/main/resources/wrk-scripts/bearer_proxied_check.lua
- benchmarks/src/main/resources/wrk-scripts/api_health_check.lua
- benchmarks/src/main/resources/wrk-scripts/health_live_check.lua
- benchmarks/src/main/resources/wrk-scripts/bearer_proxied_benchmark.sh
- benchmarks/src/main/resources/wrk-scripts/proxied_static_benchmark.sh
- benchmarks/src/test/java/de/cuioss/sheriff/api/wrk/benchmark/BearerProxiedRouteBenchmarkTest.java
- benchmarks/src/main/resources/wrk-scripts/proxied_static_check.lua
- benchmarks/src/test/java/de/cuioss/sheriff/api/wrk/benchmark/GatewayEndpointBenchmarkTest.java
OliverWolffGIP
added a commit
that referenced
this pull request
Jul 20, 2026
onStartup observed StartupEvent with the @ApplicationScoped validator as a lazy CDI client proxy, so ArC never invoked gatewayTokenValidator() at boot — tls_profile resolution was deferred to the first bearer request instead of failing startup. Invoke a method on the injected proxy to force contextual-instance creation at boot, and correct the misleading comment. Add onStartup unit tests proving the validator is now dereferenced. Addresses PR #82 CodeRabbit review (TokenValidatorProducer.java:81-83). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hh2epJFjdLAt2wZyxEeh3p
OliverWolffGIP
added a commit
that referenced
this pull request
Jul 20, 2026
…primitive JSON Three PR #82 review fixes in the k6 post-processing harness: - ComparisonSummaryWriter.readTargetSummaries now wraps each per-file parse in try/catch (JsonSyntaxException | IllegalStateException), logging the bad file via FAILED_PARSE_SUMMARY and continuing, so one truncated summary cannot abort the whole comparison (Gemini). - K6ResultPostProcessor.stringOrNull/instantOrNull now guard with isJsonPrimitive() before getAsString(), so a present-but-non-primitive metadata field yields the absent-field path instead of an uncaught UnsupportedOperationException (Gemini). - K6BenchmarkConverter.requiredString and the gateway_target read now guard with isJsonPrimitive(): a non-primitive required field is treated as missing and a non-primitive gateway_target falls back to "unknown" (Gemini). Adds unit coverage for the malformed-skip and non-primitive-guard branches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hh2epJFjdLAt2wZyxEeh3p
There was a problem hiding this comment.
🧹 Nitpick comments (3)
api-sheriff/src/main/java/de/cuioss/sheriff/api/auth/TokenValidatorProducer.java (1)
90-94: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSuppress PMD warning for intentional proxy initialization.
Invoking
validator.toString()correctly forces the Quarkus ArC client proxy to initialize the underlying bean at startup, satisfying the fail-fast requirement. However, this triggers aUselessPureMethodCallPMD warning since the return value is ignored.Consider suppressing this warning on the method to keep the static analysis build clean while preserving the eager-initialization behavior.
💡 Proposed fix
+ `@SuppressWarnings`("PMD.UselessPureMethodCall") void onStartup(`@Observes` StartupEvent event, `@GatewayValidator` TokenValidator validator) { // Invoke a method on the injected proxy to force contextual-instance creation at boot; // without this the `@ApplicationScoped` validator stays unassembled until the first request. validator.toString(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-sheriff/src/main/java/de/cuioss/sheriff/api/auth/TokenValidatorProducer.java` around lines 90 - 94, Suppress PMD’s UselessPureMethodCall warning on the onStartup method, preserving the intentional validator.toString() invocation that eagerly initializes the proxy at startup. Do not remove or replace the call.Source: Linters/SAST tools
benchmarks/src/test/java/de/cuioss/sheriff/api/k6/benchmark/K6BenchmarkConverterTest.java (2)
298-332: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin down what happens to the malformed record itself.
The test only asserts
goodRunsurvives; it never asserts whetherbadTime(non-primitivestart_time) ends up present with a fallback or is dropped entirely. That leaves the actual contract for non-primitive metadata fields unverified — add an assertion onbenchmarkNamed(json, "badTime")(eitherassertNotNull/assertNull, or a check on its metadata value) so a future regression in that fallback path is caught.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/src/test/java/de/cuioss/sheriff/api/k6/benchmark/K6BenchmarkConverterTest.java` around lines 298 - 332, Add an assertion in nonPrimitiveMetadataFieldIsSkippedWithoutCrashing for benchmarkNamed(json, "badTime") that verifies the intended contract for a non-primitive start_time—either its presence with the expected fallback metadata value or its complete omission. Keep the existing goodRun assertion and processing behavior unchanged.
210-231: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared helper for inline k6 summary fixtures.
Five test methods repeat the same
k6Dir = tempDir.resolve(...)→Files.createDirectories(k6Dir)→Files.writeString(k6Dir.resolve(name), """...""")sequence, and the "good-summary.json" text block at Lines 267-276 is duplicated verbatim at Lines 305-314. Extracting a smallwriteSummary(String fileName, String json)helper (mirroringcopyFixtureToK6Dir) would remove the boilerplate and the exact-duplicate literal.♻️ Example helper
+ private void writeSummary(String fileName, String json) throws IOException { + Path k6Dir = tempDir.resolve(K6ResultPostProcessor.K6_RESULTS_SUBDIRECTORY); + Files.createDirectories(k6Dir); + Files.writeString(k6Dir.resolve(fileName), json); + }Also applies to: 233-259, 261-296, 298-332, 334-370
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/src/test/java/de/cuioss/sheriff/api/k6/benchmark/K6BenchmarkConverterTest.java` around lines 210 - 231, Extract a private writeSummary(String fileName, String json) helper in K6BenchmarkConverterTest that resolves the k6 results directory, creates it, and writes the named summary file. Replace the repeated setup in all five affected test methods, including both duplicated good-summary.json fixtures, with this helper while preserving each test’s existing JSON content and assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/auth/TokenValidatorProducer.java`:
- Around line 90-94: Suppress PMD’s UselessPureMethodCall warning on the
onStartup method, preserving the intentional validator.toString() invocation
that eagerly initializes the proxy at startup. Do not remove or replace the
call.
In
`@benchmarks/src/test/java/de/cuioss/sheriff/api/k6/benchmark/K6BenchmarkConverterTest.java`:
- Around line 298-332: Add an assertion in
nonPrimitiveMetadataFieldIsSkippedWithoutCrashing for benchmarkNamed(json,
"badTime") that verifies the intended contract for a non-primitive
start_time—either its presence with the expected fallback metadata value or its
complete omission. Keep the existing goodRun assertion and processing behavior
unchanged.
- Around line 210-231: Extract a private writeSummary(String fileName, String
json) helper in K6BenchmarkConverterTest that resolves the k6 results directory,
creates it, and writes the named summary file. Replace the repeated setup in all
five affected test methods, including both duplicated good-summary.json
fixtures, with this helper while preserving each test’s existing JSON content
and assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3cafdc4a-bb91-4f57-84d5-9285a459342f
📒 Files selected for processing (7)
api-sheriff/src/main/java/de/cuioss/sheriff/api/auth/TokenValidatorProducer.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/auth/TokenValidatorProducerTest.javabenchmarks/src/main/java/de/cuioss/sheriff/api/k6/benchmark/ComparisonSummaryWriter.javabenchmarks/src/main/java/de/cuioss/sheriff/api/k6/benchmark/K6BenchmarkConverter.javabenchmarks/src/main/java/de/cuioss/sheriff/api/k6/benchmark/K6ResultPostProcessor.javabenchmarks/src/test/java/de/cuioss/sheriff/api/k6/benchmark/ComparisonSummaryWriterTest.javabenchmarks/src/test/java/de/cuioss/sheriff/api/k6/benchmark/K6BenchmarkConverterTest.java
🚧 Files skipped from review as they are similar to previous changes (5)
- benchmarks/src/test/java/de/cuioss/sheriff/api/k6/benchmark/ComparisonSummaryWriterTest.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/auth/TokenValidatorProducerTest.java
- benchmarks/src/main/java/de/cuioss/sheriff/api/k6/benchmark/ComparisonSummaryWriter.java
- benchmarks/src/main/java/de/cuioss/sheriff/api/k6/benchmark/K6BenchmarkConverter.java
- benchmarks/src/main/java/de/cuioss/sheriff/api/k6/benchmark/K6ResultPostProcessor.java
Author doc/plan/04b-comparative-benchmark.adoc following the existing doc/plan/*.adoc convention. It records the eight decisions this plan is built on: - D1 k6 replaces wrk, because wrk cannot serve plan 05's WebSocket/gRPC obligation; Gatling rejected on footprint, Fortio on capability; cross-gateway numbers are relative, not absolute. - D2 the six-aspect matrix (unauth, bearer, http2, graphql, upload 1MB, upload 50MB), with ws/grpc pre-provisioned as declarations and deferred to plan 05. - D3 the upload-size rationale: 1MB measures the streaming path as a request rate, 50MB measures transfer capacity at reduced concurrency. - D4 the fairness rules for the comparison lane. - D5 the K6BenchmarkConverter upstreaming proposal to benchmarking-common, explicitly a cross-repo follow-up and not implemented here. - D6 the one-time GitHub Pages baseline restart. - D7 the CI baseline coverage decision, taking -Pbenchmark from 3 wired executions today to 8, with the accepted runtime cost. - D8 the xk6-dashboard report as a comparison-run-only supplementary artifact. Update doc/plan/README.adoc: add the 04b row to the Plan Sequence table sequenced after 04, and reword the forward-looking benchmark convention to name k6 scripts plus native thresholds instead of wrk lua/sh. Plan 01's "a proxied-route WRK benchmark" phrase is left intact as a historical record of what plan 01 delivered. Co-Authored-By: Claude <noreply@anthropic.com>
Implement the k6 side of the BenchmarkConverter seam so the existing
report pipeline (badges, 10-run history, trends, GitHub Pages) consumes
k6 runs unchanged. No new Maven dependency: benchmarking-common is
already on the benchmarks classpath.
K6BenchmarkConverter maps a k6 handleSummary() export onto BenchmarkData
-- throughput score, scoreUnit, and the 50/75/90/99 latency percentiles.
k6 reports http_req_duration in milliseconds, so no unit conversion is
performed and none can regress. Only measured percentiles are emitted;
absent ones are never estimated.
K6ResultPostProcessor is the main-entry replacement for the wrk-era
processor and preserves its downstream sequence exactly, including the
exact-match Prometheus benchmark-name lookup (the nondeterministic
bidirectional contains() fallback is deliberately not reintroduced).
Summaries are read from a k6 subdirectory, mirroring the wrk contract,
and execution windows come from the summary JSON rather than a text
metadata block. A missing or empty k6 directory fails loud, so an empty
report is never published.
Two findings surfaced while pinning the tests, both worth recording:
- The report pipeline's convertBenchmarks() whitelists the fields it
emits and drops additionalData entirely. Recording gateway_target only
there would never reach a report, so the target is also encoded into
the serialized fullName (k6.<target>.<name>). name stays the bare
benchmark name, so the gh-pages history and trend keys are unaffected.
- The k6 lane uses the general MetricConversionUtil.formatThroughput
rather than the wrk-branded formatter. These agree above 2K but differ
below it ("1.50K" vs "1.5K"); the one-time gh-pages baseline restart
means no historical series depends on the old rendering.
The k6-specific log records live in a local K6Benchmark-prefixed
catalogue, since every upstream counterpart is wrk-branded and would
emit a factually wrong message for a k6 run; toolchain-neutral messages
still reuse the upstream Benchmarking catalogue. Documented in
doc/LogMessages.adoc.
Tests pin concrete P50/P75/P90/P99 values and formatted scores against
known-numbers classpath fixtures, carrying over the outgoing wrk suite's
value-asserting discipline plus its full gh-pages structure and
missing-input checks. 20 tests green in the benchmarks module.
Co-Authored-By: Claude <noreply@anthropic.com>
Give the k6 aspect matrix a working API Sheriff side, so the on-demand APISIX comparison has a route-for-route counterpart to measure against. The load-bearing change is the second token_validation issuer. The bearer aspect mints its token from the Keycloak 'benchmark' realm, whose import pins frontendUrl https://keycloak:8443 — so a minted token carries iss https://keycloak:8443/realms/benchmark regardless of the host-published 1443 mint port. gateway.yaml previously declared only the unrelated it-static issuer, so every bearer request would have been rejected 401 and the aspect would have measured the rejection path while reporting it as bearer-validation throughput. A rejection is *faster* than a validation, so that failure presents as flatteringly good numbers rather than as an error. The new benchmark-keycloak issuer declares that exact container-internal iss value and fetches JWKS over the shared api-sheriff network. it-static is retained untouched: BearerValidationIT's rejection scenarios resolve against it and stay green. Also added: - graphql and upload anchors (POST), the latter with max_body_bytes at 64 MiB so the 50 MB upload aspect is measured rather than rejected at the edge; APISIX mirrors the same limit. - tls.alpn advertising h2 alongside http/1.1, so the http2 aspect negotiates HTTP/2 instead of silently falling back to HTTP/1.1. - ws and grpc anchors as declarations only — no behaviour; the pipeline still boot-rejects those protocols and roadmap plan 05 owns them. - graphql and the two upload routes on the existing go-httpbin endpoint, each overriding the endpoint's default anchor and allow-listing Content-Type (the forward stage is deny-by-default, so without it the POST body would not be echoed as sent). go-httpbin's /anything/* echo accepts POST bodies, so no new upstream service is needed. 497 unit tests green. Note the end-to-end assertion that a benchmark-realm token returns 200 on /secure requires the running container stack and is not covered by this build. Co-Authored-By: Claude <noreply@anthropic.com>
… toolchain Replace the wrk/lua toolchain with k6 as the sole benchmark framework. The four benchmark names (healthLiveCheck, gatewayHealth, proxiedStatic, bearerProxied) are preserved verbatim so no gh-pages history or trend series is orphaned. - benchmarks/pom.xml: swap the wrk.* property block for k6.* (duration, vus, script.dir, results.dir, output.dir); retarget the three benchmark executions at `docker compose run --rm k6 run` against the new .js scripts, renamed run-k6-*; repoint the post-processing mainClass at K6ResultPostProcessor. - Add run-k6-bearer-proxied-benchmark. This is new CI coverage, not a retarget: the wrk-era bearer_proxied_benchmark.sh was registered in no Maven execution, so -Pbenchmark now carries four wired executions (3 today + 1 added). - Add the four k6 scripts plus the shared lib/summary.js handleSummary() helper emitting the cuioss summary format K6BenchmarkConverter consumes. Native k6 thresholds (http_req_failed ceiling + a checks rate gate) replace the custom WRK_MAX_ERROR_RATE lua gate wholesale, including its strict validation of a set-but-invalid override. - bearer_proxied.js mints one token from the Keycloak benchmark realm in setup() and replays it, so the run measures gateway validation rather than Keycloak issuance. Its checks threshold asserts HTTP 200 explicitly, so a 401-only run fails the build instead of reporting the fast rejection path as a good result. - Add the digest-pinned benchmarks/Dockerfile.k6 (grafana/k6 2.1.0) and replace the wrk compose service with a k6 service carrying the results bind mount. - Relocate pre-benchmark-health-check.sh from wrk-scripts/ to k6-scripts/. - Delete the whole wrk toolchain: four runner scripts, four lua gates, Dockerfile.wrk, WrkResultPostProcessor, its three test classes and the two wrk parser fixtures. The tests' coverage is not dropped -- it is carried by K6BenchmarkConverterTest against known-numbers k6 fixtures. Known behavioural change: latency_ms.stdev is absent from k6 summaries because k6 exposes no standard deviation for a trend metric and the samples are not reachable from handleSummary(). The field is omitted rather than fabricated, so the report's error / variability / confidence-band columns are zero-width for k6 runs. The D6 one-time baseline restart means no historical series depends on the wrk-era values. The single sanctioned surviving wrk token is the upstream PrometheusMetricsManager.collectMetricsForWrkBenchmark(...) call site (D5). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D5wfKpAQkWGjHN3qBmnXAp
…for-route parity Adds docker-compose.apisix.yml as a third compose overlay (not a self-contained stack: nginx-static is defined only in docker-compose.benchmark.yml and the api-sheriff network is Compose-project-scoped). The comparison lane is one three-file merge behind a dedicated 'comparison' profile, so the default stack and verify -Pintegration-tests start no APISIX service. apisix.yaml mirrors the deliverable-4 API Sheriff routes one-for-one: unauth proxied, bearer, graphql POST, and the two upload routes sharing the same 64 MiB body cap, plus ws/grpc as declarations only. The bearer route uses openid-connect in bearer_only mode with use_jwks: true so it validates offline against the same Keycloak benchmark realm -- without use_jwks the plugin falls back to introspection and would measure a per-request Keycloak round-trip. TLS could not be configured by file path as originally scoped. Verified against the pinned image's own sources: apisix/cli/ops.lua overwrites the configured apisix.ssl.ssl_cert paths with a built-in placeholder on every start, and apisix/init.lua's ssl_client_hello phase aborts any handshake matching no 'ssls' entry, whose cert/key fields accept inline PEM only (this build resolves no $env:// references). render-tls-and-start.sh therefore splices the mounted certificate into the route table at start-up, keeping src/main/docker/certificates the single source of truth rather than committing the private key a second time. Verified against a live stack: /proxy/static, /graphql, /upload/small and /upload/large all return 200, /secure returns 401 without a token and 200 with a valid benchmark-realm token. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D5wfKpAQkWGjHN3qBmnXAp
…or upstream parity The APISIX route table sent the graphql and upload aspects to go-httpbin while API Sheriff sent them to nginx-static (the benchmark overlay repoints the UPSTREAM alias for the whole endpoint, not just /proxy). That asymmetry measured go-httpbin's JSON serialization as APISIX latency and biased both aspects in API Sheriff's favour, violating the spec's SAME-upstreams fairness requirement. Repoint graphql-post, upload-small and upload-large at static-upstream so both gateways hit the identical backend, and drop the now-unused echo-upstream and the dead go-httpbin depends_on edge. The proxy-rewrite entries are kept because API Sheriff rewrites the same way and nginx-static answers 200 on any path/method. Consequence: the graphql and upload aspects measure gateway processing rather than upstream work, which is the benchmark's stated intent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D5wfKpAQkWGjHN3qBmnXAp
The benchmark aspect's `benchmark-keycloak` issuer fetches its JWKS from https://keycloak:8443, a compose service name that resolves to the bridge network's site-local 172.x address. token-sheriff's EgressPolicy.secureDefault() refuses exactly that, so the key set never loaded and every bearer request failed — the aspect would have measured a rejection path while reporting it as bearer-validation throughput. Adds the narrow widening the threat model already prescribes (GW-05 / BFF-07: "keep the secure egress defaults; widen only via allowedEgressHost for a trusted IdP") as a per-issuer config key: - gateway.schema.json: `allowed_egress_hosts` on the jwks object, which is additionalProperties: false — without the schema entry the key is a boot rejection, not a silent no-op. - IssuerConfig.Jwks: `List<String> allowedEgressHosts`, normalized to List.of() when absent. - TokenValidatorProducer: each entry forwarded to HttpJwksLoaderConfigBuilder.allowedEgressHost(String). When the list is empty no egress builder method is called at all, so the config keeps EgressPolicy.secureDefault(). Secure by default is asserted, not assumed: tests cover absent and empty lists (structurally equal to secureDefault(), and behaviourally still refusing a loopback-resolving JWKS URL), host-exactness (allowlisting one host does not exempt another), multi-entry application, YAML binding through the SNAKE_CASE strategy, and schema rejection of a non-array value. Only the `benchmark-keycloak` issuer is widened. `it-static` is deliberately untouched — it loads from a mounted file and needs no egress. No default or production config is widened. Scope: this is an accepted scope deviation. It was escalated before any code was written (the originally-planned sheriff.token.issuers.* property keys would have been a silent no-op, since the gateway drives issuers from its own YAML via @GatewayValidator, not the extension's cui.jwt.* surface) and the operator chose this option, logged as (scope-deviation:accept). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D5wfKpAQkWGjHN3qBmnXAp
…d by deployment
The JWKS client verified the IdP's certificate against the JVM default trust
store, with no way to change that from configuration. An IdP behind an internal
or corporate CA — or a cluster-internal IdP using the cluster's own authority —
therefore failed PKIX path building, the key set never loaded, and every bearer
request was rejected. That is a production gap, not a test gap: together with
the egress default (which refuses private addresses), API Sheriff could only
validate tokens from a public-internet IdP, which is backwards for a gateway
that normally sits inside the same private network as its IdP.
Adds `token_validation.issuers[].jwks.tls_profile`: the name of a logical trust
profile in API Sheriff's own vocabulary.
The design is a mapping, deliberately not a direct reference:
- gateway.yaml and gateway.schema.json name a trust profile and nothing else —
no truststore path, no password, no runtime construct. Letting the document
reference a runtime concept directly would make the product's configuration
language depend on the identity of the framework beneath it, so a change of
runtime would invalidate documents operators wrote and committed.
- JwksTrustProfileResolver is the single seam that knows the binding. It maps
the logical name to the runtime's named TLS configuration and hands the
resulting SSLContext to HttpJwksLoaderConfigBuilder.sslContext(...). No other
class knows the mapping exists; TokenValidatorProducer deals only in the
logical name.
- Trust material and passwords stay on the deployment side on purpose: secrets
never enter gateway.yaml, and keystore loading/rotation is not reimplemented.
Boundary rule — config neutral, diagnostics concrete. The schema and the
document never name the runtime; the startup error does, because an operator
who names an unbound profile needs the exact knob, not an abstraction:
Issuer 'X' names jwks.tls_profile 'P' but no such trust profile is
configured — define it via quarkus.tls.P.trust-store.*
An unresolvable profile fails startup rather than falling back to default
trust — a silent fallback would either surface later as an obscure fetch error
or succeed against anchors nobody intended. The validator is now assembled
eagerly at boot so that failure lands at startup, matching ConfigProducer's
existing refuse-to-start posture.
Absent tls_profile preserves current behaviour exactly: no SSL context is set
and the client keeps the JVM default trust store. Asserted, not assumed —
tests cover absent (identity-compared against a defined profile's context, so
the assertion cannot pass vacuously), the resolver never being consulted when
absent, applied-and-identical when named, unresolvable, unloadable material,
co-existence with allowed_egress_hosts, YAML binding, and schema rejection of
a non-string value. The registry double is hand-written; no mocking framework.
doc/configuration.adoc carries the full mapping: what tls_profile means, how a
name resolves to concrete anchors, a worked example, the failure mode and its
message, the rationale for the indirection, and how it differs from
allowed_egress_hosts ("may I connect?" vs "do I trust what I get back?").
ADR-0011 records the config-surface decision, why a direct runtime reference
was rejected, the boundary rule, and the deferred knobs — several of which
(connect/read timeouts, retryConfig, refreshIntervalSeconds, keyRotation
GracePeriod, maxRetiredKeySets) are production-significant and currently
unconfigurable; refresh interval is effectively fixed at 600s.
Benchmark stack wiring:
- gateway.yaml names tls_profile benchmark-idp on benchmark-keycloak only;
it-static is untouched (file-sourced, no egress, no TLS).
- application.properties binds that name under the `it` profile to the
already-mounted localhost truststore.
- benchmark-realm.json gains an oidc-sub-mapper on the openid client scope.
The realm's clientScopes replace Keycloak's built-ins, so the `basic` scope
that normally supplies `sub` was absent and every benchmark token was
rejected for a missing mandatory claim — the same silent-wrong-result class
as the TLS failure: the aspect would measure a rejection path while
reporting it as bearer-validation throughput.
Verified against the running stack (native runner and image rebuilt):
JWKS now loads over TLS with no handshake error, and a benchmark-realm token
returns 200 on /secure while an unauthenticated request still returns 401
(previously 401/401).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D5wfKpAQkWGjHN3qBmnXAp
…f recipe The OpenRewrite UseMapOf recipe rewrote the additionalData LinkedHashMap into new HashMap<>(Map.of(...)) without adding the java.util.HashMap import, leaving the tree broken for the next compile. Pre-commit itself passed because the rewrite lands after the module compiles, so the breakage only surfaced on the following build and would have broken CI. Constructing the map with Map.of directly satisfies the recipe, so there is nothing left to rewrite -- the defect class is removed rather than suppressed. Ordering is provably irrelevant here: the map is never iterated or mutated after construction, and convertBenchmarks() drops additionalData entirely downstream. The percentiles LinkedHashMap at line 227 is retained, where insertion order is meaningful. Resolves finding c0d56f.
…path Three InvalidExceptionUsageRecipe markers sat on HEAD in the request dispatch path. CLAUDE.md forbids committing code with markers present. Each was judged on its merits rather than blanket-suppressed: - GatewayEdgeRoute#handle (executor dispatch): NARROWED to RejectedExecutionException. Executor#execute contracts this as its only failure -- the submitted command is a non-null lambda literal, so the NPE arm is unreachable, and process(ctx) runs on the virtual thread rather than inline, so no pipeline exception can surface here. Behaviour-preserving. - GatewayEdgeRoute#process (pipeline tail): SUPPRESSED. A deliberate last-resort per-request safety net over a dozen independent stages; the failure type is genuinely unknowable. Narrowing would abandon the response without ending it (client hangs until timeout) and strand the admission permit, since the end handler never fires. - DispatchStage#guardedDispatch: SUPPRESSED. Guard#call propagates the checked Exception declared by the Callable it wraps and the method declares no throws clause, so a narrower catch would not compile. It is also the single point translating any guard failure into a mapped GatewayException. Both suppressions carry a rationale comment and sit before the enclosing try, which is where the recipe actually honours them -- placing them inside the preceding catch block left the markers being re-inserted on the next run. Verified: verify -Ppre-commit BUILD SUCCESS with zero markers re-added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D5wfKpAQkWGjHN3qBmnXAp
…arison entry point Deliverable 6 (TASK-008, TASK-009). Harness: - lib/target.js resolves GATEWAY_TARGET to a base URL, defaulting to API Sheriff so the CI lane needs no plumbing. An unknown target is fatal rather than defaulted: silently falling back would label an API Sheriff run as the other gateway, and mislabelled comparison data is worse than a failed run. - lib/summary.js now sources gatewayTarget() from target.js (single owner) and grows an opt-in throughput_mbps derived from k6's own data_sent rate, so a partially-transferred body cannot be counted as fully delivered bytes. - proxied_static.js / bearer_proxied.js resolve their host via target.js. health_live.js and gateway_health.js deliberately stay API-Sheriff-bound: they measure the Quarkus management port and /api/health, which APISIX does not expose, so making them 'neutral' would point them at absent routes. New matrix aspects (graphql, upload_small 1MB, upload_large 50MB, http2), each declaring native k6 thresholds. Two anti-silent-wrong-result gates: http2.js asserts the negotiated protocol per response, because k6 falls back to HTTP/1.1 transparently and a fallback would file HTTP/1.1 numbers under the h2 aspect; upload_large.js asserts HTTP 200 so an undersized body cap fails the run rather than reporting the faster rejection path as excellent throughput. upload_large runs at reduced concurrency (5 VUs). Every pom execution passes BENCHMARK_VUS explicitly, so a dedicated k6.vus.upload.large property is what actually keeps the shared k6.vus (50) from overriding the script default -- otherwise the aspect would measure link saturation and the generator's own per-VU 50MB payload memory instead of gateway transfer cost. run-comparison.sh is the single on-demand entry point, registered in no Maven profile and referenced by no workflow (verified). It writes to a results root segregated from both the CI baseline and the gh-pages tree, and closes the aspect matrix to the six comparable aspects so the API-Sheriff-only health benchmarks can never be driven under a non-default target label. ComparisonSummaryWriter renders the per-aspect side-by-side table, selecting MBps for the transfer-bound uploadLarge aspect and RPS elsewhere, with P50/P99 per gateway. An unmeasured metric renders n/a, never 0, so an absent measurement is not read as a measured collapse. Formatting is pinned to Locale.ROOT so a German-locale run cannot emit decimal commas into the artifact. CI lane now wires eight benchmark executions. Verified: verify -pl benchmarks -am BUILD SUCCESS; 16 tests green (7 new). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D5wfKpAQkWGjHN3qBmnXAp
…artifact Deliverable 7 (TASK-010). Dockerfile.k6-dashboard is a SECOND image beside Dockerfile.k6, never a replacement: xk6-dashboard is a k6 output extension and k6 loads output extensions only when compiled in, so the HTML report requires rebuilding k6 with the extension linked. Engine parity is the load-bearing constraint -- a dashboard run and a baseline run measured by different engines are not comparable. Two things enforce it: the builder stage pins K6_VERSION=v2.1.0 to match the version behind Dockerfile.k6's digest, and the runtime stage reuses that exact digest-pinned grafana/k6 image and replaces only the binary, so everything but the linked-in extension is byte-identical to the baseline environment. All three build versions (k6, xk6, xk6-dashboard) are explicit pins; no floating latest. run-comparison.sh grows --no-dashboard and routes through the dashboard image by default, passing web-dashboard=report=... so each aspect emits a self-contained single-file HTML report (assets inlined, no CDN, no served endpoint) per run and per aspect into the same segregated results directory. The report is purely additive: the handleSummary() JSON in the generic cuioss format is emitted either way, so the flag changes what a person can browse, never what ComparisonSummaryWriter computes from. The k6-dashboard compose service sits behind its own comparison-dashboard profile, deliberately distinct from the apisix service's comparison profile. Verified by config --services: the default stack and the comparison profile both resolve without it, and it resolves without apisix -- so the default stack, verify -Pintegration-tests and the -Pbenchmark CI lane start no dashboard container. docker compose run implicitly enables the target service's profile, which is what lets run-comparison.sh drive it directly. Verified: benchmarks/pom.xml and .github/workflows/ carry no reference to the dashboard image or profile; verify -pl benchmarks -am BUILD SUCCESS, 16 tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D5wfKpAQkWGjHN3qBmnXAp
…word the CI step Deliverable 8 (TASK-011). benchmarks/README.adoc is reframed from 'WRK-based HTTP benchmarks' to k6 and gains the full aspect matrix in place of the three-row table, plus: - Lane composition, stated precisely: the CI baseline lane runs eight executions (six matrix aspects + healthLiveCheck/gatewayHealth), the on-demand comparison lane runs the six matrix aspects only. Records WHY the health benchmarks are excluded from comparison: they have no APISIX counterpart, so driving them would mislabel API Sheriff data as apisix. - Methodology: single node/same host, identical resource envelopes, shared TLS material, identical routes, dedicated load-generator capacity, and the explicit relative-not-absolute framing for cross-gateway numbers. - The fairness invariant: BOTH sides target nginx-static, so the graphql and upload aspects measure gateway processing rather than upstream work -- stated as the benchmark's intent, not a limitation. - The accepted latency_ms.stdev fidelity loss: k6 exposes no stdev for trend metrics, so the error/variability/confidence-band columns are permanently zero-width. Documented as accepted and NOT to be worked around, with percentiles named as the surviving spread signal. - Reproducibility dependencies on jwks.tls_profile and allowed_egress_hosts (ADR-0011), and why the bearer aspect asserts HTTP 200. - Comparative runs: the exact three-file compose merge, the on-demand-only status, the segregated results layout, and the --build warning. - Dashboard reports: per-aspect self-contained HTML, offline opening, the disable flag, and the k6/xk6-dashboard version pins tracking the baseline. - The wrk-to-k6 swap and the one-time gh-pages baseline restart, including the expected one-time discontinuity at the swap commit and an explicit instruction not to investigate it as a regression. Remaining 'wrk' mentions are historical/migration references that the swap section requires; the document's framing is fully k6. benchmark.yml: step renamed to 'Run Integration Benchmarks with k6' with prose naming k6 and the eight executions; timeout-minutes 45 -> 75, with the rationale inline -- five additional runs land in the same cap, and the transfer-bound upload-50MB aspect is the dominant contributor beyond a budget originally sized for three 60-second runs. Maven invocation, history handling, artifact assembly and Pages deployment are unchanged. Verified: verify -Ppre-commit BUILD SUCCESS (520 tests, zero markers); workflow YAML valid; asciidoc validate reports 0 errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D5wfKpAQkWGjHN3qBmnXAp
Picks up two fixes relevant to this branch: - 1.5.2 excludes the defective JavaUtilAPIs/UseMapOf recipe from the pre-commit profile (cuioss/cuioss-parent-pom#1323). That recipe rewrote a LinkedHashMap into new HashMap<>(Map.of(...)) without adding the java.util.HashMap import, leaving the tree uncompilable for the next build while the rewriting build itself still reported BUILD SUCCESS. - 1.5.3 excludes Docker Compose files from OpenRewrite parsing (cuioss/cuioss-parent-pom#1329), which matters now that this branch adds docker-compose.apisix.yml and the APISIX config set. Two version vectors were checked and are unaffected: version.quarkus is pinned locally at 3.37.2 so the parent's 3.37.3 does not apply, and smallrye-config convergence is decided by quarkus-bom being declared first, not by the parent. Quarkus config-mapping augmentation runs during package and passed, which is the check that pin protects. Also carries the resulting OpenRewrite simplification: the explicit StandardCharsets.UTF_8 arguments on Files.readString/writeString are redundant, since both are specified as UTF-8 by the JDK. Verified as a stable fixed point -- a second pre-commit run produced no further rewrites, 520 tests green.
onStartup observed StartupEvent with the @ApplicationScoped validator as a lazy CDI client proxy, so ArC never invoked gatewayTokenValidator() at boot — tls_profile resolution was deferred to the first bearer request instead of failing startup. Invoke a method on the injected proxy to force contextual-instance creation at boot, and correct the misleading comment. Add onStartup unit tests proving the validator is now dereferenced. Addresses PR #82 CodeRabbit review (TokenValidatorProducer.java:81-83). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hh2epJFjdLAt2wZyxEeh3p
…primitive JSON Three PR #82 review fixes in the k6 post-processing harness: - ComparisonSummaryWriter.readTargetSummaries now wraps each per-file parse in try/catch (JsonSyntaxException | IllegalStateException), logging the bad file via FAILED_PARSE_SUMMARY and continuing, so one truncated summary cannot abort the whole comparison (Gemini). - K6ResultPostProcessor.stringOrNull/instantOrNull now guard with isJsonPrimitive() before getAsString(), so a present-but-non-primitive metadata field yields the absent-field path instead of an uncaught UnsupportedOperationException (Gemini). - K6BenchmarkConverter.requiredString and the gateway_target read now guard with isJsonPrimitive(): a non-primitive required field is treated as missing and a non-primitive gateway_target falls back to "unknown" (Gemini). Adds unit coverage for the malformed-skip and non-primitive-guard branches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hh2epJFjdLAt2wZyxEeh3p
…ve-benchmark Consolidate the duplicated has(field) && isJsonPrimitive() guard into a single helper in K6BenchmarkConverter and delegate instantOrNull to stringOrNull in K6ResultPostProcessor. Co-Authored-By: Claude <noreply@anthropic.com>
Record the operator-confirmed architectural decision that comparative cross-gateway benchmarking is a target-neutral, on-demand-only lane segregated from the authoritative CI baseline, with fairness parity as a precondition. Co-Authored-By: Claude <noreply@anthropic.com>
OliverWolffGIP
force-pushed
the
feature/plan-04b-comparative-benchmark
branch
from
July 20, 2026 17:14
10a79f2 to
68d2bbb
Compare
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
3 tasks
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
1 participant
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
Replaces
wrkwithk6as the sole benchmark framework for API Sheriff and stands upan on-demand, never-CI-bound comparative baseline against Apache APISIX on the same
upstreams, Keycloak realm, and TLS certs. Doc-first:
doc/plan/04b-comparative-benchmark.adocrecords the k6-over-wrk decision, the aspect matrix, upload-size rationale, and fairness
rules;
doc/plan/README.adoclinks 04b and moves the benchmark-convention wording fromwrk/lua to k6.
Changes
doc/plan/04b-comparative-benchmark.adoc; updateddoc/plan/README.adoc,benchmarks/README.adoc(via test/resource sweep),doc/LogMessages.adoc,doc/configuration.adoc,doc/security-threat-model.adoc, anddoc/adr/0011-gatewayyaml_exposes_JWKS_trust_and_egress_as_neutral_names_bound_by_the_deployment.adoc.K6BenchmarkConverter,K6BenchmarkLogMessages,K6ResultPostProcessor, andComparisonSummaryWriterunderbenchmarks/src/main/java/de/cuioss/sheriff/api/k6/benchmark/, wired through theexisting
BenchmarkResultProcessor/ gh-pages flow unchanged.benchmarks/src/main/resources/k6-scripts/(gateway_health,health_live, bearer_proxied, proxied_static, http2, graphql, upload_small,
upload_large, plus
lib/summary.js,lib/target.js, andpre-benchmark-health-check.sh), migrating all four existing wrk benchmarks andreplacing the custom error-rate gate with native k6 thresholds.
benchmarks/Dockerfile.wrkand allbenchmarks/src/main/resources/wrk-scripts/*(sh + lua) and their associated wrkbenchmark tests/fixtures; replaced with
benchmarks/Dockerfile.k6andbenchmarks/Dockerfile.k6-dashboard.K6BenchmarkConverterTestandComparisonSummaryWriterTestusingknown-numbers k6 summary fixtures (
k6-summary-gateway-health.json,k6-summary-health-live.json); removed the superseded wrk result-processor tests.integration-tests/docker-compose.apisix.yml,integration-tests/src/main/docker/apisix/{apisix.yaml,config.yaml,render-tls-and-start.sh},and a benchmark Keycloak realm (
integration-tests/src/main/docker/keycloak/benchmark-realm.json)with route-for-route parity against the same upstreams/TLS certs as API Sheriff
(openid-connect bearer_only, graphql POST, upload routes), plus pre-provisioned
(behavior-inert) ws/grpc routes for roadmap 05.
benchmarks/scripts/run-comparison.sh— atarget-neutral, never-CI-wired entry point (
gateway_targetrecorded in resultmetadata) that runs the full aspect matrix or a named subset against whichever stack
is started, producing a per-aspect side-by-side summary kept segregated from CI
baselines/pages.
benchmarks/pom.xmland.github/workflows/benchmark.ymlfor thek6-based benchmark toolchain; updated
pom.xml.IssuerConfig,TokenValidatorProducer,JwksTrustProfileResolver,DispatchStage,GatewayEdgeRoute,application.properties,and
schema/gateway.schema.json(plus their tests) supporting the APISIX-parity routeconfiguration and TLS/JWKS trust setup exercised by the comparison stack.
Test Plan
verify -Ppre-commit)Related Issues
N/A
Generated by plan-finalize skill
Summary by CodeRabbit