Report observed egress hosts via the metrics pipeline

[AbhishekBhaskar]

What are you trying to accomplish?

We want to restrict which outbound hosts the proxy allows a job to reach. Before enforcing anything, we need to observe the hosts each ecosystem actually contacts so we can tune the allowlist, then flip on enforcement safely. Today that observability data only lands in proxy logs, which aren't forwarded to Splunk/Kusto — so we can't analyze real egress patterns.

This wires the egress allowlist handler into the proxy's existing metrics reporting pipeline so every observed host is reported to the backend as an  egress_host  metric. The backend (dependabot-api) logs the raw host to Splunk for allowlist discovery.

The handler has two independent, flag-gated modes (both off by default / fail-open):

• observe — logs and reports non-allowlisted hosts; blocks nothing.
• enforce — returns a 403 for non-allowlisted hosts.

Anything you want to highlight for special attention from reviewers?

• Reuses the existing metrics collector instead of a dedicated pipeline. Per review feedback, the earlier dedicated egress collector +  record_egress_hosts  endpoint have been removed. Observations now flow through the same buffering / flush / retry / job-lifecycle path as other metrics — one pipeline to maintain, not two.
• Recording happens at the allowlist decision point, inside the handler. goproxy stops the request chain on the first blocking response, and the allowlist handler runs before the metrics handler — so a downstream handler would never see enforce-blocked hosts. Emitting the observation from the allowlist handler itself (before the 403) is what lets us capture blocked hosts.
• What's reported: every observed host with an  allowlisted  flag (not just non-allowlisted), giving the full egress picture per ecosystem. Tags: raw  request_host ,  allowlisted ;  package_manager  is added automatically by the collector's default tags.
• Two fixes to the metrics collector that this depends on:
• Tag-aware aggregation — the collector previously merged records by metric name + type only, so series with different tags (e.g. different hosts) were incorrectly folded together. It now also compares tags. This also corrects the existing bucketed metric.
• Bounded buffer — a cap so a job hitting many distinct hosts can't grow the buffer unbounded (new series are dropped once the cap is reached; existing ones keep aggregating).
• Gated by the existing experiment flags ( proxy_egress_observe  /  proxy_egress_enforce ); fail-open mode reports nothing.

How will you know you've accomplished your goal?

• Unit tests: the handler reports observed hosts with the correct  allowlisted  status, reports enforce-blocked hosts, and reports nothing when disabled; the collector keeps distinct-tag series separate and enforces the buffer cap.
• Full suite +  gofmt  /  go vet  pass locally ( go test ./... , incl.  -race  on the affected packages).
• End-to-end: once the API change is live, observed hosts appear in Splunk keyed by  package_manager , letting us list non-allowlisted hosts per ecosystem before enabling enforce.

Checklist

• I have run the complete test suite to ensure all tests and linters pass.
• I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
• I have written clear and descriptive commit messages.
• I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
• I have ensured that the code is well-documented and easy to understand.

[Copilot]

Copilot review overview

🟡 Changes recommended

Shutdown synchronization and failed-batch retention must be addressed to prevent buffered telemetry loss.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite (auto)
Findings: 2

Note

Copilot is running an experiment and ran this review at Lite.

New issues introduced by this change (2)

Severity	Finding
	internal/​egress/​collector.go — Shutdown can exit before the final egress flush View comment
	internal/​egress/​collector.go — Failed reports permanently discard the buffered hosts View comment

What changed in this PR

Adds batched outbound-host reporting from the proxy to the Dependabot API for Splunk/Kusto observability.

Changes:

• Collects and aggregates observed egress hosts.
• Integrates recording with allowlist handling and proxy lifecycle.
• Adds API support and unit tests for reporting.

File	Summary
proxy.go	Wires and stops the egress collector; shutdown does not wait for final flushing.
internal/​metrics/​collector_client_test.go	Updates the API client mock.
internal/​handlers/​egress_allowlist.go	Records observed hosts and allowlist status.
internal/​handlers/​egress_allowlist_test.go	Tests recording behavior.
internal/​egress/​collector.go	Implements batching and reporting; requires shutdown synchronization and failed-batch retention.
internal/​egress/​collector_test.go	Tests aggregation and flushing.
internal/​apiclient/​client.go	Adds the egress reporting endpoint.
internal/​apiclient/​client_test.go	Tests endpoint requests and payload handling.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[jakecoffman]

Keeping hosts separate exposes a bug in the existing size-limit path. flushBuffer clears the records but doesn't reset estimatedBufferSize, so that estimate accumulates across flushes. Eventually SendMetric hits the byte limit and calls flushBuffer while already holding BufferMutex. flushBuffer tries to acquire the same mutex and deadlocks.

I reproduced this with repeated batches of 900 distinct hosts, flushing between batches. Each batch stays below the new series cap, but the request eventually hangs.

Can we reset the byte estimate whenever we drain the buffer, and fix the size-triggered flush so it doesn't acquire a mutex we already hold? Please add coverage for repeated flushes and reaching the byte limit.

[AbhishekBhaskar]

Thanks for identifying this bug! I've fixed this by resetting estimatedBufferSize on every drain (in a new drainLocked helper), so it can't accumulate across flushes. Size-triggered flush no longer re-enters the mutex — SendMetric  drains into a local batch and posts it after releasing BufferMutex , so flushBuffer is never called while we hold the lock. Added test coverage for the same.

[jakecoffman]

This cap applies to all metrics, so egress observations can fill the buffer and cause us to silently drop ordinary request/response metrics.

I reproduced this by recording one HTTP 200 response, then 999 distinct egress hosts, then an HTTP 500 response. The 500 metric is dropped, and SendMetric returns success.

Can we give egress observations a separate quota within this collector, or reserve capacity for the existing metrics? Also, is 1,000 an intentional limit for this use case, or are we just inheriting the previously unused default?

[AbhishekBhaskar]

The distinct-series cap is now applied per metric name rather than globally, so egress cardinality can't drop ordinary request/response metrics. 1000 was just the inherited unused default. I've set it to 500 per name now (comfortably above what either metric emits per flush window).