Distributor: Don't drop time series with invalid exemplars #8224

aknuds1 · 2024-05-30T15:24:54Z

What this PR does

Modify the distributor to not discard time series with invalid exemplars, instead just drop affected exemplars. This is to deal with the fact that OTel sends empty exemplars by default.

Which issue(s) this PR fixes or relates to

Fixes #8210.

Checklist

Tests updated.
Documentation added.
CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
about-versioning.md updated with experimental features.

Don't discard time series with invalid exemplars, just drop affected exemplars. Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

pkg/distributor/distributor.go

CHANGELOG.md

pkg/distributor/distributor.go

pkg/distributor/distributor_test.go

Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com>

…ect-1

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

pkg/distributor/distributor_test.go

colega

LGTM, please consider my nit about the test.

Co-authored-by: Oleg Zaytsev <mail@olegzaytsev.com> Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

aknuds1 · 2024-05-31T10:20:38Z

I tested the fix in our dev environment, and it works.

* Distributor: Don't drop time series with invalid exemplars Don't discard time series with invalid exemplars, just drop affected exemplars. --------- Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com> Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com> Co-authored-by: Oleg Zaytsev <mail@olegzaytsev.com> (cherry picked from commit d319b9f)

grafanabot · 2024-05-31T12:28:44Z

The backport to r291 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-8224-to-r291 origin/r291
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x d319b9ff9d0dbdfed71e98526e2bf235b66e04dd
# Push it to GitHub
git push --set-upstream origin backport-8224-to-r291
git switch main
# Remove the local backport branch
git branch -D backport-8224-to-r291

Then, create a pull request where the base branch is r291 and the compare/head branch is backport-8224-to-r291.

grafanabot · 2024-05-31T12:29:01Z

The backport to r290 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-8224-to-r290 origin/r290
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x d319b9ff9d0dbdfed71e98526e2bf235b66e04dd
# Push it to GitHub
git push --set-upstream origin backport-8224-to-r290
git switch main
# Remove the local backport branch
git branch -D backport-8224-to-r290

Then, create a pull request where the base branch is r290 and the compare/head branch is backport-8224-to-r290.

…8236) * Distributor: Don't drop time series with invalid exemplars Don't discard time series with invalid exemplars, just drop affected exemplars. --------- Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com> Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com> Co-authored-by: Oleg Zaytsev <mail@olegzaytsev.com> (cherry picked from commit d319b9f) Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>

* Distributor: Don't drop time series with invalid exemplars Don't discard time series with invalid exemplars, just drop affected exemplars. --------- Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com> Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com> Co-authored-by: Oleg Zaytsev <mail@olegzaytsev.com> (cherry picked from commit d319b9f)

…8238) * Distributor: Don't drop time series with invalid exemplars Don't discard time series with invalid exemplars, just drop affected exemplars. --------- Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com> Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com> Co-authored-by: Oleg Zaytsev <mail@olegzaytsev.com> (cherry picked from commit d319b9f)

…8239) * Distributor: Don't drop time series with invalid exemplars Don't discard time series with invalid exemplars, just drop affected exemplars. --------- Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com> Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com> Co-authored-by: Oleg Zaytsev <mail@olegzaytsev.com> (cherry picked from commit d319b9f)

) * Distributor: Don't drop time series with invalid exemplars Don't discard time series with invalid exemplars, just drop affected exemplars. --------- Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com> Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com> Co-authored-by: Oleg Zaytsev <mail@olegzaytsev.com>

bwplotka · 2024-07-19T16:42:13Z

