Skip to content

Commit

Permalink
Sanitize labels in metrics-collector (#1296)
Browse files Browse the repository at this point in the history 
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
  • Loading branch information
@saswatamcode
saswatamcode authored Dec 13, 2023
1 parent 024c5e6 commit d56f8c4
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 22 deletions.
37 changes: 30 additions & 7 deletions collectors/metrics/pkg/metricsclient/metricsclient.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"net/http"
"os"
"path/filepath"
"sort"
"strconv"
"strings"
"time"
Expand All @@ -33,7 +34,6 @@ import (
"github.com/prometheus/prometheus/promql"

"github.com/stolostron/multicluster-observability-operator/collectors/metrics/pkg/logger"
"github.com/stolostron/multicluster-observability-operator/collectors/metrics/pkg/metricfamily"
"github.com/stolostron/multicluster-observability-operator/collectors/metrics/pkg/reader"
)

Expand Down Expand Up @@ -422,20 +422,28 @@ func convertToTimeseries(p *PartitionedMetrics, now time.Time) ([]prompb.TimeSer
for _, m := range f.Metric {
var ts prompb.TimeSeries

labelpairs := []prompb.Label{}
labelpairs := []prompb.Label{{
Name: nameLabelName,
Value: *f.Name,
}}

dedup := make(map[string]struct{})
for _, l := range m.Label {
// Skip empty labels.
if *l.Name == "" || *l.Value == "" {
continue
}
// Check for duplicates
if _, ok := dedup[*l.Name]; ok {
continue
}
labelpairs = append(labelpairs, prompb.Label{
Name: *l.Name,
Value: *l.Value,
})
dedup[*l.Name] = struct{}{}
}

labelpairs = metricfamily.InsertLabelLexicographicallyByName(labelpairs, prompb.Label{
Name: nameLabelName,
Value: *f.Name,
})

s := prompb.Sample{
Timestamp: *m.TimestampMs,
}
Expand All @@ -456,6 +464,8 @@ func convertToTimeseries(p *PartitionedMetrics, now time.Time) ([]prompb.TimeSer
}

ts.Labels = append(ts.Labels, labelpairs...)
sortLabels(ts.Labels)

ts.Samples = append(ts.Samples, s)

timeseries = append(timeseries, ts)
Expand All @@ -465,6 +475,19 @@ func convertToTimeseries(p *PartitionedMetrics, now time.Time) ([]prompb.TimeSer
return timeseries, nil
}

func sortLabels(labels []prompb.Label) {
lset := sortableLabels(labels)
sort.Sort(&lset)
}

// Extension on top of prompb.Label to allow for easier sorting.
// Based on https://github.com/prometheus/prometheus/blob/main/model/labels/labels.go#L44
type sortableLabels []prompb.Label

func (sl *sortableLabels) Len() int { return len(*sl) }
func (sl *sortableLabels) Swap(i, j int) { (*sl)[i], (*sl)[j] = (*sl)[j], (*sl)[i] }
func (sl *sortableLabels) Less(i, j int) bool { return (*sl)[i].Name < (*sl)[j].Name }

// RemoteWrite is used to push the metrics to remote thanos endpoint.
func (c *Client) RemoteWrite(ctx context.Context, req *http.Request,
families []*clientmodel.MetricFamily, interval time.Duration) error {
Expand Down
89 changes: 74 additions & 15 deletions collectors/metrics/pkg/metricsclient/metricsclient_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,19 @@ func Test_convertToTimeseries(t *testing.T) {
fooLabelValue1 := "bar"
fooLabelValue2 := "baz"

emptyLabelName := ""

barMetricName := "bar_metric"
barHelp := "bar help text"
barLabelName := "bar"
barLabelValue1 := "baz"

value42 := 42.0
value50 := 50.0
timestamp := int64(1596948588956) //15615582020000)
timestamp := int64(15615582020000)
now := time.Now()
nowTimestamp := now.UnixNano() / int64(time.Millisecond)
fmt.Println("timestamp: ", timestamp)

fmt.Println("nowTimestamp: ", nowTimestamp)
tests := []struct {
name string
in *PartitionedMetrics
Expand Down Expand Up @@ -86,13 +86,13 @@ func Test_convertToTimeseries(t *testing.T) {
},
want: []prompb.TimeSeries{{
Labels: []prompb.Label{{Name: nameLabelName, Value: fooMetricName}, {Name: fooLabelName, Value: fooLabelValue1}},
Samples: []prompb.Sample{{Value: value42, Timestamp: timestamp}},
Samples: []prompb.Sample{{Value: value42, Timestamp: nowTimestamp}},
}, {
Labels: []prompb.Label{{Name: nameLabelName, Value: fooMetricName}, {Name: fooLabelName, Value: fooLabelValue2}},
Samples: []prompb.Sample{{Value: value50, Timestamp: timestamp}},
Samples: []prompb.Sample{{Value: value50, Timestamp: nowTimestamp}},
}, {
Labels: []prompb.Label{{Name: nameLabelName, Value: barMetricName}, {Name: barLabelName, Value: barLabelValue1}},
Samples: []prompb.Sample{{Value: value42, Timestamp: timestamp}},
Samples: []prompb.Sample{{Value: value42, Timestamp: nowTimestamp}},
}},
}, {
name: "gauge",
Expand Down Expand Up @@ -123,13 +123,13 @@ func Test_convertToTimeseries(t *testing.T) {
},
want: []prompb.TimeSeries{{
Labels: []prompb.Label{{Name: nameLabelName, Value: fooMetricName}, {Name: fooLabelName, Value: fooLabelValue1}},
Samples: []prompb.Sample{{Value: value42, Timestamp: timestamp}},
Samples: []prompb.Sample{{Value: value42, Timestamp: nowTimestamp}},
}, {
Labels: []prompb.Label{{Name: nameLabelName, Value: fooMetricName}, {Name: fooLabelName, Value: fooLabelValue2}},
Samples: []prompb.Sample{{Value: value50, Timestamp: timestamp}},
Samples: []prompb.Sample{{Value: value50, Timestamp: nowTimestamp}},
}, {
Labels: []prompb.Label{{Name: nameLabelName, Value: barMetricName}, {Name: barLabelName, Value: barLabelValue1}},
Samples: []prompb.Sample{{Value: value42, Timestamp: timestamp}},
Samples: []prompb.Sample{{Value: value42, Timestamp: nowTimestamp}},
}},
}, {
name: "untyped",
Expand Down Expand Up @@ -160,13 +160,75 @@ func Test_convertToTimeseries(t *testing.T) {
},
want: []prompb.TimeSeries{{
Labels: []prompb.Label{{Name: nameLabelName, Value: fooMetricName}, {Name: fooLabelName, Value: fooLabelValue1}},
Samples: []prompb.Sample{{Value: value42, Timestamp: timestamp}},
Samples: []prompb.Sample{{Value: value42, Timestamp: nowTimestamp}},
}, {
Labels: []prompb.Label{{Name: nameLabelName, Value: fooMetricName}, {Name: fooLabelName, Value: fooLabelValue2}},
Samples: []prompb.Sample{{Value: value50, Timestamp: nowTimestamp}},
}, {
Labels: []prompb.Label{{Name: nameLabelName, Value: barMetricName}, {Name: barLabelName, Value: barLabelValue1}},
Samples: []prompb.Sample{{Value: value42, Timestamp: nowTimestamp}},
}},
}, {
name: "unsanitized",
in: &PartitionedMetrics{
Families: []*clientmodel.MetricFamily{{
Name: &fooMetricName,
Help: &fooHelp,
Type: &counter,
Metric: []*clientmodel.Metric{{
Label: []*clientmodel.LabelPair{{Name: &fooLabelName, Value: &fooLabelValue1}},
Counter: &clientmodel.Counter{Value: &value42},
TimestampMs: &timestamp,
}, {
Label: []*clientmodel.LabelPair{{Name: &fooLabelName, Value: &fooLabelValue2}},
Counter: &clientmodel.Counter{Value: &value50},
TimestampMs: &timestamp,
}, {
// With empty label.
Label: []*clientmodel.LabelPair{{Name: &emptyLabelName, Value: &fooLabelValue2}},
Counter: &clientmodel.Counter{Value: &value50},
TimestampMs: &timestamp,
},
},
}, {
Name: &barMetricName,
Help: &barHelp,
Type: &counter,
Metric: []*clientmodel.Metric{{
Label: []*clientmodel.LabelPair{{Name: &barLabelName, Value: &barLabelValue1}},
Counter: &clientmodel.Counter{Value: &value42},
TimestampMs: &timestamp,
}, {
// With duplicate labels.
Label: []*clientmodel.LabelPair{{Name: &fooLabelName, Value: &fooLabelValue2}, {Name: &fooLabelName, Value: &fooLabelValue2}},
Counter: &clientmodel.Counter{Value: &value42},
TimestampMs: &timestamp,
}, {
// With out-of-order labels.
Label: []*clientmodel.LabelPair{{Name: &fooLabelName, Value: &fooLabelValue2}, {Name: &barLabelName, Value: &barLabelValue1}},
Counter: &clientmodel.Counter{Value: &value50},
TimestampMs: &timestamp,
}},
}},
},
want: []prompb.TimeSeries{{
Labels: []prompb.Label{{Name: nameLabelName, Value: fooMetricName}, {Name: fooLabelName, Value: fooLabelValue1}},
Samples: []prompb.Sample{{Value: value42, Timestamp: nowTimestamp}},
}, {
Labels: []prompb.Label{{Name: nameLabelName, Value: fooMetricName}, {Name: fooLabelName, Value: fooLabelValue2}},
Samples: []prompb.Sample{{Value: value50, Timestamp: timestamp}},
Samples: []prompb.Sample{{Value: value50, Timestamp: nowTimestamp}},
}, {
Labels: []prompb.Label{{Name: nameLabelName, Value: fooMetricName}},
Samples: []prompb.Sample{{Value: value50, Timestamp: nowTimestamp}},
}, {
Labels: []prompb.Label{{Name: nameLabelName, Value: barMetricName}, {Name: barLabelName, Value: barLabelValue1}},
Samples: []prompb.Sample{{Value: value42, Timestamp: timestamp}},
Samples: []prompb.Sample{{Value: value42, Timestamp: nowTimestamp}},
}, {
Labels: []prompb.Label{{Name: nameLabelName, Value: barMetricName}, {Name: fooLabelName, Value: fooLabelValue2}},
Samples: []prompb.Sample{{Value: value42, Timestamp: nowTimestamp}},
}, {
Labels: []prompb.Label{{Name: nameLabelName, Value: barMetricName}, {Name: barLabelName, Value: barLabelValue1}, {Name: fooLabelName, Value: fooLabelValue2}},
Samples: []prompb.Sample{{Value: value50, Timestamp: nowTimestamp}},
}},
}}
for _, tt := range tests {
Expand All @@ -176,9 +238,6 @@ func Test_convertToTimeseries(t *testing.T) {
t.Errorf("converting timeseries errored: %v", err)
}
if ok, err := timeseriesEqual(tt.want, out); !ok {
// t.Error("want: ", tt.want)
// t.Error("out: ", out)

t.Errorf("timeseries don't match: %v", err)
}
})
Expand Down

0 comments on commit d56f8c4

Please sign in to comment.