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

Add priorities for server set errors #5359

Merged
merged 1 commit into from
Jun 13, 2023
Merged

Conversation

amarziali
Copy link
Collaborator

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:

  • The error set by the user should have higher prio than the one set according to a http response status code
  • The application server throws an error but, despite logged, the error flag should be set according to the http status code

Motivation

Additional Notes

@amarziali amarziali requested a review from a team as a code owner June 9, 2023 14:19
@pr-commenter
Copy link

pr-commenter bot commented Jun 9, 2023

Benchmarks

Parameters

Baseline Candidate
commit 1.16.0-SNAPSHOT~8ac87914bf 1.16.0-SNAPSHOT~74661c498f
config baseline candidate
See matching parameters
Baseline Candidate
module Agent Agent
parent None None

Summary

Found 0 performance improvements and 2 performance regressions! Performance is the same for 20 cases.

scenario Δ mean execution_time
scenario:Startup-base-AppSec worse
[+4.171ms; +4.996ms] or [+8.400%; +10.062%]
scenario:Startup-iast-AppSec worse
[+3.969ms; +4.905ms] or [+8.195%; +10.127%]

@amarziali amarziali force-pushed the andrea.marziali/error-prios branch 2 times, most recently from d0ea4a1 to d4961bc Compare June 9, 2023 15:03
@amarziali amarziali force-pushed the andrea.marziali/error-prios branch from d4961bc to 74661c4 Compare June 12, 2023 07:47
@@ -363,6 +365,16 @@ private AgentSpan.Context.Extracted callIGCallbackStart(AgentSpan.Context.Extrac
return context;
}

@Override
public AgentSpan onError(final AgentSpan span, final Throwable throwable) {
Copy link
Contributor

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

Copy link
Collaborator Author

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

Copy link
Contributor

@ygree ygree left a 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.

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

Successfully merging this pull request may close these issues.

2 participants