feat(discovery): Phase 1 telemetry — measure non-uniform relay binding before changing logic by metaphorics · Pull Request #235 · gosuda/portal-tunnel

metaphorics · 2026-04-30T05:44:38Z

Phase 1 of the discovery rationalization

Adds telemetry around the existing MOLSRelayPolicy to measure non-uniform relay binding before changing selection logic. Phase 2 (load-aware weighted composite) is gated on the data this PR collects.

What landed (13 atomic commits)

portal/discovery/trace.go (new) — SelectionTrace + TraceEntry. ClientHash (uint8) for sampled debug-log correlation only; LocalAddress NOT carried (PII surface).
portal/discovery/metrics.go (new) — 8 Prometheus metrics + EmitFromTrace. Cardinality bounded: max 64 unique relay URLs, overflow → relay="other". NO per-client labels.
portal/discovery/mols.go — SelectPriorityWithTrace / SelectMultiHopWithTrace siblings; existing methods delegate. Public API unchanged; new mols_test.go golden byte-equality assertions.
portal/discovery/relayset.go — PriorityRelaysWithTrace / PriorityMultiHopWithTrace; emits to metrics; sampled zerolog debug log per call.
cmd/relay-server/admin.go — /admin/metrics endpoint behind existing auth middleware (path /admin/metrics because serveAdmin is registered under types.PathAdminPrefix).
cmd/portal-tunnel/main.go — optional --metrics-addr <host:port> flag (no-op if unset).
sdk/expose.go — active_tunnels_per_relay gauge instrumentation in accept loop with closeOnce decrementer (exact-once via sync.Once).
cmd/portal-loadtest/main.go (new) — N synthetic clients with unique LocalAddress; per-relay top-pick histogram; chi-square vs uniform N/K; p-value via regularized incomplete gamma (Numerical Recipes §6.2 series + continued-fraction).
Makefile — load-test: target with %: catch-all for make load-test -- <args> passthrough.

Verification (local)

go build ./... exit 0
go vet ./... exit 0
make lint exit 0 (0 issues)
go test -count=1 ./... exit 0 (179 tests pass across 20 packages)

Phase 1 finding — validates user-reported non-uniform binding

make load-test -- -clients 1000 -relays 5 (deterministic; identical on every re-run):

relay                                          picks  expected
https://test-relay-1.example                     125     200.0
https://test-relay-2.example                     126     200.0
https://test-relay-3.example                     124     200.0
https://test-relay-4.example                     500     200.0
https://test-relay-5.example                     125     200.0

chi-square: 562.5100  df: 4  p-value: 0.0000

The MOLS deterministic GF(64) hash is structurally non-uniform on this 5-relay-URL set: relay-4 captures ~50% of synthetic clients' top picks because its hash wins ~32/64 GF(64) rows. This is the empirical baseline that Phase 2's weighted composite (additive load penalty final = mols_position + lambda * load_factor) is designed to address.

Public API

No public API changes. SelectPriority / SelectMultiHop / PriorityRelays / PriorityMultiHop signatures preserved byte-equal. The new *WithTrace siblings are additive; existing 70+ MOLS tests pass byte-identically (verified via parameterized golden byte-equality test in mols_test.go).

New direct dep

github.com/prometheus/client_golang v1.23.2 (only new direct dep, per Phase 1 plan acceptance criterion feat: add remote ID validation in RelayClient RequestConnection #8).

Out of scope (deferred to later phases)

Selector interface introduction (Phase 2; gated on this PR's telemetry).
Weighted composite + EWMA load surface on RelayState (Phase 2).
Multi-hop role separation + /16 diversity (Phase 3).
Reservation voucher (Phase 4).

Atomic-commit gate

git log --oneline origin/main..HEAD shows 13 commits, each with a single concern, no behavior+cleanup mixing. The two trailing fix: and chore: commits address an inadvertent submodule-pointer add by the git add pattern in one of the implementer agents (keyless_tls/ is a separate nested workspace, now .gitignore'd).

… surface) Adds in-memory trace types consumed by upcoming SelectPriorityWithTrace / SelectMultiHopWithTrace siblings on MOLSRelayPolicy and RelaySet, and by the metrics emitter. ClientHash carried for debug-log correlation only; LocalAddress intentionally absent (PII-leak surface). No callers in this commit; pure type addition; existing public API unchanged. Phase 1 (telemetry-only) per the discovery rationalization plan.

…EmitFromTrace)

…e siblings on MOLSRelayPolicy

…HopWithTrace; wire metrics emission and sampled debug log

…niform N/K)

