Skip to content

ChainedInstrumentation not finishing #1110

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
kclaes opened this issue Jul 4, 2018 · 3 comments
Closed

ChainedInstrumentation not finishing #1110

kclaes opened this issue Jul 4, 2018 · 3 comments

Comments

@kclaes
Copy link

kclaes commented Jul 4, 2018

Upgrading from 7.0 to 9.0.

I've been using a ChainedInstrumentation that chains MaxQueryComplexityInstrumentation, DataLoaderDispatcherInstrumentation and TracingInstrumentation. This worked well in 7.0.

Now, having updated the dependencies to 9.0 for graphql-java, 5.0.1 for graphql-servlet and 4.2.0 for graphql-spring-boot, a query that has to invoke more than one resolver sequentially, does not return.

I've narrowed it down to the following Instrumentation not working:
return new ChainedInstrumentation(Arrays.asList(new DataLoaderDispatcherInstrumentation(...)))
And the following does work:
return new DataLoaderDispatcherInstrumentation(...)

The queries that fail are hierarchical with resolvers, so for instance:
{ entity(id=1) { property } },
will work if the property is fetched by a resolver without a batchloader, but will fail if the resolver for property also uses a batchloader (it simply does not get invoked/triggered).

Any pointers or things I can try?

@andimarek
Copy link
Member

Hi,

can you please try the lastest dev build (2018-07-04T08-09-58-795c232). It maybe fixed with this PR: #1090

Thanks!

@kclaes
Copy link
Author

kclaes commented Jul 10, 2018

Confirmed fixed in 2018-07-04T08-09-58-795c232

(I searched the issues before creating this one, but I think I couldn't find anything because there's no Issue reflecting this problem, only a PR (?) )

Any timeframe for a release that contains this fix?

@andimarek
Copy link
Member

@kclaes thanks for confirming: it is released in 9.1 (released yersterday)

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

2 participants