GraphQL performance spec #1001
GraphQL performance spec #1001
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
||
| If there are hooks available and the transport layer is also instrumented (e.g. Apollo Interceptors for GraphQL and Spring), the SDK should give preference to the layer that has more information and void creating duplicate transactions/spans, or merge the information, if possible. | ||
|
|
||
| Changes in the product may be necessary, e.g. if `request.api_target` is set to `graphql`, the `request.data` and `contexts.response.data` should do syntax highlighting. |
There was a problem hiding this comment.
This is probably a TODO, @malwilley did the implementation for errors, we could reuse the code for performance.
|
|
||
| Spans should be created for [resolvers](https://www.apollographql.com/docs/apollo-server/data/resolvers/), if possible. | ||
|
|
||
| Spans should be created for [data loaders](https://graphql.org/learn/best-practices/#server-side-batching-caching/), if possible. |
There was a problem hiding this comment.
Assuming we add keys used as input for the data loader. How do we serialize non primitive keys that could bloat events?
There was a problem hiding this comment.
@adinauer I'd fall back to .toString as best effort.
There was a problem hiding this comment.
Sounds OK but could still bloat if the language simply puts everything into toString() - so something to be aware of.
For Java it's a compromise between risking bloat (just put the objects in the breadcrumb and let serialization handle it) and having useless data like MyClass@12345 (by using toString). Let's go with toString for now. We can always change if we come up with something better.
There was a problem hiding this comment.
@adinauer maybe the best decision is to not add such values then, risking the event to be dropped or having useless data does not feel good.
There was a problem hiding this comment.
I think there are cases where this data is very valuable. The useless toString output only applies to complex keys. For simple keys it should be fine. So I vote for adding keys.
Edit: should probably mention in docs that toString should be implemented for complex key objects.
Co-authored-by: Alexander Dinauer <adinauer@users.noreply.github.com>
Co-authored-by: Alexander Dinauer <adinauer@users.noreply.github.com>
Co-authored-by: Alexander Dinauer <adinauer@users.noreply.github.com>
Co-authored-by: Alexander Dinauer <adinauer@users.noreply.github.com>
Co-authored-by: Alexander Dinauer <adinauer@users.noreply.github.com>
|
I have two questions where I would not be sure how to implement it if I would. Otherwise, it's clear and understandable to me. 👍 |
There was a problem hiding this comment.
added some questions/suggestions - pretty clear to me otherwise, thanks @marandaneto 👍
No description provided.