GraphQL Deepest Path Update (#291)

TimPansino · lrafeei · umaannamalai · TimPansino · commit 3d88d9077f48 · 2021-07-29T15:18:33.000-07:00
* GraphQL cleanup changes * Fix broken tests * Add ignored fields and introspection fields * Clean up to_source object function * Clean up existing exception tests Co-authored-by: lrafeei <lrafeei@users.noreply.github.com> * Fix default values in trace * Fix logic for parsing and transaction naming * Basic async test * Update deepest path behavior * GraphQL Cleanup (#290) * GraphQL cleanup changes * Fix broken tests * Add ignored fields and introspection fields * Clean up to_source object function * Clean up existing exception tests Co-authored-by: lrafeei <lrafeei@users.noreply.github.com> * Fix default values in trace * Fix logic for parsing and transaction naming * Basic async test * Fix failing tests and update txn group. * Update non-null resolver test. * Update non-null error case. * Seperate async testing to new file. * Drop support for graphql 2.1 (3 year support policy). * Remove 2.1 from test matrix. Co-authored-by: lrafeei <lrafeei@users.noreply.github.com> Co-authored-by: Uma Annamalai <uannamalai@newrelic.com> * GraphQL cleanup changes * Fix broken tests * Add ignored fields and introspection fields * Clean up existing exception tests Co-authored-by: lrafeei <lrafeei@users.noreply.github.com> * Fix logic for parsing and transaction naming * Basic async test * Update deepest path behavior * Update branch. * Modify deepest path logic in instrumentation and tests. * Remove asyncio dependency. Co-authored-by: lrafeei <lrafeei@users.noreply.github.com> Co-authored-by: Uma Annamalai <uannamalai@newrelic.com>
diff --git a/newrelic/api/graphql_trace.py b/newrelic/api/graphql_trace.py
@@ -57,7 +57,6 @@ def finalize_data(self, transaction, exc=None, value=None, tb=None):
         # Add attributes
         self._add_agent_attribute("graphql.operation.type", self.operation_type)
         self._add_agent_attribute("graphql.operation.name", self.operation_name)
-        self._add_agent_attribute("graphql.operation.deepestPath", self.deepest_path)
 
         # Attach formatted graphql
         limit = transaction.settings.agent_limits.sql_query_length_maximum
diff --git a/newrelic/core/attribute.py b/newrelic/core/attribute.py
@@ -77,7 +77,6 @@
         'graphql.field.path',
         'graphql.operation.name',
         'graphql.operation.type',
-        'graphql.operation.deepestPath',
         'graphql.operation.query',
 ))
 
diff --git a/newrelic/hooks/framework_graphql.py b/newrelic/hooks/framework_graphql.py
@@ -69,6 +69,7 @@ def wrap_executor_context_init(wrapped, instance, args, kwargs):
             instance.field_resolver = wrap_resolver(instance.field_resolver)
             instance.field_resolver._nr_wrapped = True
 
+
     return result
 
 
@@ -91,65 +92,84 @@ def wrap_execute_operation(wrapped, instance, args, kwargs):
     except TypeError:
         operation = bind_operation_v2(*args, **kwargs)
 
-    operation_name = operation.name
-    if hasattr(operation_name, "value"):
-        operation_name = operation_name.value
+    operation_name = get_node_value(operation, "name")
     trace.operation_name = operation_name = operation_name or "<anonymous>"
 
-    operation_type = operation.operation
-    if hasattr(operation_type, "name"):
-        operation_type = operation_type.name.lower()
+    operation_type = get_node_value(operation, "operation", "name").lower()
     trace.operation_type = operation_type = operation_type or "<unknown>"
 
     if operation.selection_set is not None:
         fields = operation.selection_set.selections
-        try:
-            deepest_path = traverse_deepest_path(fields)
-        except:
-            deepest_path = []
-        trace.deepest_path = deepest_path = ".".join(deepest_path) or "<unknown>"
+        # Ignore transactions for introspection queries
+        for field in fields:
+            if get_node_value(field, "name") in GRAPHQL_INTROSPECTION_FIELDS:
+                ignore_transaction()
 
-    transaction.set_transaction_name(callable_name(wrapped), "GraphQL", priority=11)
+        deepest_path = traverse_deepest_unique_path(fields)
+        trace.deepest_path = deepest_path = ".".join(deepest_path) or ""
 
+    transaction.set_transaction_name(callable_name(wrapped), "GraphQL", priority=11)
     result = wrapped(*args, **kwargs)
-    transaction_name = "%s/%s/%s" % (operation_type, operation_name, deepest_path)
+    transaction_name = "%s/%s/%s" % (operation_type, operation_name, deepest_path) if deepest_path else "%s/%s" % (operation_type, operation_name)
     transaction.set_transaction_name(transaction_name, "GraphQL", priority=14)
 
     return result
 
 
