-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Profiler tweaks for JSON serialisation #4068
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b69dd6b
5de46cd
eb90338
fde7f50
1c76bec
7c5e387
0f22f07
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -273,12 +273,13 @@ public List<String> getInstrumentationClasses() { | |
|
|
||
| public Map<String, Object> shortSummaryMap() { | ||
| Map<String, Object> result = new LinkedHashMap<>(); | ||
| result.put("executionId", Assert.assertNotNull(executionId)); | ||
| result.put("operation", operationType + ":" + operationName); | ||
| result.put("startTime", startTime); | ||
| result.put("endTime", endTime); | ||
| result.put("totalRunTime", (endTime - startTime) + "(" + (endTime - startTime) / 1_000_000 + "ms)"); | ||
| result.put("engineTotalRunningTime", engineTotalRunningTime + "(" + engineTotalRunningTime / 1_000_000 + "ms)"); | ||
| result.put("executionId", Assert.assertNotNull(executionId).toString()); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cast to string to serialise in JSON logs |
||
| result.put("operationName", operationName); | ||
| result.put("operationType", Assert.assertNotNull(operationType).toString()); | ||
| result.put("startTimeNs", startTime); | ||
| result.put("endTimeNs", endTime); | ||
| result.put("totalRunTimeNs", endTime - startTime); | ||
| result.put("engineTotalRunningTimeNs", engineTotalRunningTime); | ||
| result.put("totalDataFetcherInvocations", totalDataFetcherInvocations); | ||
| result.put("totalCustomDataFetcherInvocations", getTotalCustomDataFetcherInvocations()); | ||
| result.put("totalTrivialDataFetcherInvocations", totalTrivialDataFetcherInvocations); | ||
|
|
@@ -310,14 +311,20 @@ public Map<String, Object> shortSummaryMap() { | |
| Assert.assertShouldNeverHappen(); | ||
| } | ||
| } | ||
| result.put("dataFetcherResultTypes", Map.of( | ||
| DataFetcherResultType.COMPLETABLE_FUTURE_COMPLETED.name(), "(count:" + completedCount + ", invocations:" + completedInvokeCount + ")", | ||
| DataFetcherResultType.COMPLETABLE_FUTURE_NOT_COMPLETED.name(), "(count:" + notCompletedCount + ", invocations:" + notCompletedInvokeCount + ")", | ||
| DataFetcherResultType.MATERIALIZED.name(), "(count:" + materializedCount + ", invocations:" + materializedInvokeCount + ")" | ||
| )); | ||
| LinkedHashMap<String, Object> dFRTinfo = new LinkedHashMap<>(3); | ||
| dFRTinfo.put(DataFetcherResultType.COMPLETABLE_FUTURE_COMPLETED.name(), createCountMap(completedCount, completedInvokeCount)); | ||
| dFRTinfo.put(DataFetcherResultType.COMPLETABLE_FUTURE_NOT_COMPLETED.name(), createCountMap(notCompletedCount, notCompletedInvokeCount)); | ||
| dFRTinfo.put(DataFetcherResultType.MATERIALIZED.name(), createCountMap(materializedCount, materializedInvokeCount)); | ||
| result.put("dataFetcherResultTypes", dFRTinfo); | ||
andimarek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return result; | ||
| } | ||
|
|
||
| private LinkedHashMap<String, Integer> createCountMap(int count, int invocations) { | ||
| LinkedHashMap<String, Integer> map = new LinkedHashMap<>(2); | ||
| map.put("count", count); | ||
| map.put("invocations", invocations); | ||
| return map; | ||
| } | ||
|
|
||
| public List<Map<String, Object>> getDispatchEventsAsMap() { | ||
| List<Map<String, Object>> result = new ArrayList<>(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| package graphql | ||
|
|
||
| import graphql.execution.ExecutionId | ||
| import graphql.execution.instrumentation.ChainedInstrumentation | ||
| import graphql.execution.instrumentation.Instrumentation | ||
| import graphql.execution.instrumentation.InstrumentationState | ||
|
|
@@ -39,9 +40,9 @@ class ProfilerTest extends Specification { | |
| def schema = TestUtil.schema(sdl, [Query: [ | ||
| hello: { DataFetchingEnvironment dfe -> return "world" } as DataFetcher | ||
| ]]) | ||
| def graphql = GraphQL.newGraphQL(schema).build(); | ||
| def graphql = GraphQL.newGraphQL(schema).build() | ||
|
|
||
| ExecutionInput ei = ExecutionInput.newExecutionInput() | ||
| ExecutionInput ei = newExecutionInput() | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't change anything, only IDE complaints |
||
| .query("{ hello }") | ||
| .profileExecution(true) | ||
| .build() | ||
|
|
@@ -68,9 +69,9 @@ class ProfilerTest extends Specification { | |
| def schema = TestUtil.schema(sdl, [Query: [ | ||
| hello: { DataFetchingEnvironment dfe -> return "world" } as DataFetcher | ||
| ]]) | ||
| def graphql = GraphQL.newGraphQL(schema).build(); | ||
| def graphql = GraphQL.newGraphQL(schema).build() | ||
|
|
||
| ExecutionInput ei = ExecutionInput.newExecutionInput() | ||
| ExecutionInput ei = newExecutionInput() | ||
| .query("{ hello __typename alias:__typename __schema {types{name}} __type(name: \"Query\") {name} }") | ||
| .profileExecution(true) | ||
| .build() | ||
|
|
@@ -98,9 +99,9 @@ class ProfilerTest extends Specification { | |
| def schema = TestUtil.schema(sdl, [Query: [ | ||
| hello: { DataFetchingEnvironment dfe -> return "world" } as DataFetcher | ||
| ]]) | ||
| def graphql = GraphQL.newGraphQL(schema).build(); | ||
| def graphql = GraphQL.newGraphQL(schema).build() | ||
|
|
||
| ExecutionInput ei = ExecutionInput.newExecutionInput() | ||
| ExecutionInput ei = newExecutionInput() | ||
| .query("{ __schema {types{name}} __type(name: \"Query\") {name} }") | ||
| .profileExecution(true) | ||
| .build() | ||
|
|
@@ -152,9 +153,9 @@ class ProfilerTest extends Specification { | |
| dog: dogDf | ||
| ]] | ||
| def schema = TestUtil.schema(sdl, dfs) | ||
| def graphql = GraphQL.newGraphQL(schema).instrumentation(instrumentation).build(); | ||
| def graphql = GraphQL.newGraphQL(schema).instrumentation(instrumentation).build() | ||
|
|
||
| ExecutionInput ei = ExecutionInput.newExecutionInput() | ||
| ExecutionInput ei = newExecutionInput() | ||
| .query("{ dog {name age} }") | ||
| .profileExecution(true) | ||
| .build() | ||
|
|
@@ -193,10 +194,10 @@ class ProfilerTest extends Specification { | |
| } | ||
| } | ||
|
|
||
| DataLoader<String, String> nameDataLoader = DataLoaderFactory.newDataLoader(batchLoader); | ||
| DataLoader<String, String> nameDataLoader = DataLoaderFactory.newDataLoader(batchLoader) | ||
|
|
||
| DataLoaderRegistry dataLoaderRegistry = new DataLoaderRegistry(); | ||
| dataLoaderRegistry.register("name", nameDataLoader); | ||
| DataLoaderRegistry dataLoaderRegistry = new DataLoaderRegistry() | ||
| dataLoaderRegistry.register("name", nameDataLoader) | ||
|
|
||
| def df1 = { env -> | ||
| def loader = env.getDataLoader("name") | ||
|
|
@@ -250,10 +251,10 @@ class ProfilerTest extends Specification { | |
| } | ||
| } | ||
|
|
||
| DataLoader<String, String> nameDataLoader = DataLoaderFactory.newDataLoader(batchLoader); | ||
| DataLoader<String, String> nameDataLoader = DataLoaderFactory.newDataLoader(batchLoader) | ||
|
|
||
| DataLoaderRegistry dataLoaderRegistry = new DataLoaderRegistry(); | ||
| dataLoaderRegistry.register("name", nameDataLoader); | ||
| DataLoaderRegistry dataLoaderRegistry = new DataLoaderRegistry() | ||
| dataLoaderRegistry.register("name", nameDataLoader) | ||
|
|
||
| def dogDF = { env -> | ||
| def loader = env.getDataLoader("name") | ||
|
|
@@ -306,17 +307,17 @@ class ProfilerTest extends Specification { | |
| def schema = TestUtil.schema(sdl, [Query: [ | ||
| hello: { DataFetchingEnvironment dfe -> return "world" } as DataFetcher | ||
| ]]) | ||
| Instrumentation fooInstrumentation = new Instrumentation() {}; | ||
| Instrumentation barInstrumentation = new Instrumentation() {}; | ||
| Instrumentation fooInstrumentation = new Instrumentation() {} | ||
| Instrumentation barInstrumentation = new Instrumentation() {} | ||
| ChainedInstrumentation chainedInstrumentation = new ChainedInstrumentation( | ||
| new ChainedInstrumentation(new SimplePerformantInstrumentation()), | ||
| new ChainedInstrumentation(fooInstrumentation, barInstrumentation), | ||
| new SimplePerformantInstrumentation()) | ||
|
|
||
|
|
||
| def graphql = GraphQL.newGraphQL(schema).instrumentation(chainedInstrumentation).build(); | ||
| def graphql = GraphQL.newGraphQL(schema).instrumentation(chainedInstrumentation).build() | ||
|
|
||
| ExecutionInput ei = ExecutionInput.newExecutionInput() | ||
| ExecutionInput ei = newExecutionInput() | ||
| .query("{ hello }") | ||
| .profileExecution(true) | ||
| .build() | ||
|
|
@@ -354,9 +355,9 @@ class ProfilerTest extends Specification { | |
| Foo : [ | ||
| bar: { DataFetchingEnvironment dfe -> dfe.source.id } as DataFetcher | ||
| ]]) | ||
| def graphql = GraphQL.newGraphQL(schema).build(); | ||
| def graphql = GraphQL.newGraphQL(schema).build() | ||
|
|
||
| ExecutionInput ei = ExecutionInput.newExecutionInput() | ||
| ExecutionInput ei = newExecutionInput() | ||
| .query("{ foo { id bar } }") | ||
| .profileExecution(true) | ||
| .build() | ||
|
|
@@ -391,22 +392,22 @@ class ProfilerTest extends Specification { | |
| // blocking the engine for 1ms | ||
| // so that engineTotalRunningTime time is more than 1ms | ||
| Thread.sleep(1) | ||
| return CompletableFuture.supplyAsync { | ||
| return supplyAsync { | ||
| Thread.sleep(500) | ||
| "1" | ||
| } | ||
| } as DataFetcher], | ||
| Foo : [ | ||
| id: { DataFetchingEnvironment dfe -> | ||
| return CompletableFuture.supplyAsync { | ||
| return supplyAsync { | ||
| Thread.sleep(500) | ||
| dfe.source | ||
| } | ||
| } as DataFetcher | ||
| ]]) | ||
| def graphql = GraphQL.newGraphQL(schema).build(); | ||
| def graphql = GraphQL.newGraphQL(schema).build() | ||
|
|
||
| ExecutionInput ei = ExecutionInput.newExecutionInput() | ||
| ExecutionInput ei = newExecutionInput() | ||
| .query("{ foo { id } }") | ||
| .profileExecution(true) | ||
| .build() | ||
|
|
@@ -455,11 +456,13 @@ class ProfilerTest extends Specification { | |
| } as DataFetcher | ||
|
|
||
| ]]) | ||
| def graphql = GraphQL.newGraphQL(schema).build(); | ||
| def graphql = GraphQL.newGraphQL(schema).build() | ||
|
|
||
| ExecutionInput ei = ExecutionInput.newExecutionInput() | ||
| ExecutionId id = ExecutionId.from("myExecutionId") | ||
| ExecutionInput ei = newExecutionInput() | ||
| .query("{ foo { id name text } foo2: foo { id name text} }") | ||
| .profileExecution(true) | ||
| .executionId(id) | ||
| .build() | ||
|
|
||
| when: | ||
|
|
@@ -477,11 +480,10 @@ class ProfilerTest extends Specification { | |
| "/foo2/text": MATERIALIZED, | ||
| "/foo2" : COMPLETABLE_FUTURE_NOT_COMPLETED, | ||
| "/foo" : COMPLETABLE_FUTURE_NOT_COMPLETED] | ||
| profilerResult.shortSummaryMap().get("dataFetcherResultTypes") == ["COMPLETABLE_FUTURE_COMPLETED" : "(count:2, invocations:4)", | ||
| "COMPLETABLE_FUTURE_NOT_COMPLETED": "(count:2, invocations:2)", | ||
| "MATERIALIZED" : "(count:2, invocations:4)"] | ||
|
|
||
|
|
||
| profilerResult.shortSummaryMap().get("dataFetcherResultTypes") == ["COMPLETABLE_FUTURE_COMPLETED" : ["count":2, "invocations":4], | ||
| "COMPLETABLE_FUTURE_NOT_COMPLETED": ["count":2, "invocations":2], | ||
| "MATERIALIZED" : ["count":2, "invocations":4]] | ||
| profilerResult.shortSummaryMap().get("executionId") == "myExecutionId" | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the only real test change |
||
| } | ||
|
|
||
| def "operation details"() { | ||
|
|
@@ -494,9 +496,9 @@ class ProfilerTest extends Specification { | |
| def schema = TestUtil.schema(sdl, [Query: [ | ||
| hello: { DataFetchingEnvironment dfe -> return "world" } as DataFetcher | ||
| ]]) | ||
| def graphql = GraphQL.newGraphQL(schema).build(); | ||
| def graphql = GraphQL.newGraphQL(schema).build() | ||
|
|
||
| ExecutionInput ei = ExecutionInput.newExecutionInput() | ||
| ExecutionInput ei = newExecutionInput() | ||
| .query("query MyQuery { hello }") | ||
| .profileExecution(true) | ||
| .build() | ||
|
|
@@ -542,10 +544,10 @@ class ProfilerTest extends Specification { | |
| } | ||
| } | ||
|
|
||
| DataLoader<String, String> nameDataLoader = DataLoaderFactory.newDataLoader(batchLoader); | ||
| DataLoader<String, String> nameDataLoader = DataLoaderFactory.newDataLoader(batchLoader) | ||
|
|
||
| DataLoaderRegistry dataLoaderRegistry = new DataLoaderRegistry(); | ||
| dataLoaderRegistry.register("name", nameDataLoader); | ||
| DataLoaderRegistry dataLoaderRegistry = new DataLoaderRegistry() | ||
| dataLoaderRegistry.register("name", nameDataLoader) | ||
|
|
||
| def dogDF = { env -> | ||
| return env.getDataLoader("name").load("Key1").thenCompose { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to nanoseconds only, if need be someone can convert to ms later
Remove string concatenation to make it easier to read