-
Notifications
You must be signed in to change notification settings - Fork 293
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
Add priorities for server set errors #5359
Conversation
BenchmarksParameters
See matching parameters
SummaryFound 0 performance improvements and 2 performance regressions! Performance is the same for 20 cases.
|
d0ea4a1
to
d4961bc
Compare
d4961bc
to
74661c4
Compare
@@ -363,6 +365,16 @@ private AgentSpan.Context.Extracted callIGCallbackStart(AgentSpan.Context.Extrac | |||
return context; | |||
} | |||
|
|||
@Override | |||
public AgentSpan onError(final AgentSpan span, final Throwable throwable) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't see any usages of this method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's used by Subclasses (e.g. datadog.trace.instrumentation.tomcat.TomcatDecorator#onResponse
calls onError
).
Being overridded now http server based decorators are inheriting this behavior
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except for onError method that I can't find any usages for.
What Does This Do
Similarly to resource names, it adds priority based error flag settings.
The http servers will set error using a lower priority than the user one. The errors set elsewhere will stay untouched (but it should be changed in the future as well).
It solves a particular two use cases:
Motivation
Additional Notes