diff --git a/cmd/metrics-v2.go b/cmd/metrics-v2.go index 3dd3ac192..bcf974240 100644 --- a/cmd/metrics-v2.go +++ b/cmd/metrics-v2.go @@ -1875,13 +1875,17 @@ func getHistogramMetrics(hist *prometheus.HistogramVec, desc MetricDescription, // add metrics with +Inf label labels1 := make(map[string]string) for _, lp := range dtoMetric.GetLabel() { - labels1[*lp.Name] = *lp.Value + if *lp.Name == "api" && toLowerAPILabels { + labels1[*lp.Name] = strings.ToLower(*lp.Value) + } else { + labels1[*lp.Name] = *lp.Value + } } labels1["le"] = fmt.Sprintf("%.3f", math.Inf(+1)) metrics = append(metrics, MetricV2{ Description: desc, VariableLabels: labels1, - Value: dtoMetric.Counter.GetValue(), + Value: float64(dtoMetric.Histogram.GetSampleCount()), }) } return metrics diff --git a/cmd/metrics-v2_test.go b/cmd/metrics-v2_test.go index 03f01e85f..1e5221ea3 100644 --- a/cmd/metrics-v2_test.go +++ b/cmd/metrics-v2_test.go @@ -18,13 +18,15 @@ package cmd import ( + "slices" + "strings" "testing" "time" "github.com/prometheus/client_golang/prometheus" ) -func TestGetHistogramMetrics(t *testing.T) { +func TestGetHistogramMetrics_BucketCount(t *testing.T) { histBuckets := []float64{0.05, 0.1, 0.25, 0.5, 0.75} labels := []string{"GetObject", "PutObject", "CopyObject", "CompleteMultipartUpload"} ttfbHist := prometheus.NewHistogramVec( @@ -83,6 +85,135 @@ func TestGetHistogramMetrics(t *testing.T) { metrics := getHistogramMetrics(ttfbHist, getBucketTTFBDistributionMD(), false) // additional labels for +Inf for all histogram metrics if expPoints := len(labels) * (len(histBuckets) + 1); expPoints != len(metrics) { - t.Fatalf("Expected %v data points but got %v", expPoints, len(metrics)) + t.Fatalf("Expected %v data points when toLowerAPILabels=false but got %v", expPoints, len(metrics)) + } + + metrics = getHistogramMetrics(ttfbHist, getBucketTTFBDistributionMD(), true) + // additional labels for +Inf for all histogram metrics + if expPoints := len(labels) * (len(histBuckets) + 1); expPoints != len(metrics) { + t.Fatalf("Expected %v data points when toLowerAPILabels=true but got %v", expPoints, len(metrics)) + } +} + +func TestGetHistogramMetrics_Values(t *testing.T) { + histBuckets := []float64{0.50, 5.00} + labels := []string{"PutObject", "CopyObject"} + ttfbHist := prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "s3_ttfb_seconds", + Help: "Time taken by requests served by current MinIO server instance", + Buckets: histBuckets, + }, + []string{"api"}, + ) + observations := []struct { + val float64 + label string + }{ + { + val: 0.02, + label: labels[0], + }, + { + val: 0.19, + label: labels[1], + }, + { + val: 0.31, + label: labels[1], + }, + { + val: 0.61, + label: labels[0], + }, + { + val: 6.79, + label: labels[1], + }, + } + ticker := time.NewTicker(1 * time.Millisecond) + defer ticker.Stop() + for _, obs := range observations { + // Send observations once every 1ms, to simulate delay between + // observations. This is to test the channel based + // synchronization used internally. + select { + case <-ticker.C: + ttfbHist.With(prometheus.Labels{"api": obs.label}).Observe(obs.val) + } + } + + // Accumulate regular-cased API label metrics for 'PutObject' for deeper verification + metrics := getHistogramMetrics(ttfbHist, getBucketTTFBDistributionMD(), false) + capitalPutObjects := make([]MetricV2, 0, len(histBuckets)+1) + for _, metric := range metrics { + if value := metric.VariableLabels["api"]; value == "PutObject" { + capitalPutObjects = append(capitalPutObjects, metric) + } + } + if expMetricsPerAPI := len(histBuckets) + 1; expMetricsPerAPI != len(capitalPutObjects) { + t.Fatalf("Expected %d api=PutObject metrics but got %d", expMetricsPerAPI, len(capitalPutObjects)) + } + + // Deterministic ordering + slices.SortFunc(capitalPutObjects, func(a MetricV2, b MetricV2) int { + le1 := a.VariableLabels["le"] + le2 := a.VariableLabels["le"] + return strings.Compare(le1, le2) + }) + if le := capitalPutObjects[0].VariableLabels["le"]; le != "0.500" { + t.Errorf("Expected le='0.050' api=PutObject metrics but got '%v'", le) + } + if value := capitalPutObjects[0].Value; value != 1 { + t.Errorf("Expected le='0.050' api=PutObject value to be 1 but got '%v'", value) + } + if le := capitalPutObjects[1].VariableLabels["le"]; le != "5.000" { + t.Errorf("Expected le='5.000' api=PutObject metrics but got '%v'", le) + } + if value := capitalPutObjects[1].Value; value != 2 { + t.Errorf("Expected le='5.000' api=PutObject value to be 2 but got '%v'", value) + } + if le := capitalPutObjects[2].VariableLabels["le"]; le != "+Inf" { + t.Errorf("Expected le='+Inf' api=PutObject metrics but got '%v'", le) + } + if value := capitalPutObjects[2].Value; value != 2 { + t.Errorf("Expected le='+Inf' api=PutObject value to be 2 but got '%v'", value) + } + + // Accumulate lower-cased API label metrics for 'copyobject' for deeper verification + metrics = getHistogramMetrics(ttfbHist, getBucketTTFBDistributionMD(), true) + lowerCopyObjects := make([]MetricV2, 0, len(histBuckets)+1) + for _, metric := range metrics { + if value := metric.VariableLabels["api"]; value == "copyobject" { + lowerCopyObjects = append(lowerCopyObjects, metric) + } + } + if expMetricsPerAPI := len(histBuckets) + 1; expMetricsPerAPI != len(lowerCopyObjects) { + t.Fatalf("Expected %d api=copyobject metrics but got %d", expMetricsPerAPI, len(lowerCopyObjects)) + } + + // Deterministic ordering + slices.SortFunc(lowerCopyObjects, func(a MetricV2, b MetricV2) int { + le1 := a.VariableLabels["le"] + le2 := a.VariableLabels["le"] + return strings.Compare(le1, le2) + }) + if le := lowerCopyObjects[0].VariableLabels["le"]; le != "0.500" { + t.Errorf("Expected le='0.050' api=copyobject metrics but got '%v'", le) + } + if value := lowerCopyObjects[0].Value; value != 2 { + t.Errorf("Expected le='0.050' api=copyobject value to be 2 but got '%v'", value) + } + if le := lowerCopyObjects[1].VariableLabels["le"]; le != "5.000" { + t.Errorf("Expected le='5.000' api=copyobject metrics but got '%v'", le) + } + if value := lowerCopyObjects[1].Value; value != 2 { + t.Errorf("Expected le='5.000' api=copyobject value to be 2 but got '%v'", value) + } + if le := lowerCopyObjects[2].VariableLabels["le"]; le != "+Inf" { + t.Errorf("Expected le='+Inf' api=copyobject metrics but got '%v'", le) + } + if value := lowerCopyObjects[2].Value; value != 3 { + t.Errorf("Expected le='+Inf' api=copyobject value to be 3 but got '%v'", value) } }