diff --git a/instrumentation/graphql-java-16.2/build.gradle b/instrumentation/graphql-java-16.2/build.gradle index 5ba4bf563c..490ba8b00c 100644 --- a/instrumentation/graphql-java-16.2/build.gradle +++ b/instrumentation/graphql-java-16.2/build.gradle @@ -5,9 +5,12 @@ dependencies { testImplementation 'org.junit.jupiter:junit-jupiter-api:5.7.2' testImplementation 'org.junit.jupiter:junit-jupiter-params:5.7.2' + testImplementation 'org.mockito:mockito-core:4.6.1' + testImplementation 'org.mockito:mockito-junit-jupiter:4.6.1' testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.7.2' - testRuntimeOnly 'org.junit.vintage:junit-vintage-engine:5.7.2'} + testRuntimeOnly 'org.junit.vintage:junit-vintage-engine:5.7.2' +} repositories { mavenCentral() diff --git a/instrumentation/graphql-java-16.2/src/main/java/com/nr/instrumentation/graphql/GraphQLSpanUtil.java b/instrumentation/graphql-java-16.2/src/main/java/com/nr/instrumentation/graphql/GraphQLSpanUtil.java index 2e56c666ee..c0c9dd7215 100644 --- a/instrumentation/graphql-java-16.2/src/main/java/com/nr/instrumentation/graphql/GraphQLSpanUtil.java +++ b/instrumentation/graphql-java-16.2/src/main/java/com/nr/instrumentation/graphql/GraphQLSpanUtil.java @@ -11,7 +11,8 @@ import graphql.execution.ExecutionStrategyParameters; import graphql.language.Document; import graphql.language.OperationDefinition; -import graphql.schema.GraphQLObjectType; +import graphql.schema.GraphQLNamedSchemaElement; +import graphql.schema.GraphQLOutputType; import static com.nr.instrumentation.graphql.GraphQLObfuscator.obfuscate; import static com.nr.instrumentation.graphql.GraphQLOperationDefinition.getOperationTypeFrom; @@ -38,9 +39,19 @@ public static void setOperationAttributes(final Document document, final String public static void setResolverAttributes(ExecutionStrategyParameters parameters) { AgentBridge.privateApi.addTracerParameter("graphql.field.path", parameters.getPath().getSegmentName()); - GraphQLObjectType type = (GraphQLObjectType) parameters.getExecutionStepInfo().getType(); - AgentBridge.privateApi.addTracerParameter("graphql.field.parentType", type.getName()); AgentBridge.privateApi.addTracerParameter("graphql.field.name", parameters.getField().getName()); + // ExecutionStepInfo is not nullable according to documentation + GraphQLOutputType graphQLOutputType = parameters.getExecutionStepInfo().getType(); + setGraphQLFieldParentTypeIfPossible(graphQLOutputType); + } + + private static void setGraphQLFieldParentTypeIfPossible(GraphQLOutputType graphQLOutputType) { + // graphql.field.parentType is NOT required according to the Agent Spec + // https://source.datanerd.us/agents/agent-specs/blob/main/GraphQL.md + if (graphQLOutputType instanceof GraphQLNamedSchemaElement) { + GraphQLNamedSchemaElement named = (GraphQLNamedSchemaElement) graphQLOutputType; + AgentBridge.privateApi.addTracerParameter("graphql.field.parentType", named.getName()); + } } private static void setOperationAttributes(String type, String name, String query) { diff --git a/instrumentation/graphql-java-16.2/src/test/java/com/nr/instrumentation/graphql/GraphQLSpanUtilTest.java b/instrumentation/graphql-java-16.2/src/test/java/com/nr/instrumentation/graphql/GraphQLSpanUtilTest.java index 189ce57a43..f2484caff2 100644 --- a/instrumentation/graphql-java-16.2/src/test/java/com/nr/instrumentation/graphql/GraphQLSpanUtilTest.java +++ b/instrumentation/graphql-java-16.2/src/test/java/com/nr/instrumentation/graphql/GraphQLSpanUtilTest.java @@ -11,21 +11,34 @@ import com.newrelic.agent.bridge.PrivateApi; import com.nr.instrumentation.graphql.helper.GraphQLTestHelper; import com.nr.instrumentation.graphql.helper.PrivateApiStub; +import graphql.execution.ExecutionStepInfo; +import graphql.execution.ExecutionStrategyParameters; +import graphql.execution.MergedField; +import graphql.execution.ResultPath; import graphql.language.Definition; import graphql.language.Document; +import graphql.schema.GraphQLNonNull; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; import java.util.Collections; import java.util.List; import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +@ExtendWith(MockitoExtension.class) public class GraphQLSpanUtilTest { private static final List NO_DEFINITIONS = Collections.emptyList(); @@ -80,4 +93,42 @@ public void testSetOperationAttributesEdgeCases(Document document, String query, assertEquals(expectedName, privateApiStub.getTracerParameterFor("graphql.operation.name")); assertEquals(expectedQuery, privateApiStub.getTracerParameterFor("graphql.operation.query")); } + + // This test verifies https://issues.newrelic.com/browse/NEWRELIC-4936 no longer exists + @Test + public void testInvalidClassExceptionFix() { + // given execution parameters with execution step info type that is NOT GraphQLNamedSchemaElement + String expectedFieldPath = "fieldPath"; + String expectedFieldName = "fieldName"; + ExecutionStrategyParameters parameters = + mockExecutionStrategyParametersForInvalidClassCastExceptionTest(expectedFieldPath, expectedFieldName); + + // when (this had ClassCastException) + GraphQLSpanUtil.setResolverAttributes(parameters); + + // then tracer parameters set correctly without exception + assertEquals(expectedFieldPath, privateApiStub.getTracerParameterFor("graphql.field.path")); + assertEquals(expectedFieldName, privateApiStub.getTracerParameterFor("graphql.field.name")); + + // and 'graphql.field.parentType' is not set + assertNull(privateApiStub.getTracerParameterFor("graphql.field.parentType"), + "'graphql.field.parentType' is not required and should be null here"); + } + + private static ExecutionStrategyParameters mockExecutionStrategyParametersForInvalidClassCastExceptionTest(String graphqlFieldPath, String graphqlFieldName) { + ExecutionStrategyParameters parameters = mock(ExecutionStrategyParameters.class); + ResultPath resultPath = mock(ResultPath.class); + MergedField mergedField = mock(MergedField.class); + ExecutionStepInfo executionStepInfo = mock(ExecutionStepInfo.class); + GraphQLNonNull notGraphQLNamedSchemaElement = mock(GraphQLNonNull.class); + + when(resultPath.getSegmentName()).thenReturn(graphqlFieldPath); + when(mergedField.getName()).thenReturn(graphqlFieldName); + when(parameters.getPath()).thenReturn(resultPath); + when(parameters.getField()).thenReturn(mergedField); + when(parameters.getExecutionStepInfo()).thenReturn(executionStepInfo); + when(executionStepInfo.getType()).thenReturn(notGraphQLNamedSchemaElement); + + return parameters; + } } diff --git a/instrumentation/graphql-java-17.0/src/main/java/com/nr/instrumentation/graphql/GraphQLSpanUtil.java b/instrumentation/graphql-java-17.0/src/main/java/com/nr/instrumentation/graphql/GraphQLSpanUtil.java index 2e56c666ee..c0c9dd7215 100644 --- a/instrumentation/graphql-java-17.0/src/main/java/com/nr/instrumentation/graphql/GraphQLSpanUtil.java +++ b/instrumentation/graphql-java-17.0/src/main/java/com/nr/instrumentation/graphql/GraphQLSpanUtil.java @@ -11,7 +11,8 @@ import graphql.execution.ExecutionStrategyParameters; import graphql.language.Document; import graphql.language.OperationDefinition; -import graphql.schema.GraphQLObjectType; +import graphql.schema.GraphQLNamedSchemaElement; +import graphql.schema.GraphQLOutputType; import static com.nr.instrumentation.graphql.GraphQLObfuscator.obfuscate; import static com.nr.instrumentation.graphql.GraphQLOperationDefinition.getOperationTypeFrom; @@ -38,9 +39,19 @@ public static void setOperationAttributes(final Document document, final String public static void setResolverAttributes(ExecutionStrategyParameters parameters) { AgentBridge.privateApi.addTracerParameter("graphql.field.path", parameters.getPath().getSegmentName()); - GraphQLObjectType type = (GraphQLObjectType) parameters.getExecutionStepInfo().getType(); - AgentBridge.privateApi.addTracerParameter("graphql.field.parentType", type.getName()); AgentBridge.privateApi.addTracerParameter("graphql.field.name", parameters.getField().getName()); + // ExecutionStepInfo is not nullable according to documentation + GraphQLOutputType graphQLOutputType = parameters.getExecutionStepInfo().getType(); + setGraphQLFieldParentTypeIfPossible(graphQLOutputType); + } + + private static void setGraphQLFieldParentTypeIfPossible(GraphQLOutputType graphQLOutputType) { + // graphql.field.parentType is NOT required according to the Agent Spec + // https://source.datanerd.us/agents/agent-specs/blob/main/GraphQL.md + if (graphQLOutputType instanceof GraphQLNamedSchemaElement) { + GraphQLNamedSchemaElement named = (GraphQLNamedSchemaElement) graphQLOutputType; + AgentBridge.privateApi.addTracerParameter("graphql.field.parentType", named.getName()); + } } private static void setOperationAttributes(String type, String name, String query) { diff --git a/instrumentation/graphql-java-17.0/src/test/java/com/nr/instrumentation/graphql/GraphQLSpanUtilTest.java b/instrumentation/graphql-java-17.0/src/test/java/com/nr/instrumentation/graphql/GraphQLSpanUtilTest.java index 189ce57a43..f61178b9c0 100644 --- a/instrumentation/graphql-java-17.0/src/test/java/com/nr/instrumentation/graphql/GraphQLSpanUtilTest.java +++ b/instrumentation/graphql-java-17.0/src/test/java/com/nr/instrumentation/graphql/GraphQLSpanUtilTest.java @@ -11,10 +11,16 @@ import com.newrelic.agent.bridge.PrivateApi; import com.nr.instrumentation.graphql.helper.GraphQLTestHelper; import com.nr.instrumentation.graphql.helper.PrivateApiStub; +import graphql.execution.ExecutionStepInfo; +import graphql.execution.ExecutionStrategyParameters; +import graphql.execution.MergedField; +import graphql.execution.ResultPath; import graphql.language.Definition; import graphql.language.Document; +import graphql.schema.GraphQLNonNull; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.CsvSource; @@ -25,6 +31,9 @@ import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class GraphQLSpanUtilTest { @@ -80,4 +89,42 @@ public void testSetOperationAttributesEdgeCases(Document document, String query, assertEquals(expectedName, privateApiStub.getTracerParameterFor("graphql.operation.name")); assertEquals(expectedQuery, privateApiStub.getTracerParameterFor("graphql.operation.query")); } + + // This test verifies https://issues.newrelic.com/browse/NEWRELIC-4936 no longer exists + @Test + public void testInvalidClassExceptionFix() { + // given execution parameters with execution step info type that is NOT GraphQLNamedSchemaElement + String expectedFieldPath = "fieldPath"; + String expectedFieldName = "fieldName"; + ExecutionStrategyParameters parameters = + mockExecutionStrategyParametersForInvalidClassCastExceptionTest(expectedFieldPath, expectedFieldName); + + // when (this had ClassCastException) + GraphQLSpanUtil.setResolverAttributes(parameters); + + // then tracer parameters set correctly without exception + assertEquals(expectedFieldPath, privateApiStub.getTracerParameterFor("graphql.field.path")); + assertEquals(expectedFieldName, privateApiStub.getTracerParameterFor("graphql.field.name")); + + // and 'graphql.field.parentType' is not set + assertNull(privateApiStub.getTracerParameterFor("graphql.field.parentType"), + "'graphql.field.parentType' is not required and should be null here"); + } + + private static ExecutionStrategyParameters mockExecutionStrategyParametersForInvalidClassCastExceptionTest(String graphqlFieldPath, String graphqlFieldName) { + ExecutionStrategyParameters parameters = mock(ExecutionStrategyParameters.class); + ResultPath resultPath = mock(ResultPath.class); + MergedField mergedField = mock(MergedField.class); + ExecutionStepInfo executionStepInfo = mock(ExecutionStepInfo.class); + GraphQLNonNull notGraphQLNamedSchemaElement = mock(GraphQLNonNull.class); + + when(resultPath.getSegmentName()).thenReturn(graphqlFieldPath); + when(mergedField.getName()).thenReturn(graphqlFieldName); + when(parameters.getPath()).thenReturn(resultPath); + when(parameters.getField()).thenReturn(mergedField); + when(parameters.getExecutionStepInfo()).thenReturn(executionStepInfo); + when(executionStepInfo.getType()).thenReturn(notGraphQLNamedSchemaElement); + + return parameters; + } }