diff --git a/.github/actions/e2e/action.yml b/.github/actions/e2e/action.yml index c6c259641..37c828f25 100644 --- a/.github/actions/e2e/action.yml +++ b/.github/actions/e2e/action.yml @@ -16,8 +16,11 @@ runs: shell: bash - name: E2E (${{ inputs.path }}) - run: > - make -C ${{ inputs.path }} - EDITION=${{ inputs.edition }} - HURL='docker run --rm --network=host --volume $$(pwd):/workspace --workdir /workspace ghcr.io/orange-opensource/hurl:8.0.1' + run: | + mkdir -p "$GITHUB_WORKSPACE/benchmark-results" + SLUG="$(printf '%s' '${{ inputs.path }}' | tr '/' '-')" + make -C ${{ inputs.path }} \ + EDITION=${{ inputs.edition }} \ + BENCHMARK_OUTPUT="$GITHUB_WORKSPACE/benchmark-results/$SLUG.json" \ + HURL='docker run --rm --network=host --volume $$(pwd):/workspace --workdir /workspace ghcr.io/orange-opensource/hurl:8.0.1' shell: bash diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9b248acff..026972283 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -157,20 +157,34 @@ jobs: edition: ${{ matrix.edition.name }} if: matrix.edition.name == 'enterprise' + # The suites measured themselves as they ran, so what is left is to take + # the indexer's own measurements and say all of it at once. This action + # keeps a checkout of the branch it records into, so it is called exactly + # once per job rather than once per kind of measurement - name: Benchmark run: | make docker-benchmark PRESET=Release ${{ matrix.edition.options }} - docker run --rm one-benchmark > benchmark.json + mkdir -p benchmark-results + docker run --rm one-benchmark > benchmark-results/index.json + ./benchmark/merge.sh benchmark-results > benchmark.json + cat benchmark.json - uses: benchmark-action/github-action-benchmark@v1 if: github.event.pull_request.head.repo.fork == false with: + # TODO: Rename to `Benchmark ()`, as this now carries + # server measurements too. The name keys the series stored on + # `gh-pages`, and only pushes to `main` write there, so renaming + # starts an empty series and suppresses the comparison comment until + # the first push after the rename lands name: Benchmark Index (${{ matrix.edition.name }}) tool: customSmallerIsBetter output-file-path: benchmark.json github-token: ${{ secrets.GITHUB_TOKEN }} auto-push: ${{ github.event_name != 'pull_request' }} benchmark-data-dir-path: benchmark/${{ matrix.edition.name }}/index - alert-threshold: '5%' + # Latency on a shared runner moves far more than an index does, and + # one threshold now covers both + alert-threshold: '25%' comment-always: true fail-on-alert: false diff --git a/CMakeLists.txt b/CMakeLists.txt index 98bf0994e..c82c608ed 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -130,7 +130,9 @@ sourcemeta_target_clang_format(SOURCES enterprise/*.h enterprise/*.cc) sourcemeta_target_shellcheck(SOURCES release.sh + contrib/*.sh test/*.sh + test/e2e/*/*.sh docker/*.sh enterprise/scripts/*.sh enterprise/e2e/*/*.sh diff --git a/benchmark/measure.py b/benchmark/measure.py new file mode 100755 index 000000000..e0c5530a1 --- /dev/null +++ b/benchmark/measure.py @@ -0,0 +1,91 @@ +#!/usr/bin/env python3 + +"""Measure how long one endpoint takes to answer. + +The connection is opened once and kept, so what is timed is answering a +request rather than establishing a conversation. Requests are made one after +another, which measures latency rather than throughput: what this answers is +how long one caller waits, not how many callers can be served at once. +""" + +import argparse +import http.client +import json +import sys +import time + + +def percentile(samples, fraction): + return samples[min(int(len(samples) * fraction), len(samples) - 1)] + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("--name", required=True) + parser.add_argument("--url", required=True) + parser.add_argument("--method", default="GET") + parser.add_argument("--header", action="append", default=[]) + parser.add_argument("--body", default=None) + parser.add_argument("--count", type=int, default=20000) + parser.add_argument("--warmup", type=int, default=1000) + options = parser.parse_args() + + _, _, rest = options.url.partition("//") + authority, _, path = rest.partition("/") + host, _, port = authority.partition(":") + path = "/" + path + + headers = {} + for header in options.header: + key, _, value = header.partition(":") + headers[key.strip()] = value.strip() + + body = options.body.encode() if options.body else None + connection = http.client.HTTPConnection(host, int(port or 80)) + connection.connect() + + def once(): + connection.request(options.method, path, body=body, headers=headers) + response = connection.getresponse() + response.read() + return response.status + + # Answering the first time builds what answering afterwards reuses, so + # what is measured is a warm instance rather than a cold one + for _ in range(options.warmup): + status = once() + if status >= 400: + sys.exit(f"{options.name}: warm up got HTTP {status}") + + samples = [] + for _ in range(options.count): + start = time.perf_counter_ns() + status = once() + samples.append((time.perf_counter_ns() - start) / 1000.0) + # A run that was refused measured something other than what it set out + # to, so it stops rather than reporting a number nobody can trust + if status >= 400: + sys.exit(f"{options.name}: got HTTP {status}") + + samples.sort() + json.dump( + [ + { + "name": f"{options.name} (p50)", + "unit": "us", + "value": round(percentile(samples, 0.50)), + }, + { + "name": f"{options.name} (p99)", + "unit": "us", + "value": round(percentile(samples, 0.99)), + }, + ], + sys.stdout, + indent=2, + ) + sys.stdout.write("\n") + + +if __name__ == "__main__": + main() diff --git a/benchmark/merge.sh b/benchmark/merge.sh new file mode 100755 index 000000000..c2b9cf552 --- /dev/null +++ b/benchmark/merge.sh @@ -0,0 +1,22 @@ +#!/bin/sh + +set -o errexit +set -o nounset + +if [ "$#" -ne 1 ] +then + echo "Usage: $0 " 1>&2 + exit 1 +fi + +DIRECTORY="$1" + +# A run where no suite declared anything is not a failure, it is a run with +# nothing to say, and an empty array is how it says so +if [ ! -d "$DIRECTORY" ] || [ -z "$(ls -A "$DIRECTORY" 2>/dev/null)" ] +then + echo "[]" + exit 0 +fi + +jq --slurp 'add' "$DIRECTORY"/*.json diff --git a/enterprise/e2e/auth-keys/benchmark.sh b/enterprise/e2e/auth-keys/benchmark.sh new file mode 100755 index 000000000..e13f6bdeb --- /dev/null +++ b/enterprise/e2e/auth-keys/benchmark.sh @@ -0,0 +1,14 @@ +#!/bin/sh + +set -o errexit +set -o nounset + +BASE="$1" +HERE="$(cd "$(dirname "$0")" && pwd)" +MEASURE="$HERE/../../../benchmark/measure.py" + +# A path a policy governs, so what is measured includes admitting the caller +# rather than only serving them +"$MEASURE" --name "Schema Fetch (gated)" \ + --header "Authorization: Bearer $ONE_E2E_VAULT_KEY" \ + --url "$BASE/vault/secret.json" diff --git a/test/e2e/common.mk b/test/e2e/common.mk index cc1522d17..d26e8201f 100644 --- a/test/e2e/common.mk +++ b/test/e2e/common.mk @@ -3,6 +3,8 @@ HURL ?= hurl NPM ?= npm NPX ?= npx ROOT := $(dir $(lastword $(MAKEFILE_LIST)))../.. +# Where this suite lives, which is what a measurement it takes is named after +SUITE := $(patsubst $(abspath $(ROOT))/%,%,$(CURDIR)) COMPOSE = compose.yml BASE ?= http://localhost @@ -16,7 +18,7 @@ export EDITION all: $(MAKE) down $(MAKE) up - $(MAKE) test-hurl test-playwright; \ + $(MAKE) test-benchmark test-hurl test-playwright; \ status=$$?; $(MAKE) down; exit $$status .PHONY: up @@ -32,6 +34,22 @@ test-hurl: $(wildcard hurl/*.all.hurl) \ $(wildcard hurl/*.$(EDITION).hurl) +# A suite that carries a script saying what it wants measured is measured +# before its tests run, so what is timed is an instance warmed deliberately +# rather than one that has just served a whole test suite. Nothing is measured +# unless somewhere to put the answer was named, so an ordinary run pays nothing +.PHONY: test-benchmark +test-benchmark: +ifneq ($(wildcard benchmark.sh),) +ifneq ($(BENCHMARK_OUTPUT),) + if [ -f environment ]; then set -a; . ./environment; set +a; fi; \ + ./benchmark.sh $(BASE):$(PORT) > $(BENCHMARK_OUTPUT).part + jq --arg suite "$(SUITE)" 'map(.name |= "\($$suite): \(.)")' \ + < $(BENCHMARK_OUTPUT).part > $(BENCHMARK_OUTPUT) + rm -f $(BENCHMARK_OUTPUT).part +endif +endif + .PHONY: test-playwright test-playwright: ifneq ($(wildcard playwright/),) diff --git a/test/e2e/html/benchmark.sh b/test/e2e/html/benchmark.sh new file mode 100755 index 000000000..50f33f900 --- /dev/null +++ b/test/e2e/html/benchmark.sh @@ -0,0 +1,10 @@ +#!/bin/sh + +set -o errexit +set -o nounset + +BASE="$1" +HERE="$(cd "$(dirname "$0")" && pwd)" +MEASURE="$HERE/../../../benchmark/measure.py" + +"$MEASURE" --name "Schema Fetch" --url "$BASE/test/bundling/single.json"