resolver instrumentation

[roippi]

Basically re-opening #94 😁

I'm understanding I/we just need to wrap resolveOrError in a pluggable before/after hook? Plus a small amount of spec work around promise/error handling.

[vladar]

One thing why this is problematic for me right now is that we may implement it in a different way comparing to the reference implementation, which will make further maintenance much harder (or will require breaking changes).

So I think it makes sense to wait until they resolve graphql/graphql-js#284 and follow up after that.

[roippi]

Ah. That issue is coming up on its two-year birthday 😢

Disappointing but understandable; if there's any way around being blocked by reference impl here, I'd be interested to hear solutions.

[roippi]

I think I've managed to implement this without framework changes by wrapping resolvers. We have a place where we ensure singleton class instances (which is mandated by the framework, of course) and I can drop something like this in the pipe:

  /**
   * @param \GraphQL\Type\Definition\Type $instance The type instance
   *
   * @return Type
   */
  protected static function maybeInstallTelemetry($instance) {
    if (self::$telemetry && $instance instanceof ObjectType) {
      foreach ($instance->getFields() as $field) {
        $resolver = $field->resolveFn;
        // TODO maybe we should instrument the default resolver too?
        if ($resolver) {
          $field->resolveFn = function(...$args) use ($resolver) {
            list($beforeHook, $afterHook) = self::$telemetry;
            $beforeHook(...$args);
            $res = $resolver(...$args);
            $afterHook(...$args);
            return $res;
          };
        }
      }
    }
    return $instance;
  }

It works well enough for my needs, I think, though I haven't done much testing yet. Is anyone else taking an approach like this?

[vladar]

Well, actually composing resolvers is a recommended way to do this right now. If you do this in a lazy type loader - it should be ok.

I prefer using class-level resolveField callback in types and don't use field-level resolvers. In this case, the number of wrapping operations is way lower. Other option would be to wrap all resolvers inside definitions (of course if you have full control over your types):

'myField' => [
    'type' => Type::string(),
    'resolve' => MyUtils::wrap(function() {
    })
]

As for default resolver - you have full access to it, so you can replace it with whatever you want.

[caseycs]

@roippi I think Laravel is doing this is the same way - every middleware is wrapping another one, so you can always make a wrapper around your resolvers to handle, for example EntityNotFound exceptions.

For my handy use-case was to do instanceof check of final resolver in order to throw UserNotAuthorized exception when resolver implements AuthorizedUserOnly interface.

[vladar]

Closing because no action is pending on this right now from our side. We'll get back to it as soon as graphql-js moves forward with logging support.

[vladar]

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