Repository: netflix/dgs-framework
Issue: bug: Handle validation of input arguments in query complexity instrumentation

For invalid input arguments (null values for non-nullable fields), query complexity instrumentation throws an exception: NonNullableValueCoercedAsNullException

This is because graphql-java validation runs after the instrumentation. We can handle this by either:
1. move variable coercion logic in graphql-java before, or 
2. push the complexity instrumentation down from beginValidation to beforeBeginExecution, or 
3. do the variable coercion validation an additional time in the complexity instrumentation. 
4. catch all the exceptions during complexity calculation and continue query processing, to let graphql-java do further handling of errors/validation.





Changed files:
graphql-dgs-spring-boot-micrometer/src/test/kotlin/com/netflix/graphql/dgs/metrics/micrometer/MicrometerServletSmokeTest.kt
graphql-dgs-spring-boot-micrometer/src/main/kotlin/com/netflix/graphql/dgs/metrics/micrometer/DgsGraphQLMetricsInstrumentation.kt

diff --git a/graphql-dgs-spring-boot-micrometer/src/main/kotlin/com/netflix/graphql/dgs/metrics/micrometer/DgsGraphQLMetricsInstrumentation.kt b/graphql-dgs-spring-boot-micrometer/src/main/kotlin/com/netflix/graphql/dgs/metrics/micrometer/DgsGraphQLMetricsInstrumentation.kt
index 16a675d5..7a6bbd2d 100644
--- a/graphql-dgs-spring-boot-micrometer/src/main/kotlin/com/netflix/graphql/dgs/metrics/micrometer/DgsGraphQLMetricsInstrumentation.kt
+++ b/graphql-dgs-spring-boot-micrometer/src/main/kotlin/com/netflix/graphql/dgs/metrics/micrometer/DgsGraphQLMetricsInstrumentation.kt
@@ -174,7 +174,6 @@ class DgsGraphQLMetricsInstrumentation(
             if (parameters.document != null) {
                 state.querySignature = optQuerySignatureRepository.flatMap { it.get(parameters.document, parameters) }
             }
-            state.queryComplexity = ComplexityUtils.resolveComplexity(parameters)
         }
     }
 
@@ -188,6 +187,7 @@ class DgsGraphQLMetricsInstrumentation(
                 state.operationName = Optional.ofNullable(parameters.executionContext.operationDefinition?.name)
             }
         }
+        state.queryComplexity = ComplexityUtils.resolveComplexity(parameters)
         return DefaultExecutionStrategyInstrumentationContext
     }
 
