feat(server): measure lifecycle operations server-side

[ferponse]

Fixes #1408.

Problem

The server exports one metric and it does not measure the server. sandbox.create.duration is a number the SDK reports to POST /metrics/events; the server only forwards it. Two consequences:

1. A deployment whose clients call the REST API directly rather than through an instrumented SDK exports nothing at all.
2. Even when clients do report, the number is the client's experience — network and SDK overhead included — so it cannot separate a slow server from a slow network.

And nothing was ever counted. SandboxErrorCodes lists ~40 failure modes (IMAGE_PULL_FAILED, K8S_POD_READY_TIMEOUT, EXECD_START_FAILED, …) and not one reached a metric, so when creation starts failing the server's telemetry stays silent and only the logs know.

What this adds

Metric	Type	Attributes
opensandbox.sandbox.operation.duration	Histogram, ms	operation, outcome
opensandbox.sandbox.operation.total	Counter	operation, outcome, error.code (failures only)

Recorded by a decorator on the eight mutating lifecycle handlers — create, update_metadata, delete, pause, resume, renew, create_snapshot, delete_snapshot. Read-only endpoints are deliberately left alone.

This makes the existing error taxonomy queryable without inventing a new one:

sum by (error_code) (rate(opensandbox_sandbox_operation_total{outcome="error"}[5m]))

Design notes

• Instrumented at the API boundary, because it is the single seam where every runtime converges and where the error code has already been decided. One decorator covers docker and kubernetes; no per-provider duplication.
• error.code on the counter only. The histogram keeps two attributes so its bucket series stay cheap; the taxonomy lives on the counter, which is one series per code.
• The code passes a shape check before becoming an attribute. Codes are source constants today, so this is normally a pass-through — but a future call site interpolating a message into detail["code"] would otherwise make the attribute unbounded. Anything not code-shaped becomes OTHER.
• The decorator cannot affect a request. It re-raises untouched, and a failure inside the recording is logged rather than propagated. My first version did not do the second part, and the test I had written asserted the wrong behaviour — instrumentation on every mutating route must not be able to 500 a request. Both directions are now pinned: a broken recorder does not fail a successful handler, and does not mask a failing one either.
• functools.wraps is load-bearing: FastAPI builds the request model from the handler signature, and inspect.signature follows __wrapped__. There is an explicit test for the signature surviving, since losing it would break routing in a way that is easy to introduce and easy to miss.
• Additive. The SDK-reported histogram is untouched, and both new instruments are None when [otel] enabled = false, exactly like the existing one.

Scope, and what I did not decide

POST /sandboxes returns 202 Accepted and provisions asynchronously, so create here is time-to-scheduled, not time-to-ready. Those are different numbers and the second is the one an operator usually wants for cold-start. Measuring it needs instrumentation inside the provisioning path, which is a design question about where the readiness boundary lives — I would rather follow your preference than pick unilaterally. Documented as a known limitation in configuration.md and in the issue.

Also not here: a gauge of live sandboxes, and provider-level timings (Kubernetes API calls, image pull). Both are straightforward once the shape above is agreed.

Testing

Full suite 1323 passed (21 new), ruff check clean. The suite already exercises these routes end to end, so it also confirms the decorator does not disturb FastAPI's request handling.

Covered: sync and async handlers, success, HTTPException with a code dict, HTTPException with a plain-string detail (falls back to HTTP_<status>), unexpected exceptions (UNKNOWN_ERROR), no-op when export is disabled, instrument failures swallowed, and the shape check accepting real codes while rejecting messages, identifiers and over-long values.

[ferponse]

Unprompted follow-up: e0f9370 documents this in docs/ as well.

Codex flagged on my sibling PRs that docs/ is the source of truth for operations-visible behavior per AGENTS.md, and the same applies here: docs/guides/sdk-telemetry.md described the SDK histogram as the only thing [otel] exports.

