Return context.Context from FieldMiddleware

[vvakame]

I'm trying to use OpenCensus for query tracing.
Problem is gqlgen can't propagate tracing span to next resolver.
We can organize resolver tracing to tree structure after this PR merged.

I think same issue has in OpenTracing at here

example stack with this PR

I have:

• Added tests covering the bug / feature (see testing)
• Updated any relevant documentation (see docs)

package gqlopencensus

import (
	"context"
	"fmt"

	"github.com/99designs/gqlgen/graphql"
	"go.opencensus.io/trace"
	"encoding/json"
)

// ResolverMiddleware add tracing for resolver.
func ResolverMiddleware() graphql.FieldMiddleware {
	return func(ctx context.Context, next graphql.Resolver) (context.Context, interface{}, error) {
		rctx := graphql.GetResolverContext(ctx)
		ctx, span := trace.StartSpan(ctx, rctx.Object+"."+rctx.Field.Name)
		defer span.End()

		span.AddAttributes(
			trace.StringAttribute("resolver.object", rctx.Object),
			trace.StringAttribute("resolver.field", rctx.Field.Name),
			trace.StringAttribute("resolver.path", fmt.Sprintf("%+v", rctx.Path())),
		)

		res, err := next(ctx)
		return ctx, res, err
	}
}

// RequestMiddleware add tracing for request.
func RequestMiddleware() graphql.RequestMiddleware {
	return func(ctx context.Context, next func(ctx context.Context) []byte) []byte {
		requestContext := graphql.GetRequestContext(ctx)
		requestName := "nameless-operation"
		if len(requestContext.Doc.Operations) != 0 {
			op := requestContext.Doc.Operations[0]
			if op.Name != "" {
				requestName = op.Name
			}
		}

		ctx, span := trace.StartSpan(ctx, requestName)
		defer span.End()

		var variables string
		b, err := json.Marshal(requestContext.Variables)
		if err != nil {
			variables = "can't marshal to JSON"
		} else {
			variables = string(b)
		}

		span.AddAttributes(
			trace.StringAttribute("request.query", requestContext.RawQuery),
			trace.StringAttribute("request.variables", variables),
		)

		return next(ctx)
	}
}

[vektah]

Lets try and keep API breaks off master until we are ready to shop the next major release, maybe bring back the next branch?

@mathewbyrne have any thoughts?

[vektah]

Why was this required?

[vvakame]

I changed to return values 1 to 2. but generated code will return return graphql.Null.
this immidiately execute function is wrapping it.

[vektah]

Its probably preferable to update ctx rather than shadowing it and disabling a bunch of linter checks?

[vvakame]

It sees that there is not much value for the this hard work... 🤔

[vektah]

Does this interface mismatch imply resolvers cant change their childs context either? Should we be thinking a little broader than just directives?

[vvakame]

It is an interesting point.
Does resolvers and directives need to modify context?
For Resolver, I can not make an idea that use case.
For directive, It doesn't have tree structure, because it doesn't need to modify context. I think directive middleware may be good.

[vvakame]

👀

[vetcher]

May be, somehow, we can add second type for middleware? One with context and the second without?

[mathewbyrne]

Closing this as I think it's been addressed in #353. @vvakame feel free to reopen if you think the other change hasn't quite covered your use-case.