obs: expose TLS/H2 cutover transport telemetry - #88
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
seonghobae
left a comment
There was a problem hiding this comment.
Exact-current technical review for a15404e7822a4e751b2f363f67a7bebf1a837fec: CI 34604676321, Supply Chain 34604676279, and PgErd bounded-origin capacity 34604676389 are terminal SUCCESS. Current branch still points to this exact head; review-thread enumeration is empty. The coverage RCA is repaired by 2e267403... without telemetry semantic changes, canonical ARCHITECTURE/TRD are code-current, and 5cdc842... removes the H2 traffic/metrics ephemeral-port TOCTOU by holding both listener reservations until spawn without changing the H2/TLS/origin/metric oracles. Effective scope remains rollback/canary transport telemetry plus its executable acceptance and documentation. Product auth/business logic, Keyverse, Wardnet/EgressWeave, certificate lifecycle, generic client-IP/trusted-hop authority, supplier framing roots, deployment identity, actual canary/rollback/cutover, and legacy removal remain outside this increment. This COMMENT is technical evidence only and is not an independent approval.
Refs #51.
Writer-safe child of #87 exact
5868fb61e8f66dc5d79bb15a6e7887156dc7684a. This increment targets rollback/canary observability without adding product labels, deployment identity, client-IP authority, Keyverse identity, or Wardnet/EgressWeave verdicts to the gateway.The aggregate request/error counters could not distinguish a TLS/H2 canary regression from healthy cleartext/H1 traffic. RED-only commit
701faf4390225ae650870dedf64dc921e838e278added a CA-verified real-wire requirement that an actual H2-over-TLS request exposecwl_pingora_gateway_requests_by_transport_total{outcome="ok",protocol="h2",transport="tls"} 1. Its hosted CI was superseded while compile/test was still running, so it is not claimed as a terminal hosted RED receipt.Production commit
f0231178a0eb21c66b3f81b6ed1239cb608ff6c1adds one finite-cardinality CounterVec with onlyoutcome={ok,error},protocol={h1,h2}, andtransport={cleartext,tls}. Protocol comes from the accepted Pingora session (Session::is_http2()); TLS comes from the accepted session digest. The eight possible series are pre-created. Routes, headers, cookies, hostnames, product/customer IDs, client IPs, certificate paths and deployment revisions remain excluded; existing aggregate counters remain backward-compatible.CI
34601751211onf023117...failed only at Rust 1.98.0cargo fmt --check;7834aad1c6b991e57b663590e240489ef06a4446applies exactly the emitted formatter diff. Exactcc9067006296b6dd1fb5f89d2b26fe5ea4e9e77athen passed formatting, compile/test including the real-wire H2/TLS metric assertion, Clippy, rustdoc, load-contract, OCI runtime, Supply Chain and bounded-origin capacity, but CI34602099784failed final complete-owned-production coverage. Artifactcoverage-cc9067006296b6dd1fb5f89d2b26fe5ea4e9e77a(sha256:39bb9923363809341feca489bdecfd95ffec1f0f355e84305b88be13021c13d2) isolated the only sub-100% file tosrc/observability.rs; the two zero-count regions were duplicate-registration failure instrumentation in the transport CounterVec initializer.Ordinary-forward
2e2674033667007872be7bc95261f1eb60f5e0e0factors CounterVec registration through a fail-closed helper and adds the missing duplicate-vector registration regression without changing metric names, labels, pre-created series, classification or failure semantics.87f354e70b4e33847110840a30c0aa1d721ff028records that RCA indocs/doctoring/CUTOVER_OBSERVABILITY_TRACEABILITY.md. A code-current sweep then repaired stale canonicalARCHITECTURE.md/TRD.mddescriptions through86e53139277f1a8055ba4041637a68a6c3da25c8and9e342abf41a95e6711f04858ee3aa39343b66623; dedicated baseline lane #61 remains untouched.A second current-head fixture review found that the older
tests/downstream_tls_h2_wire.rsreleased its ephemeral traffic/metrics reservations immediately after discovering their addresses, leaving a TOCTOU window where unrelated local activity could bind a configured port and create a non-protocol RED.5cdc84284f1eebde932d654c742ca4159ed54216adopts the already-proven H2 fixture pattern: bothTcpListenerreservations remain live until immediately before gateway spawn. Current exacta15404e7822a4e751b2f363f67a7bebf1a837fecrecords this repair in cutover TRACEABILITY. The H2/TLS, cleartext-origin proxy and exact metric oracles are unchanged.Current exact
a15404e...independently passed CI34604676321, Supply Chain34604676279, and PgErd bounded-origin capacity34604676389. CI includes formatting, compile/test, strict Clippy, warnings-denied rustdoc, complete owned-production coverage, load-contract and rootless/read-only OCI runtime. Fresh review-thread enumeration is empty. Exact-current technical COMMENT review5179290159records scope/evidence only and is not independent approval. The PR is Ready for review, open and mergeable. Organization rules still require a ruleset-valid independent approving review plus central required workflows before protected integration; no bypass, protected merge, release, canary, cutover or legacy-removal credit is claimed.