feat(Field#lazy_resove) add hook for instrumenting lazy methods

[rmosolgo]

Now, you can "hook in" to lazy_resolve similar to resolve. You get access to the same things: type, field, runtime object (eg, Promise), arguments, and field context.

I think you could use the runtime object to group lazy_resolves by actual database operation. For example, I think Promise#source will point to the Loader instance.

[rmosolgo]

Hmm, lazy_method_name is a throw-away, maybe I can find a way to search for it only once.

[rmosolgo]

cc @tmeasday relevant to your interests perhaps :P

[tmeasday]

@rmosolgo so the idea is you'd check if the field had resolve_proc or lazy_resolve_proc; in the first case you'd assume that when that function returns it is done, in the second case, you'd look at the output object, and register some callback with it to run when it resolves?

And you are saying additionally you might be able to get some information about when the "true" work of the lazy field starts via the source property?

[rmosolgo]

you'd check

The downside of the lazy API is that you can't tell ahead of time whether a field will be resolved, uh, you know, during resolve, or whether the real work will be done later. So, you have to instrument both resolve and lazy_resolve for each field, eg:

prev_resolve = field.resolve_proc 
prev_lazy_resolve = field.lazy_resolve_proc 
field.redefine {
  resolve instrument_resolve(prev_resolve)
  lazy_resolve instrument_resolve(prev_lazy_resolve)
}

Lazy resolve will only be used in the cases where some lazy value (eg Promise) is returned ... but we don't know when that will be!

when the "true" work of the lazy field starts

lazy_resolve above will capture the start, at least. lazy_resolve will contain promise.sync, which usually does the work. (But there are cases where work is still out of bounds: for example, if a field returns an ActiveRecord::Relation, no database call is made right away: instead, it's made when graphql-ruby calls #each on the relation!)

source property

I'm not exactly sure on this one. But I thought I remembered two things:

• You were interested in grouping fields by the actual database operation performed to fulfill them
• A recent graphql-batch version added Promise#source that points to the loader where the promise came from: https://github.com/Shopify/graphql-batch/blob/master/lib/graphql/batch/loader.rb#L25

I think those would have the same loader_key, so that might be useful for something but ... I'm a bit out of my element!

[tmeasday]

The downside of the lazy API is that you can't tell ahead of time whether a field will be resolved, uh, you know, during resolve, or whether the real work will be done later.

I'm pretty confused about this. Does the user not specify which? Or do they just optionally return an async object from their resolve function? (are there docs for the user visible feature anywhere?)

Is the result of lazy_resolve_proc a Promise or other async object? If so, is there code in graphql that understands how to wait on it's completion (or do we duck type result.then?)

I'm not exactly sure on this one....

No, I think you're exactly right, this gives me what I need. What I want is some way to know when the promise "actually starts". In parallel-land (Node or whatever), that typically is "right when it's created (or at least in the next tick)". In the (slightly mind bending) promise.rb land, it could be some time later however.

In the case of graphql-batch it's when loader.perform is called (for the loader the promise is attached to). If I have a reference to the loader, I can monkey patch loader.perform (or better yet, PR to get loader.onPerform or something) to get that timing. Yay!

[rmosolgo]

The docs aren't on the website yet since this feature isn't released yet, but the will-be-guide is visible on master:

https://github.com/rmosolgo/graphql-ruby/blob/master/guides/schema/lazy_execution.md

Or do they just optionally return an async object from their resolve function?

☝️ yup 😬

wait on

This was the biggest difference for promise.rb for me: as Ruby does, you don't really "wait" on it. It can't do anything until you transfer control to it. So instead, we just delay execution. In graphql-batch's case, we're giving it time to gather more info about how to hit the DB. Then when we can't proceed any further, we call promise.sync which triggers a database hit, and continue from there.

Hope those docs help! Let me know if you find any other questions on that.

[tmeasday]

Thanks for the link. One question on the feature: why not define a default lazy_resolve name? (maybe resolve or lazy_resolve, or maybe execute or perform)?

It seems like a unnecessary step to have to configure lazy_resolve(LazyFindPerson, :person) at the schema level just so I can call my function person.

---

Then when we can't proceed any further, we call promise.sync which triggers a database hit, and continue from there.

Right but I don't want to trigger the sync, I just want to register a callback to fire when the promise is done. That's what I mean "wait" on it. Ultimately promise.rb still follows the spec and is "thennable" -- it still represents data that will resolve later, it's just the mechanism is different.

So does lazy_resolve_proc always return a Promise.rb promise?

[tmeasday]

It seems like a unnecessary step to have to configure lazy_resolve(LazyFindPerson, :person) at the schema level just so I can call my function person.

Hmm, thinking about this more, this is the way you actually tell graphql that the object is "unresolved". That makes sense, ignore that part of my comment ;)

[rmosolgo]

Oh, I think here's the missing piece: Field#lazy_resolve_proc comes with a default behavior that calls the registered method on the object. So, for graphql-batch, it does a promise.public_send(:sync).

We can hook into that behavior just the same way we can hook into Field#resolve_proc:

• extract the inner proc
• build a new proc that wraps the inner one
• redefine the field with the new proc

For example, assuming the app is using graphql-batch, which uses Promise.rb for lazy values:

def instrument(type, field) 
  inner_lazy_resolve = field.lazy_resolve_proc 
  instrumented_lazy_resolve = ->(obj, args, ctx) {
    # `obj` is a Promise 
    Instrumenter.instrument { 
      # `promise.sync` hasn't been called yet
      # This will call `promise.sync`:
      result = inner_lazy_resolve.call(obj, args, ctx)
      # `promise.sync` was called, `result` is the "real" value 
      result 
    } 
  }
  field.redefine(lazy_resolve: instrumented_lazy_resolve)
end 

Now, when the call to promise.sync will happen inside the instrument { ... } block, so you can run arbitrary code before and after it.

For Promise.rb in particular, you could probably also do:

def instrument(type, field) 
  inner_lazy_resolve = field.lazy_resolve_proc 
  instrumented_lazy_resolve = ->(obj, args, ctx) {
    # make a new promise: 
    instrumented_promise = obj.then { |val| Logger.log("promise finished");  val } 
    # pass your new promise into the lazy_resolve_proc to be `sync`-d 
    inner_lazy_resolve.call(instrumented_promise, args, ctx)
  }
  field.redefine(lazy_resolve: instrumented_lazy_resolve)
end 

But this is graphql-batch specific (... not that there are alternatives at the moment 😆 )

[tmeasday]

Ohh, I understand now, thanks for bearing with me. So graphql doesn't not handle asyncronicity at all in this version, it simply allows a setTimeout(0) essentially? [1]

I understand now that lazy_resolve_proc time is the moment the "real work" starts for the field; but that subsequent fields for the same loader will be essentially noops, so I need to somehow reconcile all the start times for the fields loaded by the same loader.

Thanks again @rmosolgo

[1] I wonder would graphql-batch still need to use promises internally with this feature?

[rmosolgo]

yep, like a setTimeout(0), but ruby's version: a single thread that determines it has run out of things to do, so it starts triggering promise.syncs :P

[tmeasday]

I think maybe my code would look like this:

def instrument(type, field) 
  inner_resolve = field.resolve_proc 
  inner_lazy_resolve = field.lazy_resolve_proc 

  resolve = ->(obj, args, ctx) {
     resolve_start_time = Time.now
     result = inner_resolve(obj, args, ctx)
     resolve_end_time = Time.now
     result
  }

  instrumented_lazy_resolve = ->(obj, args, ctx) {
    # this code is `graphql-batch` specific
    if obj.fulfilled?
      # this doesn't exist but I'll try to figure out a way to do this
      work_start_time = promise.source.last_work_start_time
    else
       # we are about to fire off another batch
       work_start_time = promise.source.last_work_start_time = Time.now
    end

    result = inner_lazy_resolve.call(obj, args, ctx)
    work_end_time = Time.now
  }

  field.redefine(lazy_resolve: instrumented_lazy_resolve, resolve: instrumented_resolve)
end 

[tmeasday]

@rmosolgo is it possible to use graphql-batch with this feature right now?

[rmosolgo]

This feature isn't on Rubygems yet but you can point your gemfile to GitHub to try it out, eg

gem "graphql", github: "rmosolgo/graphql-ruby", branch: "master" 
# or this branch: 
gem "graphql", github: "rmosolgo/graphql-ruby", branch: "lazy-instrumentation"

[tmeasday]

Do I still use the executors from inside graphql-batch? Do I need to use a branch of it?

[rmosolgo]

I think graphql-batch support for this API is on rubygems already, but requires some new setup.

The new setup isn't in Readme yet since this API isn't public yet, but you can see the new setup in the test helpers:

https://github.com/Shopify/graphql-batch/blob/874be934fd109c0dc328f1cffb2e9fa92fafa6f0/test/lazy_resolve_test.rb#L14-L15

[tmeasday]

Oh nice. Thanks

…

On Sat., 3 Dec. 2016 at 9:36 am, Robert Mosolgo ***@***.***> wrote: I think graphql-batch support for this API is on rubygems already, but requires some new setup. The new setup isn't in Readme yet since this API isn't public yet, but you can see the new setup in the test helpers: https://github.com/Shopify/graphql-batch/blob/874be934fd109c0dc328f1cffb2e9fa92fafa6f0/test/lazy_resolve_test.rb#L14-L15 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#429 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIFyo00w8lzFPO0B_C72zLFkIeTCHWyks5rEJ15gaJpZM4K_v8-> .

[rmosolgo]

I'll leave this branch for a bit in case you've got any tests pointed at it :)

[tmeasday]

I don't, feel free to remove.

[tmeasday]

Ok, so I am playing with this and it works well.

Is there any way to tell easily that lazy_resolve_proc won't get called?

[rmosolgo]

There should be a way .... ok, here we go: 0f0a8d7 !

[tmeasday]

Nice! Thanks @rmosolgo