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

Make it possible to exempt some resolvers from tracing #859

Open
vlaci opened this issue May 3, 2022 · 5 comments
Open

Make it possible to exempt some resolvers from tracing #859

vlaci opened this issue May 3, 2022 · 5 comments
Labels
enhancement New feature or request help wanted Extra attention is needed to do

Comments

@vlaci
Copy link

vlaci commented May 3, 2022

The default behavior of only tracing custom resolvers is great 95% of the time, however there is a case for not to trace certain custom resolvers as well.

We have a few simple custom resolvers that can be called up to a couple thousand times per request where the tracing overhead becomes significant. Also, in our case they don't do meaningful work that would warrant tracing them.

Right now I see two ways to implement a workaround in our code-base without touching ariadne itself:

  1. Monkey-patch ariadne.contrib.tracing.should_trace function
  2. Add _ariadne_alias_resolver attribute to our own resolvers, disguising them as alias resolvers which are already exempt fro tracing.
  3. Extend the tracing extensions in our code to do the filtering.

The first two approaches are brittle and can break upon any update to ariadne. We can go forward using the third approach but we think it'd make sense to add this functionality in ariadne itself.

I'd like to contribute support for this functionality in ariadne if you accept my reasoning for the need of it. My plan is to add a filter callable to the tracing extensions that can look-up the field in a block-list or perform any other custom logic.
An even less intrusive change could also be just adding a should_trace method in the tracing extensions that can be overridden in a subclass, instead of using a non-member function.

@rafalp
Copy link
Contributor

rafalp commented May 4, 2022

Construction option (like we are already doing with args_filter) is the way to go here IMHO. This option should default to should_trace.

@rafalp rafalp added enhancement New feature or request help wanted Extra attention is needed labels May 4, 2022
@vlaci
Copy link
Author

vlaci commented May 5, 2022

I've taken a quick look and it is easy to do this way but the should_trace function having a trace_default_resolver bool argument makes it a bit smelly to me. One part of the tracing is configured via a callable, the other via a separate flag seems weird. It is easy to make should_trace be more restrictive via wrapping it to a separate function but it is hard to make it more permissive without duplicating its contents.

@rafalp
Copy link
Contributor

rafalp commented May 5, 2022

Yeah, we should drop trace_default_resolver at same time, but it would be good to have both at same time for deprecation period.

For now we could have both options but raise an error if custom tracing filter is set together with trace_default_resolver.

@rafalp
Copy link
Contributor

rafalp commented May 12, 2023

Actually, I've found a way to implement extensions that is much faster that what we currently have, and I would like to implement it in future release of Ariadne, but I've been busy with other commitments recently that prevented me from doing so.

@rafalp
Copy link
Contributor

rafalp commented Jul 21, 2023

Faster extensions landed in Ariadne 0.20, but as I've mentioned on Ariadne's blog, there's still utility in having an option to remove fields from tracing extensions for removing the especially noisy resolvers from APM.

@rafalp rafalp added the roadmap Feature that we want to have included label Jul 21, 2023
@TMuszczekk TMuszczekk added to do and removed roadmap Feature that we want to have included labels Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed to do
Projects
None yet
Development

No branches or pull requests

3 participants