Repository: mirumee/ariadne
Pull request #608 (2021-06-25 19:08:52)

Added option to trace default resolvers as well
Adding the option to trace default resolvers if someone want. This is actually very useful in tracing base field types for which resolvers are not explicitly set because those can be use in apollo studio to see which fields are being accessed and how much thereby helping api lifecycle management

nilansaha: Hi @rafalp I was wondering if you had time to take a quick look at this. IMO this is quite useful when people want to collect stats on which fields were accessed even for default resolvers and the way I added it does not cause trouble for the existing way.
rafalp: `is_default_resolver` function call should only happen if `trace_default_resolver` is set to true. Its micro-optimization, but those are justified during query execution.

Recently I've started wondering if we should use GraphQL middlewares for features like this, or if it would be better to have functions that wrap resolvers instead, eg:

```python
schema = make_executable_schema(types, resolvers)
schema = add_tracing_to_schema(schema)  # recursively walk schema and wrap resolvers in tracing logic
```

This approach would save us the need to check if resolution should be traced during query time, and enable interesting micro-optimization were we could use separate tracing logic for resolvers taking kwargs and those without kwargs, but I feel that benchmark suite should be a starting point to that.
nilansaha: Why would `is_default_resolver` function call only happen if `trace_default_resolver` is set to `true`. The default behaviour was to check if `is_default_resolver` is true and then skip tracing that query. Even if `trace_default_resolver` is `false` we need to call `is_default_resolver` so we can skip default resolver queries. Maybe I am missing something ?
rafalp: Actually it should be opposite and `trace_default_resolver` call should only happen if `trace_default_resolver == False`. Because its pointless to check if resolver is default if we are going to trace it anyway.
nilansaha: Ah good catch. Just updated the PR and this should be good to go. Let me know if there is anything else.
nilansaha: Can we get this reviewed please ? @rafalp  PR has been open for ages. I was hoping to add federated tracing after this and I think this is a very useful component in that direction
rafalp: @nilansaha can you please rebase on master and post master benchmarks vs your branch benchmarks? I am thinking I'll merge this, but I would like to also see the numbers.

Patch:
diff --git a/ariadne/contrib/tracing/apollotracing.py b/ariadne/contrib/tracing/apollotracing.py
index 3694474..edb634c 100644
--- a/ariadne/contrib/tracing/apollotracing.py
+++ b/ariadne/contrib/tracing/apollotracing.py
@@ -23,7 +23,8 @@ TIMESTAMP_FORMAT = "%Y-%m-%dT%H:%M:%S.%fZ"
 
 
 class ApolloTracingExtension(Extension):
-    def __init__(self):
+    def __init__(self, trace_default_resolver: bool = False):
+        self.trace_default_resolver = trace_default_resolver
         self.start_date = None
         self.start_timestamp = None
         self.resolvers = []
@@ -37,7 +38,7 @@ class ApolloTracingExtension(Extension):
     async def resolve(
         self, next_: Resolver, parent: Any, info: GraphQLResolveInfo, **kwargs
     ):
-        if not should_trace(info):
+        if not should_trace(info, self.trace_default_resolver):
             result = next_(parent, info, **kwargs)
             if isawaitable(result):
                 result = await result
diff --git a/ariadne/contrib/tracing/utils.py b/ariadne/contrib/tracing/utils.py
index c5594a1..93f48a9 100644
--- a/ariadne/contrib/tracing/utils.py
+++ b/ariadne/contrib/tracing/utils.py
@@ -11,16 +11,16 @@ def format_path(path: ResponsePath):
     return elements[::-1]
 
 
-def should_trace(info: GraphQLResolveInfo):
+def should_trace(info: GraphQLResolveInfo, trace_default_resolver: bool = False):
     if info.field_name not in info.parent_type.fields:
         return False
 
     resolver = info.parent_type.fields[info.field_name].resolve
-    return not (
-        resolver is None
-        or is_default_resolver(resolver)
-        or is_introspection_field(info)
-    )
+    if trace_default_resolver:
+        default_resolver_bool = False
+    else:
+        default_resolver_bool = is_default_resolver(resolver)
+    return not (default_resolver_bool or is_introspection_field(info))
 
 
 def is_introspection_field(info: GraphQLResolveInfo):
diff --git a/ariadne/resolvers.py b/ariadne/resolvers.py
index e01bdbc..de60b93 100644
--- a/ariadne/resolvers.py
+++ b/ariadne/resolvers.py
@@ -61,6 +61,6 @@ def resolve_to(field_name: str) -> Resolver:
 
 def is_default_resolver(resolver: Resolver) -> bool:
     # pylint: disable=comparison-with-callable
-    if resolver == default_field_resolver:
+    if resolver == default_field_resolver or not resolver:
         return True
     return hasattr(resolver, "_ariadne_alias_resolver")


Test patch:
diff --git a/tests/tracing/test_utils.py b/tests/tracing/test_utils.py
index 81fa12a..1cdf6a7 100644
--- a/tests/tracing/test_utils.py
+++ b/tests/tracing/test_utils.py
@@ -48,7 +48,7 @@ def test_introspection_field_is_excluded_from_tracing():
     assert not should_trace(info)
 
 
-def test_field_with_default_resolver_is_excluded_from_tracing():
+def test_field_with_default_resolver_is_excluded_from_tracing_by_default():
     path = Mock(key="name", prev=Mock(key="user", prev=None))
     info = Mock(
         field_name="name",
@@ -58,6 +58,16 @@ def test_field_with_default_resolver_is_excluded_from_tracing():
     assert not should_trace(info)
 
 
+def test_field_with_default_resolver_is_included_in_tracing_when_set():
+    path = Mock(key="name", prev=Mock(key="user", prev=None))
+    info = Mock(
+        field_name="name",
+        path=path,
+        parent_type=Mock(fields={"name": Mock(resolve=None)}),
+    )
+    assert should_trace(info, trace_default_resolver=True)
+
+
 def test_field_with_custom_resolver_is_included_in_tracing():
     path = Mock(key="name", prev=Mock(key="user", prev=None))
     info = Mock(