-def traverse_deepest_path(fields):
-    inputs = deque(((field, deque(), 0) for field in fields))
-    deepest_path_len = 0
-    deepest_path = []
-
-    while inputs:
-        field, current_path, current_path_len = inputs.pop()
-
-        field_name = field.name
-        if hasattr(field_name, "value"):
-            field_name = field_name.value
-
-        if field_name in GRAPHQL_INTROSPECTION_FIELDS:
-            # If an introspection query is identified, ignore the transaction completely.
-            # This matches the logic used in Apollo server for identifying introspection queries.
-            ignore_transaction()
-            return []
-        elif field_name not in GRAPHQL_IGNORED_FIELDS:
-            # Only add to the current path for non-ignored values.
-            current_path.append(field_name)
-            current_path_len += 1
-
-            if field.selection_set is None or len(field.selection_set.selections) == 0:
-                if deepest_path_len < current_path_len:
-                    deepest_path = current_path
-                    deepest_path_len = current_path_len
-            else:
-                for inner_field in field.selection_set.selections:
-                    inputs.append((inner_field, copy(current_path), current_path_len))
+def get_node_value(field, attr, subattr="value"):
+    field_name = getattr(field, attr, None)
+    if hasattr(field_name, subattr):
+        field_name = getattr(field_name, subattr)
+    return field_name
 
-    return deepest_path
 
+def is_fragment(field):
+    # Resolve version specific imports
+    try:
+        from graphql.language.ast import FragmentSpread, InlineFragment
+    except ImportError:
+        from graphql import FragmentSpreadNode as FragmentSpread, InlineFragmentNode as InlineFragment
+
+    _fragment_types = (InlineFragment, FragmentSpread)
+
+    return isinstance(field, _fragment_types)
+
+def is_named_fragment(field):
+    # Resolve version specific imports
+    try:
+        from graphql.language.ast import NamedType
+    except ImportError:
+        from graphql import NamedTypeNode as NamedType
+
+    return is_fragment(field) and getattr(field, "type_condition", None) is not None and isinstance(field.type_condition, NamedType)
+
+
+def traverse_deepest_unique_path(fields):
+    deepest_path = deque()
+
+    while fields is not None and len(fields) > 0:
+        fields = [f for f in fields if get_node_value(f, "name") not in GRAPHQL_IGNORED_FIELDS]
+        if len(fields) != 1:  # Either selections is empty, or non-unique
+            return deepest_path
+        field = fields[0]
+
+        field_name = get_node_value(field, "name")
+        if is_named_fragment(field):
+            name = get_node_value(field.type_condition, "name")
+            if name:
+                deepest_path.append("%s<%s>" % (deepest_path.pop(), name))
+        elif is_fragment(field):
+            break
+        else:
+            if field_name:
+                deepest_path.append(field_name)
+
+        if field.selection_set is None:
+            break
+        else:
+            fields = field.selection_set.selections
+
+    return deepest_path
 
 def bind_get_middleware_resolvers(middlewares):
     return middlewares
@@ -176,7 +196,6 @@ def wrap_middleware(wrapped, instance, args, kwargs):
             return wrapped(*args, **kwargs)
 
 
-
 def bind_get_field_resolver(field_resolver):
     return field_resolver
 
@@ -217,7 +236,9 @@ def wrap_resolver(wrapped, instance, args, kwargs):
     transaction = current_transaction()
     if transaction is None:
         return wrapped(*args, **kwargs)
+
     transaction.set_transaction_name(callable_name(wrapped), "GraphQL", priority=13)
+
     with ErrorTrace(ignore=ignore_graphql_duplicate_exception):
         return wrapped(*args, **kwargs)
 
@@ -250,8 +271,8 @@ def wrap_parse(wrapped, instance, args, kwargs):
     transaction = current_transaction()
     if transaction is None:
         return wrapped(*args, **kwargs)
-    transaction.set_transaction_name(callable_name(wrapped), "GraphQL", priority=10)
 
+    transaction.set_transaction_name(callable_name(wrapped), "GraphQL", priority=10)
     with ErrorTrace(ignore=ignore_graphql_duplicate_exception):
         return wrapped(*args, **kwargs)
 
@@ -333,7 +354,7 @@ def wrap_graphql_impl(wrapped, instance, args, kwargs):
         trace.statement = graphql_statement(query)
         with ErrorTrace(ignore=ignore_graphql_duplicate_exception):
             result = wrapped(*args, **kwargs)
-            #transaction.set_transaction_name(transaction_name, "GraphQL", priority=14)
+            # transaction.set_transaction_name(transaction_name, "GraphQL", priority=14)
             return result
 
 
