This change adds support to configurable prometheus buckets

http_request_duration has fixed, hardcoded number of buckets with no possibility to tweak them
For cases when the most of the latencies are above 1ms, with only 4 available buckets there's no good insight on OPA's performance.

This implementation:
- adds the possibility for the buckets to be configurable in ```server.metrics.prom.http_request_duration_seconds.buckets``` key
- it's not a breaking change, if the buckets are not present in the configuration, the metric is configured with the existing values as a fallback

Signed-off-by: aarnautu <aarnautu@adobe.com>

config/config.go

@@ -39,6 +39,7 @@ type Config struct {
 	DistributedTracing           json.RawMessage            `json:"distributed_tracing,omitempty"`
 	Server                       *struct {
 		Encoding json.RawMessage `json:"encoding,omitempty"`
+		Metrics  json.RawMessage `json:"metrics,omitempty"`
 	} `json:"server,omitempty"`
 	Storage *struct {
 		Disk json.RawMessage `json:"disk,omitempty"`

config/config_test.go

@@ -202,6 +202,13 @@ func TestActiveConfig(t *testing.T) {
 					"min_length": 1024,
 					"compression_level": 1
 				}
+			},
+			"metrics": {
+				"prom": {
+					"http_request_duration_seconds": {
+						"buckets": [0.1, 0.2]
+					}
+				}
 			}
 		},
 		"discovery": {"name": "config"}`
@@ -263,6 +270,13 @@ func TestActiveConfig(t *testing.T) {
 					"min_length": 1024,
 					"compression_level": 1
 				}
+			},
+			"metrics": {
+				"prom": {
+					"http_request_duration_seconds": {
+						"buckets": [0.1, 0.2]
+					}
+				}
 			}
 		},
 		"default_authorization_decision": "/system/authz/allow",

docs/content/configuration.md

@@ -991,10 +991,13 @@ See [the docs on disk storage](../misc-disk/) for details about the settings.
 
 ### Server
 
-The `server` configuration sets the gzip compression settings for `/v0/data`, `/v1/data` and `/v1/compile` HTTP `POST` endpoints
+The `server` configuration sets:
+- the gzip compression settings for `/v0/data`, `/v1/data` and `/v1/compile` HTTP `POST` endpoints
 The gzip compression settings are used when the client sends `Accept-Encoding: gzip`
+- buckets for `http_request_duration_seconds` histogram
 
-| Field                                    | Type  | Required            | Description                                                                                                                                                                                                                                  |
-|------------------------------------------|-------|---------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| `server.encoding.gzip.min_length`        | `int` | No, (default: 1024) | Specifies the minimum length of the response to compress                                                                                                                                                                                     |
-| `server.encoding.gzip.compression_level` | `int` | No, (default: 9)    | Specifies the compression level. Accepted values: a value of either 0 (no compression), 1 (best speed, lowest compression) or 9 (slowest, best compression). See https://pkg.go.dev/compress/flate#pkg-constants |
+| Field                                                       | Type        | Required                                                                  | Description                                                                                                                                                                                                               |
+|-------------------------------------------------------------|-------------|---------------------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| `server.encoding.gzip.min_length`                           | `int`       | No, (default: 1024)                                                       | Specifies the minimum length of the response to compress                                                                                                                                                                  |
+| `server.encoding.gzip.compression_level`                    | `int`       | No, (default: 9)                                                          | Specifies the compression level. Accepted values: a value of either 0 (no compression), 1 (best speed, lowest compression) or 9 (slowest, best compression). See https://pkg.go.dev/compress/flate#pkg-constants          |
+| `server.metrics.prom.http_request_duration_seconds.buckets` | `[]float64` | No, (default: [1e-6, 5e-6, 1e-5, 5e-5, 1e-4, 5e-4, 1e-3, 0.01, 0.1, 1  ]) | Specifies the buckets for the `http_request_duration_seconds` metric. Each value is a float, it is expressed in seconds and subdivisions of it. E.g `1e-6` is 1 microsecond, `1e-3` 1 millisecond, `0.01` 10 milliseconds |

internal/prometheus/prometheus.go

@@ -37,25 +37,14 @@ type Provider struct {
 type loggerFunc func(attrs map[string]interface{}, f string, a ...interface{})
 
 // New returns a new Provider object.
-func New(inner metrics.Metrics, logger loggerFunc) *Provider {
+func New(inner metrics.Metrics, logger loggerFunc, httpRequestBuckets []float64) *Provider {
 	registry := prometheus.NewRegistry()
 	registry.MustRegister(collector())
 	durationHistogram := prometheus.NewHistogramVec(
 		prometheus.HistogramOpts{
-			Name: "http_request_duration_seconds",
-			Help: "A histogram of duration for requests.",
-			Buckets: []float64{
-				1e-6, // 1 microsecond
-				5e-6,
-				1e-5,
-				5e-5,
-				1e-4,
-				5e-4,
-				1e-3, // 1 millisecond
-				0.01,
-				0.1,
-				1, // 1 second
-			},
+			Name:    "http_request_duration_seconds",
+			Help:    "A histogram of duration for requests.",
+			Buckets: httpRequestBuckets,
 		},
 		[]string{"code", "handler", "method"},
 	)

internal/prometheus/prometheus_go1.19_test.go

@@ -25,7 +25,7 @@ func TestJSONSerialization(t *testing.T) {
 		}
 	}(logging.NewNoOpLogger())
 
-	prom := New(inner, logger)
+	prom := New(inner, logger, []float64{1e-6, 5e-6, 1e-5, 5e-5, 1e-4, 5e-4, 1e-3, 0.01, 0.1, 1})
 
 	m := prom.All()
 	bs, err := json.Marshal(m)

internal/prometheus/prometheus_test.go

@@ -25,7 +25,7 @@ func TestJSONSerialization(t *testing.T) {
 		}
 	}(logging.NewNoOpLogger())
 
-	prom := New(inner, logger)
+	prom := New(inner, logger, []float64{1e-6, 5e-6, 1e-5, 5e-5, 1e-4, 5e-4, 1e-3, 0.01, 0.1, 1})
 
 	m := prom.All()
 	bs, err := json.Marshal(m)

plugins/server/metrics/config.go

@@ -0,0 +1,93 @@
+package metrics
+
+import (
+	"github.com/open-policy-agent/opa/util"
+)
+
+var defaultHTTPRequestBuckets = []float64{
+	1e-6, // 1 microsecond
+	5e-6,
+	1e-5,
+	5e-5,
+	1e-4,
+	5e-4,
+	1e-3, // 1 millisecond
+	0.01,
+	0.1,
+	1, // 1 second
+}
+
+// Config represents the configuration for the Server.Metrics settings
+type Config struct {
+	Prom *Prom `json:"prom,omitempty"`
+}
+
+// Prom represents the configuration for the Server.Metrics.Prom settings
+type Prom struct {
+	HTTPRequestDurationSeconds *HTTPRequestDurationSeconds `json:"http_request_duration_seconds,omitempty"`
+}
+
+// HTTPRequestDurationSeconds represents the configuration for the Server.Metrics.Prom.HTTPRequestDurationSeconds settings
+type HTTPRequestDurationSeconds struct {
+	Buckets []float64 `json:"buckets,omitempty"` // the float64 array of buckets representing seconds or division of a second
+}
+
+// ConfigBuilder assists in the construction of the plugin configuration.
+type ConfigBuilder struct {
+	raw []byte
+}
+
+// NewConfigBuilder returns a new ConfigBuilder to build and parse the server config
+func NewConfigBuilder() *ConfigBuilder {
+	return &ConfigBuilder{}
+}
+
+// WithBytes sets the raw server config
+func (b *ConfigBuilder) WithBytes(config []byte) *ConfigBuilder {
+	b.raw = config
+	return b
+}
+
+// Parse returns a valid Config object with defaults injected.
+func (b *ConfigBuilder) Parse() (*Config, error) {
+	if b.raw == nil {
+		defaultConfig := &Config{
+			Prom: &Prom{
+				HTTPRequestDurationSeconds: &HTTPRequestDurationSeconds{
+					Buckets: defaultHTTPRequestBuckets,
+				},
+			},
+		}
+		return defaultConfig, nil
+	}
+
+	var result Config
+
+	if err := util.Unmarshal(b.raw, &result); err != nil {
+		return nil, err
+	}
+
+	return &result, result.validateAndInjectDefaults()
+}
+
+func (c *Config) validateAndInjectDefaults() error {
+	if c.Prom == nil {
+		c.Prom = &Prom{
+			HTTPRequestDurationSeconds: &HTTPRequestDurationSeconds{
+				Buckets: defaultHTTPRequestBuckets,
+			},
+		}
+	}
+
+	if c.Prom.HTTPRequestDurationSeconds == nil {
+		c.Prom.HTTPRequestDurationSeconds = &HTTPRequestDurationSeconds{
+			Buckets: defaultHTTPRequestBuckets,
+		}
+	}
+
+	if c.Prom.HTTPRequestDurationSeconds.Buckets == nil {
+		c.Prom.HTTPRequestDurationSeconds.Buckets = defaultHTTPRequestBuckets
+	}
+
+	return nil
+}

plugins/server/metrics/config_test.go

@@ -0,0 +1,129 @@
+package metrics
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestConfigValidation(t *testing.T) {
+	tests := []struct {
+		input   string
+		wantErr bool
+	}{
+		{
+			input:   `{}`,
+			wantErr: false,
+		},
+		{
+			input:   `{"prom": {}}`,
+			wantErr: false,
+		},
+		{
+			input:   `{"prom": {"http_request_duration_seconds": {}}}`,
+			wantErr: false,
+		},
+		{
+			input:   `{"prom": {"http_request_duration_seconds": {"buckets": []}}}`,
+			wantErr: false,
+		},
+
+		{
+			input:   `{"prom": {"http_request_duration_seconds": {"buckets": ["not-a-array"]}}}`,
+			wantErr: true,
+		},
+		{
+			input:   `{"prom": {"http_request_duration_seconds": {"buckets": [1]}}}`,
+			wantErr: false,
+		},
+		{
+			input:   `{"prom": {"http_request_duration_seconds": {"buckets": "1"}}}`,
+			wantErr: true,
+		},
+		{
+			input:   `{"prom": {"http_request_duration_seconds": {"buckets": [0.001, "1", "2"]}}}`,
+			wantErr: true,
+		},
+		{
+			input:   `{"prom": {"http_request_duration_seconds": {"buckets": ["one", "two", "three"]}}}`,
+			wantErr: true,
+		},
+		{
+			input:   `{"prom": {"http_request_duration_seconds": {"buckets": ["0.1", "0.2", "0.3", "4"]}}}`,
+			wantErr: true,
+		},
+		{
+			input:   `{"prom": {"random_key": 0}}`,
+			wantErr: false,
+		},
+		{
+			input:   `{"prom": {"http_request_duration_seconds": {"random_key": 0}}}`,
+			wantErr: false,
+		},
+	}
+
+	for i, test := range tests {
+		t.Run(fmt.Sprintf("TestConfigValidation_case_%d", i), func(t *testing.T) {
+			_, err := NewConfigBuilder().WithBytes([]byte(test.input)).Parse()
+			if err != nil && !test.wantErr {
+				t.Fail()
+			}
+			if err == nil && test.wantErr {
+				t.Fail()
+			}
+		})
+	}
+}
+
+func TestConfigValue(t *testing.T) {
+	tests := []struct {
+		input         string
+		expectedValue []float64
+	}{
+		{
+			input:         `{}`,
+			expectedValue: defaultHTTPRequestBuckets,
+		},
+		{
+			input:         `{"prom": {}}`,
+			expectedValue: defaultHTTPRequestBuckets,
+		},
+		{
+			input:         `{"prom": {"http_request_duration_seconds": {}}}`,
+			expectedValue: defaultHTTPRequestBuckets,
+		},
+		{
+			input:         `{"prom": {"http_request_duration_seconds": {"buckets": []}}}`,
+			expectedValue: []float64{},
+		},
+		{
+			input:         `{"prom": {"http_request_duration_seconds": {"buckets":[0.1, 0.2, 0.3, 4]}}}`,
+			expectedValue: []float64{0.1, 0.2, 0.3, 4},
+		},
+	}
+
+	for i, test := range tests {
+		t.Run(fmt.Sprintf("TestConfigValue_case_%d", i), func(t *testing.T) {
+			config, err := NewConfigBuilder().WithBytes([]byte(test.input)).Parse()
+			if err != nil {
+				t.Fail()
+			}
+			if !valuesAreEqual(config.Prom.HTTPRequestDurationSeconds.Buckets, test.expectedValue) {
+				t.Fail()
+			}
+		})
+	}
+}
+
+func valuesAreEqual(a []float64, b []float64) bool {
+	if len(a) != len(b) {
+		return false
+	}
+
+	for i, v := range a {
+		if v != b[i] {
+			return false
+		}
+	}
+
+	return true
+}

runtime/runtime.go

@@ -32,6 +32,7 @@ import (
 	"go.uber.org/automaxprocs/maxprocs"
 
 	"github.com/open-policy-agent/opa/bundle"
+	opa_config "github.com/open-policy-agent/opa/config"
 	"github.com/open-policy-agent/opa/internal/config"
 	internal_tracing "github.com/open-policy-agent/opa/internal/distributedtracing"
 	internal_logging "github.com/open-policy-agent/opa/internal/logging"
@@ -46,6 +47,7 @@ import (
 	"github.com/open-policy-agent/opa/plugins"
 	"github.com/open-policy-agent/opa/plugins/discovery"
 	"github.com/open-policy-agent/opa/plugins/logs"
+	metrics_config "github.com/open-policy-agent/opa/plugins/server/metrics"
 	"github.com/open-policy-agent/opa/repl"
 	"github.com/open-policy-agent/opa/server"
 	"github.com/open-policy-agent/opa/storage"
@@ -347,7 +349,11 @@ func NewRuntime(ctx context.Context, params Params) (*Runtime, error) {
 		params.Router = mux.NewRouter()
 	}
 
-	metrics := prometheus.New(metrics.New(), errorLogger(logger))
+	metricsConfig, parseConfigErr := extractMetricsConfig(config, params)
+	if parseConfigErr != nil {
+		return nil, parseConfigErr
+	}
+	metrics := prometheus.New(metrics.New(), errorLogger(logger), metricsConfig.Prom.HTTPRequestDurationSeconds.Buckets)
 
 	var store storage.Store
 	if params.DiskStorage == nil {
@@ -427,6 +433,27 @@ func NewRuntime(ctx context.Context, params Params) (*Runtime, error) {
 	return rt, nil
 }
 
+// extractMetricsConfig returns the configuration for server metrics and parsing errors if any
+func extractMetricsConfig(config []byte, params Params) (*metrics_config.Config, error) {
+	var opaParsedConfig, opaParsedConfigErr = opa_config.ParseConfig(config, params.ID)
+	if opaParsedConfigErr != nil {
+		return nil, opaParsedConfigErr
+	}
+
+	var serverMetricsData []byte
+	if opaParsedConfig.Server != nil {
+		serverMetricsData = opaParsedConfig.Server.Metrics
+	}
+
+	var configBuilder = metrics_config.NewConfigBuilder()
+	var metricsParsedConfig, metricsParsedConfigErr = configBuilder.WithBytes(serverMetricsData).Parse()
+	if metricsParsedConfigErr != nil {
+		return nil, fmt.Errorf("server metrics configuration parse error: %w", metricsParsedConfigErr)
+	}
+
+	return metricsParsedConfig, nil
+}
+
 // StartServer starts the runtime in server mode. This function will block the
 // calling goroutine and will exit the program on error.
 func (rt *Runtime) StartServer(ctx context.Context) {

runtime/runtime_test.go

@@ -603,3 +603,39 @@ func TestAddrWarningMessage(t *testing.T) {
 		})
 	}
 }
+
+func TestRuntimeWithExplicitMetricConfiguration(t *testing.T) {
+	fs := map[string]string{
+		"/config.yaml": `{"server": {"metrics": {"prom": {"http_request_duration_seconds": {"buckets": [0.1, 0.2, 0.3]}}}}}`,
+	}
+
+	test.WithTempFS(fs, func(testDirRoot string) {
+		params := NewParams()
+		params.ConfigFile = filepath.Join(testDirRoot, "/config.yaml")
+
+		_, err := NewRuntime(context.Background(), params)
+		if err != nil {
+			t.Fatalf(err.Error())
+		}
+	})
+}
+
+func TestRuntimeWithExplicitBadMetricConfiguration(t *testing.T) {
+	fs := map[string]string{
+		"/config.yaml": `{"server": {"metrics": {"prom": {"http_request_duration_seconds": {"buckets": "would-not-work"}}}}}`,
+	}
+
+	test.WithTempFS(fs, func(testDirRoot string) {
+		params := NewParams()
+		params.ConfigFile = filepath.Join(testDirRoot, "/config.yaml")
+
+		_, err := NewRuntime(context.Background(), params)
+		if err == nil {
+			t.Fatalf("Expected error to be thrown on malformed metrics config")
+		}
+
+		if !strings.HasPrefix(err.Error(), "server metrics configuration parse error") {
+			t.Fatalf("Expected specific error to be thrown on malformed metrics config")
+		}
+	})
+}

server/server_test.go

@@ -3415,7 +3415,7 @@ func TestStatusV1MetricsWithSystemAuthzPolicy(t *testing.T) {
 		}
 	}(logging.NewNoOpLogger())
 
-	prom := prometheus.New(inner, logger)
+	prom := prometheus.New(inner, logger, []float64{1e-6, 5e-6, 1e-5, 5e-5, 1e-4, 5e-4, 1e-3, 0.01, 0.1, 1})
 	serverOpts := []func(s *Server){func(s *Server) { s.WithAuthorization(AuthorizationBasic) }, func(s *Server) { s.WithMetrics(prom) }}
 
 	f := newFixtureWithStore(t, store, serverOpts...)