[1.8] Instrumentation compatibility

[exAspArk]

Hey!

I have 2 issues when I'm trying to instrument fields by using the 1.8-dev branch:

1. Type class mismatch

For example, I have the following code in my app:

class User < ApplicationRecord
  ...
end

class Types::User < GraphQL::Schema::Object
  ...
end

When I'm trying to introspect a field, I'm getting the following:

def instrument(type, field)
  binding.pry
end

pry> type == Types::User
# => false
# ^ it returns "true" with v1.7 

pry> type
# => User

pry> type.class
# => GraphQL::ObjectType

2. Type instance instead of the passed object as the first argument

When I'm trying to redefine the resolve_proc, it passes the following:

->(obj, args, ctx) { binding.pry }

pry> obj.class
# => Types::User
# ^ it returns "User" with v1.7 

pry> obj.object
# => #<User:0x007fb9fa86abd0>

Are you planning to keep the instrumentation compatible with the previous gem versions? :)

[rmosolgo]

Hi, thanks for opening this issue! Yes, I intend to maintain compatibility. Right now, GitHub is running class-based types and lot of different instrumenters.

Let me share some implementation details and maybe the cause of the issue will present itself ...

• In 1.8, object classes are used as proxies, as you noticed. These proxies are applied by field instrumentation
• This instrumentation is added to BUILT_IN_INSTRUMENTERS
• BUILT_IN_INSTRUMENTERS is used during traversal to modify fields as they're added to the schema BUT they're added after user-provided instrumenters
• I think that's right: you want user-provided instrumenters to have access to un-proxied objects -- that is, the originally returned objects, not the proxies
• So, why, in your case, did you get the proxy objects instead of the original objects? 🤔

By the way, regarding type, I'm sorry but this is an odd detail for some time of compatibility. You can get from the ObjectType instance to the Class by using type.metadata[:type_class]. That will return the class.

Sometime, execution will use the classes instead of ObjectType instances ... but not yet!

[rmosolgo]

Maybe the thing to do is add an instrumenter to spec/support/jazz.rb and make sure it works properly, I'm happy to take a look there in a bit, or feel free to take a stab yourself.

[exAspArk]

Thank you for your reply! type.metadata[:type_class] works fine 👍

Sometime, execution will use the classes instead of ObjectType instances ... but not yet!

Are you still planning to do that? :)

[rmosolgo]

Yes, 1.9-dev branch has a new runtime that uses class-based types only, although in a few cases it's still supported by the compatibility layer. Hoping to remove that too, but not quite there yet!

[rmosolgo]

Removing the compat layer is WIP in #2363 , closing this issue as it doesn't have any specific work to do.