Skip to content

telemetry: log and count samples dropped at buffer capacity - #4144

Open
elitegreg wants to merge 2 commits into
mainfrom
gm/telemetry-drop-samples-metric
Open

telemetry: log and count samples dropped at buffer capacity#4144
elitegreg wants to merge 2 commits into
mainfrom
gm/telemetry-drop-samples-metric

Conversation

@elitegreg

@elitegreg elitegreg commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Resolves: #4126

Summary of Changes

  • The telemetry submitter now logs a warning and increments counters when it discards samples because a submission failed and the partition buffer is already at capacity. That path previously recycled the batch silently, so lost measurement data left no trace beyond submitter_retries_exhausted, which fires whether the batch was requeued or destroyed.
  • New counter doublezero_device_telemetry_agent_samples_dropped_total with a reason label (buffer_full), so the account-full drop path (device/telemetry: account-full path reports success and logs the wrong count #4127) can report on the same series with a different reason.
  • New submitter_buffer_full error type on the existing doublezero_device_telemetry_agent_errors_total counter, one increment per dropped batch.
  • Requeue behavior below capacity is unchanged. Neither signal fires in steady state: a partition holds 4096 samples, roughly 11 hours of backlog at 6 samples per minute.

Diff Breakdown

Category Files Lines (+/-) Net
Tests 1 +89 / -0 +89
Core logic 1 +14 / -5 +9
Scaffolding 1 +14 / -0 +14
Docs 1 +3 / -0 +3
Total 4 +120 / -5 +115

Mostly tests around a nine-line observability fix; no behavior change to submission or requeueing.

Key files (click to expand)

Testing Verification

  • New test fills a partition to exactly its capacity, fails every submission attempt, and asserts the warning is emitted with droppedSamples=2 and capacity=2, that the drop counter rises by the batch length, that the error counter rises once, and that nothing is requeued. Confirmed it fails against the pre-fix submitter.
  • Companion test at capacity 1024 asserts the below-capacity path still requeues the batch, emits no drop warning, and leaves the drop counter untouched.
  • Both counter assertions measure deltas and the subtests run serially, since the metrics are package-level and shared with the existing over-capacity subtests.
  • go test ./controlplane/telemetry/... passes except internal/netns, which fails identically on main in this environment (needs privileges).

@elitegreg
elitegreg marked this pull request as ready for review August 3, 2026 15:23
@elitegreg
elitegreg enabled auto-merge (squash) August 3, 2026 15:23
elitegreg added a commit that referenced this pull request Aug 3, 2026
Resolves: #4128

Independent of #4144 and #4145 (different file), so this one branches
from `main`.

## Summary of Changes

- `ledgerPeerDiscovery.refresh` no longer empties the peer cache before
doing work that can fail. It cleared `p.peers` under the lock and then
called `LocalNet.Interfaces()`, so a transient failure there returned
with zero peers and `Pinger.Tick` iterated an empty slice, probing
nothing until a later refresh succeeded.
- The cache is now replaced only once the new list is built, and the
lock covers just that assignment rather than the whole build. The clear
was redundant with the existing assignment at the end of the happy path.
- Success path is unchanged.

## Diff Breakdown

| Category   | Files | Lines (+/-) | Net  |
|------------|-------|-------------|------|
| Tests      |     1 | +79 / -0    |  +79 |
| Core logic |     1 | +6 / -5     |   +1 |
| Docs       |     1 | +3 / -0     |   +3 |
| **Total**  |     3 | +88 / -5    |  +83 |

A one-line behavioral fix plus the regression test that pins it.

<details>
<summary>Key files (click to expand)</summary>

-
[`controlplane/telemetry/internal/telemetry/peers.go`](https://github.com/malbeclabs/doublezero/pull/4146/files#diff-9c369dff3cb79259b8bc34d8d952b923103baeef1515402614c23a997a06c286)
— drops the `p.peers = make(...)` clear, moves the mutex to wrap only
`p.peers = peers`, and leaves a comment that nothing in the build may
clear the cache

</details>

## Testing Verification

- New test lets the first refresh discover a peer, then fails every
subsequent `LocalNet.Interfaces()` call, and asserts `GetPeers()` still
returns the fully populated peer (link, device, tunnel, TWAMP port)
after at least three failed refreshes. It fails against the pre-fix
code, which returns an empty list.
- Existing peer discovery tests pass unchanged, covering the success
path and the skip cases.
- Package passes under `-race`, since the change moves what the mutex
covers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

device/telemetry: samples silently dropped when submission fails at buffer capacity

1 participant