dd-trace-go
dd-trace-go copied to clipboard
Add option to gqlgen contrib for skipping spans for field resolvers w/o methods
Followup now that I've implemented https://github.com/DataDog/dd-trace-go/issues/2597 and started using this middleware; also somewhat related to https://github.com/DataDog/dd-trace-go/issues/2610 in that we're both aiming to reduce overly verbose traces from the same contrib package.
Problem: the contrib package for gqlgen
creates separate spans for each field being resolved, even if it can automatically be inferred by the fields on the struct (instead of requiring a separate function).
Consider the following GQL type:
type Book {
author: Author!
title: String!
}
Consider it being mapped to the following Golang type:
type Book struct {
AuthorID int
Title string
}
In this case, you would need to implement a function to resolve the book's author via the author ID; this is in contrast to the title, which gqlgen
can determine automatically.
Solution: another option, such as WithSkipFieldsWithoutMethods
, that would skip creating a span downstream of the InterceptField
method here: https://github.com/DataDog/dd-trace-go/blob/3a426cad6b642bc0dab36d49c02f8e09b40529e5/contrib/99designs/gqlgen/tracer.go#L139-L141
It would be checked with fieldCtx.IsMethod
; I'm happy to contribute and would be able to do it alongside #2610, also.
@samsullivan Thanks for reaching out! We'll take a look (cc @rarguelloF @zarirhamza). I'm a bit surprised about the fix created by Devin. Was it guided by you or did it create the PR completely autonomously?
Interesting, I didn't even notice the Devin AI actions until your comment. I am not involved and would be happy to handwrite something similar w/tests and w/o AI, if it sounds like a reasonable addition...
@samsullivan It sounds like a reasonable addition. We'll be glad to review it once it's ready.
I'm having a hard time testing this since I don't see a good way of forcing gqlgen's graphql.Fieldcontext
type to have IsMethod = true
with the middleware test library's current implementation of gqlgen in newTestClient
.
Nevermind; took a fresh look this morning and dug into the gqlgen
test server to figure out how to accurately test this case. Should be ready to review whenever, thanks! :+1: