Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Ivan <2103732+codebien@users.noreply.github.com>
  • Loading branch information
olegbespalov and codebien committed Jul 5, 2023
1 parent e7b2319 commit 5941821
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 10 deletions.
12 changes: 8 additions & 4 deletions output/cloud/expv2/flush.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (f *metricsFlusher) push(msb metricSetBuilder) error {
}

f.discardedLabels[key] = struct{}{}
f.logger.Warnf("Label %s was discarded since it can't be used with the cloud output", key)
f.logger.Warnf("Tag %s has been discarded since it is reserved for Cloud operations.", key)
}

return f.client.push(msb.MetricSet)
Expand Down Expand Up @@ -107,7 +107,7 @@ type metricSetBuilder struct {
seriesIndex map[metrics.TimeSeries]uint

// discardedLabels tracks the labels that have been discarded
// because they are not supported by the remote service.
// since they are reserved for internal usage by the Cloud service.
discardedLabels map[string]struct{}
}

Expand Down Expand Up @@ -157,12 +157,16 @@ func (msb *metricSetBuilder) addTimeBucket(bucket timeBucket) {
}
}

func (msb *metricSetBuilder) recordDiscardedLabels(labels map[string]struct{}) {
func (msb *metricSetBuilder) recordDiscardedLabels(labels []string) {
if len(labels) == 0 {
return
}

if msb.discardedLabels == nil {
msb.discardedLabels = make(map[string]struct{})
}

for key := range labels {
for _, key := range labels {
if _, ok := msb.discardedLabels[key]; ok {
continue
}
Expand Down
4 changes: 2 additions & 2 deletions output/cloud/expv2/flush_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,8 @@ func TestFlushWithReservedLabels(t *testing.T) {

// check that warnings sown only once per label
require.Len(t, loglines, 2)
testutils.LogContains(loglines, logrus.WarnLevel, "Label __name__ was discarded since it can't be used with the cloud output")
testutils.LogContains(loglines, logrus.WarnLevel, "Label test_run_id was discarded since it can't be used with the cloud output")
testutils.LogContains(loglines, logrus.WarnLevel, "Tag __name__ has been discarded since it is reserved for Cloud operations.")
testutils.LogContains(loglines, logrus.WarnLevel, "Tag test_run_id has been discarded since it is reserved for Cloud operations.")

// check that flusher is not sending labels with reserved names
require.Len(t, collected[0].Metrics, 1)
Expand Down
8 changes: 4 additions & 4 deletions output/cloud/expv2/mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import (
)

// TODO: unit test
func mapTimeSeriesLabelsProto(tags *metrics.TagSet) ([]*pbcloud.Label, map[string]struct{}) {
func mapTimeSeriesLabelsProto(tags *metrics.TagSet) ([]*pbcloud.Label, []string) {
labels := make([]*pbcloud.Label, 0, ((*atlas.Node)(tags)).Len())
var discardedLabels map[string]struct{}
var discardedLabels []string

// TODO: move this as a shared func
// https://github.com/grafana/k6/issues/2764
Expand All @@ -31,10 +31,10 @@ func mapTimeSeriesLabelsProto(tags *metrics.TagSet) ([]*pbcloud.Label, map[strin
}

if discardedLabels == nil {
discardedLabels = make(map[string]struct{})
discardedLabels = make([]string, 0, 1)
}

discardedLabels[key] = struct{}{}
discardedLabels = append(discardedLabels, key)
}
return labels, discardedLabels
}
Expand Down

0 comments on commit 5941821

Please sign in to comment.