Visualization of tracing in query result #162
Conversation
Why: Tracing [1] is optionally returned from GraphQL servers but is hard to interpret without visualization. What: Tracing visualization rendered in the footer of the GraphiQL result window. Each tracing.execution.resolvers entry is rendered as a row with a bar together with resolver path label and duration. Testing: - Chrome Browser, Version 71.0.3578.98 (Official Build) (64-bit) - Mac OSX 10.12.6 - GraphQL-java server with and without tracing turned on. Left to do: - Resizable panel where tracing visualization resides (currently max-height set to 300px to not consume all of the result window). - Clear the tracing visualization when new query has been triggered. [1] https://github.com/apollographql/apollo-tracing
|
@zippocage: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/ |
|
Thanks for working on this @zippocage! I'm still playing catch up from the holidays, but I'll dig into this shortly. In the meantime, would you be able to sign the CLA? |
|
@hwillson Thanks for the patience on the CLA delay, I am just confirming the details with my employer. Should be resolved soon. |
|
@hwillson CLA signed. |
hwillson
requested review from
justinanastos and
cheapsteak
and removed request for
hwillson and
jbaxleyiii
There was a problem hiding this comment.
Thanks for the PR! It's a cool feature to add
A few comments from me regarding some seemingly unnecessary internal states, and wonder if it's possible to avoid passing a component instance into createBridgeLink
BTW what service did you use while developing this? I'm having a bit of trouble finding a server that returns traces so I could test it out myself 🙂 edit: nvm, found one! (turned it on in fullstacktutorial)
| path: props.path, | ||
| startOffset: props.startOffset, | ||
| duration: props.duration, | ||
| totalDuration: props.totalDuration, |
There was a problem hiding this comment.
I might be mistaken, but it doesn't look like this component needs internal state?
Would it be preferable for places that reference these fields to directly refer to props instead?
If that's the case, it looks like this component could be converted from a class component to a function component as well :)
There was a problem hiding this comment.
I think you are right, I should try that.
| super(props); | ||
|
|
||
| this.state = { | ||
| tracing: props.tracing, |
There was a problem hiding this comment.
Similar question here, can this internal state be replaced by directly referencing props.tracing?
If so, let's convert this to a function component as well :)
| @@ -52,6 +53,9 @@ export const createBridgeLink = bridge => | |||
| delete result.extensions; | |||
| } | |||
| } | |||
| // send back query result to explorer for Tracing rendering | |||
| explorer.updateResult(result); | |||
There was a problem hiding this comment.
This piece feels a bit odd, to pass an instance of a component to a function so the function can call a method on the component
I wonder if there might be a different way to do this? I wouldn't block on this if not
There was a problem hiding this comment.
I am unsure how the link connection works. I was hoping you could guide me on how to make this the least intrusive.
| @@ -207,6 +220,9 @@ export class Explorer extends Component { | |||
| Load from cache | |||
| </label> | |||
| </GraphiQL.Toolbar> | |||
| <GraphiQL.Footer> | |||
| <Tracing tracing={this.state.tracingResult} /> | |||
There was a problem hiding this comment.
Screen real-estate in this panel is a bit scarce
For users who won't be able to make use of tracing (if their server doesn't return it), having a "No tracing information present" present at all times might be a bit annoying
Would you mind updating this so that component only shows up when there is something to show?
There was a problem hiding this comment.
The response has become rather difficult to see when traces are returned
I wonder what a good solution might be
edit: Perhaps a button to toggle whether we see the trace or response?
There was a problem hiding this comment.
Is there an easy way to get a resizable pane? The GraphiQL.Footer may the blocker but am not 100% sure. A button or checkbox to toggle makes sense. I would suggest showing the pane is on by default for the sake of discovery though.
| ); | ||
| } | ||
|
|
||
| pathWidthRatio() { |
Why:
Tracing [1] is optionally returned from GraphQL servers but is hard to interpret without visualization.
What:
Tracing visualization rendered in the footer of the GraphiQL result window. Each tracing.execution.resolvers entry is rendered as a row with a bar together with resolver path label and duration.
Testing:
Left to do:
[1] https://github.com/apollographql/apollo-tracing