otelriver: Track inserts skipped as duplicate due to UniqueOpts#58
Merged
brandur merged 4 commits intoJun 1, 2026
Merged
Conversation
brandur
reviewed
Jun 1, 2026
| } | ||
|
|
||
| span.SetAttributes(attrs...) // set after finalizing status | ||
| span.SetAttributes(attribute.Int64("duplicate_skipped_count", skipped)) |
Contributor
There was a problem hiding this comment.
Could you modify this and the two instances below to prefix with unique_* like the row field name? I kind of like that unique_* is on there because for someone not familiar with the property already it's strongly suggestive that it's related to unique jobs.
Contributor
Author
There was a problem hiding this comment.
Sure. I was trying to keep it short but already failed at that, so may as well go with fully verbose for clarity. Fix incoming.
Contributor
Author
There was a problem hiding this comment.
Done. I also added a CHANGELOG entry; let me know if you prefer to leave small changes like this out of the changelog and I'll remove it
Contributor
Today every job submitted to InsertMany is counted in river.insert_count
even if River dropped it via UniqueOpts. That makes the insert metric
unable to distinguish "we accepted 100 new jobs" from "we accepted 0 new
jobs because all 100 were dedup'd against in-flight work" - both look
identical on a chart.
This adds a skipped_as_duplicate boolean attribute to river.insert_count
data points and a duplicate_skipped_count span attribute on the
river.insert_many span:
- river.insert_count{skipped_as_duplicate:false} - jobs actually enqueued
- river.insert_count{skipped_as_duplicate:true} - jobs dropped by UniqueOpts
- span.duplicate_skipped_count - how many of the batch were skipped
The sum across both insert_count data points still equals len(manyParams),
so existing dashboards that sum the metric without filtering see no
change. New dashboards / monitors can now compute a duplicate rate
directly from the otel metric rather than needing a separate counter in
each service that uses UniqueOpts.
Tests cover all-duplicates, no-duplicates, and mixed-batch shapes.
Co-authored-by: Cursor <cursoragent@cursor.com>
Match the existing requireSum helper's //nolint:unparam pattern; the name parameter is constant for now but the helper is intended to grow more callers as more partitioned-by-attribute metrics show up. Co-authored-by: Cursor <cursoragent@cursor.com>
34fe2ad to
a0631e4
Compare
brandur
approved these changes
Jun 1, 2026
Merged
brandur
added a commit
that referenced
this pull request
Jun 1, 2026
Contributor
|
Published v0.9.0. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a
skipped_as_duplicateboolean attribute toriver.insert_countdata points and aduplicate_skipped_countspan attribute on theriver.insert_manyspan, so callers can tell how many of the submitted jobs were dropped byUniqueOptsvs. actually enqueued.After this change, each
InsertManycall emits tworiver.insert_countdata points:river.insert_count{skipped_as_duplicate:false}— jobs that were actually enqueuedriver.insert_count{skipped_as_duplicate:true}— jobs dropped becauseUniqueOptsmatched an in-flight or pending duplicateThe sum across both data points still equals
len(manyParams), so dashboards or monitors that sum the metric without filtering see no change. The span also carries the count directly:Why
Today every job submitted to
InsertManyis counted inriver.insert_countregardless of whether River accepted it or dropped it viaUniqueOpts. That makes the insert metric unable to distinguish "we accepted 100 new jobs" from "we accepted 0 new jobs because all 100 were dedup'd against in-flight work" — both look identical on a chart.I've been carrying a custom River middleware that emits a separate
…insert_duplicate_skipped_countcounter just to recover this information. Pushing this intootelriverremoves the need for that workaround and gives everyotelriveruser duplicate-rate visibility out of the box.Tests
Existing
InsertMany,InsertManyError, andInsertManyPanictests updated to assert against the new partitioned data points. Two new tests cover the new behaviour:InsertManyAllDuplicates— 2 submitted, both flaggedUniqueSkippedAsDuplicate: trueInsertManyMixedDuplicates— 5 submitted, 2 dedup'd / 3 enqueued; also asserts the grand-total invariantA small test helper
requireSumByAttrswas added (alongside the existingrequireSum) for asserting against a specific subset of data points by attributes, since the partitioned metric now emits multiple data points per measurement.make testandmake lintboth pass; tests also pass under-race.Notes for reviewers
Made with Cursor on behalf of @echatman.