Skip to content

Conversation

mabdinur
Copy link
Contributor

@mabdinur mabdinur commented Apr 29, 2022

Graphql-core Integration

Overview

This integration supports graphql-core 3 and graphql-core 2. By transitive dependencies executing graphql schemas in graphene>2.0 is also supported. This integration provides instrumentation for executing graphql queries using graphql.graphql(), graphql.graphql_sync(), graphql.execute(), and graphql.execution.execute(), graphql.execution.executor.execute(), graphql.execute_sync(), and graphql.execution.execute_sync().

Resolves #925.

Implementation Details

graphql-core this integration provides support for parsing, building, validating, and executing graphql schemas. It also provides additional insight into resolving GraphQLFields.

The methods listed below are the main parts of the public api which are fully supported by the ddtrace library. All other methods listed in graphql.init will not produce any instrumentation.

graphql.graphql() and graphql.graphql_sync()

When graphql.graphql() or graphql.graphql_sync (in graphql-core>=3) is called the following trace is produced:

  • span name: graphql.query, resource: graphql source string
    • span name: graphql.parse, resource: graphql.parse
      converts the body of an incoming graphql request into a graphql document
    • span name: graphql.validate, resource: graphql.validate
      ensures a graphql schema is syntactically correct and can be executed in a given context
    • span name: graphql.execute, resource: graphql source string
      executes a valid graphql schema and returns a response
      • span name: graphql.resolve, resource: name of graphql field
      • span name: graphql.resolve, resource: name of graphql field
      • ........ (a span is produced for every resolved field)
        Graphql queries can be complex and often resolve multiple Graphql fields in single request. Instrumenting resolvers provides insight on the execution of individual graphql fields and can highlight bottlenecks in fetching specific types of data. However for some applications this can produce a large number of spans.

Note: If a GraphqlError is raised while parsing or validating a graphql query then graphql.execute will not be called and instrumented. For this reason we add the graphql source string to the graphql.query span AND the graphql.execute span.

graphql.execute() and graphql.execution.execute()

When execute() is called the following trace is produced:

  • span name: graphql.execute, resource: graphql source string
    • span name: graphql.resolve, resource: name of graphql field
    • span name: graphql.resolve, resource: name of graphql field
    • ........ (a span is produced for every resolved field)

Note: graphql-core>=3.1 introduced execute_sync. Using graphql.execute_sync() and graphql.execution.execute_sync() should produce the same trace as graphql.execute().

Future Work

  • add graphql.source, graphql.operation.type and graphql.operation.name span tags to maintain feature parity with the .NET tracer.
  • instrument the execution of graphql subscriptions in graphql-core 3. Due to how we patch graphql.execution.executor.execute() in graphql-core 2 this version of the library generates graphql.execute and graphql.resolver spans when subscriptions are executed. For consistency we should extend this functionality to graphql-core 3 and offically support graphql subscriptions.
    • sample patching:
       # patch execute function used in subscribe
       if graphql_version < (3,0):
           # graphql.execution.executor.execute() is used in graphql.execution.executor.subscribe()
           # this method is patched above.
           pass
       elif graphql_version < (3,2):
           _w("graphql.subscription.subscribe", "execute", _traced_operation("graphql.execute"))
       else:
           _w("graphql.execution.subscribe", "execute", _traced_operation("graphql.execute"))
      
  • write tests for executing graphql subscriptions (graphql-core 2 and graphql-core 3)

Checklist

  • Added to the correct milestone.
  • Tests provided or description of manual testing performed is included in the code or PR.
  • Library documentation is updated.
  • Corp site documentation is updated (link to the PR).

@mabdinur mabdinur force-pushed the graphql-support branch 5 times, most recently from 6eb3d42 to 5446941 Compare May 5, 2022 19:33
@mabdinur mabdinur changed the title feat: add graphql integration [wip - do not review] feat: add graphql-core integration May 5, 2022
@mabdinur mabdinur force-pushed the graphql-support branch 3 times, most recently from b90e8a1 to 3dd8cbe Compare May 5, 2022 22:26
@mabdinur mabdinur marked this pull request as ready for review May 5, 2022 22:29
@mabdinur mabdinur requested a review from a team as a code owner May 5, 2022 22:29
assert len(spans) == 1
assert spans[0].name == "graphql"
assert spans[0].resource == test_source
assert spans[0].service == "graphql"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I plan to increase test coverage if this general approach looks good. I'll also add a sample django-graphene app with snapshot tests

