fix(grpc-proxy): emit worker tunnel close reason as metric, log and span

[balajinvda]

TL;DR

grpc-proxy already computes why it closes every worker tunnel, then writes it to a debug log and discards it. This emits it as a metric, promotes the log to info, and adds a span, so "why did the tunnel drop" becomes answerable from production data.

Observability only, no behaviour change.

Additional Details

The proxy closes worker tunnels for several ordinary reasons: cache TTL expiry, the connection going inactive, capacity eviction, and shutdown drain. mapEvictionReason turns each into a string on every close, and that string only ever reached a debug log line.

That left an entire hop unobserved. Existing metrics cover client connections and NATS; nothing covered the proxy-to-worker leg, which is exactly where connection-lifecycle failures occur. During a recent streaming-disconnect investigation the question "is the proxy dropping these connections, and if so why" stayed open for weeks, while the service computed the answer every single time it happened.

Metrics added, following the naming and pre-initialisation conventions in AGENTS.md:

nvcf_grpc_proxy_service_worker_connections_active            gauge
nvcf_grpc_proxy_service_worker_connection_opened_total       counter
nvcf_grpc_proxy_service_worker_connection_closed_total       counter, label: reason
nvcf_grpc_proxy_service_worker_connection_duration_seconds   histogram

Notes on the choices:

• reason is bounded to four values (ttl_expired, deleted, capacity_reached, unknown) and every one is pre-initialised to zero, so rate() has no gaps and absent() alerts do not misfire on a reason that has not occurred yet.
• Duration buckets deliberately straddle the two timers that can close a tunnel: the worker-side QUIC idle timeout at 8s, and this service's worker connection cache TTL at 30s. A hard cliff at either value distinguishes a timer kill from a natural end, which a default bucket set would hide.
• Open and close are counted where they balance: on successful insertion into the cache, and in the eviction handler, which fires for every entry that was inserted.

The log line is promoted from debug to info and gains held_for. The message text is deliberately unchanged so existing log searches and any downstream consumers keep working.

The span is emitted with the request id, reason and duration. Worth knowing: the eviction context is the cache's own, not the session's, so this span has no parent to attach to. The request id attribute is what makes a dropped session correlatable in tracing rather than only by grepping logs.

For the Reviewer

• proxy/director.go eviction handler is the substance.
• proxy/metrics/metrics.go for the metric definitions and the pre-initialised reason set.
• Worth a look at whether the span is wanted at all, given it cannot attach to the session trace. It is cheap and searchable by request id, but I would drop it if reviewers prefer metrics and logs only.

For QA

No QA needed; this is observability only and changes no request handling.

Verified locally:

• bazel test //... --flaky_test_attempts=3 — 9 of 9 targets pass
• go build ./..., go vet ./proxy/..., gofmt clean on all changed files
• bazel run //:gazelle for the BUILD file updates

Tests added cover the reason strings, since dashboards and alerts key on them, plus a guard that the set emitted by mapEvictionReason and the pre-initialised label set stay in agreement. A reason that is emitted but not pre-initialised would silently reintroduce the gap problem.

Issues

Closes #596

Checklist

• I am familiar with the Contributing Guidelines.
• I have signed off my commits for Developer Certificate of Origin (DCO) compliance.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.

Summary by CodeRabbit

• New Features

  • Added monitoring for worker tunnel connections, including active, opened, and closed connection counts.
  • Added closure reason tracking and connection duration measurements.
  • Added detailed logs and tracing information when worker connections close.
  • Improved visibility into connection lifecycle events for troubleshooting and operational monitoring.

• Tests

  • Added coverage for connection eviction reasons and metric label consistency.

[coderabbitai]

📝 Walkthrough

Walkthrough

Worker tunnel lifecycle telemetry now covers creation, active connections, closure reasons, duration, logging, and tracing. WorkerConnection records creation time, and tests validate eviction-reason mappings and metric labels.

Changes

Worker tunnel observability

Layer / File(s)	Summary
Observability metrics contract src/invocation-plane-services/grpc-proxy/proxy/metrics/metrics.go	Adds active, opened, closed-by-reason, and duration metrics. Initializes all supported close-reason labels.
Connection lifecycle instrumentation src/invocation-plane-services/grpc-proxy/proxy/worker/worker.go, src/invocation-plane-services/grpc-proxy/proxy/director.go	Records WorkerConnection.CreatedAt. Updates lifecycle metrics during cache insertion and eviction. Adds duration logging and an OpenTelemetry eviction span.
Eviction mapping validation src/invocation-plane-services/grpc-proxy/proxy/director_test.go, src/invocation-plane-services/grpc-proxy/proxy/BUILD.bazel	Tests all eviction mappings and metric-label coverage. Adds the required test dependencies and normalizes test formatting.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Director
  participant WorkerConnectionCache
  participant PrometheusMetrics
  participant OpenTelemetryTracer
  Director->>WorkerConnectionCache: Cache worker connection
  Director->>PrometheusMetrics: Record opened and active connections
  WorkerConnectionCache->>Director: Report eviction reason
  Director->>PrometheusMetrics: Record closure and duration
  Director->>OpenTelemetryTracer: Emit eviction span

Loading

Possibly related PRs

• NVIDIA/nvcf#525: Updates the related grpc-proxy Bazel configuration during the Bazel 9 migration.

Suggested reviewers: sanjay-saxena

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes implement the issue requirements for close-reason metrics, active connections, duration, info logging, and eviction tracing without behavior changes.
Out of Scope Changes check	✅ Passed	The Bazel, production, and test changes directly support worker tunnel lifecycle observability and remain within the linked issue scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title follows Conventional Commits and accurately describes the observability fix for worker tunnel closure reasons.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/grpc-proxy-tunnel-close-observability

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/invocation-plane-services/grpc-proxy/proxy/director.go`:
- Around line 159-166: Update the span name passed to Tracer.Start in the worker
connection cache eviction path to the stable name
grpc-proxy.worker_connection_cache_eviction, while preserving the existing
attributes and tracing flow.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2cc45584-0f6f-458e-b4ae-d0c47b29f21b

📥 Commits

Reviewing files that changed from the base of the PR and between da80672 and 5b129b5.

📒 Files selected for processing (5)

• src/invocation-plane-services/grpc-proxy/proxy/BUILD.bazel
• src/invocation-plane-services/grpc-proxy/proxy/director.go
• src/invocation-plane-services/grpc-proxy/proxy/director_test.go
• src/invocation-plane-services/grpc-proxy/proxy/metrics/metrics.go
• src/invocation-plane-services/grpc-proxy/proxy/worker/worker.go

[balajinvda]

Good catch, this one was a genuine convention violation rather than a nitpick.

AGENTS.md specifies service.operation for span names ("for example nvca.reconcile, ratelimiter.check, grpc-proxy.forward") and asks that they stay stable so dashboards do not break. I had named the span to match the log message for searchability, but log messages and span names follow different conventions and matching them to each other was the mistake.

Renamed to grpc-proxy.worker_connection_cache_eviction in bf61097.

The log message text is deliberately unchanged. That one I do want stable, since promoting it from debug to info was the point of the change and any existing log searches should keep working.