Skip to content

Commit

Permalink
Add field resolver returnType span attribute (#300)
Browse files Browse the repository at this point in the history
* Add field.returnType span attr and update error logic.

* Change solr docker image to pass health checks.
  • Loading branch information
umaannamalai authored and TimPansino committed Jul 29, 2021
1 parent f1c6af2 commit 2c83422
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 20 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ jobs:

services:
solr:
image: bitnami/solr:8
image: bitnami/solr:8.8.2
env:
SOLR_CORE: collection
ports:
Expand Down
1 change: 1 addition & 0 deletions newrelic/core/attribute.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
'graphql.field.name',
'graphql.field.parentType',
'graphql.field.path',
'graphql.field.returnType',
'graphql.operation.name',
'graphql.operation.type',
'graphql.operation.query',
Expand Down
28 changes: 19 additions & 9 deletions newrelic/hooks/framework_graphql.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,14 @@ def ignore_graphql_duplicate_exception(exc, val, tb):
# after the original exception has already been recorded.

if transaction and hasattr(val, "original_error"):
_, _, fullnames, message = parse_exc_info((None, val.original_error, None))
while hasattr(val, "original_error"):
# Unpack lowest level original error
val = val.original_error

_, _, fullnames, message = parse_exc_info((None, val, None))
fullname = fullnames[0]
for error in transaction._errors:
if error.message == message:
if error.type == fullname and error.message == message:
return True

return None # Follow original exception matching rules
Expand Down Expand Up @@ -93,6 +97,11 @@ def wrap_execute_operation(wrapped, instance, args, kwargs):
except TypeError:
operation = bind_operation_v2(*args, **kwargs)

if graphql_version() < (3, 0, 0):
execution_context = args[0]
else:
execution_context = instance

operation_name = get_node_value(operation, "name")
trace.operation_name = operation_name = operation_name or "<anonymous>"

Expand All @@ -106,12 +115,7 @@ def wrap_execute_operation(wrapped, instance, args, kwargs):
if get_node_value(field, "name") in GRAPHQL_INTROSPECTION_FIELDS:
ignore_transaction()

if graphql_version() <= (3, 0, 0):
fragments = args[
0
].fragments # In v2, args[0] is the ExecutionContext object
else:
fragments = instance.fragments # instance is the ExecutionContext object
fragments = execution_context.fragments
deepest_path = traverse_deepest_unique_path(fields, fragments)
trace.deepest_path = deepest_path = ".".join(deepest_path) or ""

Expand All @@ -122,7 +126,9 @@ def wrap_execute_operation(wrapped, instance, args, kwargs):
if deepest_path
else "%s/%s" % (operation_type, operation_name)
)
transaction.set_transaction_name(transaction_name, "GraphQL", priority=14)

if not execution_context.errors:
transaction.set_transaction_name(transaction_name, "GraphQL", priority=14)

return result

Expand Down Expand Up @@ -359,10 +365,14 @@ def wrap_resolve_field(wrapped, instance, args, kwargs):
parent_type, field_asts, field_path = bind_resolve_field(*args, **kwargs)

field_name = field_asts[0].name.value
field_def = parent_type.fields.get(field_name)
field_return_type = str(field_def.type) if field_def else "<unknown>"

with GraphQLResolverTrace(field_name) as trace:
with ErrorTrace(ignore=ignore_graphql_duplicate_exception):
trace._add_agent_attribute("graphql.field.parentType", parent_type.name)
trace._add_agent_attribute("graphql.field.returnType", field_return_type)

if isinstance(field_path, list):
trace._add_agent_attribute("graphql.field.path", field_path[0])
else:
Expand Down
25 changes: 16 additions & 9 deletions tests/framework_graphql/test_application.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def error_middleware(next, root, info, **args):


@dt_enabled
def test_basic(app, graphql_run):
def test_basic(app, graphql_run, is_graphql_2):
from graphql import __version__ as version

FRAMEWORK_METRICS = [
Expand Down Expand Up @@ -114,6 +114,7 @@ def test_basic(app, graphql_run):
"graphql.field.name": "storage_add",
"graphql.field.parentType": "Mutation",
"graphql.field.path": "storage_add",
"graphql.field.returnType": "[String]" if is_graphql_2 else "String",
}
_expected_query_operation_attributes = {
"graphql.operation.type": "query",
Expand All @@ -123,6 +124,7 @@ def test_basic(app, graphql_run):
"graphql.field.name": "storage",
"graphql.field.parentType": "Query",
"graphql.field.path": "storage",
"graphql.field.returnType": "[String]",
}