@mabdinur mabdinur requested a review from brettlangdon May 6, 2022 17:48
@mabdinur mabdinur force-pushed the graphql-support branch 3 times, most recently from 6c81d91 to c7bb795 Compare May 6, 2022 19:44
@mabdinur mabdinur force-pushed the graphql-support branch 4 times, most recently from b10f07d to 35b530c Compare May 9, 2022 16:05
@mabdinur mabdinur requested a review from Kyle-Verhoog May 9, 2022 16:06
@mabdinur mabdinur force-pushed the graphql-support branch 7 times, most recently from bb2cfeb to 16dfc0f Compare May 10, 2022 00:59
@mabdinur mabdinur removed the request for review from paullegranddc May 18, 2022 14:37
@EtienneLamoureux
Copy link

EtienneLamoureux commented May 20, 2022

I'm very excited for this change. Will instrumentation of all GraphQL APIs come free with using ddtrace-run or will we have to annotate the resolvers in some fashion?

@mabdinur mabdinur force-pushed the graphql-support branch 3 times, most recently from a75bffd to 1c27a34 Compare May 23, 2022 14:06
@mabdinur mabdinur force-pushed the graphql-support branch 3 times, most recently from 5ed26c5 to 209d645 Compare May 23, 2022 17:48
@mabdinur
Copy link
Contributor Author

I'm very excited for this change. Will instrumentation of all GraphQL APIs come free with using ddtrace-run or will we have to annotate the resolvers in some fashion?

Hi @EtienneLamoureux, thanks for reaching out. The current plan is to instrument all resolvers using ddtrace-run but this approach could be noisy for some customers. If we decide not to auto instrument all resolvers in the initial release of this integration you could use @tracer.wrap() to instrument individual resolvers.

Would you like to test out a POC of this integration and let us know what you think? We'd greatly appreciate any feedback.

@EtienneLamoureux
Copy link

EtienneLamoureux commented May 25, 2022

Hi @mabdinur,

Manually tagging my resolvers is perfectly acceptable for my use-case, no problem.

And I'd gladly test a POC of this feature! What's the best way to get a hold of a build with it in it?

@mabdinur
Copy link
Contributor Author

Hi @mabdinur,

Manually tagging my resolvers is perfectly acceptable for my use-case, no problem.

And I'd gladly test a POC of this feature! What's the best way to get a hold of a build with it in it?

Hey @EtienneLamoureux, sorry about the delay. Here's a link to our public slack channel: https://chat.datadoghq.com/. Feel free to join and we can continue our conversation there. Thanks again helping us test the graphql integration 😊

@mabdinur mabdinur force-pushed the graphql-support branch 3 times, most recently from 409211d to 6476619 Compare June 9, 2022 18:01
_w("graphql", "execute_sync", _traced_operation("graphql.execute"))
_w("graphql.execution", "execute_sync", _traced_operation("graphql.execute"))

# patch all resolvers
Copy link
Contributor Author

@mabdinur mabdinur Jun 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of monkey patching resolve_field I could instead pass a trace middleware to graphql.execute(). This middleware would wrap resolvers in a span when ExecutionContext.resolve_field is called. Both approaches produce equivalent instrumentation. Although using a trace middleware could provide more accurate span durations it comes with some performance costs. We will need to iterated over existing middleware and apply the TracedResolverMiddleware everytime graphql.execute is called. I think the current monkey patching approach is easier to follow.

ex:

import graphql

class TracedResolverMiddleware(object):
    def resolve(self, next, root, info, **args):
        pin = Pin.get_from(graphql)
        if not pin or not pin.enabled():
            return next(root, info, **args)

        with pin.tracer.trace(
            name="graphql.resolve", 
            resource=info.field_name,
            service=trace_utils.int_service(pin, config.graphql),
            span_type=SpanTypes.WEB,
        ) as span:
            _init_span(span)
            return next(root, info, **args)

# wraps all resolvers in the schema
result = graphql.execute(schema, source, middleware=[TracedResolverMiddleware()])

@mabdinur
Copy link
Contributor Author

The graphql integration will be introduced in the following PR: #3878

@mabdinur mabdinur closed this Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add graphql support
4 participants