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

Allow user to enable/disable storing entire stack trace in spans #431

Open
dneray opened this issue May 23, 2020 · 10 comments
Open

Allow user to enable/disable storing entire stack trace in spans #431

dneray opened this issue May 23, 2020 · 10 comments
Labels
enhancement New feature or request

Comments

@dneray
Copy link
Contributor

dneray commented May 23, 2020

When using the @WithSpan annotation in a high load system, it can be expensive to serialize all stack traces. While these are obviously incredibly valuable, it would be nice to have a way for users to disable it.

I'm not sure if it makes more sense to have this part of the annotation itself or a global property.

@iNikem
Copy link
Contributor

iNikem commented May 23, 2020

Can please specify, what stacktraces are talking about?

@dneray
Copy link
Contributor Author

dneray commented May 23, 2020

If a method annotated with @WithSpan throws an exception, the entire stack trace is stored in an attribute called error.stacktrace.

I believe this happens in io.opentelemetry.auto.bootstrap.instrumentation.decorator.BaseDecorator, within the addThrowable method.

@trask
Copy link
Member

trask commented May 23, 2020

Hi @dneray, does sampling address your concern?

If not, I could see this being part of the annotation, e.g. for methods where exceptions are fairly common and the stack trace is not really useful, but you still want to capture timing and success/failure (we would still mark the span as an error and capture the exception message).

public @interface WithSpan {
  String value() default "";
  boolean captureStackTraceOnThrowable() default true;
}

@dneray
Copy link
Contributor Author

dneray commented May 23, 2020

Thanks @trask sampling would help reduce the load but ultimately I think that addition to the WithSpan annotation would be the ideal solution.

Ideally the backend wouldn't be throwing exceptions frequently with useless stack traces but unfortunately many applications do and it would be a lot of overhead to serialize them even with sampling enabled.

@trask
Copy link
Member

trask commented May 23, 2020

@dneray In your system, does this concern also apply to spans from the built-in instrumentation (e.g. servlets, http clients)? If so, maybe this is a global setting that says "don't capture any exception stack traces"? (or maybe a max frames setting? we'd have to decide what that means for caused by chains)

@dneray
Copy link
Contributor Author

dneray commented May 23, 2020

In my system so far, I have not seen anywhere where this concern is applicable it the built-in instrumentations but I can see how it might for others.

Now that I think about it, for my case another option that would work very well would be to be able to ignore certain types of exceptions i.e. similar to the TRACE_CLASSES_EXCLUDE and TRACE_METHODS_EXCLUDE there could be something like a TRACE_THROWABLES_IGNORE.

Not sure if that would work best for everyone though, I could imagine that there are codebases full of generic exceptions that are thrown from everywhere.

@iNikem
Copy link
Contributor

iNikem commented May 23, 2020

There are many options how to solve this issue.

  1. Have a new attribute on @WithSpan annotation to drop exception stack trace
  2. Have a new attribute on @WithSpsn annotation to ignore some classes of exception
  3. Have a global configuration option to limit the number of frames captured for all exceptions
  4. Have a global configuration option to ignore some classes of exceptions
  5. Use OpenTelemetry Collector to drop unwanted span attributes in this case error.message

First of all I would suggest to see if option number 5 solves your concern. It is the fastest option in your case.

Next, ignoring specific exceptions would probably mean as well changing the status of span from one of the error to one of the normal operation. It depends on your use case if you want this or not. I have a request from our customers to do exactly this: treat some exceptions as “normal”, nor errors.

I don’t think that @WithSpan notation is special enough to deserve the preferential treatment of option 1 or 2 from above. If do something along these lines, then I prefer either option 3 or 4. Meaning global configuration.

@dneray
Copy link
Contributor Author

dneray commented May 23, 2020

Thanks @iNikem

I am already using the collector but unfortunately it still causes excessive GC pauses to send these traces out of the java app.

Option 4 is preferable for me, I understand that it would treat these exceptions as "normal". For my use case, I would rather have the option to use the manual instrumentation to control how these are captured in the spans.

@trask
Copy link
Member

trask commented May 23, 2020

@dneray Using the OpenTelemetry API seems like it might be a good option here as an alternative to using @WithSpan. See https://github.com/open-telemetry/opentelemetry-auto-instr-java#manually-instrumenting.

@dneray
Copy link
Contributor Author

dneray commented May 23, 2020

Thanks, yes that is what I have done for now but I would really like to be able to use the @WithSpan annotation instead.

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

No branches or pull requests

3 participants