Adds new metric for dropped samples in ingester #4503

pedro-stanaka · 2021-09-30T21:22:16Z

What this PR does: As described in #3954, this PR adds a new metric distributor_discarded_samples_total to represent
the number of dropped samples due to relabeling configuration.

Which issue(s) this PR fixes:
Fixes #3954

Checklist

Tests updated
Documentation added (N/A ?)
CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

pkg/distributor/distributor.go

bboreham

Thanks for the PR; I think there are a number of unnecessary changes, or if they are necessary they should go in a different PR.

A good cross-check is to make sure all changes are explained in the commit message.

pkg/distributor/distributor.go

pkg/distributor/distributor_test.go

pkg/util/validation/validate.go

pedro-stanaka · 2021-10-12T06:15:28Z

@bboreham @alvinlin123 anything else I should be doing here? Can I have further feedback on the PR?

pedro-stanaka · 2021-11-03T07:49:53Z

Lint output:

test output:

bboreham

Sorry I let this languish for a while.
Insisting on __name__ is optional -

cortex/pkg/util/validation/limits.go

Line 137 in 4e9fc3a

    
           f.BoolVar(&l.EnforceMetricName, "validation.enforce-metric-name", true, "Enforce every sample has a metric name.")

so please don't add more checks for __name__. Just check whether any labels remain.

bboreham · 2021-12-01T19:11:31Z

pkg/distributor/distributor.go

