Conversation
Test Results 322 files 322 suites 3m 24s ⏱️ Results for commit 0f22f07. ♻️ This comment has been updated with latest results. |
| 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)"); |
There was a problem hiding this comment.
Change to nanoseconds only, if need be someone can convert to ms later
Remove string concatenation to make it easier to read
| result.put("executionId", Assert.assertNotNull(executionId)); | ||
| result.put("executionId", Assert.assertNotNull(executionId).toString()); | ||
| result.put("operation", operationType + ":" + operationName); | ||
| result.put("startTime", startTime); |
There was a problem hiding this comment.
actually we should suffix startTime and endTime also with Ns to be clear here
| Map<String, Object> result = new LinkedHashMap<>(); | ||
| result.put("executionId", Assert.assertNotNull(executionId)); | ||
| result.put("executionId", Assert.assertNotNull(executionId).toString()); | ||
| result.put("operation", operationType + ":" + operationName); |
There was a problem hiding this comment.
should that be two fields? operation Type and operation name? or one Map inside "operation"?
There was a problem hiding this comment.
Two fields are better, and better to be clearer on operation name vs type.
There was a problem hiding this comment.
I see what you meant, should they be nested or not
We've done both: nesting some information, but keeping all the timing information at the top level. I think operation name is one of the most important keys and I'd like to have that at the top level.
| def graphql = GraphQL.newGraphQL(schema).build() | ||
|
|
||
| ExecutionInput ei = ExecutionInput.newExecutionInput() | ||
| ExecutionInput ei = newExecutionInput() |
There was a problem hiding this comment.
This doesn't change anything, only IDE complaints
| 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" |
There was a problem hiding this comment.
This is the only real test change
| 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()); |
There was a problem hiding this comment.
Cast to string to serialise in JSON logs
3 participants
We're putting the finishing touches on the Profiler. We expect users will use the Profiler to sample logs and we're giving that a try ourselves.
ExecutionIdis better cast to a string for the output map, otherwise a logger can complain it doesn't know how to serialise this type to JSONAlso changing some formatted strings into map values that's easier to parse and analyse later