diff --git a/tests/framework_graphql/_target_application.py b/tests/framework_graphql/_target_application.py
@@ -21,17 +21,64 @@
     GraphQLObjectType,
     GraphQLSchema,
     GraphQLString,
+    GraphQLUnionType,
 )
 
-libraries = [
+
+books = [
     {
         "id": 1,
-        "name": "NYC Public Library",
-        "book": [{"name": "A", "author": "B", "id": 1}, {"name": "C", "author": "D", "id": 2}],
+        "name": 'Python Agent: The Book',
+        "isbn": 'a-fake-isbn',
+        "author": 'Sentient Bits',
+        "branch": 'riverside'
+    },
+    {
+        "id": 2,
+        "name": 'Ollies for O11y: A Sk8er\'s Guide to Observability',
+        "isbn": 'a-second-fake-isbn',
+        "author": 'Faux Hawk',
+        "branch": 'downtown'
+    },
+    {
+        "id": 3,
+        "name": '[Redacted]',
+        "isbn": 'a-third-fake-isbn',
+        "author": 'Closed Telemetry',
+        "branch": 'riverside'
+    },
+]
+
+magazines = [
+    {
+        "id": 1,
+        "name": 'Reli Updates Weekly',
+        "issue": 1,
+        "branch": 'riverside'
+    },
+    {
+        "id": 2,
+        "name": 'Reli Updates Weekly',
+        "issue": 2,
+        "branch": 'downtown'
+    },
+    {
+        "id": 3,
+        "name": 'Node Weekly',
+        "issue": 1,
+        "branch": 'riverside'
     },
-    {"id": 2, "name": "Portland Public Library", "book": [{"name": "E", "author": "F", "id": 1}]},
 ]
 
+
+libraries = ['riverside', 'downtown']
+libraries = [{
+    "id": i + 1,
+    "branch": branch,
+    "magazine": [m for m in magazines if m["branch"] == branch],
+    "book": [b for b in books if b["branch"] == branch],
+} for i, branch in enumerate(libraries)]
+
 storage = []
 
 
@@ -45,22 +92,41 @@ def resolve_storage_add(parent, info, string):
 def resolve_storage(parent, info):
     return storage
 
+def resolve_search(parent, info, contains):
+    search_books = [b for b in books if contains in b["name"]]
+    search_magazines = [m for m in magazines if contains in m["name"]]
+    return search_books + search_magazines
+
 
 Book = GraphQLObjectType(
     "Book",
     {
         "id": GraphQLField(GraphQLInt),
         "name": GraphQLField(GraphQLString),
+        "isbn": GraphQLField(GraphQLString),
         "author": GraphQLField(GraphQLString),
+        "branch": GraphQLField(GraphQLString),
     },
 )
 
+Magazine = GraphQLObjectType(
+    "Magazine",
+    {
+        "id": GraphQLField(GraphQLInt),
+        "name": GraphQLField(GraphQLString),
+        "issue": GraphQLField(GraphQLInt),
+        "branch": GraphQLField(GraphQLString),
+    },
+)
+
+
 Library = GraphQLObjectType(
     "Library",
     {
         "id": GraphQLField(GraphQLInt),
         "name": GraphQLField(GraphQLString),
         "book": GraphQLField(GraphQLList(Book)),
+        "magazine": GraphQLField(GraphQLList(Book)),
     },
 )
 
@@ -86,6 +152,10 @@ def resolve_error(root, info):
         resolver=resolve_library,
         args={"index": GraphQLArgument(GraphQLNonNull(GraphQLInt))},
     )
+    search_field = GraphQLField(
+        GraphQLList(GraphQLUnionType("Item", (Book, Magazine), resolve_type=resolve_search)),
+        args={"contains": GraphQLArgument(GraphQLNonNull(GraphQLString))},
+    )
     echo_field = GraphQLField(
         GraphQLString,
         resolver=resolve_echo,
@@ -111,6 +181,10 @@ def resolve_error(root, info):
         resolve=resolve_library,
         args={"index": GraphQLArgument(GraphQLNonNull(GraphQLInt))},
     )
+    search_field = GraphQLField(
+        GraphQLList(GraphQLUnionType("Item", (Book, Magazine), resolve_type=resolve_search)),
+        args={"contains": GraphQLArgument(GraphQLNonNull(GraphQLString))},
+    )
     echo_field = GraphQLField(
         GraphQLString,
         resolve=resolve_echo,
@@ -135,6 +209,7 @@ def resolve_error(root, info):
     fields={
         "hello": hello_field,
         "library": library_field,
+        "search": search_field,
         "echo": echo_field,
         "storage": storage_field,
         "error": error_field,
diff --git a/tests/framework_graphql/test_application.py b/tests/framework_graphql/test_application.py
diff --git a/tests/framework_graphql/test_application_async.py b/tests/framework_graphql/test_application_async.py