I kept to the repo's existing pattern for server config — docs/ links to server/configuration.md rather than duplicating the reference (four pages already do this). What docs/ gains is the part a table cannot convey: that the SDK histogram only exists where clients report it, so a dashboard built on it alone looks empty rather than healthy, and that the server-measured create is time-to-scheduled while the SDK number approximates time-to-ready.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e0f9370f4b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Correct the documented create metric boundary

For current Docker and Kubernetes create paths, this note under-describes what the new metric measures: DockerSandboxService.create_sandbox awaits the provisioning thread's result from _provision_sandbox, and KubernetesSandboxService.create_sandbox explicitly awaits _wait_for_sandbox_ready before returning, so operation.duration{operation="create"} includes readiness/provisioning time rather than only scheduling time. Leaving this text here (and the matching note in docs/guides/sdk-telemetry.md) will steer operators away from the only server-side cold-start signal this change adds.

AGENTS.md reference: server/AGENTS.md:L40-L42

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 208834f092

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Count requests rejected before the handler runs

When FastAPI rejects a lifecycle request during parsing or Pydantic validation, this wrapper is never invoked, so neither operation.duration nor operation.total records an error. For example, an authenticated POST /v1/sandboxes body that fails CreateSandboxRequest validation returns 422 before create_sandbox is called, leaving error-rate dashboards built from opensandbox.sandbox.operation.total undercounting bad create requests; instrumenting at the APIRoute/middleware layer would cover those API-boundary failures too.

Useful? React with 👍 / 👎.

[ferponse]

You're right and I had this backwards — fixed in 208834f.

I inferred "asynchronous" from the 202 Accepted status without checking the handler, and the status code is a convention here rather than a description. Verified now:

• KubernetesSandboxService.create_sandbox awaits _wait_for_sandbox_ready (kubernetes_service.py:947) before returning, so the sample includes pod readiness.
• DockerSandboxService.create_sandbox spawns the provisioning thread and then return await future (docker_service.py:680), so it includes container start and egress sidecar readiness — _start_egress_sidecar internally waits on _wait_for_egress_sidecar_ready.

So operation.duration{operation="create"} is a genuine server-side cold-start signal, which is the opposite of what my note claimed — and, as you say, the note would have steered operators away from the only cold-start metric here that does not depend on clients reporting anything. Corrected in server/configuration.md, docs/guides/sdk-telemetry.md and the decorator docstring, all three of which said the wrong thing.

What I kept is the boundary that is real: the measurement ends when the sandbox is ready, so it excludes whatever the workload does afterwards inside the sandbox.

[ferponse]

Correct — fixed in 9ad80dc, and I took your suggested layer.

A decorator on the endpoint cannot see a request FastAPI never delivers: validation raises before the handler is called, so a malformed create body returned 422 with nothing recorded. And it undercounts precisely the failures a caller is most likely to produce, which is the worst place to have a blind spot in an error rate.

Recording now lives in an APIRoute subclass, and the decorator is reduced to a marker that tags the handler with its operation name. The route covers:

Failure	error.code
RequestValidationError	HTTP_422
HTTPException with a code dict	the server code
HTTPException with a string detail	HTTP_<status>
anything unexpected	SANDBOX::UNKNOWN_ERROR
an error status returned rather than raised	HTTP_<status>

Unmarked read-only routes get the untouched handler back, so they pay nothing.

A side benefit: dropping the wrapper removes the functools.wraps dependency I had flagged as load-bearing. Nothing stands between FastAPI and the handler signature any more.

Two of the new tests guard the wiring rather than the logic, because that is where this construction fails silently — a marker applied in the wrong decorator order, or a router that forgets route_class, records nothing while every other test still passes. One pins the eight marked handlers and that the real lifecycle router uses InstrumentedRoute; the other pins that a single call is recorded exactly once, since a marker that also wrapped would double every count. I checked both fail when those mistakes are introduced, and that the 422 test fails without the RequestValidationError branch.

Suite is 1327 passing, ruff clean.