Skip to content

Instrumentation for performance metrics like Apollo Optics #94

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

Closed
adri opened this issue Mar 8, 2017 · 5 comments
Closed

Instrumentation for performance metrics like Apollo Optics #94

adri opened this issue Mar 8, 2017 · 5 comments

Comments

@adri
Copy link
Contributor

adri commented Mar 8, 2017

Hi :) First of all, congrats to an awesome library.

I saw Apollo Optics for performance monitoring. There are clients for nodejs and ruby, both based on the general agent description.

When adding a PHP agent, I was wondering where would be a good place to hook into instrumentation. As far as I can see mostly a wrapper around field resolvers to time them would be handy.

Any ideas or tips how to add this instrumentation?
PS: I'm not affiliated with Apollo.

@vladar
Copy link
Member

vladar commented Mar 9, 2017

Hey thanks!

I looked into node agent and it seems to modify original schema by looping through all fields and wrapping their resolvers with optics functions. Technically you can do the same with graphql-php schema.

But on the other hand iterating over hundreds or thousands of fields is not the best way to do it in PHP. So it might make sense to provide hooks before/after field execution on library level.

PR for this feature is welcome!

If you decide to prepare such PR I suggest you to wrap this call with before and after callbacks and introduce static method on Executor to set these callbacks (like setFieldHooks(callable $beforeResolve, callable $afterResolve)).

Prefer static at this point because we may refactor executor later and it will be way easier to maintain backwards compatibility for simple static hooks. But if you have other opinion - I am open to discussion.

@vladar
Copy link
Member

vladar commented Mar 9, 2017

One more thing to keep in mind for your optics agent is that resolvers may return promises. So promiseAdapter must be also passed to hooks (in addition to other arguments) in order to check for isThenable and act appropriately.

@adri
Copy link
Contributor Author

adri commented Mar 26, 2017

Thanks a lot for your input :) I think for now I don't have the time to work on this.
I might pick it up in a few months!

@adri adri closed this as completed Mar 26, 2017
@jasonbahl
Copy link

jasonbahl commented Sep 18, 2017

@adri @vladar I've got Instrumentation setup for WPGraphQL. I've followed the proposed spec here: https://github.com/apollographql/apollo-tracing

You can see my implementation here: https://github.com/wp-graphql/wp-graphql-insights

It's super early (just worked on it a few days last week), but it follows the same approach as the node instrumentations I've seen where it wraps the entire schema. So far, I've not seen any noticeable performance differences with the tracing enabled vs disabled. . .but, I haven't done any significant testing. . .

I'm not sure what the best approach would be to create something that works for any implementation of this library though. I'm making use of WordPress hooks/filters to accomplish my implementation.

Feel free to use anything I've done in my implementation, or even make suggestions on how my implementation could be better.

Thanks!

====

Update: I should mention that right now this just adds tracing. It doesn't send the trace data to Optics (yet). I've been discussing with @martijnwalraven and @rohit2b from the Apollo team to figure out options for transporting the traces to Optics.

At the moment they're transitioning from their Optics Agent model that sends protobuf-encoded trace data via a POST request to a proxy model where your endpoint would be proxied by an "Optics Engine" which would handle getting trace data to Optics and offer additional things, such as full-query caching, etc.

Not sure how I'll end up handling getting the data to Optics. The proxy engine sounds cool, but not ideal for the WordPress community at large, so we'll see where I land on that 🤔

@vladar
Copy link
Member

vladar commented Jun 23, 2018

For anyone interested, there is another proposal on how to enable apollo tracing for any existing schema: see #289 (comment) PRs are welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants