Skip to content

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

Merged
merged 1 commit into from
Jun 24, 2025

Conversation

manuel-alvarez-alvarez
Copy link
Member

@manuel-alvarez-alvarez manuel-alvarez-alvarez commented Jun 16, 2025

What Does This Do

Store the http.route tag value inside the appsec request context in Play framework instrumentation

Motivation

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 the http.route we have to store it in the appsec request context before the sampling decision is made.

Additional Notes

Contributor Checklist

Jira ticket: APPSEC-56869

@manuel-alvarez-alvarez manuel-alvarez-alvarez added inst: play framework Play Framework instrumentation type: enhancement Enhancements and improvements labels Jun 16, 2025
@manuel-alvarez-alvarez manuel-alvarez-alvarez marked this pull request as ready for review June 16, 2025 12:33
@manuel-alvarez-alvarez manuel-alvarez-alvarez requested review from a team as code owners June 16, 2025 12:33
@pr-commenter
Copy link

pr-commenter bot commented Jun 16, 2025

Benchmarks

Startup

Load

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master malvarez/http-route-play
git_commit_date 1750705003 1750705386
git_commit_sha 1f2b612 cf0f8aa
release_version 1.51.0-SNAPSHOT~1f2b612b52 1.51.0-SNAPSHOT~cf0f8aa954
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1750707207 1750707207
ci_job_id 994468334 994468334
ci_pipeline_id 68519370 68519370
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
kernel_version Linux runner-ecr1ksdw-project-304-concurrent-2-2c8li3wn 6.8.0-1029-aws #31~22.04.1-Ubuntu SMP Thu Apr 24 21:16:18 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux Linux runner-ecr1ksdw-project-304-concurrent-2-2c8li3wn 6.8.0-1029-aws #31~22.04.1-Ubuntu SMP Thu Apr 24 21:16:18 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux

Summary

Found 1 performance improvements and 3 performance regressions! Performance is the same for 8 metrics, 12 unstable metrics.

