-
Notifications
You must be signed in to change notification settings - Fork 308
Store the http.route tag value inside the appsec request context in Play #8991
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
Conversation
e06c22a
to
e5e0581
Compare
BenchmarksStartupLoadParameters
See matching parameters
SummaryFound 1 performance improvements and 3 performance regressions! Performance is the same for 8 metrics, 12 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.51.0-SNAPSHOT~cf0f8aa954, baseline=1.51.0-SNAPSHOT~1f2b612b52
dateFormat X
axisFormat %s
section baseline
no_agent (4.261 ms) : 4214, 4308
. : milestone, 4261,
iast (8.941 ms) : 8797, 9084
. : milestone, 8941,
iast_FULL (13.674 ms) : 13405, 13942
. : milestone, 13674,
iast_GLOBAL (9.973 ms) : 9796, 10149
. : milestone, 9973,
profiling (8.552 ms) : 8422, 8683
. : milestone, 8552,
tracing (7.436 ms) : 7329, 7542
. : milestone, 7436,
section candidate
no_agent (4.601 ms) : 4547, 4655
. : milestone, 4601,
iast (8.997 ms) : 8854, 9140
. : milestone, 8997,
iast_FULL (14.241 ms) : 13954, 14527
. : milestone, 14241,
iast_GLOBAL (10.247 ms) : 10069, 10424
. : milestone, 10247,
profiling (8.747 ms) : 8614, 8880
. : milestone, 8747,
tracing (8.173 ms) : 8056, 8290
. : milestone, 8173,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.51.0-SNAPSHOT~cf0f8aa954, baseline=1.51.0-SNAPSHOT~1f2b612b52
dateFormat X
axisFormat %s
section baseline
no_agent (37.404 ms) : 37095, 37712
. : milestone, 37404,
appsec (47.466 ms) : 47031, 47902
. : milestone, 47466,
code_origins (45.189 ms) : 44824, 45555
. : milestone, 45189,
iast (44.688 ms) : 44308, 45067
. : milestone, 44688,
profiling (48.936 ms) : 48491, 49382
. : milestone, 48936,
tracing (44.657 ms) : 44303, 45011
. : milestone, 44657,
section candidate
no_agent (36.216 ms) : 35921, 36511
. : milestone, 36216,
appsec (46.955 ms) : 46546, 47363
. : milestone, 46955,
code_origins (46.672 ms) : 46270, 47074
. : milestone, 46672,
iast (43.879 ms) : 43503, 44254
. : milestone, 43879,
profiling (49.95 ms) : 49493, 50406
. : milestone, 49950,
tracing (43.425 ms) : 43062, 43787
. : milestone, 43425,
Dacapo |
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.
The change looks OK. However this is a breaking change since the route will change the resource hence the RED metrics. Please do not forget to mention in the release note. You can also leave a notice in the PR that, to disable this behaviour, the user can disable the route based resource naming.
5097a43
to
f980f9c
Compare
Thanks for the review @amarziali, I did update the PR so we don´t break any customer. The change is not clean as before but it works fine for us. Thanks again! |
956faea
to
0078896
Compare
9b5f0d7
to
ad0bc38
Compare
a9e3f59
to
ad782b6
Compare
ad0bc38
to
7c8620c
Compare
5ae3d48
to
aac9883
Compare
6a23408
to
d5eeb4d
Compare
aac9883
to
ad5e01d
Compare
ad5e01d
to
cf0f8aa
Compare
#9105) What Does This Do Store the http.route tag value inside the iast request context in Play framework instrumentation Move PlayHttpServerDecorator.onRequest to onEnter advice from onExit advice. We need to send the event on enter to have the info in the context available when vulns are detected during the requests Motivation IAST sampling algorithm requires the http.route span tag to be set on the local root span so it can be used for its sampling decision. Since Play does not use the local root span for the http.route we have to store it in the iast request context before the sampling decision is made. Additional Notes related with #8991
What Does This Do
Store the
http.route
tag value inside the appsec request context in Play framework instrumentationMotivation
AppSec API protection requires the
http.route
span tag to be set on the local root span so it can be used for its sampling decision. Since Play does not use the local root span for thehttp.route
we have to store it in the appsec request context before the sampling decision is made.Additional Notes
Contributor Checklist
type:
and (comp:
orinst:
) labels in addition to any usefull labelsclose
,fix
or any linking keywords when referencing an issue.Use
solves
instead, and assign the PR milestone to the issueJira ticket: APPSEC-56869