Repository navigation
Conversation
…lient-closed The reverse proxy (shared by client-proxy and the orchestrator sandbox proxy) had no request-outcome metric — only connection gauges. There was no way to tell, on a dashboard, whether a proxied request failed because the destination could not be resolved (500), the node was unreachable (502), the client went away (499), or the sandbox was not found. During a node scale-down this is the difference between stale routing record and real routing bug, and it could only be recovered by scraping logs. Add a request-outcome counter, recorded at the points where a request's fate is decided: - handler() pre-routing terminal branches: client_closed (ctx ended before routing), sandbox_not_found, too_many_connections, upstream_unreachable (owner acquire failed), routing_failed (unexpected resolve error). - pool ErrorHandler: client_closed (context.Canceled / ECONNRESET / EOF) vs upstream_unreachable (everything else), and routing_failed when the destination is missing from context. - ModifyResponse: ok (the upstream answered). The shared proxy stays telemetry-agnostic: it classifies and calls an OnRequestOutcome(ctx, outcome) callback (mirroring the existing ConnectionLimitConfig callbacks). Each service owns its counter and name: client_proxy.request.outcome and orchestrator.proxy.request.outcome, labeled outcome = ok | routing_failed | upstream_unreachable | client_closed | sandbox_not_found | too_many_connections. Fixes e2b-dev#3757
The request-outcome counter shows that upstream-unreachable dials are
happening but not which node they target. During a scale-down a drained
node's routing record stays live (TTL = sandbox_max_length_in_hours,
deleted only on a clean MarkStopping), so client-proxy keeps resolving
sandboxes to it: resolution succeeds, and the failure surfaces only at dial
time. Without a node label the dashboard shows a 502 spike but not the
offending node.
Attribute each outcome to its destination node:
- add Node to pool.Destination; client-proxy sets it to the resolved
SandboxInfo.OrchestratorID (worker instance, bounded cardinality — the
same identity the hugepage-ceiling metric labels by).
- extend OnRequestOutcome to carry the node; the pool's ErrorHandler /
ModifyResponse read it off the Destination, pre-routing handler branches
pass (recorded as node=unknown).
- client-proxy labels client_proxy.request.outcome by outcome + node; the
orchestrator sandbox proxy leaves Node empty (its node is itself), so its
metric is unchanged.
So client_proxy.request.outcome{outcome="upstream_unreachable",node=...}
isolates a drained/stale node directly.
Fixes e2b-dev#3759
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Fixes #3759. Stacked on #3758 — review/merge that first; this branch builds on it and its diff includes #3758's commit until it lands.
#3758 added
client_proxy.request.outcomelabeledoutcome, which shows thatupstream_unreachableis spiking but not which orchestrator node the unreachable dials target. During a node scale-down a drained node's routing record stays live (TTL =sandbox_max_length_in_hours, deleted only on a cleanMarkStopping), so client-proxy keeps resolving sandboxes to it — resolution succeeds, and the failure surfaces only at dial time. Without a node label the dashboard shows a 502 spike but not the offending node, so on-call still scrapes logs fordial tcp <nodeIP>:5007.Changes
Nodetopool.Destination. client-proxy sets it to the resolvedSandboxInfo.OrchestratorID— the worker instance, bounded cardinality, the same identity the hugepage-ceiling metric labels by asnode.id.OnRequestOutcomecallback (from feat(proxy): request-outcome metric distinguishing routing (500) / upstream (502) / client-closed (499) #3758) to carry the node. The pool'sErrorHandler/ModifyResponsealready hold theDestination, so they passt.Node; the pre-routing branches inhandler()(no destination resolved) pass"", recorded asnode="unknown".client_proxy.request.outcomebyoutcome+node. The orchestrator sandbox proxy leavesNodeempty (its node is itself — not meaningful), soorchestrator.proxy.request.outcomeis unchanged.Result:
client_proxy.request.outcome{outcome="upstream_unreachable", node="<orchestrator-id>"}isolates a drained/stale node directly — the missing half of the node-churn observability story (#3692 split the message, #3758 added the outcome, this adds the node).Tests
outcome_test.go: theokcase now setsDestination.Nodeand asserts it reaches the recorder; existing outcome cases updated for the new callback signature.go test ./pkg/proxy/...+ telemetry desc/unit test pass;go vet+golangci-lintclean on all changed packages; client-proxy and orchestrator build.Refs #3757, #3691.