scenario Δ mean http_req_duration Δ mean throughput candidate mean http_req_duration candidate mean throughput baseline mean http_req_duration baseline mean throughput
scenario:load:insecure-bank:no_agent:high_load worse
[+285.516µs; +394.470µs] or [+6.701%; +9.258%]
unstable
[-211.156op/s; +54.594op/s] or [-19.613%; +5.071%]
4.601ms 998.344op/s 4.261ms 1076.625op/s
scenario:load:insecure-bank:tracing:high_load worse
[+617.159µs; +857.559µs] or [+8.300%; +11.533%]
unstable
[-137.560op/s; +26.185op/s] or [-22.098%; +4.206%]
8.173ms 566.812op/s 7.436ms 622.500op/s
scenario:load:petclinic:no_agent:high_load better
[-1.513ms; -0.863ms] or [-4.045%; -2.308%]
unstable
[-5.208op/s; +13.408op/s] or [-4.166%; +10.725%]
36.216ms 129.113op/s 37.404ms 125.013op/s
scenario:load:petclinic:code_origins:high_load worse
[+1.069ms; +1.896ms] or [+2.366%; +4.195%]
unstable
[-10.764op/s; +4.189op/s] or [-10.397%; +4.046%]
46.672ms 100.250op/s 45.189ms 103.537op/s
Request duration reports for insecure-bank
Loading
gantt
    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,
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 4.261 ms [4.214 ms, 4.308 ms] -
iast 8.941 ms [8.797 ms, 9.084 ms] 4.68 ms (109.8%)
iast_FULL 13.674 ms [13.405 ms, 13.942 ms] 9.413 ms (220.9%)
iast_GLOBAL 9.973 ms [9.796 ms, 10.149 ms] 5.712 ms (134.0%)
profiling 8.552 ms [8.422 ms, 8.683 ms] 4.291 ms (100.7%)
tracing 7.436 ms [7.329 ms, 7.542 ms] 3.175 ms (74.5%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 4.601 ms [4.547 ms, 4.655 ms] -
iast 8.997 ms [8.854 ms, 9.14 ms] 4.396 ms (95.5%)
iast_FULL 14.241 ms [13.954 ms, 14.527 ms] 9.64 ms (209.5%)
iast_GLOBAL 10.247 ms [10.069 ms, 10.424 ms] 5.646 ms (122.7%)
profiling 8.747 ms [8.614 ms, 8.88 ms] 4.146 ms (90.1%)
tracing 8.173 ms [8.056 ms, 8.29 ms] 3.572 ms (77.6%)
Request duration reports for petclinic
Loading
gantt
    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,
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 37.404 ms [37.095 ms, 37.712 ms] -
appsec 47.466 ms [47.031 ms, 47.902 ms] 10.063 ms (26.9%)
code_origins 45.189 ms [44.824 ms, 45.555 ms] 7.786 ms (20.8%)
iast 44.688 ms [44.308 ms, 45.067 ms] 7.284 ms (19.5%)
profiling 48.936 ms [48.491 ms, 49.382 ms] 11.533 ms (30.8%)
tracing 44.657 ms [44.303 ms, 45.011 ms] 7.253 ms (19.4%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 36.216 ms [35.921 ms, 36.511 ms] -
appsec 46.955 ms [46.546 ms, 47.363 ms] 10.739 ms (29.7%)
code_origins 46.672 ms [46.27 ms, 47.074 ms] 10.456 ms (28.9%)
iast 43.879 ms [43.503 ms, 44.254 ms] 7.663 ms (21.2%)
profiling 49.95 ms [49.493 ms, 50.406 ms] 13.734 ms (37.9%)
tracing 43.425 ms [43.062 ms, 43.787 ms] 7.209 ms (19.9%)

Dacapo

Copy link
Contributor

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

@amarziali amarziali added the tag: breaking change Breaking changes label Jun 17, 2025
@manuel-alvarez-alvarez manuel-alvarez-alvarez requested a review from a team as a code owner June 17, 2025 09:52
@manuel-alvarez-alvarez manuel-alvarez-alvarez changed the title Use the local root span for the http.route tag in Play Store the http.route tag value inside the appsec request context in Play Jun 17, 2025
@manuel-alvarez-alvarez
Copy link
Member Author

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.

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!

@manuel-alvarez-alvarez manuel-alvarez-alvarez force-pushed the malvarez/http-route-play branch 3 times, most recently from 956faea to 0078896 Compare June 17, 2025 13:45
@manuel-alvarez-alvarez manuel-alvarez-alvarez changed the base branch from master to malvarez/vertx-http-route June 17, 2025 13:45
@manuel-alvarez-alvarez manuel-alvarez-alvarez force-pushed the malvarez/http-route-play branch 2 times, most recently from a9e3f59 to ad782b6 Compare June 17, 2025 17:26
@manuel-alvarez-alvarez manuel-alvarez-alvarez force-pushed the malvarez/vertx-http-route branch 2 times, most recently from 6a23408 to d5eeb4d Compare June 23, 2025 10:34
Base automatically changed from malvarez/vertx-http-route to master June 23, 2025 18:56
@manuel-alvarez-alvarez manuel-alvarez-alvarez added comp: asm waf Application Security Management (WAF) tag: no release notes Changes to exclude from release notes labels Jun 23, 2025
@manuel-alvarez-alvarez manuel-alvarez-alvarez merged commit 6553226 into master Jun 24, 2025
654 of 655 checks passed
@manuel-alvarez-alvarez manuel-alvarez-alvarez deleted the malvarez/http-route-play branch June 24, 2025 07:55
@github-actions github-actions bot added this to the 1.51.0 milestone Jun 24, 2025
jandro996 added a commit that referenced this pull request Jul 10, 2025
#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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: asm waf Application Security Management (WAF) inst: play framework Play Framework instrumentation tag: no release notes Changes to exclude from release notes type: enhancement Enhancements and improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants