Skip to content

Conversation

@amarziali
Copy link
Contributor

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
}

@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
Contributor 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

Labels

type: bug Bug report and fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants