From 1a55b960710e8860f2b9001dd0cb29537d17f264 Mon Sep 17 00:00:00 2001 From: David Ackroyd <23301187+dackroyd@users.noreply.github.com> Date: Thu, 1 Oct 2020 23:17:54 +1000 Subject: [PATCH] Add context to validation tracing Context is needed for tracing to access the current span, in order to add tags to it, or create child spans. As presently defined (without a context), this cannot be done: new spans could be created, but they would not be associated with the existing request trace. OpenTracingTracer implements the new interface (it never implemented the old one). Via this 'extension interface', the tracer configured (or the default tracer) will be used as the validation tracer if: * The tracer implements the (optional) interface; and * A validation tracer isn't provided using the deprecated option What this means is that the deprecated option is _preferred_ as an override. This allows users to migrate in a non-breaking, non-behaviour changing way, until such time as they intentionally remove the use of the deprecated option. For those who are currently using the default tracer, and not supplying a validation tracer, validation will be traced immediately with no change required to configuration options. --- graphql.go | 32 ++++++++++++++++++++++++-------- subscriptions.go | 2 +- trace/trace.go | 16 ++++++++++++++++ trace/validation_trace.go | 8 ++++++++ 4 files changed, 49 insertions(+), 9 deletions(-) diff --git a/graphql.go b/graphql.go index 61d0a4d9..a2f21f84 100644 --- a/graphql.go +++ b/graphql.go @@ -25,16 +25,23 @@ import ( // resolver, then the schema can not be executed, but it may be inspected (e.g. with ToJSON). func ParseSchema(schemaString string, resolver interface{}, opts ...SchemaOpt) (*Schema, error) { s := &Schema{ - schema: schema.New(), - maxParallelism: 10, - tracer: trace.OpenTracingTracer{}, - validationTracer: trace.NoopValidationTracer{}, - logger: &log.DefaultLogger{}, + schema: schema.New(), + maxParallelism: 10, + tracer: trace.OpenTracingTracer{}, + logger: &log.DefaultLogger{}, } for _, opt := range opts { opt(s) } + if s.validationTracer == nil { + if tracer, ok := s.tracer.(trace.ValidationTracerContext); ok { + s.validationTracer = tracer + } else { + s.validationTracer = &validationBridgingTracer{tracer: trace.NoopValidationTracer{}} + } + } + if err := s.schema.Parse(schemaString, s.useStringDescriptions); err != nil { return nil, err } @@ -68,7 +75,7 @@ type Schema struct { maxDepth int maxParallelism int tracer trace.Tracer - validationTracer trace.ValidationTracer + validationTracer trace.ValidationTracerContext logger log.Logger useStringDescriptions bool disableIntrospection bool @@ -117,9 +124,10 @@ func Tracer(tracer trace.Tracer) SchemaOpt { } // ValidationTracer is used to trace validation errors. It defaults to trace.NoopValidationTracer. +// Deprecated: context is needed to support tracing correctly. Use a Tracer which implements trace.ValidationTracerContext. func ValidationTracer(tracer trace.ValidationTracer) SchemaOpt { return func(s *Schema) { - s.validationTracer = tracer + s.validationTracer = &validationBridgingTracer{tracer: tracer} } } @@ -186,7 +194,7 @@ func (s *Schema) exec(ctx context.Context, queryString string, operationName str return &Response{Errors: []*errors.QueryError{qErr}} } - validationFinish := s.validationTracer.TraceValidation() + validationFinish := s.validationTracer.TraceValidation(ctx) errs := validation.Validate(s.schema, doc, variables, s.maxDepth) validationFinish(errs) if len(errs) != 0 { @@ -272,6 +280,14 @@ func (s *Schema) validateSchema() error { return nil } +type validationBridgingTracer struct { + tracer trace.ValidationTracer +} + +func (t *validationBridgingTracer) TraceValidation(context.Context) trace.TraceValidationFinishFunc { + return t.tracer.TraceValidation() +} + func validateRootOp(s *schema.Schema, name string, mandatory bool) error { t, ok := s.EntryPoints[name] if !ok { diff --git a/subscriptions.go b/subscriptions.go index 0709796a..013039bd 100644 --- a/subscriptions.go +++ b/subscriptions.go @@ -36,7 +36,7 @@ func (s *Schema) subscribe(ctx context.Context, queryString string, operationNam return sendAndReturnClosed(&Response{Errors: []*qerrors.QueryError{qErr}}) } - validationFinish := s.validationTracer.TraceValidation() + validationFinish := s.validationTracer.TraceValidation(ctx) errs := validation.Validate(s.schema, doc, variables, s.maxDepth) validationFinish(errs) if len(errs) != 0 { diff --git a/trace/trace.go b/trace/trace.go index 68b856ae..8d5d8a71 100644 --- a/trace/trace.go +++ b/trace/trace.go @@ -67,6 +67,22 @@ func (OpenTracingTracer) TraceField(ctx context.Context, label, typeName, fieldN } } +func (OpenTracingTracer) TraceValidation(ctx context.Context) TraceValidationFinishFunc { + span, _ := opentracing.StartSpanFromContext(ctx, "Validate Query") + + return func(errs []*errors.QueryError) { + if len(errs) > 0 { + msg := errs[0].Error() + if len(errs) > 1 { + msg += fmt.Sprintf(" (and %d more errors)", len(errs)-1) + } + ext.Error.Set(span, true) + span.SetTag("graphql.error", msg) + } + span.Finish() + } +} + func noop(*errors.QueryError) {} type NoopTracer struct{} diff --git a/trace/validation_trace.go b/trace/validation_trace.go index e223f0fd..bce7a9a4 100644 --- a/trace/validation_trace.go +++ b/trace/validation_trace.go @@ -1,17 +1,25 @@ package trace import ( + "context" + "github.com/graph-gophers/graphql-go/errors" ) type TraceValidationFinishFunc = TraceQueryFinishFunc +// Deprecated: use ValidationTracerContext. type ValidationTracer interface { TraceValidation() TraceValidationFinishFunc } +type ValidationTracerContext interface { + TraceValidation(ctx context.Context) TraceValidationFinishFunc +} + type NoopValidationTracer struct{} +// Deprecated: use a Tracer which implements ValidationTracerContext. func (NoopValidationTracer) TraceValidation() TraceValidationFinishFunc { return func(errs []*errors.QueryError) {} }