Skip to content
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

Tracing inconsistency #3393

Closed
eapache opened this issue Mar 16, 2021 · 2 comments · Fixed by #3408
Closed

Tracing inconsistency #3393

eapache opened this issue Mar 16, 2021 · 2 comments · Fixed by #3408

Comments

@eapache
Copy link
Contributor

eapache commented Mar 16, 2021

Describe the bug

The order of tracing events is inconsistent depending on how you execute a query.

If you instantiate a GraphQL::Query with a query string and schema, then call #result on it, then the lex and parse trace events fire outside of execute_multiplex.

If instead you call Schema.execute or Schema.multiplex, then the lex and parse trace events fire inside of execute_multiplex.

This is because Query calls Multiplex.run_queries inside of with_prepared_ast:

with_prepared_ast {
Execution::Multiplex.run_queries(@schema, [self], context: @context)
}

Where-as Schema.multiplex calls Multiplex.run_all directly which fires the execute_multiplex tracer before it does anything that triggers lexing and parsing.

@rmosolgo maybe this isn't technically a bug but it's surprising enough it should at least be called out in the documentation.

Versions

1.12.5, efcedef, probably master

cc @chrisbutcher

@eapache
Copy link
Contributor Author

eapache commented Mar 16, 2021

It might be as simple as not calling with_prepared_ast in Query.result?

@rmosolgo
Copy link
Owner

Thanks for reporting this -- I agree it would be better if it was consistent!

It might be as simple as not calling with_prepared_ast in Query.result?

I believe it -- since execution will eventually call some other method that causes the query to prepare its AST.

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

Successfully merging a pull request may close this issue.

2 participants