@validate_transaction_metrics(
Expand Down Expand Up @@ -189,14 +191,15 @@ def test_exception_in_middleware(app, graphql_run):
_test_exception_rollup_metrics = [
("Errors/all", 1),
("Errors/allOther", 1),
("Errors/OtherTransaction/GraphQL/query/MyQuery/%s" % field, 1),
("Errors/OtherTransaction/GraphQL/test_application:error_middleware", 1),
] + _test_exception_scoped_metrics

# Attributes
_expected_exception_resolver_attributes = {
"graphql.field.name": field,
"graphql.field.parentType": "Query",
"graphql.field.path": field,
"graphql.field.returnType": "String",
}
_expected_exception_operation_attributes = {
"graphql.operation.type": "query",
Expand All @@ -205,7 +208,7 @@ def test_exception_in_middleware(app, graphql_run):
}

@validate_transaction_metrics(
"query/MyQuery/hello",
"test_application:error_middleware",
"GraphQL",
scoped_metrics=_test_exception_scoped_metrics,
rollup_metrics=_test_exception_rollup_metrics + _graphql_base_rollup_metrics,
Expand All @@ -224,13 +227,10 @@ def _test():

@pytest.mark.parametrize("field", ("error", "error_non_null"))
@dt_enabled
def test_exception_in_resolver(app, graphql_run, is_graphql_2, field):
def test_exception_in_resolver(app, graphql_run, field):
query = "query MyQuery { %s }" % field

if is_graphql_2 and field == "error_non_null":
txn_name = "_target_application:resolve_error"
else:
txn_name = "query/MyQuery/%s" % field
txn_name = "_target_application:resolve_error"

# Metrics
_test_exception_scoped_metrics = [
Expand All @@ -248,6 +248,7 @@ def test_exception_in_resolver(app, graphql_run, is_graphql_2, field):
"graphql.field.name": field,
"graphql.field.parentType": "Query",
"graphql.field.path": field,
"graphql.field.returnType": "String!" if "non_null" in field else "String",
}
_expected_exception_operation_attributes = {
"graphql.operation.type": "query",
Expand Down Expand Up @@ -366,6 +367,7 @@ def test_field_resolver_metrics_and_attrs(app, graphql_run):
"graphql.field.name": "hello",
"graphql.field.parentType": "Query",
"graphql.field.path": "hello",
"graphql.field.returnType": "String",
}

@validate_transaction_metrics(
Expand Down Expand Up @@ -462,8 +464,13 @@ def _test():
@dt_enabled
@pytest.mark.parametrize("query,expected_path", _test_queries)
def test_deepest_unique_path(app, graphql_run, query, expected_path):
if expected_path == "/error":
txn_name = "_target_application:resolve_error"
else:
txn_name = "query/<anonymous>%s" % expected_path

@validate_transaction_metrics(
"query/<anonymous>%s" % expected_path,
txn_name,
"GraphQL",
background_task=True,
)
Expand Down
6 changes: 5 additions & 1 deletion tests/framework_graphql/test_application_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
from testing_support.validators.validate_span_events import validate_span_events
from newrelic.api.background_task import background_task

from test_application import is_graphql_2


@pytest.fixture(scope="session")
def graphql_run_async():
Expand All @@ -25,7 +27,7 @@ def graphql_run(*args, **kwargs):


@dt_enabled
def test_basic_async(app, graphql_run_async):
def test_basic_async(app, graphql_run_async, is_graphql_2):
from graphql import __version__ as version

FRAMEWORK_METRICS = [
Expand Down Expand Up @@ -53,6 +55,7 @@ def test_basic_async(app, graphql_run_async):
"graphql.field.name": "storage_add",
"graphql.field.parentType": "Mutation",
"graphql.field.path": "storage_add",
"graphql.field.returnType": "[String]" if is_graphql_2 else "String",
}
_expected_query_operation_attributes = {
"graphql.operation.type": "query",
Expand All @@ -62,6 +65,7 @@ def test_basic_async(app, graphql_run_async):
"graphql.field.name": "storage",
"graphql.field.parentType": "Query",
"graphql.field.path": "storage",
"graphql.field.returnType": "[String]",
}

@validate_transaction_metrics(
Expand Down

0 comments on commit 2c83422

Please sign in to comment.