Skip to content

docs: research unified v1 LMCache observability for #141 - #156

Open
rickisba wants to merge 2 commits into
mainfrom
codex/research-observability-for-issue-#141
Open

docs: research unified v1 LMCache observability for #141#156
rickisba wants to merge 2 commits into
mainfrom
codex/research-observability-for-issue-#141

Conversation

@rickisba

Copy link
Copy Markdown
Collaborator

Motivation

  • Provide a research/design deliverable for Issue v0.1.10-4: Add Unified v1 LMCache Gateway, Tier, Adapter, and Queue Observability #141 to audit current observability surfaces and propose a canonical, dependency-light v1 observability model without implementing runtime instrumentation.
  • Capture current behavior, ambiguity, and compatibility constraints so subsequent small incremental PRs can implement contracts and adapters safely.
  • Preserve existing runtime behavior and compatibility (no changes to routing, queue code, production Gateway I/O, or the root README).

Description

  • Add a single research document docs/research/issue-141-unified-observability.md that inventories request IDs and correlation fields, lists every ProxyTask.trace key, documents separately stored prediction/state fields, queue timestamps/durations, KDN injection and fallback markers, first-token/prefill/decode/completion markers, LMCache/vLLM metrics usage, and places where context is currently lost.
  • Classify every existing trace/metric field as predicted, observed, actual, inferred, legacy-projected, or ambiguous, and call out violations (e.g. overwritten predictions, mixed types in trace, and wall-clock-derived durations).
  • Recommend a canonical package boundary (core/observability/) and provide implementation-ready v1 model schemas for TraceContext, TraceStageName, TraceValueKind, TraceStageOutcome, TraceProvenance, TraceMeasurement, TraceStage, RequestTrace, CacheOperationTrace, and an OperationWaiterLink correlation design.
  • Define a stable ordered stage vocabulary, monotonic/UTC clock semantics, compatibility-first context propagation (internal envelope + allow-listed HTTP headers), shared-operation attach/detach semantics, metric-attribution rules for the validator, a four-phase additive rollout, an exact first-PR scope (contracts + CPU-only tests only), a precise file-plan for later phases, a CPU-only test matrix, and a list of unresolved maintainer decisions.
  • This change is research-only and additive documentation: it does not modify any production logic, telemetry emitters, gateway adapters, or the root README, and it preserves cacheroute_meta and existing trace dictionary shape for migration.

Testing

  • Ran an AST-based automated audit that located all literal ProxyTask.trace keys in proxy/proxy.py and proxy/queue/manager.py and asserted they are documented in the new research file; the check succeeded.
  • Performed repository safety checks including git diff --cached --check and validated that the new file is staged cleanly and that the repository root README.md was not modified; these checks passed.
  • Verified basic repository metadata (status/log/show) to confirm the documentation addition is present and isolated; no runtime or external-service tests were required because the PR contains documentation only and introduces no codepaths.

Codex Task

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ebeee57480

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +69 to +71
Both streaming and non-streaming Proxy responses expose exactly: `request_id`,
the entire mutable `trace` dictionary, `kv_ack`, `kv_ready_kids`,
`text_only_kids`, `miss_kids`, and `error`. Streaming inserts an

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Correct the documented legacy metadata shape

The baseline proxy/proxy.py::build_cacheroute_meta returns only trace, kv_ack, the three KID lists, and error; request_id remains solely on ProxyTask and is not exposed in either response form. Calling it part of the existing compatibility contract, while later directing Phase 2 to leave that shape unchanged, will cause the rollout and its tests to assume a correlation field that clients never receive. Either remove it from this inventory or explicitly make adding it a documented wire-format change.

AGENTS.md reference: AGENTS.md:L255-L261

Useful? React with 👍 / 👎.

Comment on lines +354 to +357
Required: `context`, ordered tuple `stages`, `exported_at` UTC, `complete: bool`,
and schema version. Optional: `source_components`, sanitized top-level
`error_code`, `legacy_trace` (JSON-safe copy of current dictionary), and
`legacy_cacheroute_meta` during migration. Canonical ordering is

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Deep-freeze legacy mappings in exported snapshots

These optional fields reintroduce mutable dictionaries into a snapshot that the proposal and test matrix require to be immutable. Pydantic's ConfigDict(frozen=True) prevents rebinding a model field but does not prevent mutations such as request_trace.legacy_trace["proxy_recv_ms"] = 0 (and nested values in legacy_cacheroute_meta have the same problem), so consumers can silently corrupt an exported research timeline. The contract needs to require a recursively immutable representation or an equivalent defensive export boundary rather than merely a JSON-safe dictionary copy.

Useful? React with 👍 / 👎.

Comment on lines +45 to +47
-> runtime_profile + created_at
-> targeted calls add compatibility_profile_id, endpoint_id, generation
-> Gateway responses preserve those fields

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use the actual versioned-message timestamp fields

The audited VersionedMessage contract has a timestamp field, not created_at, and GatewayAdapterBase._response constructs a fresh response without preserving the request timestamp. Thus this inventory both names a nonexistent envelope field and implies timestamp preservation that does not occur; instrumentation based on it may fail or incorrectly correlate request and response times. Document timestamp as a per-message timestamp and reserve created_at for nested domain objects that actually define it.

AGENTS.md reference: AGENTS.md:L273-L277

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant