Fix v2 metrics: Send all ttfb api labels (#20191)

Fix a regression in #19733 where TTFB metrics for all APIs except
GetObject were removed in v2 and v3 metrics. This causes breakage for
existing v2 metrics users. Instead we continue to send TTFB for all APIs
in V2 but only send for GetObject in V3.

cmd/http-stats.go

@@ -27,10 +27,6 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 )
 
-const (
-	apiGetObject = "GetObject"
-)
-
 // connStats - Network statistics
 // Count total input/output transferred bytes during
 // the server's life.
@@ -132,7 +128,7 @@ func (bh *bucketHTTPStats) updateHTTPStats(bucket, api string, w *xhttp.Response
 		return
 	}
 
-	if w != nil && api == apiGetObject {
+	if w != nil {
 		// Increment the prometheus http request response histogram with API, Bucket
 		bucketHTTPRequestsDuration.With(prometheus.Labels{
 			"api":    api,
@@ -437,9 +433,7 @@ func (st *HTTPStats) updateStats(api string, w *xhttp.ResponseRecorder) {
 	st.totalS3Requests.Inc(api)
 
 	// Increment the prometheus http request response histogram with appropriate label
-	if api == apiGetObject {
-		httpRequestsDuration.With(prometheus.Labels{"api": api}).Observe(w.TimeToFirstByte.Seconds())
-	}
+	httpRequestsDuration.With(prometheus.Labels{"api": api}).Observe(w.TimeToFirstByte.Seconds())
 
 	code := w.StatusCode
 

cmd/metrics-v3-api.go

@@ -19,6 +19,8 @@ package cmd
 
 import (
 	"context"
+
+	"github.com/minio/minio-go/v7/pkg/set"
 )
 
 const (
@@ -127,7 +129,8 @@ func loadAPIRequestsHTTPMetrics(ctx context.Context, m MetricValues, _ *metricsC
 // This is a `MetricsLoaderFn`.
 func loadAPIRequestsTTFBMetrics(ctx context.Context, m MetricValues, _ *metricsCache) error {
 	renameLabels := map[string]string{"api": "name"}
-	m.SetHistogram(apiRequestsTTFBSecondsDistribution, httpRequestsDuration, renameLabels, nil,
+	labelsFilter := map[string]set.StringSet{"api": set.CreateStringSet("GetObject")}
+	m.SetHistogram(apiRequestsTTFBSecondsDistribution, httpRequestsDuration, labelsFilter, renameLabels, nil,
 		"type", "s3")
 	return nil
 }
@@ -214,7 +217,8 @@ func loadBucketAPIHTTPMetrics(ctx context.Context, m MetricValues, _ *metricsCac
 // This is a `MetricsLoaderFn`.
 func loadBucketAPITTFBMetrics(ctx context.Context, m MetricValues, _ *metricsCache, buckets []string) error {
 	renameLabels := map[string]string{"api": "name"}
-	m.SetHistogram(apiRequestsTTFBSecondsDistribution, bucketHTTPRequestsDuration, renameLabels,
+	labelsFilter := map[string]set.StringSet{"api": set.CreateStringSet("GetObject")}
+	m.SetHistogram(apiRequestsTTFBSecondsDistribution, bucketHTTPRequestsDuration, labelsFilter, renameLabels,
 		buckets, "type", "s3")
 	return nil
 }

cmd/metrics-v3-types.go

@@ -23,6 +23,7 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/minio/minio-go/v7/pkg/set"
 	"github.com/minio/minio/internal/logger"
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus"
@@ -247,28 +248,38 @@ func (m *MetricValues) Set(name MetricName, value float64, labels ...string) {
 // SetHistogram - sets values for the given MetricName using the provided
 // histogram.
 //
+// `filterByLabels` is a map of label names to list of allowed label values to
+// filter by. Note that this filtering happens before any renaming of labels.
+//
 // `renameLabels` is a map of label names to rename. The keys are the original
 // label names and the values are the new label names.
 //
-// TODO: bucketFilter doc
+// `bucketFilter` is a list of bucket values to filter. If this is non-empty,
+// only metrics for the given buckets are added.
 //
 // `extraLabels` are additional labels to add to each metric. They are ordered
 // label name and value pairs.
 func (m *MetricValues) SetHistogram(name MetricName, hist *prometheus.HistogramVec,
-	renameLabels map[string]string, bucketFilter []string, extraLabels ...string,
+	filterByLabels map[string]set.StringSet, renameLabels map[string]string, bucketFilter []string,
+	extraLabels ...string,
 ) {
 	if _, ok := m.descriptors[name]; !ok {
 		panic(fmt.Sprintf("metric has no description: %s", name))
 	}
 	dummyDesc := MetricDescription{}
 	metricsV2 := getHistogramMetrics(hist, dummyDesc, false)
+mainLoop:
 	for _, metric := range metricsV2 {
+		for label, allowedValues := range filterByLabels {
+			if !allowedValues.Contains(metric.VariableLabels[label]) {
+				continue mainLoop
+			}
+		}
+
 		// If a bucket filter is provided, only add metrics for the given
 		// buckets.
-		if len(bucketFilter) > 0 {
-			if !slices.Contains(bucketFilter, metric.VariableLabels["bucket"]) {
-				continue
-			}
+		if len(bucketFilter) > 0 && !slices.Contains(bucketFilter, metric.VariableLabels["bucket"]) {
+			continue
 		}
 
 		labels := make([]string, 0, len(metric.VariableLabels)*2)