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

Excluding Trace Methods #779

Closed
ethranes opened this issue Aug 5, 2019 · 7 comments
Closed

Excluding Trace Methods #779

ethranes opened this issue Aug 5, 2019 · 7 comments
Labels

Comments

@ethranes
Copy link

ethranes commented Aug 5, 2019

Is your feature request related to a problem? Please describe.
When instrumenting code with the trace_methods property there is no way of excluding a class/method, even though it may just be a single class that you're trying to exclude from a directory that has a large amount of other classes.

Describe the solution you'd like
I am suggesting an option to exclude specific classes from being instrumented even if the class exists in a location where trace_methods is being implemented. The syntax could be: trace_methods=com.example.*, -com.example.foo.*.

Describe alternatives you've considered
An alternate solution for this would be to list each class individually that you want instrumented in the trace_method property, which isn't feasible for larger projects, it's better to list what you don't want instrumented instead of listing what you do want.

@christiangalsterer
Copy link

In our company we would also be interested in this feature. I would also submit a PR but would like to get some feedback on the following implementation ideas:

  1. Specify classes to include and exclude in the trace_methods parameter, using "+" prefix or no prefix for includes and "-" for excludes (original proposal).
  2. Use two separate parameter for includes and excludes, similar to profiling_inferred_spans_included_classes and profiling_inferred_spans_excluded_classes.

In order to keep consistency I would have preference for option 2 but open for any ideas, proposals. Please let me know if you would be interested in a contribution for this issue.

@felixbarny
Copy link
Member

Out of curiosity: what’s your use case for trace_methods now that there are profiler inferred spans?

Both approaches sound fine to me, I don’t have a strong preference.

@christiangalsterer
Copy link

According to the documentation profiler inferred spans are experimental and they have a trade off between accuracy and performance. So I was wondering if the feature is mature enough for a large scale production usage and hence was looking into extending the existing feature. Do you have any insides if and when profiler inferred span become stable and if they would replace the existing stable feature?

@felixbarny
Copy link
Member

Great question.

Inferred spans are not experimental because we think they may cause stability issues with the application. It's built on the widely used profiler async-profiler which does the heavy lifting of efficiently collecting stack traces.

The main reasons for it being experimental are:

  • We want to be able to change configuration options and their defaults (for example, we did a major change in 1.15.0).
  • There are some issues with parent/child relationships which we want to fix before declaring it a generally available feature: Reparenting spans to support inferred spans apm#247
    The impact is basically that some spans are displayed out-of-order in the UI.
  • We're working on better documentation for the feature.

If you are fine which those caveats, please give it a try and we'd be happy to get your feedback.
But we also don't plan to remove trace_methods.

@christiangalsterer
Copy link

Thanks for sharing the insides. We will give it a try and will let you know about our experiences.

@SylvainJuge
Copy link
Member

Hi @ethranes and @christiangalsterer , this issue has been open for a while and I was wondering if you managed to use inferred spans for your use-cases and had any feedback about it. Right now this issue isn't really actionable as-is, thus we are quite likely to close it without further updates.

Also, async-profiler that is used by the agent has been updated in latest (1.19.0) version of the agent, thus in case you had any stability issues with it, it might be better now.

@SylvainJuge
Copy link
Member

This issue hasn't been updated in a while, thus we'll close it for now.

The way to find slow/relevant methods is now properly documented here.

Also, ff you have time to provide feedback, can you describe your use-case for using trace_methods in the first place here ?

  • instrument a library/framework that is not currently natively supported through auto-instrumentation
  • instrument a part of your application that needs to be monitored with dedicated spans/transactions: for example, custom batch execution, or internal delegation to a sub-component of the application.
  • something else

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants