feat: Change metrics for specific rules to histograms, rename option … (

#42) - Change some counters to histogram to gain insights in time spend per protection - Cleanup unused code - Rename `fail_unknown_operations` to `reject_on_failure` for persisted operations to be consistent with other protection configuration naming - Update documentation --------- Co-authored-by: ldebruijn <[email protected]>
ldebruijn · Jan 22, 2024 · 1f437b3 · 1f437b3
1 parent 6192d7d
commit 1f437b3
Show file tree

Hide file tree

Showing 10 changed files with 7,415 additions and 61 deletions.
diff --git a/cmd/main.go b/cmd/main.go
@@ -38,17 +38,10 @@ import (
 )
 
 var (
-	shortHash   = "develop"
-	build       = "develop"
-	configPath  = ""
-	httpCounter = prometheus.NewCounterVec(prometheus.CounterOpts{
-		Namespace: "go_graphql_armor",
-		Subsystem: "http",
-		Name:      "count",
-		Help:      "HTTP request counts",
-	},
-		[]string{"route"},
-	)
+	shortHash  = "develop"
+	build      = "develop"
+	configPath = ""
+
 	httpDuration = prometheus.NewHistogramVec(prometheus.HistogramOpts{
 		Namespace: "go_graphql_armor",
 		Subsystem: "http",
@@ -70,7 +63,7 @@ var (
 )
 
 func init() {
-	prometheus.MustRegister(httpCounter, httpDuration, appInfo)
+	prometheus.MustRegister(httpDuration, appInfo)
 }
 
 func main() {
@@ -242,7 +235,7 @@ func ValidationRules(schema *schema.Provider, tks *tokens.MaxTokensRule, batch *
 					err = tks.Validate(operationSource)
 					if err != nil {
 						errs = append(errs, gqlerror.Wrap(err))
-						continue
+						continue // we could consider break-ing here. That would short-circuit on error, with the downside of not returning all potential errors
 					}
 
 					var query, err = parser.ParseQuery(operationSource)

diff --git a/cmd/main_test.go b/cmd/main_test.go
@@ -169,7 +169,7 @@ type Product {
 				cfgOverrides: func(cfg *config.Config) *config.Config {
 					cfg.PersistedOperations.Enabled = true
 					cfg.PersistedOperations.Store = "./"
-					cfg.PersistedOperations.FailUnknownOperations = false
+					cfg.PersistedOperations.RejectOnFailure = false
 					return cfg
 				},
 				mockResponse: map[string]interface{}{
@@ -369,7 +369,7 @@ type Product {
 				cfgOverrides: func(cfg *config.Config) *config.Config {
 					cfg.PersistedOperations.Enabled = true
 					cfg.PersistedOperations.Store = "./"
-					cfg.PersistedOperations.FailUnknownOperations = false
+					cfg.PersistedOperations.RejectOnFailure = false
 					return cfg
 				},
 				mockResponse: map[string]interface{}{
@@ -433,7 +433,7 @@ type Product {
 				cfgOverrides: func(cfg *config.Config) *config.Config {
 					cfg.PersistedOperations.Enabled = true
 					cfg.PersistedOperations.Store = "./"
-					cfg.PersistedOperations.FailUnknownOperations = false
+					cfg.PersistedOperations.RejectOnFailure = false
 					return cfg
 				},
 				mockResponse: map[string]interface{}{

diff --git a/docs/configuration.md b/docs/configuration.md
@@ -44,7 +44,7 @@ persisted_operations:
   # Enable or disable the feature, enabled by default
   enabled: true
   # Fail unknown operations, disable this feature to allow unknown operations to reach your GraphQL API
-  fail_unknown_operations: true
+  reject_on_failure: true
   # Store is the location on local disk where go-graphql-armor can find the persisted operations, it loads any `*.json` files on disk
   store: "./store"
   reload:

diff --git a/docs/persisted_operations.md b/docs/persisted_operations.md
@@ -19,7 +19,7 @@ persisted_operations:
   # Enable or disable the feature, enabled by default
   enabled: "true"
   # Fail unknown operations, disable this feature to allow unknown operations to reach your GraphQL API
-  fail_unknown_operations: "true"
+  reject_on_failure: "true"
   # Store is the location on local disk where go-graphql-armor can find the persisted operations, it loads any `*.json` files on disk
   store: "./store"
   reload:
@@ -88,10 +88,10 @@ This rule produces metrics to help you gain insights into the behavior of the ru
 go_graphql_armor_persisted_operations_results{state, result}
 ```
 
-| `state`    | Description                                                                                                                                                   |
-|----------|---------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| `unknown`   | The rule was not able to do its job. This happens either when `fail_unknown_operations` is set to `false` or the rule was not able to deserialize the request. |
-| `errored` | The rule caught an error during request body mutation.                                                                                                        |
+| `state`  | Description                                                                                                                                                   |
+|---------|---------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| `unknown` | The rule was not able to do its job. This happens either when `reject_on_failure` is set to `false` or the rule was not able to deserialize the request. |
+| `error` | The rule caught an error during request body mutation.                                                                                                        |
 | `known` | The rule received a hash for which it had a known operation                                                                                                   |
 
 

diff --git a/internal/business/aliases/aliases.go b/internal/business/aliases/aliases.go
@@ -5,15 +5,18 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/vektah/gqlparser/v2/ast"
 	"github.com/vektah/gqlparser/v2/validator"
+	"time"
 )
 
-var resultCounter = prometheus.NewCounterVec(prometheus.CounterOpts{
-	Namespace: "go_graphql_armor",
-	Subsystem: "max_aliases",
-	Name:      "results",
-	Help:      "The results of the max aliases rule",
-},
-	[]string{"result"},
+var (
+	resultHistogram = prometheus.NewHistogramVec(prometheus.HistogramOpts{
+		Namespace: "go_graphql_armor",
+		Subsystem: "max_aliases",
+		Name:      "results",
+		Help:      "The results of the max aliases rule",
+	},
+		[]string{"result"},
+	)
 )
 
 type Config struct {
@@ -23,7 +26,7 @@ type Config struct {
 }
 
 func init() {
-	prometheus.MustRegister(resultCounter)
+	prometheus.MustRegister(resultHistogram)
 }
 
 func NewMaxAliasesRule(cfg Config) {
@@ -44,6 +47,8 @@ func NewMaxAliasesRule(cfg Config) {
 			})
 
 			observers.OnOperation(func(walker *validator.Walker, operation *ast.OperationDefinition) {
+				start := time.Now()
+
 				aliases += countAliases(operation)
 
 				if aliases > cfg.Max {
@@ -53,12 +58,12 @@ func NewMaxAliasesRule(cfg Config) {
 							validator.Message(err),
 							validator.At(operation.Position),
 						)
-						resultCounter.WithLabelValues("rejected").Inc()
+						resultHistogram.WithLabelValues("rejected").Observe(time.Since(start).Seconds())
 					} else {
-						resultCounter.WithLabelValues("failed").Inc()
+						resultHistogram.WithLabelValues("failed").Observe(time.Since(start).Seconds())
 					}
 				} else {
-					resultCounter.WithLabelValues("allowed").Inc()
+					resultHistogram.WithLabelValues("allowed").Observe(time.Since(start).Seconds())
 				}
 			})
 		})

diff --git a/internal/business/max_depth/max_depth.go b/internal/business/max_depth/max_depth.go
@@ -5,9 +5,10 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/vektah/gqlparser/v2/ast"
 	"github.com/vektah/gqlparser/v2/validator"
+	"time"
 )
 
-var resultCounter = prometheus.NewCounterVec(prometheus.CounterOpts{
+var resultHistogram = prometheus.NewHistogramVec(prometheus.HistogramOpts{
 	Namespace: "go_graphql_armor",
 	Subsystem: "max_depth",
 	Name:      "results",
@@ -23,13 +24,15 @@ type Config struct {
 }
 
 func init() {
-	prometheus.MustRegister(resultCounter)
+	prometheus.MustRegister(resultHistogram)
 }
 
 func NewMaxDepthRule(cfg Config) {
 	if cfg.Enabled {
 		validator.AddRule("MaxDepth", func(observers *validator.Events, addError validator.AddErrFunc) {
 			observers.OnOperation(func(walker *validator.Walker, operation *ast.OperationDefinition) {
+				start := time.Now()
+
 				var maxDepth = countDepth(operation.SelectionSet)
 
 				if maxDepth > cfg.Max {
@@ -39,12 +42,12 @@ func NewMaxDepthRule(cfg Config) {
 							validator.Message(err),
 							validator.At(operation.Position),
 						)
-						resultCounter.WithLabelValues("rejected").Inc()
+						resultHistogram.WithLabelValues("rejected").Observe(time.Since(start).Seconds())
 					} else {
-						resultCounter.WithLabelValues("failed").Inc()
+						resultHistogram.WithLabelValues("failed").Observe(time.Since(start).Seconds())
 					}
 				} else {
-					resultCounter.WithLabelValues("allowed").Inc()
+					resultHistogram.WithLabelValues("allowed").Observe(time.Since(start).Seconds())
 				}
 			})
 		})

diff --git a/internal/business/persisted_operations/persisted_operations.go b/internal/business/persisted_operations/persisted_operations.go
@@ -19,7 +19,7 @@ import (
 )
 
 var (
-	persistedOpsCounter = prometheus.NewCounterVec(prometheus.CounterOpts{
+	persistedOpsHistogram = prometheus.NewHistogramVec(prometheus.HistogramOpts{
 		Namespace: "go_graphql_armor",
 		Subsystem: "persisted_operations",
 		Name:      "counter",
@@ -59,7 +59,7 @@ type Config struct {
 	Remote struct {
 		GcpBucket string `conf:"your_bucket_name" yaml:"gcp_bucket"`
 	}
-	FailUnknownOperations bool `conf:"default:false" yaml:"fail_unknown_operations"`
+	RejectOnFailure bool `conf:"default:false" yaml:"reject_on_failure"`
 }
 
 var ErrNoLoaderSupplied = errors.New("no remoteLoader supplied")
@@ -84,7 +84,7 @@ type PersistedOperationsHandler struct {
 }
 
 func init() {
-	prometheus.MustRegister(persistedOpsCounter, reloadCounter)
+	prometheus.MustRegister(persistedOpsHistogram, reloadCounter)
 }
 
 func NewPersistedOperations(log *slog.Logger, cfg Config, loader LocalLoader, remoteLoader RemoteLoader) (*PersistedOperationsHandler, error) {
@@ -149,6 +149,8 @@ func (p *PersistedOperationsHandler) Execute(next http.Handler) http.Handler { /
 			return
 		}
 
+		start := time.Now()
+
 		var errs gqlerror.List
 
 		payload, err := gql.ParseRequestPayload(r)
@@ -159,14 +161,14 @@ func (p *PersistedOperationsHandler) Execute(next http.Handler) http.Handler { /
 		}
 
 		for i, data := range payload {
-			if !p.cfg.FailUnknownOperations && data.Query != "" {
-				persistedOpsCounter.WithLabelValues("unknown", "allowed").Inc()
+			if !p.cfg.RejectOnFailure && data.Query != "" {
+				persistedOpsHistogram.WithLabelValues("unknown", "allowed").Observe(time.Since(start).Seconds())
 				continue
 			}
 
 			hash, err := hashFromPayload(data)
 			if err != nil {
-				persistedOpsCounter.WithLabelValues("unknown", "rejected").Inc()
+				persistedOpsHistogram.WithLabelValues("error", "rejected").Observe(time.Since(start).Seconds())
 				errs = append(errs, gqlerror.Wrap(ErrPersistedQueryNotFound))
 				continue
 			}
@@ -177,7 +179,7 @@ func (p *PersistedOperationsHandler) Execute(next http.Handler) http.Handler { /
 
 			if !ok {
 				// hash not found, fail
-				persistedOpsCounter.WithLabelValues("unknown", "rejected").Inc()
+				persistedOpsHistogram.WithLabelValues("unknown", "rejected").Observe(time.Since(start).Seconds())
 				errs = append(errs, gqlerror.Wrap(ErrPersistedOperationNotFound))
 				continue
 			}
@@ -186,7 +188,7 @@ func (p *PersistedOperationsHandler) Execute(next http.Handler) http.Handler { /
 			payload[i].Query = query
 			payload[i].Extensions.PersistedQuery = nil
 
-			persistedOpsCounter.WithLabelValues("known", "allowed").Inc()
+			persistedOpsHistogram.WithLabelValues("known", "allowed").Observe(time.Since(start).Seconds())
 		}
 
 		if len(errs) > 0 {

diff --git a/internal/business/persisted_operations/persisted_operations_test.go b/internal/business/persisted_operations/persisted_operations_test.go
@@ -44,8 +44,8 @@ func TestNewPersistedOperations(t *testing.T) {
 			name: "Allows unpersisted requests if configured",
 			args: args{
 				cfg: Config{
-					Enabled:               true,
-					FailUnknownOperations: true,
+					Enabled:         true,
+					RejectOnFailure: true,
 				},
 				payload: func() []byte {
 					data := gql.RequestData{
@@ -75,8 +75,8 @@ func TestNewPersistedOperations(t *testing.T) {
 			name: "Returns error if no hash match is found and unpersisted operations are not allowed",
 			args: args{
 				cfg: Config{
-					Enabled:               true,
-					FailUnknownOperations: false,
+					Enabled:         true,
+					RejectOnFailure: false,
 				},
 				payload: func() []byte {
 					data := gql.RequestData{
@@ -114,8 +114,8 @@ func TestNewPersistedOperations(t *testing.T) {
 			name: "Swaps in query payload if hash operation is known, updates content length accordingly",
 			args: args{
 				cfg: Config{
-					Enabled:               true,
-					FailUnknownOperations: false,
+					Enabled:         true,
+					RejectOnFailure: false,
 				},
 				payload: func() []byte {
 					data := gql.RequestData{
@@ -157,8 +157,8 @@ func TestNewPersistedOperations(t *testing.T) {
 			name: "Swaps in batched query payload if hash operation is known, updates content length accordingly",
 			args: args{
 				cfg: Config{
-					Enabled:               true,
-					FailUnknownOperations: false,
+					Enabled:         true,
+					RejectOnFailure: false,
 				},
 				payload: func() []byte {
 					data := []gql.RequestData{
@@ -203,8 +203,8 @@ func TestNewPersistedOperations(t *testing.T) {
 			name: "fails entire batch if one operation is unknown",
 			args: args{
 				cfg: Config{
-					Enabled:               true,
-					FailUnknownOperations: false,
+					Enabled:         true,
+					RejectOnFailure: false,
 				},
 				payload: func() []byte {
 					data := []gql.RequestData{

diff --git a/internal/business/tokens/tokens.go b/internal/business/tokens/tokens.go
@@ -5,9 +5,10 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/vektah/gqlparser/v2/ast"
 	"github.com/vektah/gqlparser/v2/lexer"
+	"time"
 )
 
-var resultCounter = prometheus.NewCounterVec(prometheus.CounterOpts{
+var resultHistogram = prometheus.NewHistogramVec(prometheus.HistogramOpts{
 	Namespace: "go_graphql_armor",
 	Subsystem: "max_tokens",
 	Name:      "results",
@@ -23,7 +24,7 @@ type Config struct {
 }
 
 func init() {
-	prometheus.MustRegister(resultCounter)
+	prometheus.MustRegister(resultHistogram)
 }
 
 type MaxTokensRule struct {
@@ -41,6 +42,8 @@ func (t *MaxTokensRule) Validate(source *ast.Source) error {
 		return nil
 	}
 
+	start := time.Now()
+
 	lex := lexer.New(source)
 	count := 0
 
@@ -60,11 +63,11 @@ func (t *MaxTokensRule) Validate(source *ast.Source) error {
 
 	if count > t.cfg.Max {
 		if t.cfg.RejectOnFailure {
-			resultCounter.WithLabelValues("rejected").Inc()
+			resultHistogram.WithLabelValues("rejected").Observe(time.Since(start).Seconds())
 			return fmt.Errorf("operation has exceeded maximum tokens. found [%d], max [%d]", count, t.cfg.Max)
 		}
-		resultCounter.WithLabelValues("failed").Inc()
+		resultHistogram.WithLabelValues("failed").Observe(time.Since(start).Seconds())
 	}
-	resultCounter.WithLabelValues("allowed").Inc()
+	resultHistogram.WithLabelValues("allowed").Observe(time.Since(start).Seconds())
 	return nil
 }