contrib/99designs/gqlgen: additional options to reduce noise

[samsullivan]

What does this PR do?

Adds two new configuration options to contrib/99designs/gqlgen, which have a similar end goal of reducing noise:

• SkipFieldsForIntrospectionQuery
  • if the OperationName is IntrospectionQuery, spans are not created for each field
• SkipFieldsWithTrivialResolver
  • if the field resolver is trivial (doesn't require a function / can be accessed directly from the parent object), spans are not created

Motivation

We started using contrib/99designs/gqlgen middleware recently and noticed it creates way more spans than we intended, most of which have little value for our needs.

Fixes #2684. Fixes #2610.

Reviewer's Checklist

• Changed code has unit tests for its functionality at or near 100% coverage.
• System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
• There is a benchmark for any new code, or changes to existing code.
• If this interacts with the agent in a new way, a system test has been added.
• Add an appropriate team label so this PR gets put in the right place for the release notes.
• Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

[samsullivan]

TODO: Is this accurate?

Seems like it to me:

• https://graphql.org/learn/execution/#trivial-resolvers
• https://github.com/99designs/gqlgen/blob/master/graphql/context_field.go#L29-L32

[samsullivan]

Alternate naming: WithOmitTrivialFields and WithOmitIntrospectionQuery 🤷

[seh]

Alternate naming: WithOmitTrivialFields and WithOmitIntrospectionQuery 🤷

How about WithTrivialFieldsOmitted and WithIntrospectionQueriesOmitted?

What else remains to settle in order to merge this patch?

[samsullivan]

Alternate naming: WithOmitTrivialFields and WithOmitIntrospectionQuery 🤷

How about WithTrivialFieldsOmitted and WithIntrospectionQueriesOmitted?

Can push tomorrow.

What else remains to settle in order to merge this patch?

Nothing but review, as far as I'm aware.

[samsullivan]

@darccio I was just signing on for the day; did you have any opinion on the naming?

[darccio]

@darccio I was just signing on for the day; did you have any opinion on the naming?

Being honest, I would drop the With from both options, but I decided to keep them as you all seemed comfortable with them 😁 If you meant between Skip and Omit I prefer Skip at the beginning of the name, so the user can be sure about what it does (and Skip sounds more clear to me than Omit).

[samsullivan]

@darccio I was just signing on for the day; did you have any opinion on the naming?

Being honest, I would drop the With from both options, but I decided to keep them as you all seemed comfortable with them 😁 If you meant between Skip and Omit I prefer Skip at the beginning of the name, so the user can be sure about what it does (and Skip sounds more clear to me than Omit).

Agreed, but had simply been trying to follow other patterns existing elsewhere in the repo; pushed to shorten these by removing With prefix (fns start w/Skip).

[rarguelloF]

Overall LGTM! Just a small change to keep the existing options naming convention.

[samsullivan]

Thanks for the review @rarguelloF; should be good to go now 👍

[samsullivan]

Thanks!