Skip to content

Commit

Permalink
add test for discarded samples
Browse files Browse the repository at this point in the history
  • Loading branch information
@ying-jeanne
ying-jeanne committed Dec 26, 2024
1 parent a191044 commit 2bb1845
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 28 deletions.
8 changes: 4 additions & 4 deletions pkg/costattribution/manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (

func newTestManager() *Manager {
logger := log.NewNopLogger()
limits, _ := testutils.GetMockCostAttributionLimits(0)
limits, _ := testutils.NewMockCostAttributionLimits(0)
reg := prometheus.NewRegistry()
manager, err := NewManager(5*time.Second, time.Second, 10*time.Second, logger, limits, reg)
if err != nil {
Expand Down Expand Up @@ -81,7 +81,7 @@ func TestManager_CreateDeleteTracker(t *testing.T) {

t.Run("Disabling user cost attribution", func(t *testing.T) {
var err error
manager.limits, err = testutils.GetMockCostAttributionLimits(1)
manager.limits, err = testutils.NewMockCostAttributionLimits(1)
assert.NoError(t, err)
assert.NoError(t, manager.purgeInactiveAttributionsUntil(time.Unix(11, 0).Unix()))
assert.Equal(t, 1, len(manager.trackersByUserID))
Expand All @@ -96,7 +96,7 @@ func TestManager_CreateDeleteTracker(t *testing.T) {

t.Run("Updating user cardinality and labels", func(t *testing.T) {
var err error
manager.limits, err = testutils.GetMockCostAttributionLimits(2)
manager.limits, err = testutils.NewMockCostAttributionLimits(2)
assert.NoError(t, err)
assert.NoError(t, manager.purgeInactiveAttributionsUntil(time.Unix(12, 0).Unix()))
assert.Equal(t, 1, len(manager.trackersByUserID))
Expand Down Expand Up @@ -147,7 +147,7 @@ func TestManager_PurgeInactiveAttributionsUntil(t *testing.T) {

t.Run("Purge after inactive timeout", func(t *testing.T) {
// disable cost attribution for user1 to test purging
manager.limits, _ = testutils.GetMockCostAttributionLimits(1)
manager.limits, _ = testutils.NewMockCostAttributionLimits(1)
assert.NoError(t, manager.purgeInactiveAttributionsUntil(time.Unix(5, 0).Unix()))

// User3's tracker should remain since it's active, user1's tracker should be removed
Expand Down
9 changes: 7 additions & 2 deletions pkg/costattribution/testutils/test_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,20 @@ package testutils

import "github.com/grafana/mimir/pkg/util/validation"

func GetMockCostAttributionLimits(idx int) (*validation.Overrides, error) {
func NewMockCostAttributionLimits(idx int, lvs ...string) (*validation.Overrides, error) {
baseLimits := map[string]*validation.Limits{
"user1": {MaxCostAttributionCardinalityPerUser: 5, CostAttributionLabels: []string{"team"}},
"user2": {MaxCostAttributionCardinalityPerUser: 2, CostAttributionLabels: []string{}},
"user3": {MaxCostAttributionCardinalityPerUser: 2, CostAttributionLabels: []string{"department", "service"}},
"user4": {MaxCostAttributionCardinalityPerUser: 5, CostAttributionLabels: []string{"platform"}},
"user5": {MaxCostAttributionCardinalityPerUser: 10, CostAttributionLabels: []string{"a"}},
}

if len(lvs) > 0 {
baseLimits[lvs[0]] = &validation.Limits{
MaxCostAttributionCardinalityPerUser: 10,
CostAttributionLabels: lvs[1:],
}
}
switch idx {
case 1:
baseLimits["user1"].CostAttributionLabels = []string{}
Expand Down
7 changes: 6 additions & 1 deletion pkg/costattribution/tracker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,12 @@ func (t *Tracker) updateObservations(key []byte, ts int64, activeSeriesIncrement
}
if discardedSampleIncrement > 0 && reason != nil {
o.discardedSampleMtx.Lock()
o.discardedSample[*reason] = *atomic.NewFloat64(discardedSampleIncrement)
if v, ok := o.discardedSample[*reason]; ok {
v.Add(discardedSampleIncrement)
o.discardedSample[*reason] = v
} else {
o.discardedSample[*reason] = *atomic.NewFloat64(discardedSampleIncrement)
}
o.discardedSampleMtx.Unlock()
}
} else if len(t.observed) < t.maxCardinality*2 && createIfDoesNotExist {
Expand Down
64 changes: 45 additions & 19 deletions pkg/distributor/validate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"time"
"unicode/utf8"

"github.com/go-kit/log"
"github.com/gogo/protobuf/proto"
"github.com/grafana/dskit/grpcutil"
"github.com/grafana/dskit/httpgrpc"
Expand All @@ -25,6 +26,8 @@ import (
grpcstatus "google.golang.org/grpc/status"
golangproto "google.golang.org/protobuf/proto"

"github.com/grafana/mimir/pkg/costattribution"
catestutils "github.com/grafana/mimir/pkg/costattribution/testutils"
"github.com/grafana/mimir/pkg/mimirpb"
"github.com/grafana/mimir/pkg/util/validation"
)
Expand Down Expand Up @@ -75,8 +78,13 @@ func TestValidateLabels(t *testing.T) {

cfg.maxLabelValueLength = 25
cfg.maxLabelNameLength = 25
cfg.maxLabelNamesPerSeries = 2
cfg.maxLabelNamesPerInfoSeries = 3
cfg.maxLabelNamesPerSeries = 3
cfg.maxLabelNamesPerInfoSeries = 4
limits, _ := catestutils.NewMockCostAttributionLimits(0, userID, "team")
careg := prometheus.NewRegistry()
manager, err := costattribution.NewManager(5*time.Second, time.Second, 10*time.Second, log.NewNopLogger(), limits, careg)
require.NoError(t, err)
cat := manager.Tracker(userID)

for _, c := range []struct {
metric model.Metric
Expand All @@ -85,25 +93,25 @@ func TestValidateLabels(t *testing.T) {
err error
}{
{
metric: map[model.LabelName]model.LabelValue{},
metric: map[model.LabelName]model.LabelValue{"team": "a"},
skipLabelNameValidation: false,
skipLabelCountValidation: false,
err: errors.New(noMetricNameMsgFormat),
},
{
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: " "},
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: " ", "team": "a"},
skipLabelNameValidation: false,
skipLabelCountValidation: false,
err: fmt.Errorf(invalidMetricNameMsgFormat, " "),
},
{
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "metric_name_with_\xb0_invalid_utf8_\xb0"},
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "metric_name_with_\xb0_invalid_utf8_\xb0", "team": "a"},
skipLabelNameValidation: false,
skipLabelCountValidation: false,
err: fmt.Errorf(invalidMetricNameMsgFormat, "metric_name_with__invalid_utf8_ (non-ascii characters removed)"),
},
{
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "valid", "foo ": "bar"},
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "valid", "foo ": "bar", "team": "a"},
skipLabelNameValidation: false,
skipLabelCountValidation: false,
err: fmt.Errorf(
Expand All @@ -113,33 +121,35 @@ func TestValidateLabels(t *testing.T) {
[]mimirpb.LabelAdapter{
{Name: model.MetricNameLabel, Value: "valid"},
{Name: "foo ", Value: "bar"},
{Name: "team", Value: "a"},
},
),
),
},
{
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "valid"},
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "valid", "team": "c"},
skipLabelNameValidation: false,
skipLabelCountValidation: false,
err: nil,
},
{
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "badLabelName", "this_is_a_really_really_long_name_that_should_cause_an_error": "test_value_please_ignore"},
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "badLabelName", "this_is_a_really_really_long_name_that_should_cause_an_error": "test_value_please_ignore", "team": "biz"},
skipLabelNameValidation: false,
skipLabelCountValidation: false,
err: fmt.Errorf(
labelNameTooLongMsgFormat,
"this_is_a_really_really_long_name_that_should_cause_an_error",
mimirpb.FromLabelAdaptersToString(
[]mimirpb.LabelAdapter{
{Name: "team", Value: "biz"},
{Name: model.MetricNameLabel, Value: "badLabelName"},
{Name: "this_is_a_really_really_long_name_that_should_cause_an_error", Value: "test_value_please_ignore"},
},
),
),
},
{
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "badLabelValue", "much_shorter_name": "test_value_please_ignore_no_really_nothing_to_see_here"},
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "badLabelValue", "much_shorter_name": "test_value_please_ignore_no_really_nothing_to_see_here", "team": "biz"},
skipLabelNameValidation: false,
skipLabelCountValidation: false,
err: fmt.Errorf(
Expand All @@ -150,12 +160,13 @@ func TestValidateLabels(t *testing.T) {
[]mimirpb.LabelAdapter{
{Name: model.MetricNameLabel, Value: "badLabelValue"},
{Name: "much_shorter_name", Value: "test_value_please_ignore_no_really_nothing_to_see_here"},
{Name: "team", Value: "biz"},
},
),
),
},
{
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "foo", "bar": "baz", "blip": "blop"},
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "foo", "bar": "baz", "blip": "blop", "team": "plof"},
skipLabelNameValidation: false,
skipLabelCountValidation: false,
err: fmt.Errorf(
Expand All @@ -165,21 +176,22 @@ func TestValidateLabels(t *testing.T) {
{Name: model.MetricNameLabel, Value: "foo"},
{Name: "bar", Value: "baz"},
{Name: "blip", Value: "blop"},
{Name: "team", Value: "plof"},
},
2,
3,
)...,
),
},
{
// *_info metrics have higher label limits.
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "foo_info", "bar": "baz", "blip": "blop"},
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "foo_info", "bar": "baz", "blip": "blop", "team": "a"},
skipLabelNameValidation: false,
skipLabelCountValidation: false,
err: nil,
},
{
// *_info metrics have higher label limits.
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "foo_info", "bar": "baz", "blip": "blop", "blap": "blup"},
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "foo_info", "bar": "baz", "blip": "blop", "blap": "blup", "team": "a"},
skipLabelNameValidation: false,
skipLabelCountValidation: false,
err: fmt.Errorf(
Expand All @@ -190,31 +202,32 @@ func TestValidateLabels(t *testing.T) {
{Name: "bar", Value: "baz"},
{Name: "blip", Value: "blop"},
{Name: "blap", Value: "blup"},
{Name: "team", Value: "a"},
},
3,
4,
)...,
),
},
{
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "foo", "bar": "baz", "blip": "blop"},
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "foo", "bar": "baz", "blip": "blop", "team": "a"},
skipLabelNameValidation: false,
skipLabelCountValidation: true,
err: nil,
},
{
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "foo", "invalid%label&name": "bar"},
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "foo", "invalid%label&name": "bar", "team": "biz"},
skipLabelNameValidation: true,
skipLabelCountValidation: false,
err: nil,
},
{
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "foo", "label1": "你好"},
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "foo", "label1": "你好", "team": "plof"},
skipLabelNameValidation: false,
skipLabelCountValidation: false,
err: nil,
},
{
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "foo", "label1": "abc\xfe\xfddef"},
metric: map[model.LabelName]model.LabelValue{model.MetricNameLabel: "foo", "label1": "abc\xfe\xfddef", "team": "plof"},
skipLabelNameValidation: false,
skipLabelCountValidation: false,
err: fmt.Errorf(
Expand All @@ -229,7 +242,7 @@ func TestValidateLabels(t *testing.T) {
err: nil,
},
} {
err := validateLabels(s, cfg, userID, "custom label", mimirpb.FromMetricsToLabelAdapters(c.metric), c.skipLabelNameValidation, c.skipLabelCountValidation, nil, ts)
err := validateLabels(s, cfg, userID, "custom label", mimirpb.FromMetricsToLabelAdapters(c.metric), c.skipLabelNameValidation, c.skipLabelCountValidation, cat, ts)
assert.Equal(t, c.err, err, "wrong error")
}