The keyless_tls/ directory is a separate nested git repository (not part of the portal module). It was inadvertently staged as a submodule pointer in b5ce160 because that commit's tooling did a broad add. Removing the gitlink entry; the working-tree directory is unaffected. Untracked-state restored.

Prevents the keyless_tls/ directory (a separate nested git repository for an unrelated project) from being staged by broad git-add operations.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5c37195d84

ℹ️ 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".

chatgpt-codex-connector · 2026-04-30T05:49:17Z

+%:
+	@:


Remove global catch-all rule that masks unknown make targets

The new %: fallback makes every unknown target succeed as a no-op, so typos in local/CI commands silently pass instead of failing fast (e.g., make definitely-not-a-target now exits 0). That can skip intended build/test/vet steps without any signal, which is a reliability regression beyond the load-test passthrough use case this rule was added for.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-30T05:49:17Z

+	if *relays <= 0 {
+		fmt.Fprintln(os.Stderr, "portal-loadtest: -relays must be > 0")
+		os.Exit(1)


Reject relay count below two for chi-square calculation

The CLI currently accepts -relays=1, but the chi-square test then uses df := relays - 1 (so df=0) and calls igamc(0, ...), which immediately returns 1.0 via the s <= 0 guard; this prints a valid-looking p-value for a statistically undefined case. This can mislead users running the probe with small K values, so input validation should require at least 2 relays (or explicitly special-case the output).

Useful? React with 👍 / 👎.

gosunuts · 2026-04-30T08:30:45Z

+	"sync"
+
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promauto"


Prometheus metrics should not live in discovery; this introduces import-time global registration and makes discovery own telemetry transport details. Consider a separate metrics package.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 57040792a3

ℹ️ 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".

chatgpt-codex-connector · 2026-04-30T17:00:50Z

+		discovery.ActiveTunnelsPerRelay.WithLabelValues(relayURL).Inc()
+		wrappedConn := &tunnelCounterConn{
+			Conn: conn,
+			decr: func() {
+				discovery.ActiveTunnelsPerRelay.WithLabelValues(relayURL).Dec()


Bound active-tunnel relay labels before updating gauge

Using relayURL directly in ActiveTunnelsPerRelay.WithLabelValues(...) creates a new Prometheus time series for every unique relay ever seen, and those series persist even after the gauge returns to 0. With discovery enabled (or relay URL churn/malicious announce data), this can grow metric cardinality and process memory without bound; it also bypasses the 64-label cap implemented via boundedRelay in portal/discovery/metrics.go for other relay-labeled metrics.

Useful? React with 👍 / 👎.

gg582 · 2026-05-01T09:28:41Z

Proposal: Priority Transposition Logic based on EWMA RTT Analysis

To address the structural non-uniformity confirmed in Phase 1, I propose implementing a dynamic priority transposition mechanism as part of the Phase 2 weighted composite selector.

1. Rationale

While the MOLS algorithm provides a deterministic baseline, mathematical uniformity does not account for real-world network degradation. By monitoring RTT "stretching," we can bridge the gap between theoretical priority and actual user experience.

2. Implementation Details

EWMA-based Latency Tracking: Maintain a smoothed RTT for each relay node using an Exponential Moving Average (EWMA).
- This filters out transient network jitter while identifying nodes suffering from persistent congestion or "stretching."
Priority Transposition:
- Logic: If a high-priority node (according to MOLS) exhibits an EWMA RTT that consistently exceeds a performance threshold, its position should be swapped (transposed) with a lower-priority node that shows superior stability and speed.
- Trigger: This swap ensures that high-quality nodes are exhausted before failing over to congested paths, regardless of their initial mathematical index.
Hysteresis and Stability: To prevent frequent flapping of relay selections, the transposition should only occur when the RTT difference between candidate nodes exceeds a specific delta ($\Delta$).

3. Expected Benefits

Direct Quality Impact: Dynamically bypasses congested network paths that a purely static or deterministic model would otherwise favor.
Hybrid Resilience: Enhances the "Fairly Small" design philosophy by adding a lightweight, reactive feedback loop to the existing MOLS framework.
Measurable Improvement: We can validate this by tracking the reduction in tail latency (p99) alongside the existing chi-square uniformity metrics.

Recommendation: We should extend the Phase 1 telemetry to include RTT distribution logs. This data will be critical for tuning the EWMA weight ($\alpha$) and transposition thresholds before the Phase 2 rollout.

gg582 · 2026-05-01T09:28:51Z

@metaphorics

gg582 · 2026-05-01T13:01:21Z

LGTM

Since MOLS is a mere algorithm, it does not help without additional layer.

Let's dive into the "Phase 2"...and I guess that load-awareness should not rely on some constants by approximation.
@gosunuts

gg582 · 2026-05-01T13:02:00Z

Also, please detach telemetry codes from discovery :)

gg582 · 2026-05-02T01:29:19Z

@metaphorics Hi, I've refactored your code, and there are some structure changes...
Now, telemetry is moved to portal/telemetry, as a result this is a separate package rather than a part of discovery.
Please read this change thoroughly and feedback when you are available.

Thanks a lot.
Lee Yunjin(@gg582)

metaphorics added 13 commits April 30, 2026 04:06

build: add prometheus/client_golang as direct dep

8c4c941

feat(discovery): add Phase 1 Prometheus metrics surface (8 metrics + …

404b6cf

…EmitFromTrace)

fix(discovery): strengthen metrics_test presence check + errorlint fix

9befcd9

feat(discovery): add SelectPriorityWithTrace + SelectMultiHopWithTrac…

b5ce160

…e siblings on MOLSRelayPolicy

feat(relay-server): mount /metrics on admin endpoint (auth-gated)

e20e9f9

feat(portal-tunnel): add optional --metrics-addr flag

6eb0144

feat(sdk): track active_tunnels_per_relay gauge in accept loop

ef4b980

feat(discovery): add RelaySet.PriorityRelaysWithTrace + PriorityMulti…

2b5b6fb

…HopWithTrace; wire metrics emission and sampled debug log

feat(loadtest): add portal-loadtest uniformity probe (chi-square vs u…

a3e3646

…niform N/K)

build: add Makefile load-test target

8502e23

chore: gitignore keyless_tls/ (separate nested workspace)

5c37195

Prevents the keyless_tls/ directory (a separate nested git repository for an unrelated project) from being staged by broad git-add operations.

chatgpt-codex-connector Bot reviewed Apr 30, 2026

View reviewed changes

gosunuts requested changes Apr 30, 2026

View reviewed changes

This was referenced Apr 30, 2026

feat(discovery): Phase 2 weighted composite selector + Selector seam #237

Open

feat(discovery): Phase 4 reservation voucher protocol surface (EXPERIMENTAL) #240

Open

Merge branch 'main' into discovery-phase-1-telemetry

5704079

chatgpt-codex-connector Bot reviewed Apr 30, 2026

View reviewed changes

Merge branch 'main' into discovery-phase-1-telemetry

8c40440

gg582 added 2 commits May 2, 2026 10:19

refact: extract telemetry from discovery into new package

020e055

lint: fix lint

24e6b38

Navigation Menu

Uh oh!

feat(discovery): Phase 1 telemetry — measure non-uniform relay binding before changing logic - #235

feat(discovery): Phase 1 telemetry — measure non-uniform relay binding before changing logic#235
metaphorics wants to merge 17 commits into
mainfrom
discovery-phase-1-telemetry

metaphorics commented Apr 30, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Apr 30, 2026

Uh oh!

chatgpt-codex-connector Bot Apr 30, 2026

Uh oh!

gosunuts Apr 30, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Apr 30, 2026

Uh oh!

gg582 commented May 1, 2026

Uh oh!

gg582 commented May 1, 2026

Uh oh!

gg582 commented May 1, 2026

Uh oh!

gg582 commented May 1, 2026

Uh oh!

gg582 commented May 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

metaphorics commented Apr 30, 2026

Phase 1 of the discovery rationalization

What landed (13 atomic commits)

Verification (local)

Phase 1 finding — validates user-reported non-uniform binding

Public API

New direct dep

Out of scope (deferred to later phases)

Atomic-commit gate

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

gosunuts Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

gg582 commented May 1, 2026

Proposal: Priority Transposition Logic based on EWMA RTT Analysis

1. Rationale

2. Implementation Details

3. Expected Benefits

Uh oh!

gg582 commented May 1, 2026

Uh oh!

gg582 commented May 1, 2026

Uh oh!

gg582 commented May 1, 2026

Uh oh!

gg582 commented May 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants