Skip to content

Store the http.route tag value inside the appsec request context in Play - #8991

Merged
manuel-alvarez-alvarez merged 1 commit into
masterfrom
malvarez/http-route-play
Jun 24, 2025
Merged

Store the http.route tag value inside the appsec request context in Play#8991
manuel-alvarez-alvarez merged 1 commit into
masterfrom
malvarez/http-route-play

Conversation

@manuel-alvarez-alvarez

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

Copy link
Copy Markdown
Member

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-alvarezmanuel-alvarez-alvarez added inst: play framework Play Framework instrumentation type: feature 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
@pr-commenter

pr-commenterBot commented Jun 16, 2025

Copy link
Copy Markdown

Benchmarks

Startup

Load

Parameters

BaselineCandidate
baseline_or_candidatebaselinecandidate
git_branchmastermalvarez/http-route-play
git_commit_date17507050031750705386
git_commit_sha1f2b612cf0f8aa
release_version1.51.0-SNAPSHOT~1f2b612b521.51.0-SNAPSHOT~cf0f8aa954
See matching parameters
BaselineCandidate
applicationinsecure-bankinsecure-bank
ci_job_date17507072071750707207
ci_job_id994468334994468334
ci_pipeline_id6851937068519370
cpu_modelIntel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHzIntel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
kernel_versionLinux 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/LinuxLinux 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 throughputcandidate mean http_req_durationcandidate mean throughputbaseline mean http_req_durationbaseline mean throughput
scenario:load:insecure-bank:no_agent:high_loadworse
[+285.516µs; +394.470µs] or [+6.701%; +9.258%]
unstable
[-211.156op/s; +54.594op/s] or [-19.613%; +5.071%]
4.601ms998.344op/s4.261ms1076.625op/s
scenario:load:insecure-bank:tracing:high_loadworse
[+617.159µs; +857.559µs] or [+8.300%; +11.533%]
unstable
[-137.560op/s; +26.185op/s] or [-22.098%; +4.206%]
8.173ms566.812op/s7.436ms622.500op/s
scenario:load:petclinic:no_agent:high_loadbetter
[-1.513ms; -0.863ms] or [-4.045%; -2.308%]
unstable
[-5.208op/s; +13.408op/s] or [-4.166%; +10.725%]
36.216ms129.113op/s37.404ms125.013op/s
scenario:load:petclinic:code_origins:high_loadworse
[+1.069ms; +1.896ms] or [+2.366%; +4.195%]
unstable
[-10.764op/s; +4.189op/s] or [-10.397%; +4.046%]
46.672ms100.250op/s45.189ms103.537op/s
Request duration reports for insecure-bank
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,
Loading
  • baseline results
VariantRequest duration [CI 0.99]Δ no_agent
no_agent4.261 ms [4.214 ms, 4.308 ms]-
iast8.941 ms [8.797 ms, 9.084 ms]4.68 ms (109.8%)
iast_FULL13.674 ms [13.405 ms, 13.942 ms]9.413 ms (220.9%)
iast_GLOBAL9.973 ms [9.796 ms, 10.149 ms]5.712 ms (134.0%)
profiling8.552 ms [8.422 ms, 8.683 ms]4.291 ms (100.7%)
tracing7.436 ms [7.329 ms, 7.542 ms]3.175 ms (74.5%)
  • candidate results
VariantRequest duration [CI 0.99]Δ no_agent
no_agent4.601 ms [4.547 ms, 4.655 ms]-
iast8.997 ms [8.854 ms, 9.14 ms]4.396 ms (95.5%)
iast_FULL14.241 ms [13.954 ms, 14.527 ms]9.64 ms (209.5%)
iast_GLOBAL10.247 ms [10.069 ms, 10.424 ms]5.646 ms (122.7%)
profiling8.747 ms [8.614 ms, 8.88 ms]4.146 ms (90.1%)
tracing8.173 ms [8.056 ms, 8.29 ms]3.572 ms (77.6%)
Request duration reports for petclinic
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,
Loading
  • baseline results
VariantRequest duration [CI 0.99]Δ no_agent
no_agent37.404 ms [37.095 ms, 37.712 ms]-
appsec47.466 ms [47.031 ms, 47.902 ms]10.063 ms (26.9%)
code_origins45.189 ms [44.824 ms, 45.555 ms]7.786 ms (20.8%)
iast44.688 ms [44.308 ms, 45.067 ms]7.284 ms (19.5%)
profiling48.936 ms [48.491 ms, 49.382 ms]11.533 ms (30.8%)
tracing44.657 ms [44.303 ms, 45.011 ms]7.253 ms (19.4%)
  • candidate results
VariantRequest duration [CI 0.99]Δ no_agent
no_agent36.216 ms [35.921 ms, 36.511 ms]-
appsec46.955 ms [46.546 ms, 47.363 ms]10.739 ms (29.7%)
code_origins46.672 ms [46.27 ms, 47.074 ms]10.456 ms (28.9%)
iast43.879 ms [43.503 ms, 44.254 ms]7.663 ms (21.2%)
profiling49.95 ms [49.493 ms, 50.406 ms]13.734 ms (37.9%)
tracing43.425 ms [43.062 ms, 43.787 ms]7.209 ms (19.9%)

Dacapo

@amarzialiamarziali 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.

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.

@amarzialiamarziali 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 ownerJune 17, 2025 09:52
@manuel-alvarez-alvarezmanuel-alvarez-alvarez changed the title Use the local root span for the http.route tag in PlayStore the http.route tag value inside the appsec request context in PlayJun 17, 2025
@manuel-alvarez-alvarez

Copy link
Copy Markdown
MemberAuthor

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-alvarezforce-pushed the malvarez/http-route-play branch 3 times, most recently from 956faea to 0078896CompareJune 17, 2025 13:45
@manuel-alvarez-alvarez
manuel-alvarez-alvarez changed the base branch from master to malvarez/vertx-http-routeJune 17, 2025 13:45
@manuel-alvarez-alvarez
manuel-alvarez-alvarezforce-pushed the malvarez/http-route-play branch 2 times, most recently from a9e3f59 to ad782b6CompareJune 17, 2025 17:26
@manuel-alvarez-alvarez
manuel-alvarez-alvarezforce-pushed the malvarez/vertx-http-route branch 2 times, most recently from 6a23408 to d5eeb4dCompareJune 23, 2025 10:34
Base automatically changed from malvarez/vertx-http-route to masterJune 23, 2025 18:56
@manuel-alvarez-alvarezmanuel-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 deleted the malvarez/http-route-play branch June 24, 2025 07:55
@github-actionsgithub-actionsBot 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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: asm wafApplication Security Management (WAF)inst: play frameworkPlay Framework instrumentationtag: no release notesChanges to exclude from release notestype: featureEnhancements and improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@manuel-alvarez-alvarez@amarziali@jandro996