Skip to content

Commit c29bd27

Browse files
committed
Do not check for name label, adding new test
Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>
1 parent 75fbf41 commit c29bd27

File tree

2 files changed

+44
-31
lines changed

2 files changed

+44
-31
lines changed

pkg/distributor/distributor.go

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -638,7 +638,7 @@ func (d *Distributor) Push(ctx context.Context, req *cortexpb.WriteRequest) (*co
638638
if mrc := d.limits.MetricRelabelConfigs(userID); len(mrc) > 0 {
639639
l := relabel.Process(cortexpb.FromLabelAdaptersToLabels(ts.Labels), mrc...)
640640
if len(l) == 0 {
641-
// all labels are gone, therefore the __name__ label is not present, samples will be discarded
641+
// all labels are gone, samples will be discarded
642642
validation.DiscardedSamples.WithLabelValues(
643643
validation.DroppedByRelabelConfiguration,
644644
userID,
@@ -659,7 +659,7 @@ func (d *Distributor) Push(ctx context.Context, req *cortexpb.WriteRequest) (*co
659659
removeLabel(labelName, &ts.Labels)
660660
}
661661

662-
if len(ts.Labels) == 0 || wasNameLabelRemoved(ts.Labels) {
662+
if len(ts.Labels) == 0 {
663663
validation.DiscardedExemplars.WithLabelValues(
664664
validation.DroppedByUserConfigurationOverride,
665665
userID,
@@ -793,19 +793,6 @@ func (d *Distributor) Push(ctx context.Context, req *cortexpb.WriteRequest) (*co
793793
return &cortexpb.WriteResponse{}, firstPartialErr
794794
}
795795

796-
func wasNameLabelRemoved(labels []cortexpb.LabelAdapter) bool {
797-
const nameLabel = "__name__"
798-
799-
for i := 0; i < len(labels); i++ {
800-
pair := labels[i]
801-
if pair.Name == nameLabel {
802-
return false
803-
}
804-
}
805-
806-
return true
807-
}
808-
809796
func sortLabelsIfNeeded(labels []cortexpb.LabelAdapter) {
810797
// no need to run sort.Slice, if labels are already sorted, which is most of the time.
811798
// we can avoid extra memory allocations (mostly interface-related) this way.

pkg/distributor/distributor_test.go

Lines changed: 42 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1102,17 +1102,6 @@ func TestDistributor_Push_LabelRemoval(t *testing.T) {
11021102
{Name: "__name__", Value: "some_metric"},
11031103
},
11041104
},
1105-
// Edge case: remove label __name__ will drop all user metrics
1106-
{
1107-
removeReplica: true,
1108-
removeLabels: []string{"__name__"},
1109-
inputSeries: labels.Labels{
1110-
{Name: "__name__", Value: "some_metric"},
1111-
{Name: "cluster", Value: "one"},
1112-
{Name: "__replica__", Value: "two"},
1113-
},
1114-
expectedSeries: labels.Labels{},
1115-
},
11161105
// Remove multiple labels and replica.
11171106
{
11181107
removeReplica: true,
@@ -1165,22 +1154,59 @@ func TestDistributor_Push_LabelRemoval(t *testing.T) {
11651154
_, err = ds[0].Push(ctx, req)
11661155
require.NoError(t, err)
11671156

1168-
expectedTimeseriesLen := 0
1169-
if len(tc.expectedSeries) > 0 {
1170-
expectedTimeseriesLen = 1
1171-
}
11721157
// Since each test pushes only 1 series, we do expect the ingester
11731158
// to have received exactly 1 series
11741159
for i := range ingesters {
11751160
timeseries := ingesters[i].series()
1176-
assert.Equal(t, expectedTimeseriesLen, len(timeseries))
1161+
assert.Equal(t, 1, len(timeseries))
11771162
for _, v := range timeseries {
11781163
assert.Equal(t, tc.expectedSeries, cortexpb.FromLabelAdaptersToLabels(v.Labels))
11791164
}
11801165
}
11811166
}
11821167
}
11831168

1169+
func TestDistributor_Push_LabelRemoval_RemovingNameLabelWillError(t *testing.T) {
1170+
ctx := user.InjectOrgID(context.Background(), "user")
1171+
type testcase struct {
1172+
inputSeries labels.Labels
1173+
expectedSeries labels.Labels
1174+
removeReplica bool
1175+
removeLabels []string
1176+
}
1177+
1178+
tc := testcase{
1179+
removeReplica: true,
1180+
removeLabels: []string{"__name__"},
1181+
inputSeries: labels.Labels{
1182+
{Name: "__name__", Value: "some_metric"},
1183+
{Name: "cluster", Value: "one"},
1184+
{Name: "__replica__", Value: "two"},
1185+
},
1186+
expectedSeries: labels.Labels{},
1187+
}
1188+
1189+
var err error
1190+
var limits validation.Limits
1191+
flagext.DefaultValues(&limits)
1192+
limits.DropLabels = tc.removeLabels
1193+
limits.AcceptHASamples = tc.removeReplica
1194+
1195+
ds, _, _ := prepare(t, prepConfig{
1196+
numIngesters: 2,
1197+
happyIngesters: 2,
1198+
numDistributors: 1,
1199+
shardByAllLabels: true,
1200+
limits: &limits,
1201+
})
1202+
1203+
// Push the series to the distributor
1204+
req := mockWriteRequest([]labels.Labels{tc.inputSeries}, 1, 1)
1205+
_, err = ds[0].Push(ctx, req)
1206+
require.Error(t, err)
1207+
assert.Equal(t, "rpc error: code = Code(400) desc = sample missing metric name", err.Error())
1208+
}
1209+
11841210
func TestDistributor_Push_ShouldGuaranteeShardingTokenConsistencyOverTheTime(t *testing.T) {
11851211
ctx := user.InjectOrgID(context.Background(), "user")
11861212
tests := map[string]struct {

0 commit comments

Comments
 (0)