Expand All @@ -250,6 +263,19 @@ func TestValidateLabels(t *testing.T) {
cortex_discarded_samples_total{group="custom label",reason="random reason",user="different user"} 1
`), "cortex_discarded_samples_total"))

require.NoError(t, testutil.GatherAndCompare(careg, strings.NewReader(`
# HELP cortex_discarded_attributed_samples_total The total number of samples that were discarded per attribution.
# TYPE cortex_discarded_attributed_samples_total counter
cortex_discarded_attributed_samples_total{reason="label_invalid",team="a",tenant="testUser",tracker="cost-attribution"} 1
cortex_discarded_attributed_samples_total{reason="label_name_too_long",team="biz",tenant="testUser",tracker="cost-attribution"} 1
cortex_discarded_attributed_samples_total{reason="label_value_invalid",team="plof",tenant="testUser",tracker="cost-attribution"} 1
cortex_discarded_attributed_samples_total{reason="label_value_too_long",team="biz",tenant="testUser",tracker="cost-attribution"} 1
cortex_discarded_attributed_samples_total{reason="max_label_names_per_info_series",team="a",tenant="testUser",tracker="cost-attribution"} 1
cortex_discarded_attributed_samples_total{reason="max_label_names_per_series",team="plof",tenant="testUser",tracker="cost-attribution"} 1
cortex_discarded_attributed_samples_total{reason="metric_name_invalid",team="a",tenant="testUser",tracker="cost-attribution"} 2
cortex_discarded_attributed_samples_total{reason="missing_metric_name",team="a",tenant="testUser",tracker="cost-attribution"} 1
`), "cortex_discarded_attributed_samples_total"))

s.deleteUserMetrics(userID)

require.NoError(t, testutil.GatherAndCompare(reg, strings.NewReader(`
Expand Down
4 changes: 2 additions & 2 deletions pkg/ingester/activeseries/active_series_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ type mockIndex struct {
existingLabels map[storage.SeriesRef]labels.Labels
}

func (m *mockIndex) Series(ref storage.SeriesRef, builder *labels.ScratchBuilder, chks *[]chunks.Meta) error {
func (m *mockIndex) Series(ref storage.SeriesRef, builder *labels.ScratchBuilder, _ *[]chunks.Meta) error {
if ls, ok := m.existingLabels[ref]; ok {
builder.Assign(ls)
return nil
Expand All @@ -250,7 +250,7 @@ func (m *mockIndex) Series(ref storage.SeriesRef, builder *labels.ScratchBuilder
}

func TestActiveSeries_UpdateSeries_WithCostAttribution(t *testing.T) {
limits, _ := catestutils.GetMockCostAttributionLimits(0)
limits, _ := catestutils.NewMockCostAttributionLimits(0)
reg := prometheus.NewRegistry()
manager, err := costattribution.NewManager(5*time.Second, time.Second, 10*time.Second, log.NewNopLogger(), limits, reg)
require.NoError(t, err)
Expand Down

0 comments on commit 2bb1845

Please sign in to comment.