feat(personhog): make gRPC rejections visible, and share the serving layer

[benjackwhite]

Problem

A usage-ingestion caller whose billing records are rejected gets no server-side signal saying why. ingest_billing_usage refuses an empty or oversized batch, a malformed record, and a team with no organization mapping. The only trace is a producer-side drop counter that says a record died, not what killed it.

grpc_server_requests_total carries method and client, so a rejected request looks the same as a served one on every PostHog gRPC server, not just this one.

usage-ingestion cannot use the layer that would fix this. It lives in personhog-common, so depending on it also pulls in personhog-proto, sqlx, and personhog's persons and partitioning modules.

Changes

• An operator can now split grpc_server_requests_total by code and see which rejections a server returns.
• usage-ingestion reports gRPC request metrics for the first time. It had no layers at all.
• The generic serving layer moves to a new common-grpc crate, next to common-metrics and common-health. Any service can take it without taking personhog.

A failed gRPC call is a trailers-only response: an empty body with grpc-status in the headers. tonic gives a handler error that shape, GrpcLoadShedLayer synthesises it when it sheds, and the router builds its own errors in it. So the layer reads the status at the head and never polls the body.

An absent header means the handler returned Ok, whose grpc-status: 0 rides the body trailers. This holds while every method is unary, and no proto in the repo declares a stream. A status header that will not parse counts as non_status rather than passing for a success.

personhog-common keeps the semantic refusal helpers, which encode the router's retry contract for handoff fences and ownership races. ClientInFlightGuard stays with them because it emits personhog_router_client_requests_in_flight, and moving it would put a personhog metric name in a shared crate. GZIP_OVERHEAD_HEADER moves next to the AsyncGzipLayer that sets it, so nothing about response compression leaves personhog-common.

The three commits are separable and review in order: the move is mechanical, the label and the wiring are not.

Note

The code label splits every existing grpc_server_requests_total series. An aggregating query is unaffected. A dashboard or alert that reads the raw series now gets one series per observed code.

No metric is renamed. cymbal was also on personhog_common::grpc and moves with the rest.

Cardinality is safe. prod-us carries 911 series today, 592 on the replica and 289 on the router, and only observed codes create new ones.

Nothing user-visible changes, so there are no screenshots.

How did you test this code?

Three tests in common-grpc, each red-checked by breaking response_code and confirming the predicted failure:

• handler_error_reads_as_its_own_code pins the label against tonic::Status::into_http rather than a hand-built response. The whole design rests on tonic putting the status in the headers.
• shed_response_reads_as_unavailable runs the stacked layers, because the metrics layer sits above the load shed layer in every server.
• response_code_vocabulary covers absent, 0, real codes, an out-of-range code, and an unparseable one.

The 16 moved tests still pass in their new home.

Manual run against the local stack

The usage-ingestion binary ran against local Postgres and Kafka. Four requests drove every path: an empty batch, an empty record_id, an unmapped team, and a valid record.

grpc_server_requests_total{method="IngestBillingUsage",client="unknown",code="ok"} 1
grpc_server_requests_total{method="IngestBillingUsage",client="unknown",code="invalid_argument"} 2
grpc_server_requests_total{method="IngestBillingUsage",client="unknown",code="not_found"} 1
grpc_server_request_duration_ms_count{method="IngestBillingUsage",client="unknown"} 4

Not checked: the personhog e2e harness gates and the etcd-backed integration suites. This change does not touch the coordination protocol.

personhog-router backend::discovery::sync_active_* fails under parallel load and passes alone. The same four tests fail the same way on a clean origin/master worktree, so this branch does not cause it.

Automatic notifications

• Publish to changelog?

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Claude Code (Opus 5) wrote this branch under Ben's direction.

Skills invoked: /reviewing-personhog-protocol, /reviewing-before-pr, /writing-pr-descriptions.

hogli review could not run. The Greptile CLI fails to install in this environment with EPERM on ~/.config/posthog/tools/greptile. The local pass was the harness fallback over the branch diff, so the independent review is still the bot's, and the no-greptile label is not applied.

The task specified that an absent grpc-status should count as non_status. That premise does not hold: tonic's Status::into_http puts a handler error in the response headers, which personhog-router/src/grpc_http.rs already documents and relies on. Labelling an absent header non_status would tag almost every request as a non-status failure. The shipped code labels it ok and reserves non_status for a header that will not parse.

Reading the body trailers instead was the rejected alternative. It would change when the counter fires, add a body wrapper and a new trait bound to every caller's stack, and reach into the AsyncGzipLayer that wraps the metrics layer on the replica and the leader. It would only buy coverage for a mid-stream failure, and no proto here declares a stream.

[trunk-io]

Merging to master in this repository is managed by Trunk.

• To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

[pr-assigner-resolver-posthog]

👀 Auto-assigned reviewers

These soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:

• @PostHog/team-error-tracking (rust/owners.yaml)

Soft owners come from each directory's owners.yaml and each product's product.yaml (resolved nearest-file-wins). The locator after each owner is the file that decided it. Generated files and lockfiles are ignored when deciding ownership.

[github-actions]

🤖 CI report

✅ Trunk lane — non-backend lane

This PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.

[greptile-apps]

Prompt To Fix All With AI

### Issue 1
rust/common/grpc/src/lib.rs:411-413
**Trailer errors report success**

If response-size enforcement is enabled, `GrpcMetricsLayer` records the handler response as `code="ok"` before the outer `AsyncGzipLayer` replaces an oversized body with `grpc-status: 11` trailers, causing clients to receive `OUT_OF_RANGE` while the rejection metric reports success.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "feat(usage-ingestion): report gRPC reque..." | Re-trigger Greptile}

[greptile-apps]

Trailer errors report success

If response-size enforcement is enabled, GrpcMetricsLayer records the handler response as code="ok" before the outer AsyncGzipLayer replaces an oversized body with grpc-status: 11 trailers, causing clients to receive OUT_OF_RANGE while the rejection metric reports success.

Prompt To Fix With AI

This is a comment left during a code review.
Path: rust/common/grpc/src/lib.rs
Line: 411-413

Comment:
**Trailer errors report success**

If response-size enforcement is enabled, `GrpcMetricsLayer` records the handler response as `code="ok"` before the outer `AsyncGzipLayer` replaces an oversized body with `grpc-status: 11` trailers, causing clients to receive `OUT_OF_RANGE` while the rejection metric reports success.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[trunk-io]

   

View Full Report ↗︎ ⋅ Docs