Skip to content

zio: fix context propagation #6442

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 9, 2024
Merged

zio: fix context propagation #6442

merged 3 commits into from
Jan 9, 2024

Conversation

amarziali
Copy link
Collaborator

@amarziali amarziali commented Jan 4, 2024 •
edited by mcculls
Loading

What Does This Do

Provides several fixes to the zio instrumentation hence resolving as well the flakiness of tests.

  • Excludes zio.internal.FiberRuntime from context propagation since this is done via the instrumentation itself (supervisor)

Motivation

Additional Notes

Jira ticket: [PROJ-IDENT]

@pr-commenter
Copy link

pr-commenter bot commented Jan 4, 2024 •
edited
Loading

Benchmarks

Startup

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master andrea.marziali/zio-test
git_commit_date 1704464857 1704469011
git_commit_sha 260cceb dc56eac
release_version 1.28.0-SNAPSHOT~260cceba39 1.28.0-SNAPSHOT~dc56eac7c7
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1704471628 1704471628
ci_job_id 401714532 401714532
ci_pipeline_id 25961787 25961787
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 0 performance regressions! Performance is the same for 47 metrics, 7 unstable metrics.

Startup time reports for petclinic 
gantt
    title petclinic - global startup overhead: candidate=1.28.0-SNAPSHOT~dc56eac7c7, baseline=1.28.0-SNAPSHOT~260cceba39

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.057 s) : 0, 1056763
Total [baseline] (9.381 s) : 0, 9380787
Agent [candidate] (1.058 s) : 0, 1058189
Total [candidate] (9.366 s) : 0, 9366075
section appsec
Agent [baseline] (1.146 s) : 0, 1146095
Total [baseline] (9.431 s) : 0, 9430915
Agent [candidate] (1.148 s) : 0, 1148151
Total [candidate] (9.471 s) : 0, 9471120
section iast
Agent [baseline] (1.169 s) : 0, 1169389
Total [baseline] (9.615 s) : 0, 9615488
Agent [candidate] (1.179 s) : 0, 1179387
Total [candidate] (9.632 s) : 0, 9632196
section profiling
Agent [baseline] (1.28 s) : 0, 1279556
Total [baseline] (9.661 s) : 0, 9660796
Agent [candidate] (1.283 s) : 0, 1282575
Total [candidate] (9.704 s) : 0, 9704354
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.057 s -
Agent appsec 1.146 s 89.332 ms (8.5%)
Agent iast 1.169 s 112.626 ms (10.7%)
Agent profiling 1.28 s 222.793 ms (21.1%)
Total tracing 9.381 s -
Total appsec 9.431 s 50.128 ms (0.5%)
Total iast 9.615 s 234.701 ms (2.5%)
Total profiling 9.661 s 280.009 ms (3.0%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.058 s -
Agent appsec 1.148 s 89.961 ms (8.5%)
Agent iast 1.179 s 121.197 ms (11.5%)
Agent profiling 1.283 s 224.385 ms (21.2%)
Total tracing 9.366 s -
Total appsec 9.471 s 105.045 ms (1.1%)
Total iast 9.632 s 266.121 ms (2.8%)
Total profiling 9.704 s 338.279 ms (3.6%) 
gantt
    title petclinic - break down per module: candidate=1.28.0-SNAPSHOT~dc56eac7c7, baseline=1.28.0-SNAPSHOT~260cceba39

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (654.244 ms) : 0, 654244
BytebuddyAgent [candidate] (655.032 ms) : 0, 655032
GlobalTracer [baseline] (309.248 ms) : 0, 309248
GlobalTracer [candidate] (309.767 ms) : 0, 309767
AppSec [baseline] (50.89 ms) : 0, 50890
AppSec [candidate] (50.881 ms) : 0, 50881
Remote Config [baseline] (679.46 µs) : 0, 679
Remote Config [candidate] (680.385 µs) : 0, 680
Telemetry [baseline] (7.254 ms) : 0, 7254
Telemetry [candidate] (7.287 ms) : 0, 7287
section appsec
BytebuddyAgent [baseline] (648.969 ms) : 0, 648969
BytebuddyAgent [candidate] (650.863 ms) : 0, 650863
GlobalTracer [baseline] (307.31 ms) : 0, 307310
GlobalTracer [candidate] (306.854 ms) : 0, 306854
AppSec [baseline] (148.084 ms) : 0, 148084
AppSec [candidate] (148.637 ms) : 0, 148637
Remote Config [baseline] (644.195 µs) : 0, 644
Remote Config [candidate] (643.933 µs) : 0, 644
Telemetry [baseline] (6.879 ms) : 0, 6879
Telemetry [candidate] (6.882 ms) : 0, 6882
section iast
BytebuddyAgent [baseline] (770.589 ms) : 0, 770589
BytebuddyAgent [candidate] (777.826 ms) : 0, 777826
GlobalTracer [baseline] (285.63 ms) : 0, 285630
GlobalTracer [candidate] (287.735 ms) : 0, 287735
AppSec [baseline] (52.478 ms) : 0, 52478
AppSec [candidate] (53.708 ms) : 0, 53708
Remote Config [baseline] (571.424 µs) : 0, 571
Remote Config [candidate] (569.265 µs) : 0, 569
Telemetry [baseline] (7.282 ms) : 0, 7282
Telemetry [candidate] (6.395 ms) : 0, 6395
IAST [baseline] (18.56 ms) : 0, 18560
IAST [candidate] (18.519 ms) : 0, 18519
section profiling
BytebuddyAgent [baseline] (664.229 ms) : 0, 664229
BytebuddyAgent [candidate] (666.601 ms) : 0, 666601
GlobalTracer [baseline] (376.877 ms) : 0, 376877
GlobalTracer [candidate] (377.712 ms) : 0, 377712
AppSec [baseline] (51.669 ms) : 0, 51669
AppSec [candidate] (51.349 ms) : 0, 51349
Remote Config [baseline] (1.006 ms) : 0, 1006
Remote Config [candidate] (1.01 ms) : 0, 1010
Telemetry [baseline] (7.342 ms) : 0, 7342
Telemetry [candidate] (7.245 ms) : 0, 7245
ProfilingAgent [baseline] (123.823 ms) : 0, 123823
ProfilingAgent [candidate] (123.917 ms) : 0, 123917
Profiling [baseline] (123.848 ms) : 0, 123848
Profiling [candidate] (123.942 ms) : 0, 123942
Loading
Startup time reports for insecure-bank 
gantt
    title insecure-bank - global startup overhead: candidate=1.28.0-SNAPSHOT~dc56eac7c7, baseline=1.28.0-SNAPSHOT~260cceba39

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.052 s) : 0, 1051738
Total [baseline] (8.755 s) : 0, 8754606
Agent [candidate] (1.058 s) : 0, 1058337
Total [candidate] (8.746 s) : 0, 8745543
section iast
Agent [baseline] (1.171 s) : 0, 1170808
Total [baseline] (9.258 s) : 0, 9257782
Agent [candidate] (1.178 s) : 0, 1178053
Total [candidate] (9.286 s) : 0, 9286367
section iast_TELEMETRY_OFF
Agent [baseline] (1.171 s) : 0, 1171338
Total [baseline] (9.29 s) : 0, 9290301
Agent [candidate] (1.165 s) : 0, 1164559
Total [candidate] (9.277 s) : 0, 9276758
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.052 s -
Agent iast 1.171 s 119.07 ms (11.3%)
Agent iast_TELEMETRY_OFF 1.171 s 119.6 ms (11.4%)
Total tracing 8.755 s -
Total iast 9.258 s 503.176 ms (5.7%)
Total iast_TELEMETRY_OFF 9.29 s 535.695 ms (6.1%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.058 s -
Agent iast 1.178 s 119.716 ms (11.3%)
Agent iast_TELEMETRY_OFF 1.165 s 106.222 ms (10.0%)
Total tracing 8.746 s -
Total iast 9.286 s 540.824 ms (6.2%)
Total iast_TELEMETRY_OFF 9.277 s 531.215 ms (6.1%) 
gantt
    title insecure-bank - break down per module: candidate=1.28.0-SNAPSHOT~dc56eac7c7, baseline=1.28.0-SNAPSHOT~260cceba39

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (650.01 ms) : 0, 650010
BytebuddyAgent [candidate] (655.359 ms) : 0, 655359
GlobalTracer [baseline] (308.629 ms) : 0, 308629
GlobalTracer [candidate] (309.405 ms) : 0, 309405
AppSec [baseline] (50.981 ms) : 0, 50981
AppSec [candidate] (51.044 ms) : 0, 51044
Remote Config [baseline] (690.925 µs) : 0, 691
Remote Config [candidate] (681.519 µs) : 0, 682
Telemetry [baseline] (7.226 ms) : 0, 7226
Telemetry [candidate] (7.306 ms) : 0, 7306
section iast
BytebuddyAgent [baseline] (770.802 ms) : 0, 770802
BytebuddyAgent [candidate] (777.715 ms) : 0, 777715
GlobalTracer [baseline] (286.023 ms) : 0, 286023
GlobalTracer [candidate] (286.358 ms) : 0, 286358
AppSec [baseline] (51.061 ms) : 0, 51061
AppSec [candidate] (52.265 ms) : 0, 52265
Remote Config [baseline] (576.026 µs) : 0, 576
Remote Config [candidate] (561.994 µs) : 0, 562
Telemetry [baseline] (8.105 ms) : 0, 8105
Telemetry [candidate] (6.48 ms) : 0, 6480
IAST [baseline] (19.968 ms) : 0, 19968
IAST [candidate] (19.922 ms) : 0, 19922
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (769.188 ms) : 0, 769188
BytebuddyAgent [candidate] (764.473 ms) : 0, 764473
GlobalTracer [baseline] (288.224 ms) : 0, 288224
GlobalTracer [candidate] (286.708 ms) : 0, 286708
AppSec [baseline] (49.509 ms) : 0, 49509
AppSec [candidate] (49.448 ms) : 0, 49448
Remote Config [baseline] (574.845 µs) : 0, 575
Remote Config [candidate] (567.838 µs) : 0, 568
Telemetry [baseline] (6.471 ms) : 0, 6471
Telemetry [candidate] (6.454 ms) : 0, 6454
IAST [baseline] (22.799 ms) : 0, 22799
IAST [candidate] (22.587 ms) : 0, 22587
Loading

Load

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
end_time 2024-01-05T15:59:30 2024-01-05T16:16:10
git_branch master andrea.marziali/zio-test
git_commit_date 1704464857 1704469011
git_commit_sha 260cceb dc56eac
release_version 1.28.0-SNAPSHOT~260cceba39 1.28.0-SNAPSHOT~dc56eac7c7
start_time 2024-01-05T15:59:16 2024-01-05T16:15:57
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1704471628 1704471628
ci_job_id 401714532 401714532
ci_pipeline_id 25961787 25961787
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 9 metrics, 13 unstable metrics.

Request duration reports for petclinic 
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.28.0-SNAPSHOT~dc56eac7c7, baseline=1.28.0-SNAPSHOT~260cceba39
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.367 ms) : 1347, 1386
.   : milestone, 1367,
appsec (1.794 ms) : 1769, 1820
.   : milestone, 1794,
iast (1.516 ms) : 1492, 1540
.   : milestone, 1516,
profiling (1.537 ms) : 1511, 1562
.   : milestone, 1537,
tracing (1.505 ms) : 1480, 1530
.   : milestone, 1505,
section candidate
no_agent (1.37 ms) : 1351, 1389
.   : milestone, 1370,
appsec (1.785 ms) : 1759, 1810
.   : milestone, 1785,
iast (1.545 ms) : 1521, 1569
.   : milestone, 1545,
profiling (1.589 ms) : 1562, 1616
.   : milestone, 1589,
tracing (1.497 ms) : 1471, 1523
.   : milestone, 1497,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.367 ms [1.347 ms, 1.386 ms] -
appsec 1.794 ms [1.769 ms, 1.82 ms] 427.526 µs (31.3%)
iast 1.516 ms [1.492 ms, 1.54 ms] 149.294 µs (10.9%)
profiling 1.537 ms [1.511 ms, 1.562 ms] 169.949 µs (12.4%)
tracing 1.505 ms [1.48 ms, 1.53 ms] 138.558 µs (10.1%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.37 ms [1.351 ms, 1.389 ms] -
appsec 1.785 ms [1.759 ms, 1.81 ms] 414.694 µs (30.3%)
iast 1.545 ms [1.521 ms, 1.569 ms] 175.083 µs (12.8%)
profiling 1.589 ms [1.562 ms, 1.616 ms] 219.053 µs (16.0%)
tracing 1.497 ms [1.471 ms, 1.523 ms] 127.132 µs (9.3%)
Request duration reports for insecure-bank 
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.28.0-SNAPSHOT~dc56eac7c7, baseline=1.28.0-SNAPSHOT~260cceba39
    dateFormat X
    axisFormat %s
section baseline
no_agent (371.336 µs) : 351, 392
.   : milestone, 371,
iast (490.052 µs) : 469, 511
.   : milestone, 490,
iast_FULL (542.966 µs) : 523, 563
.   : milestone, 543,
iast_INACTIVE (451.273 µs) : 431, 471
.   : milestone, 451,
iast_TELEMETRY_OFF (482.365 µs) : 462, 503
.   : milestone, 482,
tracing (443.537 µs) : 423, 464
.   : milestone, 444,
section candidate
no_agent (373.029 µs) : 352, 394
.   : milestone, 373,
iast (485.115 µs) : 465, 506
.   : milestone, 485,
iast_FULL (543.05 µs) : 523, 564
.   : milestone, 543,
iast_INACTIVE (457.553 µs) : 436, 479
.   : milestone, 458,
iast_TELEMETRY_OFF (474.052 µs) : 453, 495
.   : milestone, 474,
tracing (444.898 µs) : 424, 466
.   : milestone, 445,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 371.336 µs [351.02 µs, 391.652 µs] -
iast 490.052 µs [469.481 µs, 510.623 µs] 118.716 µs (32.0%)
iast_FULL 542.966 µs [522.636 µs, 563.296 µs] 171.63 µs (46.2%)
iast_INACTIVE 451.273 µs [431.114 µs, 471.432 µs] 79.937 µs (21.5%)
iast_TELEMETRY_OFF 482.365 µs [461.587 µs, 503.143 µs] 111.029 µs (29.9%)
tracing 443.537 µs [422.862 µs, 464.211 µs] 72.2 µs (19.4%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 373.029 µs [351.777 µs, 394.281 µs] -
iast 485.115 µs [464.525 µs, 505.704 µs] 112.086 µs (30.0%)
iast_FULL 543.05 µs [522.559 µs, 563.541 µs] 170.021 µs (45.6%)
iast_INACTIVE 457.553 µs [436.009 µs, 479.097 µs] 84.524 µs (22.7%)
iast_TELEMETRY_OFF 474.052 µs [453.367 µs, 494.738 µs] 101.024 µs (27.1%)
tracing 444.898 µs [424.176 µs, 465.62 µs] 71.869 µs (19.3%)

@amarziali amarziali force-pushed the andrea.marziali/zio-test branch 2 times, most recently from 34611dd to 668432c Compare January 5, 2024 09:16
@amarziali amarziali added tag: flaky test Flaky tests type: bug inst: jax-ws JAX-WS instrumentation labels Jan 5, 2024
@amarziali amarziali changed the title add scala instrumentations to zio tests zio: fix context propagation Jan 5, 2024
@amarziali amarziali added inst: others All other instrumentations and removed inst: jax-ws JAX-WS instrumentation labels Jan 5, 2024
@amarziali amarziali marked this pull request as ready for review January 5, 2024 09:22
@amarziali amarziali requested a review from a team as a code owner January 5, 2024 09:22
@amarziali amarziali force-pushed the andrea.marziali/zio-test branch from 668432c to b102938 Compare January 5, 2024 09:55
@amarziali amarziali enabled auto-merge (squash) January 5, 2024 10:44
@amarziali amarziali disabled auto-merge January 5, 2024 14:44
@amarziali amarziali force-pushed the andrea.marziali/zio-test branch 2 times, most recently from 7d0ad79 to c065e43 Compare January 5, 2024 15:36
@amarziali amarziali force-pushed the andrea.marziali/zio-test branch from c065e43 to dc56eac Compare January 5, 2024 15:37
@amarziali amarziali enabled auto-merge (squash) January 5, 2024 16:12
@amarziali amarziali requested a review from mcculls January 8, 2024 09:41
@mcculls
Copy link
Contributor

mcculls commented Jan 9, 2024

I'm fine with the change to exclude zio.internal.FiberRuntime

I'm less confident about the (existing) suspend and resume logic. Can suspend...resume be called multiple times during the lifetime of a FiberContext? Because AFAICT that isn't handled by the code - the continuation is only captured once during construction and only activated once on the first resume.

However if a new FiberContext is created for each suspend...resume pairing then it makes more sense.

@amarziali
Copy link
Collaborator Author

I'm fine with the change to exclude zio.internal.FiberRuntime

I'm less confident about the (existing) suspend and resume logic. Can suspend...resume be called multiple times during the lifetime of a FiberContext? Because AFAICT that isn't handled by the code - the continuation is only captured once during construction and only activated once on the first resume.

However if a new FiberContext is created for each suspend...resume pairing then it makes more sense.

@mcculls Stuart as far I understood this is done on purpose. The continuation is captured on creation and restored on the new state on the first resume. The others resume / supend states seems to be handled through scope states. I tried also to create a new fibercontext on resume but test then fail

@amarziali amarziali merged commit e539aff into master Jan 9, 2024
@amarziali amarziali deleted the andrea.marziali/zio-test branch January 9, 2024 11:33
@github-actions github-actions bot added this to the 1.28.0 milestone Jan 9, 2024
@pallamidessi pallamidessi mentioned this pull request Mar 5, 2024
Open
@guizmaii guizmaii mentioned this pull request Jul 9, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcculls mcculls mcculls approved these changes

@dougqh dougqh Awaiting requested review from dougqh dougqh was automatically assigned from DataDog/apm-java

@nayeem-kamal nayeem-kamal Awaiting requested review from nayeem-kamal nayeem-kamal was automatically assigned from DataDog/apm-java

Assignees
No one assigned
Labels
inst: others All other instrumentations tag: flaky test Flaky tests type: bug
Projects
None yet
Milestone
1.28.0
Development

Successfully merging this pull request may close these issues.
2 participants
@amarziali @mcculls