From efe64f80a639388e19c1da04e1e5879a98eb201d Mon Sep 17 00:00:00 2001 From: Lars de Bruijn <9264036+ldebruijn@users.noreply.github.com> Date: Mon, 22 Jan 2024 18:42:39 +0100 Subject: [PATCH] =?UTF-8?q?feat:=20Change=20metrics=20for=20specific=20rul?= =?UTF-8?q?es=20to=20histograms,=20rename=20option=20=E2=80=A6=20(#42)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- cmd/main.go | 19 +++++-------- cmd/main_test.go | 6 ++--- docs/configuration.md | 2 +- docs/persisted_operations.md | 10 +++---- internal/business/aliases/aliases.go | 27 +++++++++++-------- internal/business/max_depth/max_depth.go | 13 +++++---- .../persisted_operations.go | 18 +++++++------ .../persisted_operations_test.go | 20 +++++++------- internal/business/tokens/tokens.go | 13 +++++---- 9 files changed, 67 insertions(+), 61 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index cc7bbef..e5ce177 100644 --- 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 index 1641700..236d6f8 100644 --- 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 index 067d6db..f6b7fd6 100644 --- 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 index 2987d28..4ea52bb 100644 --- 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 index e43ce42..0779617 100644 --- 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 index 4964721..4b6e3c2 100644 --- 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 index 4d20583..9d67caf 100644 --- 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 index b6ae470..28cad8b 100644 --- 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 index 743dd80..d00f1ad 100644 --- 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 }