Revert "fix: add classic histogram buckets to perf insights metric"

CHANGELOG.md

@@ -8,11 +8,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Added support for YAML-based validation files in DevContext (https://github.com/authzed/spicedb/pull/3024)
 - Added support for YAML-based validation files in the Language Server (https://github.com/authzed/spicedb/pull/3024)
 
-### Fixed
-- Perf insights not producing buckets when Prometheus native histograms is disabled (https://github.com/authzed/spicedb/pull/3027)
+### Changed
 - Removed MySQL metrics prefixed with `go_sql_stats_connections_*` in favor of those prefixed with `go_sql_*` (https://github.com/authzed/spicedb/pull/2980)
 - Removed support for Spanner flag value `--datastore-spanner-metrics=deprecated-prometheus`; please use values `otel` or `native` (https://github.com/authzed/spicedb/pull/2980)
 - Reduced binary size (https://github.com/authzed/spicedb/pull/3005)
+
+### Fixed
 - On a Postgres setup with read replicas, some requests may silently swallow errors of sort "revision not found in replica" (https://github.com/authzed/spicedb/pull/2979)
 - Use cgroup-aware memory detection for cache and watch buffer sizing in containerized environments (https://github.com/authzed/spicedb/pull/3000)
 - Upgraded the spanner client, which changed the internal implementation to not use a session pool. This means that the `--datastore-spanner-max-sessions` and `--datastore-spanner-min-sessions` flags are now deprecated and no-op. We also strongly recommend using [Application Default Credentials](https://docs.cloud.google.com/docs/authentication/client-libraries#adc) in favor of a credentials file. (https://github.com/authzed/spicedb/pull/3038)

internal/middleware/perfinsights/perfinsights.go

@@ -54,25 +54,20 @@ func NoLabels() APIShapeLabels {
 // of the API call that are not the specific object IDs, e.g. the resource type,
 // the relation, the subject type, etc.
 //
-// NOTE: This metric uses both classic and native histogram buckets. Classic buckets
-// ensure compatibility with standard PromQL queries (e.g. histogram_quantile), while
-// native histograms provide higher resolution when supported by the Prometheus server.
+// NOTE: This metric is a *native* histogram, which means that instead of predefined
+// the buckets, it uses a factor to determine the buckets. This is useful for latency-based
+// metrics, as the latency can vary widely and having a fixed set of buckets can lead to
+// imprecise results.
 //
-// To make use of native histograms, a special flag must be set on Prometheus:
+// To use make use of native histograms, a special flag must be set on Prometheus:
 // https://prometheus.io/docs/prometheus/latest/feature_flags/#native-histograms
 var APIShapeLatency = promauto.NewHistogramVec(prometheus.HistogramOpts{
-	Namespace:                      "spicedb",
-	Subsystem:                      "perf_insights",
-	Name:                           "api_shape_latency_seconds",
-	Help:                           "The latency of API calls, by shape",
-	NativeHistogramBucketFactor:    1.1,
-	NativeHistogramMaxBucketNumber: 100,
-	// Bucket boundaries: sub-second values match the gRPC server handling
-	// time histogram in pkg/cmd/server/defaults.go (createServerMetrics);
-	// 1–10s buckets added for slow query visibility in Perf Insights.
-	Buckets: []float64{
-		.001, .003, .006, .010, .018, .024, .032, .042, .056, .075, .100, .178, .316, .562, 1, 2, 3, 5, 7, 10,
-	},
+	Namespace:                   "spicedb",
+	Subsystem:                   "perf_insights",
+	Name:                        "api_shape_latency_seconds",
+	Help:                        "The latency of API calls, by shape",
+	Buckets:                     nil,
+	NativeHistogramBucketFactor: 1.1,
 }, append([]string{"api_kind"}, allLabels...))
 
 var tracer = otel.Tracer("spicedb/internal/middleware")

internal/middleware/perfinsights/perfinsights_test.go

@@ -67,15 +67,12 @@ func TestObserveShapeLatency(t *testing.T) {
 	reg := prometheus.NewRegistry()
 
 	metric := prometheus.NewHistogramVec(prometheus.HistogramOpts{
-		Namespace:                      "spicedb",
-		Subsystem:                      "perf_insights",
-		Name:                           "api_shape_latency_seconds",
-		Help:                           "The latency of API calls, by shape",
-		NativeHistogramBucketFactor:    1.1,
-		NativeHistogramMaxBucketNumber: 100,
-		Buckets: []float64{
-			.001, .003, .006, .010, .018, .024, .032, .042, .056, .075, .100, .178, .316, .562, 1, 2, 3, 5, 7, 10,
-		},
+		Namespace:                   "spicedb",
+		Subsystem:                   "perf_insights",
+		Name:                        "api_shape_latency_seconds",
+		Help:                        "The latency of API calls, by shape",
+		Buckets:                     nil,
+		NativeHistogramBucketFactor: 1.1,
 	}, append([]string{"api_kind"}, allLabels...))
 	require.NoError(t, reg.Register(metric))
 
@@ -102,12 +99,6 @@ func TestObserveShapeLatency(t *testing.T) {
 		require.Equal(t, float64(0.1), metric.GetMetric()[0].Histogram.GetSampleSum()) //nolint:testifylint // this value is set directly by test code
 		require.Equal(t, "testMethod", metric.GetMetric()[0].Label[0].GetValue())
 
-		// Verify classic buckets are populated (not just +Inf).
-		buckets := metric.GetMetric()[0].Histogram.GetBucket()
-		require.Len(t, buckets, 20)
-		require.InDelta(t, 0.001, buckets[0].GetUpperBound(), 1e-9)
-		require.InDelta(t, float64(10), buckets[len(buckets)-1].GetUpperBound(), 1e-9)
-
 		for _, label := range metric.GetMetric()[0].Label {
 			if label.GetName() == "api_kind" {
 				require.Equal(t, "testMethod", label.GetValue())