@@ -637,6 +637,14 @@ func (d *Distributor) Push(ctx context.Context, req *cortexpb.WriteRequest) (*co

 		if mrc := d.limits.MetricRelabelConfigs(userID); len(mrc) > 0 {
 			l := relabel.Process(cortexpb.FromLabelAdaptersToLabels(ts.Labels), mrc...)
+			if len(l) == 0 {
+				// all labels are gone, therefore the __name__ label is not present, samples will be discarded


remove mention of __name__

bboreham · 2021-12-01T19:11:49Z

pkg/distributor/distributor.go

@@ -651,7 +659,12 @@ func (d *Distributor) Push(ctx context.Context, req *cortexpb.WriteRequest) (*co
 			removeLabel(labelName, &ts.Labels)
 		}

-		if len(ts.Labels) == 0 {
+		if len(ts.Labels) == 0 || wasNameLabelRemoved(ts.Labels) {


Don't look at __name__.

Sure thing, done.

pedro-stanaka · 2021-12-03T08:51:22Z

I changed as suggested, I think this is finally good to go. Please let me know if I should do anything else. On my machine, linting and tests are passing.

…ow of distributor Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

…rom other test runs Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

pedro-stanaka · 2021-12-14T07:31:45Z

@bboreham sorry for the endless ping, but I just want to make sure you get notifications. I implemented your last comments a while ago, checking back to see if this is good to go.

bboreham

LGTM, thanks!

pedro-stanaka · 2022-01-03T06:41:38Z

@alvinlin123 can you please recheck as well so we can merge this? I am going to do yet another rebase.

* Adding test case for dropping metrics by name to understand better flow of distributor Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com> * Adding test case and new metric for dropped samples Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com> * Updating CHANGELOG with new changes Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com> * Fixing linting problem on distributor file Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com> * Reusing discarded samples metric from validate package Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com> * Compare labelset with len() instead of comparing to nil Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com> * Undoing unnecessary changes on tests and distributor Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com> * Small rename on comment Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com> * Fixing linting offenses Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com> * Reseting validation dropped samples metric to avoid getting metrics from other test runs Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com> * Resolving problems after rebase conflicts Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com> * Registering counter for dropped metrics in test Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com> * Checking if user label drop configuration did not drop __name__ label Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com> * Do not check for name label, adding new test Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com> Signed-off-by: Alvin Lin <alvinlin@amazon.com>

pull-request-size bot added the size/L label Sep 30, 2021

pedro-stanaka mentioned this pull request Sep 30, 2021

Add a metric to track samples dropped in distributor by metric_relabel #3954

Closed

alvinlin123 reviewed Oct 1, 2021

View reviewed changes

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

pedro-stanaka force-pushed the feature/metric-relabel-new-metric-droped-samples branch from 6f29446 to cba3971 Compare October 1, 2021 07:03

bboreham reviewed Oct 1, 2021

View reviewed changes

pedro-stanaka force-pushed the feature/metric-relabel-new-metric-droped-samples branch from 07b6257 to 740296a Compare October 1, 2021 20:45

pedro-stanaka force-pushed the feature/metric-relabel-new-metric-droped-samples branch 2 times, most recently from c45ca83 to 9a28d7c Compare October 14, 2021 10:24

pedro-stanaka force-pushed the feature/metric-relabel-new-metric-droped-samples branch 2 times, most recently from 3c1a0b8 to 847d6c6 Compare November 3, 2021 06:44

pedro-stanaka force-pushed the feature/metric-relabel-new-metric-droped-samples branch from 847d6c6 to 459dce2 Compare December 1, 2021 08:20

pedro-stanaka requested review from bboreham and alvinlin123 December 1, 2021 09:02

bboreham reviewed Dec 1, 2021

View reviewed changes

pedro-stanaka added 13 commits December 14, 2021 08:30

Adding test case for dropping metrics by name to understand better fl…

62e3990

…ow of distributor Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

Adding test case and new metric for dropped samples

f70552a

Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

Updating CHANGELOG with new changes

550d5b6

Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

Fixing linting problem on distributor file

a6c5884

Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

Reusing discarded samples metric from validate package

65c70d1

Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

Compare labelset with len() instead of comparing to nil

10efbbb

Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

Undoing unnecessary changes on tests and distributor

3d430b1

Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

Small rename on comment

3767e36

Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

Fixing linting offenses

4725dc7

Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

Reseting validation dropped samples metric to avoid getting metrics f…

c1f9f80

…rom other test runs Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

Resolving problems after rebase conflicts

7112549

Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

Registering counter for dropped metrics in test

76b68c1

Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

Checking if user label drop configuration did not drop __name__ label

75fbf41

Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

Do not check for name label, adding new test

c29bd27

Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>

pedro-stanaka force-pushed the feature/metric-relabel-new-metric-droped-samples branch from 50d2281 to c29bd27 Compare December 14, 2021 07:30

bboreham approved these changes Dec 16, 2021

View reviewed changes

alvinlin123 approved these changes Jan 3, 2022

View reviewed changes

alvinlin123 merged commit dd698bc into cortexproject:master Jan 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds new metric for dropped samples in ingester #4503

Adds new metric for dropped samples in ingester #4503

pedro-stanaka commented Sep 30, 2021 •

edited

Loading

bboreham left a comment

pedro-stanaka commented Oct 12, 2021

pedro-stanaka commented Nov 3, 2021

bboreham left a comment

bboreham Dec 1, 2021

pedro-stanaka Dec 3, 2021

bboreham Dec 1, 2021

pedro-stanaka Dec 3, 2021

pedro-stanaka commented Dec 3, 2021

pedro-stanaka commented Dec 14, 2021

bboreham left a comment

pedro-stanaka commented Jan 3, 2022

Adds new metric for dropped samples in ingester #4503

Adds new metric for dropped samples in ingester #4503

Conversation

pedro-stanaka commented Sep 30, 2021 • edited Loading

bboreham left a comment

Choose a reason for hiding this comment

pedro-stanaka commented Oct 12, 2021

pedro-stanaka commented Nov 3, 2021

bboreham left a comment

Choose a reason for hiding this comment

bboreham Dec 1, 2021

Choose a reason for hiding this comment

pedro-stanaka Dec 3, 2021

Choose a reason for hiding this comment

bboreham Dec 1, 2021

Choose a reason for hiding this comment

pedro-stanaka Dec 3, 2021

Choose a reason for hiding this comment

pedro-stanaka commented Dec 3, 2021

pedro-stanaka commented Dec 14, 2021

bboreham left a comment

Choose a reason for hiding this comment

pedro-stanaka commented Jan 3, 2022

pedro-stanaka commented Sep 30, 2021 •

edited

Loading