API: Prometheus metrics, DB pool tuning, header/contract docs, health/ready split - #374
Merged
Conversation
…metadata handlers
|
@Emrys02 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Seven conflicts, including two add/add where both branches independently created services/api/middleware/metrics.go. The two metrics implementations are complementary, not competing, so both are kept: - dev's hand-rolled counters render into the public GET /metrics route via handlers.MetricsHandler, and handlers/stats.go calls its WriteHTTPMetrics. - this branch's NewMetrics(mux) records to a Prometheus registry served on the separate METRICS_PORT listener (issue Telocel-Labs#58), and is wired as the outermost middleware so requests shed by the global concurrency limiter are still counted. They share no state and use different exposition paths. This branch's version moved to metrics_registry.go so the boundary between the two stays visible, rather than interleaving them in one file. Other resolutions: - main.go — kept both sides: this branch's pgxpool saturation poller (Telocel-Labs#238) and dev's usage-rollup loops, which are unrelated background jobs. - api/openapi.yaml — kept dev's clearer batch-endpoint description together with this branch's X-RateLimit-* response headers. - docs/deployment.md — this branch is right that the Fly check now probes /v1/ready, and fly/api.toml on this branch does exactly that (Telocel-Labs#243), so the endpoint is this branch's; dev's gRPC TCP-check caveat is kept alongside it. - go.mod / go.sum — took this branch's newer dependency set (it adds the Prometheus client) and reconciled with `go mod tidy`. Two stale references fixed, both pre-existing here and only reachable once the branch compiled against dev: - contract_test.go called handlers.Health(nil, nil, nil) after this branch refactored Health() to take no arguments; main.go and health_test.go were updated at the time, this call site was not. - TestContract_RouteParity failed on "GET /v1/ready documented but not registered". It was registered — the test's expectedRoutes map is hand-maintained and had not been updated when the route was added. Verified: go vet and go test across all twelve services/api packages, cargo fmt --check, cargo clippy --workspace --all-targets -D warnings, and api/openapi.yaml parses.
…el-Labs#331/Telocel-Labs#353/Telocel-Labs#359/Telocel-Labs#378/Telocel-Labs#380) Six PRs landed on dev since the first merge, so this branch went dirty again. One conflict: services/api/handlers/health_test.go, add/add — both sides created it. The two files are not alternatives. dev's is `package handlers_test` with a single table-driven TestHealthHandler_TableDriven; this branch's is `package handlers` with six tests covering the /v1/health and /v1/ready split it introduces (Telocel-Labs#243). Kept this branch's file and dropped dev's, having checked what that costs. dev's four cases — all dependencies reachable, db down, grpc down, redis down — map one-to-one onto TestReady_AllHealthy_Returns200, TestReady_PostgresDown_Returns503, TestReady_GRPCDown_Returns503, and TestReady_RedisDown_Returns503. This branch adds a fifth (TestReady_NilDependencies_Returns503) and TestHealth_AlwaysReturns200. Keeping dev's file was not an option regardless: it asserts against handlers.HealthResponse, a type this branch deliberately split into LivenessResponse and ReadyResponse, and it expects /v1/health to return 503 when a dependency is down — the exact behaviour Telocel-Labs#243 changes, since a liveness probe must not fail because Postgres is unreachable. Verified: go vet and go test across all twelve services/api packages, cargo fmt --check, cargo clippy --workspace --all-targets -D warnings, and the REST handler coverage gate added by Telocel-Labs#380 still passes (ListEvents 100.0%, GetEvent 90.5%, Health 100.0%).
Depo-dev
added a commit
that referenced
this pull request
Jul 31, 2026
Correcting my previous commit, which fixed a quarter of the problem. The OpenAPI Spec job does not run openapi-typescript directly any more — #353 replaced it with `python3 scripts/generate_sdk_models.py`, which regenerates four files and diffs all of them: sdk/typescript/src/api-types.gen.ts sdk/go/openapi/models_gen.go sdk/python/src/trident_indexer/openapi_models_gen.py sdk/rust/src/openapi_models_gen.rs I regenerated only the TypeScript one, so the job stayed red. Running the real generator produces 958 lines across all four. The trigger is #374's liveness/readiness split (#243): `HealthResponse` in the spec became `LivenessResponse` plus `ReadyResponse`/`ReadyChecks`, and `/v1/ready` was documented as a new path. That rename also broke the Python package: `src/trident_indexer/__init__.py` re-exported `HealthResponse`, which the generator no longer emits, so `mypy --strict` failed with attr-defined and every pytest collection errored. Updated the re-export and `__all__` to the three new names. Verified per SDK: cargo clippy -p trident-sdk -D warnings; go build and go vet in sdk/go; tsc --noEmit and 49 tests in sdk/typescript; mypy --strict and 38 tests in sdk/python.
Depo-dev
added a commit
that referenced
this pull request
Jul 31, 2026
…#384) The `OpenAPI Spec` job was failing on `dev` at **Check for uncommitted changes**. #374's liveness/readiness split (#243) renamed `HealthResponse` in the spec to `LivenessResponse` plus `ReadyResponse`/`ReadyChecks` and documented `GET /v1/ready`, but did not regenerate the SDK models. That check exists to catch exactly this. ## What was stale All four files the generator owns, not just TypeScript — 958 lines total: - `sdk/typescript/src/api-types.gen.ts` - `sdk/go/openapi/models_gen.go` - `sdk/python/src/trident_indexer/openapi_models_gen.py` - `sdk/rust/src/openapi_models_gen.rs` The rename also broke the Python package: `__init__.py` re-exported `HealthResponse`, which the generator no longer emits, so `mypy --strict` failed and every pytest collection errored. Updated the re-export and `__all__`. ## A real conflict between two CI jobs Regenerating alone was not enough. The job kept failing on one line: ``` -use serde::{Deserialize, Serialize}; +use serde::{Serialize, Deserialize}; ``` quicktype emits `{Serialize, Deserialize}`; `cargo fmt --all` reorders it alphabetically, and `openapi_models_gen.rs` is reachable from `lib.rs` so it is not skipped. The Rust job and this job were asserting opposite content for the same file — whichever ran last left the other red. Fixed in the generator rather than around it: `generate_rust` now runs `rustfmt` over its output, so the committed file is simultaneously what quicktype produces and what `cargo fmt` wants. Verified idempotent — regenerating twice gives byte-identical output. (`ignore` in `rustfmt.toml` was the first attempt; that option is nightly-only and CI runs stable, so it silently did nothing.) ## Still red, out of scope Two checks fail for reasons that predate this PR and need dependency work, tracked with the backlog in #373: - **Dependency audit** — `govulncheck` reports a `google.golang.org/grpc` advisory reachable from `gen/trident_grpc.pb.go`. - **SBOM + scan (go-api)** — trivy CVEs in the go-api image. Verified per SDK: `cargo clippy -p trident-sdk -D warnings`; `go build`/`go vet` in sdk/go; `tsc --noEmit` and 49 tests in sdk/typescript; `mypy --strict` and 38 tests in sdk/python.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four related Stellar Wave API hardening/observability issues, implemented together since they share the same
internal/metricsregistry and touch overlapping files (main.go,handlers/,middleware/).client_golang) on a new internal port (METRICS_PORT, default 9091), additive to the existing hand-rolled/metrics: HTTP request count/latency per route+status, WebSocket active connections + message totals, outbound gRPC call metrics, rate-limit rejections.MinConns/MaxConnLifetime+jitter/MaxConnIdleTime/HealthCheckPeriod, all env-configurable) andstatement_timeout/idle_in_transaction_session_timeoutviaAfterConnect, shared env vars with the Rust indexer. Per-call context deadlines added to handler DB paths that previously rode the full 30s request budget. Pool saturation metrics (trident_db_pool_*) exported via feat(metrics): Prometheus metrics on the Go REST API #58's registry.X-RateLimit-*,X-Cache,Retry-After, and cursor fields documented inapi/openapi.yaml(including a realafter→cursorparam drift fix), plus a newinternal/contracttestsuite (kin-openapi) that validates live responses against the spec — no such suite existed despite the issue referencing one from Testing: contract tests between OpenAPI, handlers, gRPC proto, and SDKs #324./v1/healthsplit into a cheap liveness check (no dependency calls) and a new/v1/readyreadiness check (Postgres/Redis/gRPC, 503 on failure). Helm, Fly, and Compose health-probe configs updated so liveness and readiness no longer point at the same expensive endpoint.Two real bugs were caught and fixed along the way, both surfaced by the new contract tests:
ContractsStatsserializedcontractsas JSONnullinstead of[]on empty results./v1/health's OpenAPI schema documented a response shape (indexerobject) that never matched what the handler actually returned.Test plan
go build ./... && go vet ./... && gofmt -l .— clean (pre-existing unformatted files elsewhere untouched).go test -race ./...— full suite green, including new packages (internal/metrics,internal/contracttest) and new test files (pool config, health/ready healthy+unhealthy states, contract validation againstapi/openapi.yaml).curl :9091/metrics(new metrics), legacy:3000/metricsunchanged,/v1/healthvs/v1/readybehavior.pg_sleep/SHOW statement_timeoutend-to-end check from API: connection pool sizing + statement timeouts for Postgres in the Go layer #238's plan should be re-verified against a real DB before/after merge.Closes #58, closes #238, closes #242, closes #243.