Skip to content

Add priorities for server set errors - #5359

Merged
amarziali merged 1 commit into
masterfrom
andrea.marziali/error-prios
Jun 13, 2023
Merged

Add priorities for server set errors#5359
amarziali merged 1 commit into
masterfrom
andrea.marziali/error-prios

Conversation

@amarziali

Copy link
Copy Markdown
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

pr-commenter Bot commented Jun 9, 2023

Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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

@ygree ygree left a comment

Copy link
Copy Markdown
Contributor

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants