[KaloyanTanev]

Add libp2p wire observability metrics, instrumented at the shared Send/SendReceive/RegisterHandler choke points, covering all runtime charon protocols (the DKG-ceremony-only dkg/sync protocol registers its own raw stream handler and is out of scope):

• p2p_received_message_size_bytes{protocol, peer} and p2p_sent_message_size_bytes{protocol}: histograms of logical message sizes, with bucket edges at 8MiB/32MiB/128MiB so read-limit questions are exact bucket arithmetic.
• p2p_message_read_errors_total{protocol, peer}: read failures excluding benign relay resets, including messages exceeding a protocol read limit.
• p2p_inflight_requests{protocol, peer}, p2p_concurrent_requests{protocol, peer} and p2p_handler_duration_seconds{protocol}: inbound handling concurrency (the histogram observes queue depth at every arrival, catching bursts between scrapes) and processing time.

This provides the fleet data needed to pick the transport limits tracked in #4679 without touching honest traffic, and to validate the 32MB parsigex read limit proposed in #4637.

category: feature
ticket: #4679

[Copilot]

🟡 Changes recommended

The new inflight gauge update logic in observeHandlerStart can report incorrect values under concurrent handlers due to out-of-order Set calls.

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

Pull request overview

This PR adds Prometheus observability for libp2p “wire-level” behavior at the shared p2p choke points (Send, SendReceive, RegisterHandler) to support selecting safe per-protocol transport limits (per #4679).

Changes:

• Add histograms for logical protobuf message sizes (sent/received) and a counter for message read errors.
• Add inbound handling concurrency/latency instrumentation (in-flight gauge, concurrent-at-arrival histogram, handler duration histogram).
• Add internal tests covering the new metrics and update metrics documentation.

File summaries

File	Description
p2p/sender.go	Observes sent request/message sizes and received response sizes; increments read-error counter on response read failure.
p2p/receive.go	Instruments inbound handler concurrency/duration; increments read-error counter on request read failures; observes received request and sent response sizes.
p2p/metrics.go	Adds the new Prometheus metrics plus helper functions and inflight tracking.
p2p/metrics_internal_test.go	Adds coverage validating size histograms, read-error counter behavior, and inflight/concurrency/duration metrics.
docs/metrics.md	Documents the newly added p2p_* metrics.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

---

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

[Copilot]

🟡 Changes recommended

There are a few correctness/operational issues in the new metrics instrumentation (sent-size observation timing, inflight map growth, and misleading sent-size help/docs) that should be fixed before approval.

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

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

docs/metrics.md:119

• This description says sent messages are not labelled by peer because they are broadcast identically, but sent_message_size_bytes is also recorded for per-peer responses written by handlers. Reword to avoid implying per-peer uniformity and instead note it’s omitted to reduce cardinality.
  p2p/metrics.go:115
• sent_message_size_bytes is also observed for per-peer stream responses (see receive.go calling observeSentMessage on resp), so the current help text is misleading when it claims the lack of a peer label is because messages are broadcast identically to all peers. Consider rewording to reflect the actual reason (cardinality reduction) without implying per-peer uniformity.

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Relay read failures and the DKG sync protocol remain unobserved, leaving the advertised metrics incomplete.

Review details

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

p2p/receive.go:57

• This does not cover every Charon protocol as stated: dkg/sync/server.go:355 registers /charon/dkg/sync/1.0.0/ directly with SetStreamHandler, and its client opens streams directly at dkg/sync/client.go:231. Consequently that protocol contributes none of the new inbound concurrency/read/size metrics or outbound size metrics, leaving a gap in the fleet transport data. Instrument the sync protocol too, or move the observations to a layer shared by both handler implementations.
  p2p/receive.go:88
• Relay resets are still excluded from this counter because the preceding IsRelayError branch returns first. Those are ReadMsg failures (and the outbound SendReceive path counts them), so p2p_message_read_errors_total currently has inconsistent and incomplete semantics. Increment the counter before returning from that branch as well.
  p2p/metrics_internal_test.go:138
• This test sends only one request, so it never exercises the concurrent increment/decrement ordering or verifies that overlapping arrivals produce depths 1, 2, etc.—the central behavior of these new metrics. Please block the handler, issue multiple simultaneous requests, assert the peak gauge/histogram observations, then release them and assert the gauge returns to zero.

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[codecov]

Codecov Report

❌ Patch coverage is 94.11765% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.47%. Comparing base (7798cdd) to head (f3ae169).

Files with missing lines	Patch %	Lines
p2p/receive.go	77.77%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4680      +/-   ##
==========================================
+ Coverage   65.45%   65.47%   +0.01%     
==========================================
  Files         246      246              
  Lines       30020    30053      +33     
==========================================
+ Hits        19651    19678      +27     
- Misses      10368    10374       +6     
  Partials        1        1              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[KaloyanTanev]

Addressed the suppressed comments in b0dfc5d: relay resets now excluded consistently on both read paths, and added a concurrency test asserting depths 1..3 and gauge reset. dkg/sync is deliberately out of scope (ceremony-only, raw stream handler) — PR body updated to say so.

[Copilot]

🟡 Changes recommended

Direct-stream resets are incorrectly excluded from the read-error metrics.

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

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

p2p/receive.go:88

• This skips metrics for resets on direct streams too: IsRelayError matches the transport-agnostic network.ErrReset and ErrResourceScopeClosed without verifying that the connection is relayed (p2p/ping.go:131-133). As a result, genuine direct-peer read failures never reach either new increment. Restrict the early return to relayed connections.

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Structurally invalid but successfully read messages are omitted from both size and read-error telemetry.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

p2p/receive.go:108

• This observation runs only after protonil.Check, so a protobuf that is read and decoded successfully but fails the required-field check is omitted from the size histogram. Since that case is not a read error either, malformed-but-readable traffic becomes invisible to both new metrics. Record the decoded size immediately after the read-error branches, before application-level validation.

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[KaloyanTanev]

Addressed in the latest commit — received size is now observed before protonil.Check, so malformed-but-readable messages appear in the size histogram. Deliberately not counting protonil failures as read errors: required-field mismatches fire between honest peers during rolling upgrades.

[Copilot]

🟢 Approval recommended

The implementation and coverage align with the stated scope; only a minor timeout comment needs correction.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

p2p/metrics.go:142

• This comment says the default receive timeout is about 10s, but defaultRcvTimeout is 5s in p2p/sender.go:28. Please keep the bucket rationale aligned with the actual default.

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud