diff --git a/collectors/metrics/pkg/metricsclient/metricsclient.go b/collectors/metrics/pkg/metricsclient/metricsclient.go index 24f1b76bc..746eade23 100644 --- a/collectors/metrics/pkg/metricsclient/metricsclient.go +++ b/collectors/metrics/pkg/metricsclient/metricsclient.go @@ -17,6 +17,7 @@ import ( "net/http" "os" "path/filepath" + "sort" "strconv" "strings" "time" @@ -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" ) @@ -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, } @@ -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) @@ -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 { diff --git a/collectors/metrics/pkg/metricsclient/metricsclient_test.go b/collectors/metrics/pkg/metricsclient/metricsclient_test.go index 44488a6c0..65776185f 100644 --- a/collectors/metrics/pkg/metricsclient/metricsclient_test.go +++ b/collectors/metrics/pkg/metricsclient/metricsclient_test.go @@ -40,6 +40,8 @@ func Test_convertToTimeseries(t *testing.T) { fooLabelValue1 := "bar" fooLabelValue2 := "baz" + emptyLabelName := "" + barMetricName := "bar_metric" barHelp := "bar help text" barLabelName := "bar" @@ -47,12 +49,10 @@ func Test_convertToTimeseries(t *testing.T) { 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 @@ -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", @@ -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", @@ -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: ×tamp, + }, { + Label: []*clientmodel.LabelPair{{Name: &fooLabelName, Value: &fooLabelValue2}}, + Counter: &clientmodel.Counter{Value: &value50}, + TimestampMs: ×tamp, + }, { + // With empty label. + Label: []*clientmodel.LabelPair{{Name: &emptyLabelName, Value: &fooLabelValue2}}, + Counter: &clientmodel.Counter{Value: &value50}, + TimestampMs: ×tamp, + }, + }, + }, { + Name: &barMetricName, + Help: &barHelp, + Type: &counter, + Metric: []*clientmodel.Metric{{ + Label: []*clientmodel.LabelPair{{Name: &barLabelName, Value: &barLabelValue1}}, + Counter: &clientmodel.Counter{Value: &value42}, + TimestampMs: ×tamp, + }, { + // With duplicate labels. + Label: []*clientmodel.LabelPair{{Name: &fooLabelName, Value: &fooLabelValue2}, {Name: &fooLabelName, Value: &fooLabelValue2}}, + Counter: &clientmodel.Counter{Value: &value42}, + TimestampMs: ×tamp, + }, { + // With out-of-order labels. + Label: []*clientmodel.LabelPair{{Name: &fooLabelName, Value: &fooLabelValue2}, {Name: &barLabelName, Value: &barLabelValue1}}, + Counter: &clientmodel.Counter{Value: &value50}, + TimestampMs: ×tamp, + }}, + }}, + }, + 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 { @@ -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) } })