Skip to content

tools: add duplicate-samples-generator - #16465

Open
dmatth1 wants to merge 2 commits into
mainfrom
dan/duplicate-samples-generator
Open

tools: add duplicate-samples-generator#16465
dmatth1 wants to merge 2 commits into
mainfrom
dan/duplicate-samples-generator

Conversation

@dmatth1

@dmatth1 dmatth1 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Adds tool to remote-write requests containing duplicate-timestamp samples and assert how Mimir accounts for them.

What it asserts

3 things per flow:

  1. The expected reason on cortex_discarded_samples_total gains exactly one count per duplicate.
  2. No other reason moved.
  3. cortex_ingester_ingested_samples_total grows by the non-duplicate samples only.

Validation

Run against a local Mimir, both before and after the ingester fixes in #16347 and grafana/mimir-prometheus#1259.

Before the fix - 14 of 24 flows fail (the ones expected):

FAIL: 14 of 24 flows did not match expectations
same-request/same-value/float            same request / in order / same value
    sent                         10 requests / 1000 samples (500 duplicates)
    same-value-for-timestamp     want      500  observed        0  MISMATCH
    ingested samples             want      500  observed     1000  MISMATCH

Nothing recorded as discarded and 1000 counted as ingested where only 500 were stored.

After the fix — all 24 pass:

24 flows across float and histogram
   8 to the distributor at http://localhost:8080
  16 to the ingester at localhost:9095 over gRPC

PASS: all 24 flows matched expectations

The same flow now reads want 500 / observed 500 on both assertions.

Also run against a dev cell through the gateway at replication factor 3, scraping the distributor and all three ingesters: all 24 flows passed, and the stored values were confirmed to be the first-seen ones.

Adds a tool that sends remote-write requests containing duplicate-timestamp
samples and asserts how Mimir accounts for them, covering the flows enumerated
in the Dropped Samples design doc.

Each flow asserts that the expected reason on cortex_discarded_samples_total
gains exactly one count per duplicate, that no other reason moved, and that
cortex_ingester_ingested_samples_total grows by the non-duplicate samples only.

Duplicates are produced in six shapes, with and without a value conflict, from
float samples or native histograms. Shapes that keep the duplicate inside one
TimeSeries object are collapsed by the distributor and are sent to -address;
the rest have to reach the ingester and are sent to -ingester-address over its
Push RPC, since a distributor would otherwise collapse them first.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dmatth1
dmatth1 requested a review from a team as a code owner August 25, 2026 00:34
@dmatth1 dmatth1 added the changelog-not-needed PRs that don't need a CHANGELOG.md entry label Aug 25, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4e3971f. Configure here.

Comment thread tools/duplicate-samples-generator/main.go Outdated
Comment thread tools/duplicate-samples-generator/main.go
… check

- flag.Parse is blocklisted by faillint; use dskit flagext.ParseFlagsWithoutArguments.
- Attributed discards always read zero: scrape keys the metric by reason, while
  report read the bare name. cortex_discarded_attributed_samples_total also names
  the tenant "tenant" rather than "user", so the tenant filter skipped it.
- -series-per-request 1 is valid for shapes that send the duplicate in its own
  request; only same-request and ooo-same-request need room for a pair.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog-not-needed PRs that don't need a CHANGELOG.md entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant