Skip to content

Armeria Jetty: ensure spans are finished#6291

Merged
amarziali merged 1 commit into
masterfrom
andrea.marziali/armeria-jetty-server
Dec 5, 2023
Merged

Armeria Jetty: ensure spans are finished#6291
amarziali merged 1 commit into
masterfrom
andrea.marziali/armeria-jetty-server

Conversation

@amarziali

@amarziali amarziali commented Nov 29, 2023

Copy link
Copy Markdown
Contributor

What Does This Do

Armeria is using the underlying jetty engine without calling recycle that's what we instrument as a span termination pointcut. Not finishing that root span will prevent the full trace from being sent.

This PR fixes #6248 for jetty 11 and jetty 9

Motivation

Additional Notes

I had to shortcut some query encoding tests that are not passing probably because armeria adapt the request to jetty in a different way. However this should be perfectly fine (and anyway out of scope on this PR) since we only want to test that the spans opened in most common use cases are finished.

Jira ticket: [PROJ-IDENT]

@amarziali
amarziali requested a review from a team as a code owner November 29, 2023 13:11
@amarziali amarziali added inst: armeria Armeria Instrumentation type: bug fix Bug fix labels Nov 29, 2023
@amarziali
amarziali force-pushed the andrea.marziali/armeria-jetty-server branch from cc64f2f to 7a65883 Compare November 29, 2023 13:26
@pr-commenter

pr-commenter Bot commented Nov 29, 2023 •
edited
Loading

Copy link
Copy Markdown

Benchmarks

Startup

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master andrea.marziali/armeria-jetty-server
git_commit_date 1701269143 1701271951
git_commit_sha 92c67e8 31133c7
release_version 1.25.0-SNAPSHOT~92c67e8373 1.25.0-SNAPSHOT~31133c7040
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1701274729 1701274729
ci_job_id 377865527 377865527
ci_pipeline_id 23934088 23934088
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
module Agent Agent
parent None None
variant iast iast

Summary

Found 0 performance improvements and 1 performance regressions! Performance is the same for 43 metrics, 10 unstable metrics.

scenario Δ mean execution_time candidate mean execution_time baseline mean execution_time
scenario:startup:insecure-bank:iast_TELEMETRY_OFF:AppSec worse
[+3.681ms; +7.494ms] or [+7.956%; +16.200%]		 51.851ms 46.264ms
Startup time reports for petclinic 
gantt
    title petclinic - global startup overhead: candidate=1.25.0-SNAPSHOT~31133c7040, baseline=1.25.0-SNAPSHOT~92c67e8373

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.058 s) : 0, 1057854
Total [baseline] (9.396 s) : 0, 9396324
Agent [candidate] (1.035 s) : 0, 1034674
Total [candidate] (9.316 s) : 0, 9316302
section appsec
Agent [baseline] (1.12 s) : 0, 1119650
Total [baseline] (9.41 s) : 0, 9409989
Agent [candidate] (1.125 s) : 0, 1124898
Total [candidate] (9.474 s) : 0, 9474362
section iast
Agent [baseline] (1.153 s) : 0, 1153422
Total [baseline] (9.526 s) : 0, 9526284
Agent [candidate] (1.154 s) : 0, 1153689
Total [candidate] (9.558 s) : 0, 9558278
section profiling
Agent [baseline] (1.221 s) : 0, 1221274
Total [baseline] (9.596 s) : 0, 9595866
Agent [candidate] (1.229 s) : 0, 1228930
Total [candidate] (9.635 s) : 0, 9635290
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.058 s -
Agent appsec 1.12 s 61.796 ms (5.8%)
Agent iast 1.153 s 95.568 ms (9.0%)
Agent profiling 1.221 s 163.42 ms (15.4%)
Total tracing 9.396 s -
Total appsec 9.41 s 13.665 ms (0.1%)
Total iast 9.526 s 129.959 ms (1.4%)
Total profiling 9.596 s 199.541 ms (2.1%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.035 s -
Agent appsec 1.125 s 90.224 ms (8.7%)
Agent iast 1.154 s 119.015 ms (11.5%)
Agent profiling 1.229 s 194.256 ms (18.8%)
Total tracing 9.316 s -
Total appsec 9.474 s 158.061 ms (1.7%)
Total iast 9.558 s 241.976 ms (2.6%)
Total profiling 9.635 s 318.988 ms (3.4%) 
gantt
    title petclinic - break down per module: candidate=1.25.0-SNAPSHOT~31133c7040, baseline=1.25.0-SNAPSHOT~92c67e8373

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (662.297 ms) : 0, 662297
BytebuddyAgent [candidate] (645.115 ms) : 0, 645115
GlobalTracer [baseline] (303.403 ms) : 0, 303403
GlobalTracer [candidate] (298.023 ms) : 0, 298023
AppSec [baseline] (48.955 ms) : 0, 48955
AppSec [candidate] (49.274 ms) : 0, 49274
Remote Config [baseline] (701.811 µs) : 0, 702
Remote Config [candidate] (689.62 µs) : 0, 690
Telemetry [baseline] (7.344 ms) : 0, 7344
Telemetry [candidate] (7.319 ms) : 0, 7319
section appsec
BytebuddyAgent [baseline] (643.516 ms) : 0, 643516
BytebuddyAgent [candidate] (647.169 ms) : 0, 647169
GlobalTracer [baseline] (297.886 ms) : 0, 297886
GlobalTracer [candidate] (298.875 ms) : 0, 298875
AppSec [baseline] (136.575 ms) : 0, 136575
AppSec [candidate] (137.045 ms) : 0, 137045
Remote Config [baseline] (667.603 µs) : 0, 668
Remote Config [candidate] (646.855 µs) : 0, 647
Telemetry [baseline] (6.805 ms) : 0, 6805
Telemetry [candidate] (6.784 ms) : 0, 6784
section iast
BytebuddyAgent [baseline] (765.27 ms) : 0, 765270
BytebuddyAgent [candidate] (766.778 ms) : 0, 766778
GlobalTracer [baseline] (278.67 ms) : 0, 278670
GlobalTracer [candidate] (278.655 ms) : 0, 278655
AppSec [baseline] (48.158 ms) : 0, 48158
AppSec [candidate] (47.002 ms) : 0, 47002
IAST [baseline] (14.836 ms) : 0, 14836
IAST [candidate] (17.599 ms) : 0, 17599
Remote Config [baseline] (571.774 µs) : 0, 572
Remote Config [candidate] (579.081 µs) : 0, 579
Telemetry [baseline] (11.584 ms) : 0, 11584
Telemetry [candidate] (8.746 ms) : 0, 8746
section profiling
BytebuddyAgent [baseline] (654.233 ms) : 0, 654233
BytebuddyAgent [candidate] (657.36 ms) : 0, 657360
GlobalTracer [baseline] (368.627 ms) : 0, 368627
GlobalTracer [candidate] (371.104 ms) : 0, 371104
AppSec [baseline] (48.632 ms) : 0, 48632
AppSec [candidate] (49.272 ms) : 0, 49272
Remote Config [baseline] (694.371 µs) : 0, 694
Remote Config [candidate] (710.644 µs) : 0, 711
Telemetry [baseline] (7.344 ms) : 0, 7344
Telemetry [candidate] (7.448 ms) : 0, 7448
ProfilingAgent [baseline] (87.798 ms) : 0, 87798
ProfilingAgent [candidate] (88.854 ms) : 0, 88854
Profiling [baseline] (87.821 ms) : 0, 87821
Profiling [candidate] (88.877 ms) : 0, 88877
Loading
Startup time reports for insecure-bank 
gantt
    title insecure-bank - global startup overhead: candidate=1.25.0-SNAPSHOT~31133c7040, baseline=1.25.0-SNAPSHOT~92c67e8373

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.031 s) : 0, 1031198
Total [baseline] (8.676 s) : 0, 8675925
Agent [candidate] (1.035 s) : 0, 1034844
Total [candidate] (8.682 s) : 0, 8682496
section iast
Agent [baseline] (1.151 s) : 0, 1151261
Total [baseline] (9.208 s) : 0, 9208023
Agent [candidate] (1.159 s) : 0, 1158526
Total [candidate] (9.269 s) : 0, 9268793
section iast_TELEMETRY_OFF
Agent [baseline] (1.143 s) : 0, 1143087
Total [baseline] (9.201 s) : 0, 9200955
Agent [candidate] (1.145 s) : 0, 1145174
Total [candidate] (9.21 s) : 0, 9210016
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.031 s -
Agent iast 1.151 s 120.062 ms (11.6%)
Agent iast_TELEMETRY_OFF 1.143 s 111.889 ms (10.9%)
Total tracing 8.676 s -
Total iast 9.208 s 532.098 ms (6.1%)
Total iast_TELEMETRY_OFF 9.201 s 525.03 ms (6.1%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.035 s -
Agent iast 1.159 s 123.682 ms (12.0%)
Agent iast_TELEMETRY_OFF 1.145 s 110.331 ms (10.7%)
Total tracing 8.682 s -
Total iast 9.269 s 586.296 ms (6.8%)
Total iast_TELEMETRY_OFF 9.21 s 527.519 ms (6.1%) 
gantt
    title insecure-bank - break down per module: candidate=1.25.0-SNAPSHOT~31133c7040, baseline=1.25.0-SNAPSHOT~92c67e8373

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (643.286 ms) : 0, 643286
BytebuddyAgent [candidate] (644.729 ms) : 0, 644729
GlobalTracer [baseline] (297.627 ms) : 0, 297627
GlobalTracer [candidate] (299.144 ms) : 0, 299144
AppSec [baseline] (48.175 ms) : 0, 48175
AppSec [candidate] (48.632 ms) : 0, 48632
Remote Config [baseline] (684.759 µs) : 0, 685
Remote Config [candidate] (685.602 µs) : 0, 686
Telemetry [baseline] (7.17 ms) : 0, 7170
Telemetry [candidate] (7.258 ms) : 0, 7258
section iast
BytebuddyAgent [baseline] (764.804 ms) : 0, 764804
BytebuddyAgent [candidate] (769.803 ms) : 0, 769803
GlobalTracer [baseline] (277.794 ms) : 0, 277794
GlobalTracer [candidate] (279.877 ms) : 0, 279877
AppSec [baseline] (48.037 ms) : 0, 48037
AppSec [candidate] (48.58 ms) : 0, 48580
IAST [baseline] (17.624 ms) : 0, 17624
IAST [candidate] (17.04 ms) : 0, 17040
Remote Config [baseline] (583.401 µs) : 0, 583
Remote Config [candidate] (579.249 µs) : 0, 579
Telemetry [baseline] (8.002 ms) : 0, 8002
Telemetry [candidate] (8.078 ms) : 0, 8078
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (757.781 ms) : 0, 757781
BytebuddyAgent [candidate] (758.766 ms) : 0, 758766
GlobalTracer [baseline] (278.432 ms) : 0, 278432
GlobalTracer [candidate] (278.718 ms) : 0, 278718
AppSec [baseline] (46.264 ms) : 0, 46264
AppSec [candidate] (51.851 ms) : 0, 51851
IAST [baseline] (19.341 ms) : 0, 19341
IAST [candidate] (13.828 ms) : 0, 13828
Remote Config [baseline] (554.778 µs) : 0, 555
Remote Config [candidate] (548.532 µs) : 0, 549
Telemetry [baseline] (6.383 ms) : 0, 6383
Telemetry [candidate] (7.091 ms) : 0, 7091
Loading

Load

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
end_time 2023-11-29T15:58:04 2023-11-29T16:14:36
git_branch master andrea.marziali/armeria-jetty-server
git_commit_date 1701269143 1701271951
git_commit_sha 92c67e8 31133c7
release_version 1.25.0-SNAPSHOT~92c67e8373 1.25.0-SNAPSHOT~31133c7040
start_time 2023-11-29T15:57:51 2023-11-29T16:14:23
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1701274729 1701274729
ci_job_id 377865527 377865527
ci_pipeline_id 23934088 23934088
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
variant iast iast

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 8 metrics, 14 unstable metrics.

Request duration reports for petclinic 
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.25.0-SNAPSHOT~31133c7040, baseline=1.25.0-SNAPSHOT~92c67e8373
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.346 ms) : 1327, 1365
.   : milestone, 1346,
appsec (1.756 ms) : 1732, 1780
.   : milestone, 1756,
iast (1.495 ms) : 1471, 1519
.   : milestone, 1495,
profiling (1.547 ms) : 1521, 1574
.   : milestone, 1547,
tracing (1.507 ms) : 1483, 1531
.   : milestone, 1507,
section candidate
no_agent (1.345 ms) : 1325, 1364
.   : milestone, 1345,
appsec (1.756 ms) : 1731, 1781
.   : milestone, 1756,
iast (1.534 ms) : 1510, 1558
.   : milestone, 1534,
profiling (1.491 ms) : 1466, 1516
.   : milestone, 1491,
tracing (1.521 ms) : 1497, 1546
.   : milestone, 1521,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.346 ms [1.327 ms, 1.365 ms] -
appsec 1.756 ms [1.732 ms, 1.78 ms] 410.225 µs (30.5%)
iast 1.495 ms [1.471 ms, 1.519 ms] 149.199 µs (11.1%)
profiling 1.547 ms [1.521 ms, 1.574 ms] 201.466 µs (15.0%)
tracing 1.507 ms [1.483 ms, 1.531 ms] 161.033 µs (12.0%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.345 ms [1.325 ms, 1.364 ms] -
appsec 1.756 ms [1.731 ms, 1.781 ms] 411.281 µs (30.6%)
iast 1.534 ms [1.51 ms, 1.558 ms] 189.174 µs (14.1%)
profiling 1.491 ms [1.466 ms, 1.516 ms] 146.775 µs (10.9%)
tracing 1.521 ms [1.497 ms, 1.546 ms] 176.759 µs (13.1%)
Request duration reports for insecure-bank 
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.25.0-SNAPSHOT~31133c7040, baseline=1.25.0-SNAPSHOT~92c67e8373
    dateFormat X
    axisFormat %s
section baseline
no_agent (370.855 µs) : 350, 391
.   : milestone, 371,
iast (472.507 µs) : 452, 493
.   : milestone, 473,
iast_FULL (533.843 µs) : 513, 554
.   : milestone, 534,
iast_INACTIVE (437.06 µs) : 416, 458
.   : milestone, 437,
iast_TELEMETRY_OFF (461.483 µs) : 441, 482
.   : milestone, 461,
tracing (441.582 µs) : 420, 463
.   : milestone, 442,
section candidate
no_agent (369.65 µs) : 348, 391
.   : milestone, 370,
iast (471.201 µs) : 451, 492
.   : milestone, 471,
iast_FULL (528.26 µs) : 508, 549
.   : milestone, 528,
iast_INACTIVE (443.758 µs) : 423, 465
.   : milestone, 444,
iast_TELEMETRY_OFF (468.454 µs) : 448, 489
.   : milestone, 468,
tracing (438.181 µs) : 417, 459
.   : milestone, 438,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 370.855 µs [350.327 µs, 391.383 µs] -
iast 472.507 µs [452.151 µs, 492.864 µs] 101.652 µs (27.4%)
iast_FULL 533.843 µs [513.221 µs, 554.466 µs] 162.988 µs (43.9%)
iast_INACTIVE 437.06 µs [416.305 µs, 457.815 µs] 66.205 µs (17.9%)
iast_TELEMETRY_OFF 461.483 µs [441.135 µs, 481.831 µs] 90.628 µs (24.4%)
tracing 441.582 µs [420.389 µs, 462.775 µs] 70.727 µs (19.1%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 369.65 µs [347.81 µs, 391.49 µs] -
iast 471.201 µs [450.664 µs, 491.738 µs] 101.551 µs (27.5%)
iast_FULL 528.26 µs [507.971 µs, 548.548 µs] 158.61 µs (42.9%)
iast_INACTIVE 443.758 µs [422.622 µs, 464.895 µs] 74.108 µs (20.0%)
iast_TELEMETRY_OFF 468.454 µs [447.617 µs, 489.29 µs] 98.804 µs (26.7%)
tracing 438.181 µs [417.241 µs, 459.12 µs] 68.531 µs (18.5%)

@amarziali
amarziali force-pushed the andrea.marziali/armeria-jetty-server branch from 4ec5a2a to 31133c7 Compare November 29, 2023 15:32
nayeem-kamal
nayeem-kamal approved these changes Dec 5, 2023
View reviewed changes

@nayeem-kamal nayeem-kamal 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

@amarziali
amarziali merged commit d3754fe into master Dec 5, 2023
@amarziali
amarziali deleted the andrea.marziali/armeria-jetty-server branch December 5, 2023 15:46
@github-actions github-actions Bot added this to the 1.26.0 milestone Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dougqh dougqh Awaiting requested review from dougqh dougqh is a code owner automatically assigned from DataDog/apm-java
1 more reviewer
@nayeem-kamal nayeem-kamal nayeem-kamal approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

inst: armeria Armeria Instrumentation type: bug fix Bug fix

Projects

None yet

Milestone

1.26.0

Development

Successfully merging this pull request may close these issues.

Unfinished spans in armeria

2 participants

@amarziali @nayeem-kamal