pkg/distributor/distributor_test.go

 		"rejects exemplar with too many series labels": {
-			req:    makeWriteRequestExemplar(manyLabels, 0, nil),
-			errMsg: "received a series whose number of labels exceeds the limit",
-			errID:  globalerror.MaxLabelNamesPerSeries,
+			req:            makeWriteRequestExemplar(manyLabels, 0, 1, nil),
+			expectedErrMsg: "received a series whose number of labels exceeds the limit",
+			expectedErrID:  globalerror.MaxLabelNamesPerSeries,
 		},
 		"rejects exemplar with duplicate series labels": {
-			req:    makeWriteRequestExemplar([]string{model.MetricNameLabel, "test", "foo", "bar", "foo", "bar"}, 0, nil),
-			errMsg: "received a series with duplicate label name",
-			errID:  globalerror.SeriesWithDuplicateLabelNames,
+			req:            makeWriteRequestExemplar([]string{model.MetricNameLabel, "test", "foo", "bar", "foo", "bar"}, 0, 1, nil),
+			expectedErrMsg: "received a series with duplicate label name",
+			expectedErrID:  globalerror.SeriesWithDuplicateLabelNames,
 		},
 		"rejects exemplar with empty series label name": {
-			req:    makeWriteRequestExemplar([]string{model.MetricNameLabel, "test", "", "bar"}, 0, nil),
-			errMsg: "received a series with an invalid label",
-			errID:  globalerror.SeriesInvalidLabel,
+			req:            makeWriteRequestExemplar([]string{model.MetricNameLabel, "test", "", "bar"}, 0, 1, nil),
+			expectedErrMsg: "received a series with an invalid label",
+			expectedErrID:  globalerror.SeriesInvalidLabel,


Just curious, why inconsistency with the above? (we are looking on this with @cstyan regarding Prometheus PRW 2.0 handler.

aknuds1 added bug Something isn't working component/distributor labels May 30, 2024

aknuds1 force-pushed the arve/fix-exemplar-reject branch 2 times, most recently from 67088c1 to 0996c39 Compare May 30, 2024 16:51

aknuds1 changed the title ~~WIP: Distributor: Don't drop time series with invalid exemplars~~ Distributor: Don't drop time series with invalid exemplars May 30, 2024

Distributor: Don't drop time series with invalid exemplars

1576a50

Don't discard time series with invalid exemplars, just drop affected exemplars. Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

aknuds1 force-pushed the arve/fix-exemplar-reject branch from 0996c39 to 1576a50 Compare May 30, 2024 16:54

aknuds1 marked this pull request as ready for review May 30, 2024 16:54

aknuds1 requested a review from a team as a code owner May 30, 2024 16:54

ying-jeanne reviewed May 30, 2024

View reviewed changes

pkg/distributor/distributor.go Show resolved Hide resolved

56quarters reviewed May 30, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

pkg/distributor/distributor.go Outdated Show resolved Hide resolved

pkg/distributor/distributor_test.go Outdated Show resolved Hide resolved

pkg/distributor/distributor_test.go Outdated Show resolved Hide resolved

aknuds1 and others added 6 commits May 30, 2024 19:32

Update CHANGELOG.md

6679d5a

Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com>

Update pkg/distributor/distributor.go

f33e229

Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com>

Merge remote-tracking branch 'origin/main' into arve/fix-exemplar-rej…

80e58a5

…ect-1

Apply reviewer feedback

6e407d6

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Fix test

76b417c

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Fix test

3328b82

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

aknuds1 requested review from 56quarters, colega and ying-jeanne May 31, 2024 08:43

aknuds1 force-pushed the arve/fix-exemplar-reject branch from 5b359d6 to 4144fc9 Compare May 31, 2024 08:45

Fully verify exemplars

2c89323

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

aknuds1 force-pushed the arve/fix-exemplar-reject branch from 4144fc9 to 2c89323 Compare May 31, 2024 08:47

colega reviewed May 31, 2024

View reviewed changes

pkg/distributor/distributor_test.go Outdated Show resolved Hide resolved

colega approved these changes May 31, 2024

View reviewed changes

Update pkg/distributor/distributor_test.go

ce8e471

Co-authored-by: Oleg Zaytsev <mail@olegzaytsev.com> Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

aknuds1 force-pushed the arve/fix-exemplar-reject branch from 20b0acd to ce8e471 Compare May 31, 2024 09:25

aknuds1 merged commit d319b9f into main May 31, 2024
29 checks passed

aknuds1 deleted the arve/fix-exemplar-reject branch May 31, 2024 10:33

aknuds1 added backport-r292 backport r292 backport r291 backport r290 labels May 31, 2024

grafanabot mentioned this pull request May 31, 2024

[r292] Distributor: Don't drop time series with invalid exemplars #8236

Merged

grafanabot added the backport-failed label May 31, 2024

aknuds1 removed backport-failed backport-r292 labels May 31, 2024

grafanabot added the backport-failed label May 31, 2024

aknuds1 mentioned this pull request May 31, 2024

Distributor: Don't drop time series with invalid exemplars (#8224) #8238

Merged

4 tasks

aknuds1 mentioned this pull request May 31, 2024

Distributor: Don't drop time series with invalid exemplars (#8224) #8239

Merged

4 tasks

gouthamve mentioned this pull request Jun 26, 2024

Mimir rejects valid exemplars with err-mimir-exemplar-labels-missing #7904

Closed

cstyan mentioned this pull request Jul 19, 2024

[PRW 2.0] Added Sender and RW Handler support for Response Stats. prometheus/prometheus#14444

Merged

bwplotka reviewed Jul 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Distributor: Don't drop time series with invalid exemplars #8224

Distributor: Don't drop time series with invalid exemplars #8224

aknuds1 commented May 30, 2024 •

edited

Loading

colega left a comment

aknuds1 commented May 31, 2024

grafanabot commented May 31, 2024

grafanabot commented May 31, 2024

bwplotka Jul 19, 2024

Distributor: Don't drop time series with invalid exemplars #8224

Distributor: Don't drop time series with invalid exemplars #8224

Conversation

aknuds1 commented May 30, 2024 • edited Loading

What this PR does

Which issue(s) this PR fixes or relates to

Checklist

colega left a comment

Choose a reason for hiding this comment

aknuds1 commented May 31, 2024

grafanabot commented May 31, 2024

grafanabot commented May 31, 2024

bwplotka Jul 19, 2024

Choose a reason for hiding this comment

aknuds1 commented May 30, 2024 •

edited

Loading