From e8d04f13043f22ce46497148c0606eeb17b021d5 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 27 Oct 2026 20:19:01 +0100 Subject: [PATCH] Give every API value with a unit as a number (closes #199) A value with a unit is now a number, and its field name ends in the unit, as the vendored API_POLICIES.md requires. @meta.execution_time becomes execution_time_ms. In the stats answers, uptime, connection_duration, memory_usage and whois_stats.current_interval become uptime_seconds, connection_duration_seconds, memory_usage_bytes and whois_stats.current_interval_seconds. connection_duration_seconds is null while the feed is not connected. The status page formats the numbers itself. REPO_POLICIES.md is re-vendored from the same sneak/prompts commit. Model: opus-5-5 --- API_POLICIES.md | 20 +++ README.md | 29 +++- REPO_POLICIES.md | 114 +++++++++++--- TODO.md | 9 ++ internal/server/handlers.go | 74 +++++---- internal/server/handlers_internal_test.go | 166 +++++++++++++++++++- internal/server/middleware.go | 17 +- internal/server/middleware_internal_test.go | 76 +++++++++ internal/templates/status.html | 25 ++- 9 files changed, 456 insertions(+), 74 deletions(-) create mode 100644 API_POLICIES.md diff --git a/API_POLICIES.md b/API_POLICIES.md new file mode 100644 index 0000000..6dd6f73 --- /dev/null +++ b/API_POLICIES.md @@ -0,0 +1,20 @@ +--- +title: API Policies +last_modified: 2026-10-27 +--- + +A value in an API response is never a string with a label or unit in it. A value +of a native type is sent as that type, and its unit goes in the field name, for +example `_ms`. + +Wrong: + +```json +{ "execution_time": "0 ms" } +``` + +Right: + +```json +{ "execution_time_ms": 0 } +``` diff --git a/README.md b/README.md index e1382c5..528106d 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,13 @@ one of its peers sends an update. figures. The route figures come from the routing table. `oldest_route` and `newest_route` are the times of the oldest and the newest kept routes. `origins_without_kept_route` counts, across all prefixes, the origin ASes that - have no kept route. + have no kept route. Other figures include these: + - `uptime_seconds`: the number of seconds since the RIS Live feed last + connected, or 0 when it has never connected + - `memory_usage_bytes`: the size of the Go heap in use, in bytes + - `whois_stats.current_interval_seconds`: the WHOIS fetcher's wait between + two fetches, in seconds, which can include a fraction of a second + - `GET /.well-known/healthcheck.json` - Health check (JSON): 200 when the database answers and the RIS Live feed is connected, otherwise 503; `data.status` is `ok` or `error` and `data.checks` says which check failed @@ -136,9 +142,10 @@ one of its peers sends an update. ### API v1 - `GET /api/v1/stats` - The `/status.json` figures plus connection, garbage - collector, stream and handler queue statistics. `stream.route_churn_per_min` - is the number of prefix announcements and withdrawals, from all peers, in the - last full clock minute. + collector, stream and handler queue statistics. `connection_duration_seconds` + is the number of seconds the RIS Live feed has been connected, or null while + it is not connected. `stream.route_churn_per_min` is the number of prefix + announcements and withdrawals, from all peers, in the last full clock minute. - `GET /api/v1/ip/{ip}` - Looks up each address. `{ip}` is an IP address, a host name, or a comma-separated list of at most 10 of these. `results` has one entry for each address. Every entry has the same fields: @@ -207,10 +214,16 @@ one of its peers sends an update. missing, empty or only spaces answers 400. A JSON answer is `{"status": "ok", "data": ...}`, or for an error -`{"status": "error", "error": {"msg": ..., "code": ...}}`, and carries an -`@meta` object with the time zone, the API version and the time the request -took. The health check is the exception: its 503 answer is also -`{"status": "ok", "data": ...}`, with `error` in `data.status`. +`{"status": "error", "error": {"msg": ..., "code": ...}}`. The health check is +the exception: its 503 answer is also `{"status": "ok", "data": ...}`, with +`error` in `data.status`. Every JSON answer also carries an `@meta` object: + +- `time_zone`: always `UTC` +- `api_version`: `1` +- `execution_time_ms`: the number of milliseconds the request took + +Every value with a unit is a number whose field name ends in the unit, such as +`_ms`, `_seconds` or `_bytes`. A request that runs past its time limit answers 504: the limit is 30 seconds, and 4 seconds for the database reads behind `/status.json` and `/api/v1/stats`. diff --git a/REPO_POLICIES.md b/REPO_POLICIES.md index 1067698..3e8c97f 100644 --- a/REPO_POLICIES.md +++ b/REPO_POLICIES.md @@ -1,6 +1,6 @@ --- title: Repository Policies -last_modified: 2026-10-18 +last_modified: 2026-10-27 --- This document covers repository structure, tooling, and workflow standards. Code @@ -12,6 +12,7 @@ style conventions are in separate documents: - [JavaScript](https://git.eeqj.de/sneak/prompts/raw/branch/main/prompts/CODE_STYLEGUIDE_JS.md) - [Python](https://git.eeqj.de/sneak/prompts/raw/branch/main/prompts/CODE_STYLEGUIDE_PYTHON.md) - [Go HTTP Server Conventions](https://git.eeqj.de/sneak/prompts/raw/branch/main/prompts/GO_HTTP_SERVER_CONVENTIONS.md) +- [API Policies](https://git.eeqj.de/sneak/prompts/raw/branch/main/prompts/API_POLICIES.md) --- @@ -174,27 +175,32 @@ style conventions are in separate documents: `script/cibuild`. `script/lint`, `script/test`, `script/cibuild` and `script/docker` are the four, and there is no fifth — `script/check` runs the two gate phases and `script/fmt-check`, and builds no image of its own. In - every stage of the `Dockerfile`, `ARG CACHEBUST` comes right after the steps - that install system packages and download Go's modules, or right after `FROM` - in a stage with neither, and every step that lints, tests or compiles comes - after it. A step that installs anything else as well, such as a - `script/bootstrap` that also installs node packages, comes after it too. A new - value reruns every `RUN` step after that line; `COPY` steps are compared by - content and need nothing. Docker keeps the layers above the line until the - base image, their own lines or the files they copy change. A stage without the - `ARG` line is served from cache on an unchanged tree, so a repository - re-vendors the scripts and adds the `ARG` lines in one commit. The scripts do - not pass `--no-cache`: it reruns every step too, but it also builds the layers - above the line again and empties every cache mount, so Go's build cache and - golangci-lint's, which the canonical Go `Dockerfile` below mounts so that they - survive between check builds, would start empty every time. A bare - `docker build .` is not evidence that anything ran: a sub-second build - reporting success is a cache hit, not a result. Never invalidate by pruning — - `docker builder prune` and friends destroy a build cache shared with every - other build on the host. When a check is added or changed, prove it works by - planting a defect it must catch and watching the run fail on it, then revert - the defect. A green run alone shows neither that the check ran nor that it - covers what it should. + every stage of the `Dockerfile`, every step that lints, tests or compiles + comes after `ARG CACHEBUST`. `ARG CACHEBUST` comes right after these steps: + - the steps that install system packages + - the steps that download Go's modules + - the step that runs `script/bootstrap` to install a Python `test` phase's + packages from `Pipfile.lock` + + A stage with none of these steps declares `ARG CACHEBUST` right after + `FROM`. A step that installs anything else as well, such as a + `script/bootstrap` that also installs node packages, comes after it too. A + new value reruns every `RUN` step after that line; `COPY` steps are compared + by content and need nothing. Docker keeps the layers above the line until + the base image, their own lines or the files they copy change. A stage + without the `ARG` line is served from cache on an unchanged tree, so a + repository re-vendors the scripts and adds the `ARG` lines in one commit. + The scripts do not pass `--no-cache`: it reruns every step too, but it also + builds the layers above the line again and empties every cache mount, so + Go's build cache and golangci-lint's, which the canonical Go `Dockerfile` + below mounts so that they survive between check builds, would start empty + every time. A bare `docker build .` is not evidence that anything ran: a + sub-second build reporting success is a cache hit, not a result. Never + invalidate by pruning — `docker builder prune` and friends destroy a build + cache shared with every other build on the host. When a check is added or + changed, prove it works by planting a defect it must catch and watching the + run fail on it, then revert the defect. A green run alone shows neither that + the check ran nor that it covers what it should. - **The gate phases are separate stages, and the build stage depends on both.** The lint phase is based on the `golangci/golangci-lint` image (pinned by @@ -309,6 +315,9 @@ style conventions are in separate documents: a package's findings only when the package's files and those of every package it imports, `go.mod`, the linters and their settings in `.golangci.yml`, and the golangci-lint version are all unchanged. + - In a Python `test` phase, the layer of the step that runs + `script/bootstrap` to install the packages from `Pipfile.lock`, as + described for that phase below. - If the project uses `//go:embed` directives that reference build artifacts (e.g. a web frontend compiled in a separate stage), the lint phase must create placeholder files so the embed directives resolve. Example: @@ -363,6 +372,67 @@ style conventions are in separate documents: `--build-arg VERSION=...`, as `script/docker` and `script/cibuild` already do. +- **A Python `test` phase that installs its packages from `Pipfile.lock` does it + above `ARG CACHEBUST`, so check builds keep them.** The steps are + `COPY script/ script/`, `COPY Pipfile Pipfile.lock ./` and + `RUN script/bootstrap`, with `ARG CACHEBUST` right after them. Docker keeps + what that `RUN` installed until the base image, `script/`, `Pipfile`, + `Pipfile.lock` or a line up to and including the `RUN` changes. The tests run + after `ARG CACHEBUST`, on every check build. In every other stage, a step that + installs Python packages comes after `ARG CACHEBUST`. + - When `script/bootstrap` runs in that step, it installs nothing beyond + system packages, pipenv and its dependencies, and the packages from + `Pipfile.lock`. It installs no Python there: the tests use the base + image's Python, which the image's hash pins. If it installs anything else + there, the step goes after `ARG CACHEBUST`. + - Every external reference must be pinned by hash, so `script/bootstrap` + installs pipenv and its dependencies by hash. It does so with + `pip install --require-hashes -r` and a requirements file in `script/` + that lists each of them with `==` and its `--hash`. pip refuses to install + a dependency the file does not list. + - It then runs `pipenv install --deploy`, with `--dev` when the tests need + the development packages. `--deploy` installs only what `Pipfile.lock` + names, checks every package against the lockfile's hashes, and fails when + the lockfile is out of date with `Pipfile`. + - A kept layer holds nothing a new install would refuse: pip checked pipenv + and its dependencies, and pipenv checked the lockfile's packages, against + their hashes. pipenv creates its virtual environment with the `virtualenv` + package. The packages that `virtualenv` adds come from wheels inside that + package, so the hash pip checked for it covers them. Two things in the + layer are not fixed by a hash. A package with no ready-built wheel for the + image is built from source. pip downloads the build tools without a hash + check, so a kept layer's build of such a package may differ from a new + one. And system packages keep the versions the package manager gave when + the layer was built; a new install may get newer ones. + - As with Go's modules, the packages are kept in a layer, not a cache mount. + A cache mount is shared by every build on the host, and nothing checks its + files again. + + In this example, `script/bootstrap` finds `make` and `git` already + installed, installs pipenv and its dependencies by hash, and runs + `pipenv install --dev --deploy`: + + ```dockerfile + # Test phase. The 90-second timeout covers both pytest runs. + # python:3.x-slim, YYYY-MM-DD + FROM python@sha256:... AS test + # make and git are base tools that script/bootstrap installs when they + # are missing. Installed here, they stay in their own layer when + # script/ changes. + RUN apt-get update \ + && apt-get install -y --no-install-recommends make git \ + && rm -rf /var/lib/apt/lists/* + WORKDIR /app + COPY script/ script/ + COPY Pipfile Pipfile.lock ./ + RUN script/bootstrap + ARG CACHEBUST + COPY . . + RUN timeout 90 sh -c 'pipenv run pytest || { \ + echo "--- Rerunning with -v for details ---"; \ + pipenv run pytest -v; exit 1; }' + ``` + - Every repo should have a Gitea Actions workflow (`.gitea/workflows/`) that runs `script/cibuild` on push, and checks out the repo as its only other step, with `persist-credentials: false`: `script/cibuild` needs no token, and diff --git a/TODO.md b/TODO.md index b224207..e4761d2 100644 --- a/TODO.md +++ b/TODO.md @@ -35,6 +35,15 @@ tested. # Completed Steps +- 2026-10-27: every value in a JSON answer that has a unit is a number, with the + unit at the end of its field name. `@meta.execution_time` is now + `execution_time_ms`. The stats' `uptime`, `connection_duration` and + `memory_usage` are now `uptime_seconds`, `connection_duration_seconds` and + `memory_usage_bytes`. `whois_stats.current_interval` is now + `current_interval_seconds`. They used to be strings such as "12 ms", + "1h18m34s" or "1.1 GB". `connection_duration_seconds` is null while the feed + is not connected. `API_POLICIES.md` is vendored from `sneak/prompts`, and + `REPO_POLICIES.md` re-vendored (closes #199) - 2026-10-27: every result of the IP lookup has the same fields. A field without a value is null. It used to be left out. An address with no live route has every route and AS field set to null, and `ptr` is an empty list when there diff --git a/internal/server/handlers.go b/internal/server/handlers.go index 0c66da6..cf9b9bc 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -23,7 +23,6 @@ import ( "git.eeqj.de/sneak/routewatch/internal/streamer" "git.eeqj.de/sneak/routewatch/internal/templates" asinfo "git.eeqj.de/sneak/routewatch/pkg/asinfo" - "github.com/dustin/go-humanize" "github.com/go-chi/chi/v5" ) @@ -226,18 +225,23 @@ type WHOISStatsInfo struct { NeverFetched int `json:"never_fetched"` SuccessesLastHour int `json:"successes_last_hour"` ErrorsLastHour int `json:"errors_last_hour"` - CurrentInterval string `json:"current_interval"` ConsecutiveFails int `json:"consecutive_fails"` FreshPercent float64 `json:"fresh_percent"` + + // CurrentIntervalSeconds is the wait between two fetches. It can have a + // fractional part, such as 22.5. + CurrentIntervalSeconds float64 `json:"current_interval_seconds"` } // Stats is the /status.json answer: uptime, message counts, database stats, -// and the routing table's figures. OldestRoute and NewestRoute are the times -// of the oldest and the newest kept routes. +// and the routing table's figures. UptimeSeconds is the time since the stream +// last connected, or 0 if it has never connected. MemoryUsageBytes is the Go +// heap in use. OldestRoute and NewestRoute are the times of the oldest and the +// newest kept routes. // //nolint:tagliatelle // field names are the existing JSON API type Stats struct { - Uptime string `json:"uptime"` + UptimeSeconds int64 `json:"uptime_seconds"` TotalMessages uint64 `json:"total_messages"` TotalBytes uint64 `json:"total_bytes"` TotalWireBytes uint64 `json:"total_wire_bytes"` @@ -247,7 +251,7 @@ type Stats struct { Connected bool `json:"connected"` GoVersion string `json:"go_version"` Goroutines int `json:"goroutines"` - MemoryUsage string `json:"memory_usage"` + MemoryUsageBytes uint64 `json:"memory_usage_bytes"` ASNs int `json:"asns"` Prefixes int `json:"prefixes"` IPv4Prefixes int `json:"ipv4_prefixes"` @@ -362,7 +366,7 @@ func (s *Server) statusStats( runtime.ReadMemStats(&memStats) return Stats{ - Uptime: streamUptime(streamMetrics.ConnectedSince), + UptimeSeconds: uptimeSeconds(streamMetrics.ConnectedSince), TotalMessages: streamMetrics.TotalMessages, TotalBytes: streamMetrics.TotalBytes, TotalWireBytes: streamMetrics.TotalWireBytes, @@ -372,7 +376,7 @@ func (s *Server) statusStats( Connected: streamMetrics.Connected, GoVersion: runtime.Version(), Goroutines: runtime.NumGoroutine(), - MemoryUsage: humanize.Bytes(memStats.Alloc), + MemoryUsageBytes: memStats.Alloc, ASNs: dbStats.ASNs, Prefixes: counts.IPv4Prefixes + counts.IPv6Prefixes, IPv4Prefixes: counts.IPv4Prefixes, @@ -439,30 +443,31 @@ func (s *Server) getWHOISStats(ctx context.Context) *WHOISStatsInfo { NeverFetched: dbStats.NeverFetched, SuccessesLastHour: fetcherStats.SuccessesLastHour, ErrorsLastHour: fetcherStats.ErrorsLastHour, - CurrentInterval: fetcherStats.CurrentInterval.String(), ConsecutiveFails: fetcherStats.ConsecutiveFails, FreshPercent: freshPercent, + + CurrentIntervalSeconds: fetcherStats.CurrentInterval.Seconds(), } } -// streamUptime is how long the stream has been connected, to the second, or -// "0s" if it has never connected. -func streamUptime(connectedSince time.Time) string { +// uptimeSeconds is the number of whole seconds since the stream last +// connected, or 0 if it has never connected. +func uptimeSeconds(connectedSince time.Time) int64 { if connectedSince.IsZero() { - return "0s" + return 0 } - return time.Since(connectedSince).Truncate(time.Second).String() + return int64(time.Since(connectedSince) / time.Second) } -// connectionDuration is how long the stream has been connected, to the -// second, or "disconnected". -func connectionDuration(connected bool, connectedSince time.Time) string { +// connectionDurationSeconds is the number of whole seconds the stream has +// been connected, or nil while it is not connected. +func connectionDurationSeconds(connected bool, connectedSince time.Time) *int64 { if !connected || connectedSince.IsZero() { - return "disconnected" + return nil } - return time.Since(connectedSince).Truncate(time.Second).String() + return new(uptimeSeconds(connectedSince)) } // whoisStaleThreshold matches the fetcher's threshold for consistency. @@ -512,7 +517,7 @@ type StreamStats struct { // //nolint:tagliatelle // field names are the existing JSON API type StatsResponse struct { - Uptime string `json:"uptime"` + UptimeSeconds int64 `json:"uptime_seconds"` TotalMessages uint64 `json:"total_messages"` TotalBytes uint64 `json:"total_bytes"` TotalWireBytes uint64 `json:"total_wire_bytes"` @@ -520,11 +525,10 @@ type StatsResponse struct { MbitsPerSec float64 `json:"mbits_per_sec"` WireMbitsPerSec float64 `json:"wire_mbits_per_sec"` Connected bool `json:"connected"` - ConnectionDuration string `json:"connection_duration"` ReconnectCount uint64 `json:"reconnect_count"` GoVersion string `json:"go_version"` Goroutines int `json:"goroutines"` - MemoryUsage string `json:"memory_usage"` + MemoryUsageBytes uint64 `json:"memory_usage_bytes"` GC GCStats `json:"gc"` Stream StreamStats `json:"stream"` ASNs int `json:"asns"` @@ -546,6 +550,9 @@ type StatsResponse struct { IPv6PrefixDistribution []PrefixDistribution `json:"ipv6_prefix_distribution"` WHOISStats *WHOISStatsInfo `json:"whois_stats,omitempty"` + // ConnectionDurationSeconds is null while the stream is not connected. + ConnectionDurationSeconds *int64 `json:"connection_duration_seconds"` + OriginsWithoutKeptRoute int `json:"origins_without_kept_route"` } @@ -684,20 +691,18 @@ func (s *Server) statsResponse( runtime.ReadMemStats(&memStats) return StatsResponse{ - Uptime: streamUptime(streamMetrics.ConnectedSince), - TotalMessages: streamMetrics.TotalMessages, - TotalBytes: streamMetrics.TotalBytes, - TotalWireBytes: streamMetrics.TotalWireBytes, - MessagesPerSec: streamMetrics.MessagesPerSec, - MbitsPerSec: streamMetrics.BitsPerSec / bitsPerMegabit, - WireMbitsPerSec: streamMetrics.WireBitsPerSec / bitsPerMegabit, - Connected: streamMetrics.Connected, - ConnectionDuration: connectionDuration( - streamMetrics.Connected, streamMetrics.ConnectedSince), + UptimeSeconds: uptimeSeconds(streamMetrics.ConnectedSince), + TotalMessages: streamMetrics.TotalMessages, + TotalBytes: streamMetrics.TotalBytes, + TotalWireBytes: streamMetrics.TotalWireBytes, + MessagesPerSec: streamMetrics.MessagesPerSec, + MbitsPerSec: streamMetrics.BitsPerSec / bitsPerMegabit, + WireMbitsPerSec: streamMetrics.WireBitsPerSec / bitsPerMegabit, + Connected: streamMetrics.Connected, ReconnectCount: streamMetrics.ReconnectCount, GoVersion: runtime.Version(), Goroutines: runtime.NumGoroutine(), - MemoryUsage: humanize.Bytes(memStats.Alloc), + MemoryUsageBytes: memStats.Alloc, GC: gcStats(&memStats), Stream: s.streamStats(), ASNs: dbStats.ASNs, @@ -719,6 +724,9 @@ func (s *Server) statsResponse( IPv6PrefixDistribution: prefixDistribution(counts.IPv6PrefixesByLength), WHOISStats: whoisStats, + ConnectionDurationSeconds: connectionDurationSeconds( + streamMetrics.Connected, streamMetrics.ConnectedSince), + OriginsWithoutKeptRoute: counts.OriginsWithoutKeptRoute, } } diff --git a/internal/server/handlers_internal_test.go b/internal/server/handlers_internal_test.go index 4857b6a..642f057 100644 --- a/internal/server/handlers_internal_test.go +++ b/internal/server/handlers_internal_test.go @@ -73,6 +73,7 @@ func emptyTable() *routetable.Table { // Paths several tests request. const ( asPagePath = "/as/64500" + asJSONPath = "/api/v1/as/64500" prefixLengthPagePath = "/prefixlength/24" statusJSONPath = "/status.json" statsPath = "/api/v1/stats" @@ -295,6 +296,167 @@ func TestStatsAnswerCarriesChurnPerMinute(t *testing.T) { } } +// fixedIntervalASNFetcher is a WHOIS fetcher that does nothing and reports +// interval as its wait between fetches. +type fixedIntervalASNFetcher struct { + idleASNFetcher + + interval time.Duration +} + +func (f fixedIntervalASNFetcher) GetStats() ASNFetcherStats { + return ASNFetcherStats{CurrentInterval: f.interval} +} + +// unitStats holds the figures of a stats answer that carry a unit. Each is a +// pointer, so that a figure missing from the answer is nil. +// +//nolint:tagliatelle // field names are the existing JSON API +type unitStats struct { + UptimeSeconds *int64 `json:"uptime_seconds"` + MemoryUsageBytes *uint64 `json:"memory_usage_bytes"` + WHOISStats struct { + CurrentIntervalSeconds *float64 `json:"current_interval_seconds"` + } `json:"whois_stats"` + GC struct { + HeapAllocBytes uint64 `json:"heap_alloc_bytes"` + } `json:"gc"` +} + +// TestStatsAnswersGiveUnitsInFieldNames checks that the figures of both stats +// answers that carry a unit are numbers, with the unit at the end of their +// names. They used to be strings with the unit in them, such as "1h18m34s" or +// "1.1 GB" (https://git.eeqj.de/sneak/routewatch/issues/199). The stream has +// never connected, so the uptime is 0. The memory use in /api/v1/stats is the +// same number of bytes as its gc.heap_alloc_bytes. +func TestStatsAnswersGiveUnitsInFieldNames(t *testing.T) { + t.Parallel() + + const fetchInterval = 18200 * time.Millisecond + + s := newServerWithTestRoutes(t) + s.SetASNFetcher(fixedIntervalASNFetcher{interval: fetchInterval}) + + for _, path := range []string{statusJSONPath, statsPath} { + answer := get(t, s, path).Body.Bytes() + + var body struct { + Data unitStats `json:"data"` + } + + err := json.Unmarshal(answer, &body) + if err != nil { + t.Errorf("%s: figures are not numbers: %v; answer %s", path, err, answer) + + continue + } + + got := body.Data + if got.UptimeSeconds == nil || *got.UptimeSeconds != 0 { + t.Errorf("%s: uptime_seconds is not 0: %s", path, answer) + } + + if got.MemoryUsageBytes == nil || *got.MemoryUsageBytes == 0 { + t.Errorf("%s: memory_usage_bytes is not a number of bytes: %s", + path, answer) + } + + interval := got.WHOISStats.CurrentIntervalSeconds + if interval == nil || *interval != fetchInterval.Seconds() { + t.Errorf("%s: whois_stats.current_interval_seconds is not %v: %s", + path, fetchInterval.Seconds(), answer) + } + + if path == statsPath && got.MemoryUsageBytes != nil && + *got.MemoryUsageBytes != got.GC.HeapAllocBytes { + t.Errorf("%s: memory_usage_bytes %d, want gc.heap_alloc_bytes %d", + path, *got.MemoryUsageBytes, got.GC.HeapAllocBytes) + } + } +} + +// TestStatsAnswerGivesConnectionDurationInSeconds checks that the +// connection_duration_seconds of /api/v1/stats is null while the stream is not +// connected, and a whole number of seconds once it is. It used to be the +// string "disconnected", or a Go duration such as "1h18m34s" +// (https://git.eeqj.de/sneak/routewatch/issues/199). +func TestStatsAnswerGivesConnectionDurationInSeconds(t *testing.T) { + t.Parallel() + + s := newServerWithTestRoutes(t) + + connectionDuration := func() json.RawMessage { + answer := get(t, s, statsPath).Body.Bytes() + + var body struct { + Data map[string]json.RawMessage `json:"data"` + } + + err := json.Unmarshal(answer, &body) + if err != nil { + t.Fatalf("decoding the answer: %v; answer %s", err, answer) + } + + duration, ok := body.Data["connection_duration_seconds"] + if !ok { + t.Fatalf("no connection_duration_seconds: %s", answer) + } + + return duration + } + + disconnected := connectionDuration() + if string(disconnected) != "null" { + t.Errorf("disconnected: connection_duration_seconds = %s, want null", + disconnected) + } + + s.streamer.GetMetricsTracker().SetConnected(true) + + var seconds int64 + + connected := connectionDuration() + + err := json.Unmarshal(connected, &seconds) + if err != nil || seconds != 0 { + t.Errorf("just connected: connection_duration_seconds = %s, want 0", + connected) + } +} + +// TestStreamDurationsCountSeconds checks that the uptime and the connection +// duration of a stream that connected 90 seconds ago are both 90. A stream +// that has never connected has an uptime of 0, and one that is not connected +// has no connection duration. +func TestStreamDurationsCountSeconds(t *testing.T) { + t.Parallel() + + const connectedFor = 90 * time.Second + + connectedSince := time.Now().Add(-connectedFor) + want := int64(connectedFor / time.Second) + + if got := uptimeSeconds(connectedSince); got != want { + t.Errorf("uptimeSeconds = %d, want %d", got, want) + } + + if got := uptimeSeconds(time.Time{}); got != 0 { + t.Errorf("uptimeSeconds before any connection = %d, want 0", got) + } + + switch got := connectionDurationSeconds(true, connectedSince); { + case got == nil: + t.Errorf("connectionDurationSeconds = nil, want %d", want) + case *got != want: + t.Errorf("connectionDurationSeconds = %d, want %d", *got, want) + } + + if got := connectionDurationSeconds(false, connectedSince); got != nil { + t.Errorf("connectionDurationSeconds while not connected = %d, want nil", + *got) + } +} + // newTestTable returns a routing table that holds a route for testPrefixV4, // with origin AS 64500, and one for testPrefixV6, with origin AS 64501. func newTestTable() *routetable.Table { @@ -766,7 +928,7 @@ func TestRoutesAnswer504WhenDatabaseReadIsCutOff(t *testing.T) { {path: "/ip/" + addrInTestPrefixV4, wantJSONBody: true}, {path: statsPath, wantJSONBody: true}, {path: "/api/v1/ip/" + addrInTestPrefixV4, wantJSONBody: true}, - {path: "/api/v1/as/64500", wantJSONBody: true}, + {path: asJSONPath, wantJSONBody: true}, } for _, tt := range tests { @@ -950,7 +1112,7 @@ func TestJSONRoutesDoNotSendErrorDetails(t *testing.T) { {"/.well-known/healthcheck.json", http.StatusServiceUnavailable}, {statusJSONPath, http.StatusInternalServerError}, {statsPath, http.StatusInternalServerError}, - {"/api/v1/as/64500", http.StatusInternalServerError}, + {asJSONPath, http.StatusInternalServerError}, {"/api/v1/ip/" + addrInTestPrefixV4, http.StatusInternalServerError}, {"/api/v1/prefix/foo/24", http.StatusBadRequest}, } diff --git a/internal/server/middleware.go b/internal/server/middleware.go index a0eab9f..ebe0227 100644 --- a/internal/server/middleware.go +++ b/internal/server/middleware.go @@ -53,6 +53,15 @@ func (rw *responseWriter) Header() http.Header { return rw.ResponseWriter.Header() } +// The @meta object JSONResponseMiddleware adds to a JSON answer: its key, and +// the keys of its fields. +const ( + jsonKeyMeta = "@meta" + metaKeyTimeZone = "time_zone" + metaKeyAPIVersion = "api_version" + metaKeyExecutionTimeMs = "execution_time_ms" +) + // JSONResponseMiddleware is an HTTP middleware that wraps all JSON responses // with a @meta field containing execution metadata. The metadata includes the // time zone (always UTC), API version, and request execution time in milliseconds. @@ -106,10 +115,10 @@ func JSONResponseMiddleware(next http.Handler) http.Handler { } // Add @meta field - originalResponse["@meta"] = map[string]any{ - "time_zone": "UTC", - "api_version": 1, - "execution_time": fmt.Sprintf("%d ms", execTime.Milliseconds()), + originalResponse[jsonKeyMeta] = map[string]any{ + metaKeyTimeZone: "UTC", + metaKeyAPIVersion: 1, + metaKeyExecutionTimeMs: execTime.Milliseconds(), } // Write the enhanced response diff --git a/internal/server/middleware_internal_test.go b/internal/server/middleware_internal_test.go index 39fa7d8..88d2bf6 100644 --- a/internal/server/middleware_internal_test.go +++ b/internal/server/middleware_internal_test.go @@ -10,6 +10,7 @@ import ( "net/http/httptest" "net/netip" "os" + "regexp" "strings" "testing" "time" @@ -100,6 +101,81 @@ func TestJSONValidationKeeps4xxWithoutBody(t *testing.T) { checkJSONErrorMessage(t, name, rec, http.StatusText(http.StatusBadRequest)) } +// TestJSONAnswersPutNoUnitInAString sends a request to each JSON route and +// checks each answer. Its @meta.execution_time_ms is a number, and none of its +// strings is a number with a unit, such as "12 ms", "1.1 GB" or "1h18m34s" +// (https://git.eeqj.de/sneak/routewatch/issues/199). The health check answers +// 503, since the stream is not connected, and the last path matches no route. +func TestJSONAnswersPutNoUnitInAString(t *testing.T) { + t.Parallel() + + numberWithUnit := regexp.MustCompile( + `^([0-9]+(\.[0-9]+)? ?(ns|µs|ms|s|m|h|B|kB|MB|GB|TB))+$`) + + s := newServerWithTestRoutes(t) + s.resolver = &fakeResolver{} + s.SetASNFetcher(fixedIntervalASNFetcher{interval: time.Second}) + + paths := []string{ + statusJSONPath, + statsPath, + "/.well-known/healthcheck.json", + "/api/v1/ip/" + untrustedPeerAddr, + "/ip/" + untrustedPeerAddr, + asJSONPath, + "/api/v1/prefix/" + testPrefixV4, + "/api/v1/search/as?q=google", + "/api/v1/no-such-route", + } + + for _, path := range paths { + rec := httptest.NewRecorder() + s.router.ServeHTTP(rec, httptest.NewRequestWithContext( + t.Context(), http.MethodGet, path, nil)) + + var answer map[string]any + + err := json.Unmarshal(rec.Body.Bytes(), &answer) + if err != nil { + t.Errorf("%s: decoding the answer: %v; answer %s", path, err, rec.Body) + + continue + } + + meta, _ := answer["@meta"].(map[string]any) + if _, ok := meta["execution_time_ms"].(float64); !ok { + t.Errorf("%s: @meta.execution_time_ms is not a number: %s", + path, rec.Body) + } + + for _, value := range stringsIn(answer) { + if numberWithUnit.MatchString(value) { + t.Errorf("%s: %q is a number with a unit in a string", path, value) + } + } + } +} + +// stringsIn returns every string in a decoded JSON value, at any depth. +func stringsIn(value any) []string { + var found []string + + switch v := value.(type) { + case string: + found = append(found, v) + case []any: + for _, item := range v { + found = append(found, stringsIn(item)...) + } + case map[string]any: + for _, item := range v { + found = append(found, stringsIn(item)...) + } + } + + return found +} + // TestPagesCarryTheCSPNonce checks that every script and style block of each // page carries the nonce its Content-Security-Policy header names, so that the // browser runs and applies them, that each answer has a nonce of its own, and diff --git a/internal/templates/status.html b/internal/templates/status.html index b277a22..3463f4a 100644 --- a/internal/templates/status.html +++ b/internal/templates/status.html @@ -401,6 +401,8 @@ const REFRESH_PERIOD_MS = 2000; const STALE_AFTER_REFRESHES = 3; const MS_PER_SECOND = 1000; + const SECONDS_PER_MINUTE = 60; + const SECONDS_PER_HOUR = 3600; const SPINNER_FRAMES = ['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏']; const SPINNER_COLOURS = ['#e74c3c', '#e67e22', '#f1c40f', '#27ae60', '#3498db', '#9b59b6']; @@ -443,7 +445,18 @@ function formatNumber(num) { return num.toLocaleString(); } - + + // formatDuration writes a number of seconds in hours, minutes and + // seconds, such as "1h 18m 34s" or "18.2s", with at most one decimal. + function formatDuration(totalSeconds) { + const hours = Math.floor(totalSeconds / SECONDS_PER_HOUR); + const minutes = Math.floor(totalSeconds % SECONDS_PER_HOUR / SECONDS_PER_MINUTE); + const seconds = parseFloat((totalSeconds % SECONDS_PER_MINUTE).toFixed(1)); + if (hours > 0) return hours + 'h ' + minutes + 'm ' + seconds + 's'; + if (minutes > 0) return minutes + 'm ' + seconds + 's'; + return seconds + 's'; + } + function formatProcessingTime(ms) { if (ms < 0.001) { return (ms * 1000).toFixed(0) + ' µs'; @@ -619,11 +632,13 @@ noteMessageCount(data.total_messages); // Update all metrics - document.getElementById('uptime').textContent = data.uptime; + document.getElementById('uptime').textContent = formatDuration(data.uptime_seconds); document.getElementById('go_version').textContent = data.go_version; document.getElementById('goroutines').textContent = formatNumber(data.goroutines); - document.getElementById('memory_usage').textContent = data.memory_usage; - document.getElementById('connection_duration').textContent = data.connection_duration; + document.getElementById('memory_usage').textContent = formatBytes(data.memory_usage_bytes); + // connection_duration_seconds is null while the stream is not connected. + document.getElementById('connection_duration').textContent = + data.connection_duration_seconds === null ? 'disconnected' : formatDuration(data.connection_duration_seconds); document.getElementById('reconnect_count').textContent = formatNumber(data.reconnect_count); document.getElementById('total_messages').textContent = formatNumber(data.total_messages); document.getElementById('messages_per_sec').textContent = formatNumber(Math.round(data.messages_per_sec)); @@ -672,7 +687,7 @@ const errorsEl = document.getElementById('whois_errors'); errorsEl.textContent = formatNumber(data.whois_stats.errors_last_hour); errorsEl.className = 'metric-value' + (data.whois_stats.errors_last_hour > 0 ? ' disconnected' : ''); - document.getElementById('whois_interval').textContent = data.whois_stats.current_interval; + document.getElementById('whois_interval').textContent = formatDuration(data.whois_stats.current_interval_seconds); } // Update handler stats