@@ -266,7 +266,7 @@ class DgsGraphQLMetricsInstrumentation(
 
         private val queryComplexityBuckets = listOf(5, 10, 25, 50, 100, 200, 500, 1000, 2000, 5000, 10000)
 
-        fun resolveComplexity(parameters: InstrumentationValidationParameters): Optional<Int> {
+        fun resolveComplexity(parameters: InstrumentationExecutionStrategyParameters): Optional<Int> {
             try {
                 val queryTraverser: QueryTraverser = newQueryTraverser(parameters)
                 val valuesByParent: MutableMap<QueryVisitorFieldEnvironment?, Int?> =
@@ -300,13 +300,13 @@ class DgsGraphQLMetricsInstrumentation(
             return fieldComplexityCalculator(childComplexity)
         }
 
-        private fun newQueryTraverser(parameters: InstrumentationValidationParameters): QueryTraverser {
+        private fun newQueryTraverser(parameters: InstrumentationExecutionStrategyParameters): QueryTraverser {
             return QueryTraverser
                 .newQueryTraverser()
-                .schema(parameters.schema)
-                .document(parameters.document)
-                .operationName(parameters.operation)
-                .variables(parameters.variables)
+                .schema(parameters.executionContext.graphQLSchema)
+                .document(parameters.executionContext.document)
+                .operationName(parameters.executionContext.operationDefinition.name)
+                .variables(parameters.executionContext.variables)
                 .build()
         }
     }
diff --git a/graphql-dgs-spring-boot-micrometer/src/test/kotlin/com/netflix/graphql/dgs/metrics/micrometer/MicrometerServletSmokeTest.kt b/graphql-dgs-spring-boot-micrometer/src/test/kotlin/com/netflix/graphql/dgs/metrics/micrometer/MicrometerServletSmokeTest.kt
index a08f9e4c..80e60c47 100644
--- a/graphql-dgs-spring-boot-micrometer/src/test/kotlin/com/netflix/graphql/dgs/metrics/micrometer/MicrometerServletSmokeTest.kt
+++ b/graphql-dgs-spring-boot-micrometer/src/test/kotlin/com/netflix/graphql/dgs/metrics/micrometer/MicrometerServletSmokeTest.kt
@@ -397,6 +397,65 @@ class MicrometerServletSmokeTest {
             )
     }
 
+    @Test
+    fun `Assert metrics for a bad input error`() {
+        mvc.perform(
+            MockMvcRequestBuilders
+                .post("/graphql")
+                .content("""{ "query": "{ hello }" }""")
+        ).andExpect(status().isOk)
+            .andExpect(
+                content().json(
+                    """
+                    |   { "errors": [
+                    |       { 
+                    |           "message": "Validation error of type MissingFieldArgument: Missing field argument name @ 'hello'",
+                    |           "locations": [ 
+                    |               {
+                    |                   "line":1,
+                    |                   "column":3
+                    |               }
+                    |           ],
+                    |           "extensions": { "classification":"ValidationError" }
+                    |       }
+                    |     ]
+                    |   }
+                    """.trimMargin(),
+                    false
+                )
+            )
+        val meters = fetchMeters("gql.")
+
+        assertThat(meters).containsOnlyKeys("gql.error", "gql.query")
+
+        assertThat(meters["gql.error"]).isNotNull.hasSize(1)
+        assertThat(meters["gql.error"]?.first()?.id?.tags)
+            .containsAll(
+                Tags.of("execution-tag", "foo")
+                    .and("contextual-tag", "foo")
+                    .and("gql.operation", "none")
+                    .and("gql.operation.name", "anonymous")
+                    .and("gql.query.complexity", "none")
+                    .and("gql.query.sig.hash", "none")
+                    .and("gql.errorDetail", "none")
+                    .and("gql.errorCode", "ValidationError")
+                    .and("gql.path", "[hello]")
+                    .and("outcome", "failure")
+            )
+
+        assertThat(meters["gql.query"]).isNotNull.hasSize(1)
+        assertThat(meters["gql.query"]?.first()?.id?.tags)
+            .containsAll(
+                Tags.of("execution-tag", "foo")
+                    .and("contextual-tag", "foo")
+                    .and("outcome", "failure")
+                    .and("gql.operation", "none")
+                    .and("gql.operation.name", "anonymous")
+                    .and("gql.query.complexity", "none")
+                    .and("gql.query.sig.hash", "none")
+            )
+    }
+
     @Test
     fun `Assert metrics for internal error`() {
         mvc.perform(
@@ -468,7 +527,7 @@ class MicrometerServletSmokeTest {
     }
 
     @Test
-    fun `Assert metrics for a DGS bad-request error`() {
+    fun `Assert metrics for a bad-request error`() {
         mvc.perform(
             MockMvcRequestBuilders
                 .post("/graphql")
@@ -747,6 +806,7 @@ class MicrometerServletSmokeTest {
                 |type Query{
                 |    ping:String
                 |    someTrivialThings: String 
+                |    hello(name: String!): String 
                 |    transform(input:[String]): [StringTransformation]
                 |    triggerInternalFailure: String
                 |    triggerBadRequestFailure:String
@@ -772,6 +832,11 @@ class MicrometerServletSmokeTest {
                 return "pong"
             }
 
+            @DgsQuery()
+            fun hello(@InputArgument name: String): String {
+                return "Hello $name"
+            }
+
             @DgsQuery()
             @DgsEnableDataFetcherInstrumentation(false)
             fun someTrivialThings(): String {