SRE-884: Remove the legacy health endpoints - #9133
Conversation
Sentry was authoring the HTTP server spans. Setting `tracesSampleRate` at all — `0` included, since the check is for nullish — enables Sentry's server-span and performance integrations, and those write into the globally registered provider with Sentry's own semantics, where a 404 becomes an error. `skipOpenTelemetrySetup` keeps them from ever reaching Sentry, so the only effect was skewing what Tempo derives from them. Kratos answers `GET /sessions/whoami` with 401 for "no session" and 403 for AAL1, both defined outcomes rather than failed calls. A rule table marks those successful and tags them `hash.expected_client_status`, because `parseResponseStatus` never yields OK on its own and the status would otherwise be indistinguishable from a suppressed failure. `getUserAndSession` also stops asking Kratos when there is neither a cookie nor a session token, which can only be answered with a 401. The auth middleware runs before routing, so every anonymous request paid that round trip, including ones for paths matching no route.
Every GraphQL request shares one server span name, and that span is what Tempo's `span_metrics` and `service_graphs` processors read, so per-operation views could not be built from them. The operation goes on as an attribute rather than into the span name, because operation names come from the client and a name would let a caller mint a metric series per request.
Probe paths differed per service — `/health` on the graph binaries and the
Temporal workers, `/health-check` on the Node API, nothing at all on
petrinaut-opt — so every consumer carried per-service special cases. Each
service now answers `GET /health` with `{"status":"pass"}` as
`application/health+json`, following the only member
`draft-inadarei-api-health-check` requires.
The old paths stay in place so the probe definitions in internal-infra can
switch independently of this deploy.
Probes are excluded from tracing in the two places that create HTTP server
spans: `HttpTracingLayer` returns a disabled span for the path, and
`createHttpInstrumentation` ignores it, which covers the Node API and both
workers at once. Without that, a probe answered every few seconds per task
would dominate the RED metrics derived from spans. The graph API's `/health`
was already untraced, but only as a side effect of being merged after the
layers, which the explicit filter no longer depends on.
Probes stay free of dependency checks: one that reaches through to Postgres
takes every task out of rotation as soon as Postgres is slow. The worker
health server now also answers HEAD, which is what the `hash-graph`
healthcheck subcommand and `wget --spider` issue.
petrinaut-opt's `/status` is unrelated to health — it returns the status of
every optimization run — so it keeps it and gains `/health`.
Also renames the Atlas healthcheck test, which named the endpoint it probes.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
`HEALTHCHECK` ships inside the image, and the same image now serves `/health`, so the container probe can move with it. The graph binaries have no `HEALTHCHECK` — their compose healthchecks call `hash-graph … --healthcheck`, which follows the path change on its own — and both Temporal workers were already on `/health`.
`/health-check` on the Node API and `/status` on Atlas were kept alive only so the probe definitions in internal-infra could move to `/health` independently of that deploy. Both are now unused. petrinaut-opt keeps its `/status`, which is not a probe — it returns the status of every optimization run.
49a09e6 to
38b2829
Compare
PR SummaryMedium Risk Overview The Node API no longer serves Deploy coordination: merging before internal-infra retargets ALB/container health checks will break probes on the old paths. Reviewed by Cursor Bugbot for commit 20acae5. Bugbot is set up for automated code reviews on this repo. Configure here. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9133 +/- ##
==========================================
- Coverage 59.57% 59.55% -0.02%
==========================================
Files 1413 1409 -4
Lines 138053 137832 -221
Branches 6510 6442 -68
==========================================
- Hits 82240 82092 -148
+ Misses 54771 54731 -40
+ Partials 1042 1009 -33
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@copilot resolve the merge conflicts in this pull request |
Benchmark results
|
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2002 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 1002 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 3314 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 1527 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 2078 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 1033 | Flame Graph |
policy_resolution_medium
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 102 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 269 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 108 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 133 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 63 | Flame Graph |
policy_resolution_none
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 8 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 3 | Flame Graph |
policy_resolution_small
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 26 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 94 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 27 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 66 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 29 | Flame Graph |
read_scaling_complete
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id;one_depth | 1 entities | Flame Graph | |
| entity_by_id;one_depth | 10 entities | Flame Graph | |
| entity_by_id;one_depth | 25 entities | Flame Graph | |
| entity_by_id;one_depth | 5 entities | Flame Graph | |
| entity_by_id;one_depth | 50 entities | Flame Graph | |
| entity_by_id;two_depth | 1 entities | Flame Graph | |
| entity_by_id;two_depth | 10 entities | Flame Graph | |
| entity_by_id;two_depth | 25 entities | Flame Graph | |
| entity_by_id;two_depth | 5 entities | Flame Graph | |
| entity_by_id;two_depth | 50 entities | Flame Graph | |
| entity_by_id;zero_depth | 1 entities | Flame Graph | |
| entity_by_id;zero_depth | 10 entities | Flame Graph | |
| entity_by_id;zero_depth | 25 entities | Flame Graph | |
| entity_by_id;zero_depth | 5 entities | Flame Graph | |
| entity_by_id;zero_depth | 50 entities | Flame Graph |
read_scaling_linkless
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | 1 entities | Flame Graph | |
| entity_by_id | 10 entities | Flame Graph | |
| entity_by_id | 100 entities | Flame Graph | |
| entity_by_id | 1000 entities | Flame Graph | |
| entity_by_id | 10000 entities | Flame Graph |
representative_read_entity
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/block/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/book/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/building/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/organization/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/page/v/2
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/person/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/playlist/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/song/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/uk-address/v/1
|
Flame Graph |
representative_read_entity_type
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| get_entity_type_by_id | Account ID: bf5a9ef5-dc3b-43cf-a291-6210c0321eba
|
Flame Graph |
representative_read_multiple_entities
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_property | traversal_paths=0 | 0 | |
| entity_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=0 | 0 | |
| link_by_source_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true |
scenarios
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| full_test | query-limited | Flame Graph | |
| full_test | query-unlimited | Flame Graph | |
| linked_queries | query-limited | Flame Graph | |
| linked_queries | query-unlimited | Flame Graph |
🌟 What is the purpose of this PR?
Removes the probe endpoints that #9132 kept alive only so the probe definitions in internal-infra could move to
GET /healthindependently of that deploy.Do not merge before internal-infra has switched. Merging early takes the probes out from under the ALB target groups and container healthchecks that still point at the old paths.
🔗 Related links
🚫 Blocked by
catalog/services/app/api.tf,catalog/services/ingress/defaults.tf,catalog/services/app/petrinaut_opt.tf) targeting/health🔍 What does this change?
/health-checkfrom the Node API./statusfrom Atlas, leaving the health probe as its only route./status, which is not a probe — it returns the status of every optimization run.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
dropped_spansmap in internal-infra lists span names for the removed endpoints. Those entries become dead, but its own validation rejects an empty map, so emptying it is not an option — the internal-infra change has to keep a placeholder or relax the Alloy filter.🐾 Next steps
🛡 What tests cover this?
apps/hash-graph/src/subcommand/atlas.rscontinues to cover the remaining one.❓ How to test this?
curl -i localhost:5001/health-checkanswers404while/healthstill answers200.cargo run --bin hash-graph -- atlasand confirm the same for/statusagainst/health.📹 Demo
Not applicable — no user-facing surface.