Skip to content
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

Attach command execution spans to current span if it exists #6481

Merged
merged 3 commits into from
Jan 25, 2024
Merged

Attach command execution spans to current span if it exists #6481

merged 3 commits into from
Jan 25, 2024

Conversation

smola
Copy link
Member

@smola smola commented Jan 12, 2024 •
edited
Loading

What Does This Do

If an active spans exists, append the span for subprocess execution to it. This may delay sending the parent trace if the command runs for longer. Although this has not been a problem for other languages so far, and it seems to be a more general problem also applying to HTTP client requests and others.

Motivation

Application security research team uses this to detect certain attacks in the backend, and for it to be effective, it needs to be correlated with the request.

Additional Notes

@smola smola added the run-tests: all Run all tests label Jan 12, 2024
@pr-commenter
Copy link

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

Benchmarks

Startup

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master smola/APPSEC-10243-process-parent-span
git_commit_date 1706108160 1706108693
git_commit_sha 3dc58c7 bf15270
release_version 1.29.0-SNAPSHOT~3dc58c7020 1.29.0-SNAPSHOT~bf15270539
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1706111310 1706111310
ci_job_id 417456764 417456764
ci_pipeline_id 27106629 27106629
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 44 metrics, 10 unstable metrics.

Startup time reports for insecure-bank 
gantt
    title insecure-bank - global startup overhead: candidate=1.29.0-SNAPSHOT~bf15270539, baseline=1.29.0-SNAPSHOT~3dc58c7020

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.063 s) : 0, 1062611
Total [baseline] (8.751 s) : 0, 8751430
Agent [candidate] (1.058 s) : 0, 1057704
Total [candidate] (8.745 s) : 0, 8745076
section iast
Agent [baseline] (1.201 s) : 0, 1200918
Total [baseline] (9.285 s) : 0, 9284827
Agent [candidate] (1.176 s) : 0, 1175594
Total [candidate] (9.289 s) : 0, 9288676
section iast_TELEMETRY_OFF
Agent [baseline] (1.167 s) : 0, 1167274
Total [baseline] (9.28 s) : 0, 9279933
Agent [candidate] (1.178 s) : 0, 1178372
Total [candidate] (9.287 s) : 0, 9286946
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.063 s -
Agent iast 1.201 s 138.307 ms (13.0%)
Agent iast_TELEMETRY_OFF 1.167 s 104.663 ms (9.8%)
Total tracing 8.751 s -
Total iast 9.285 s 533.397 ms (6.1%)
Total iast_TELEMETRY_OFF 9.28 s 528.503 ms (6.0%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.058 s -
Agent iast 1.176 s 117.89 ms (11.1%)
Agent iast_TELEMETRY_OFF 1.178 s 120.668 ms (11.4%)
Total tracing 8.745 s -
Total iast 9.289 s 543.6 ms (6.2%)
Total iast_TELEMETRY_OFF 9.287 s 541.87 ms (6.2%) 
gantt
    title insecure-bank - break down per module: candidate=1.29.0-SNAPSHOT~bf15270539, baseline=1.29.0-SNAPSHOT~3dc58c7020

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (670.599 ms) : 0, 670599
BytebuddyAgent [candidate] (667.556 ms) : 0, 667556
GlobalTracer [baseline] (297.476 ms) : 0, 297476
GlobalTracer [candidate] (295.96 ms) : 0, 295960
AppSec [baseline] (51.845 ms) : 0, 51845
AppSec [candidate] (51.642 ms) : 0, 51642
Remote Config [baseline] (690.801 µs) : 0, 691
Remote Config [candidate] (693.507 µs) : 0, 694
Telemetry [baseline] (7.562 ms) : 0, 7562
Telemetry [candidate] (7.498 ms) : 0, 7498
section iast
BytebuddyAgent [baseline] (791.528 ms) : 0, 791528
BytebuddyAgent [candidate] (774.23 ms) : 0, 774230
GlobalTracer [baseline] (291.089 ms) : 0, 291089
GlobalTracer [candidate] (285.45 ms) : 0, 285450
AppSec [baseline] (56.982 ms) : 0, 56982
AppSec [candidate] (52.811 ms) : 0, 52811
IAST [baseline] (18.826 ms) : 0, 18826
IAST [candidate] (20.194 ms) : 0, 20194
Remote Config [baseline] (617.146 µs) : 0, 617
Remote Config [candidate] (605.617 µs) : 0, 606
Telemetry [baseline] (6.775 ms) : 0, 6775
Telemetry [candidate] (8.163 ms) : 0, 8163
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (766.349 ms) : 0, 766349
BytebuddyAgent [candidate] (774.431 ms) : 0, 774431
GlobalTracer [baseline] (286.101 ms) : 0, 286101
GlobalTracer [candidate] (288.279 ms) : 0, 288279
AppSec [baseline] (54.551 ms) : 0, 54551
AppSec [candidate] (56.451 ms) : 0, 56451
IAST [baseline] (17.551 ms) : 0, 17551
IAST [candidate] (17.608 ms) : 0, 17608
Remote Config [baseline] (620.675 µs) : 0, 621
Remote Config [candidate] (647.589 µs) : 0, 648
Telemetry [baseline] (7.904 ms) : 0, 7904
Telemetry [candidate] (6.349 ms) : 0, 6349
Loading
Startup time reports for petclinic 
gantt
    title petclinic - global startup overhead: candidate=1.29.0-SNAPSHOT~bf15270539, baseline=1.29.0-SNAPSHOT~3dc58c7020

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.052 s) : 0, 1051707
Total [baseline] (9.347 s) : 0, 9346997
Agent [candidate] (1.065 s) : 0, 1065349
Total [candidate] (9.356 s) : 0, 9356164
section appsec
Agent [baseline] (1.162 s) : 0, 1161870
Total [baseline] (9.5 s) : 0, 9500468
Agent [candidate] (1.152 s) : 0, 1152420
Total [candidate] (9.449 s) : 0, 9448627
section iast
Agent [baseline] (1.175 s) : 0, 1174724
Total [baseline] (9.629 s) : 0, 9629303
Agent [candidate] (1.181 s) : 0, 1180924
Total [candidate] (9.681 s) : 0, 9681193
section profiling
Agent [baseline] (1.279 s) : 0, 1278664
Total [baseline] (9.592 s) : 0, 9592177
Agent [candidate] (1.302 s) : 0, 1302187
Total [candidate] (9.805 s) : 0, 9804656
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.052 s -
Agent appsec 1.162 s 110.164 ms (10.5%)
Agent iast 1.175 s 123.017 ms (11.7%)
Agent profiling 1.279 s 226.958 ms (21.6%)
Total tracing 9.347 s -
Total appsec 9.5 s 153.471 ms (1.6%)
Total iast 9.629 s 282.306 ms (3.0%)
Total profiling 9.592 s 245.179 ms (2.6%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.065 s -
Agent appsec 1.152 s 87.071 ms (8.2%)
Agent iast 1.181 s 115.575 ms (10.8%)
Agent profiling 1.302 s 236.838 ms (22.2%)
Total tracing 9.356 s -
Total appsec 9.449 s 92.463 ms (1.0%)
Total iast 9.681 s 325.029 ms (3.5%)
Total profiling 9.805 s 448.491 ms (4.8%) 
gantt
    title petclinic - break down per module: candidate=1.29.0-SNAPSHOT~bf15270539, baseline=1.29.0-SNAPSHOT~3dc58c7020

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (663.273 ms) : 0, 663273
BytebuddyAgent [candidate] (672.491 ms) : 0, 672491
GlobalTracer [baseline] (294.672 ms) : 0, 294672
GlobalTracer [candidate] (298.153 ms) : 0, 298153
AppSec [baseline] (51.384 ms) : 0, 51384
AppSec [candidate] (51.847 ms) : 0, 51847
Remote Config [baseline] (691.852 µs) : 0, 692
Remote Config [candidate] (692.732 µs) : 0, 693
Telemetry [baseline] (7.47 ms) : 0, 7470
Telemetry [candidate] (7.599 ms) : 0, 7599
section appsec
BytebuddyAgent [baseline] (671.486 ms) : 0, 671486
BytebuddyAgent [candidate] (664.971 ms) : 0, 664971
GlobalTracer [baseline] (297.549 ms) : 0, 297549
GlobalTracer [candidate] (295.643 ms) : 0, 295643
AppSec [baseline] (150.685 ms) : 0, 150685
AppSec [candidate] (150.24 ms) : 0, 150240
Remote Config [baseline] (702.458 µs) : 0, 702
Remote Config [candidate] (697.635 µs) : 0, 698
Telemetry [baseline] (6.814 ms) : 0, 6814
Telemetry [candidate] (6.765 ms) : 0, 6765
section iast
BytebuddyAgent [baseline] (773.393 ms) : 0, 773393
BytebuddyAgent [candidate] (776.348 ms) : 0, 776348
GlobalTracer [baseline] (285.175 ms) : 0, 285175
GlobalTracer [candidate] (288.145 ms) : 0, 288145
AppSec [baseline] (53.69 ms) : 0, 53690
AppSec [candidate] (54.613 ms) : 0, 54613
Remote Config [baseline] (606.394 µs) : 0, 606
Remote Config [candidate] (594.207 µs) : 0, 594
Telemetry [baseline] (6.644 ms) : 0, 6644
Telemetry [candidate] (6.661 ms) : 0, 6661
IAST [baseline] (20.984 ms) : 0, 20984
IAST [candidate] (20.224 ms) : 0, 20224
section profiling
BytebuddyAgent [baseline] (663.73 ms) : 0, 663730
BytebuddyAgent [candidate] (678.184 ms) : 0, 678184
GlobalTracer [baseline] (376.58 ms) : 0, 376580
GlobalTracer [candidate] (380.926 ms) : 0, 380926
AppSec [baseline] (52.326 ms) : 0, 52326
AppSec [candidate] (53.058 ms) : 0, 53058
Remote Config [baseline] (672.388 µs) : 0, 672
Remote Config [candidate] (679.376 µs) : 0, 679
Telemetry [baseline] (7.525 ms) : 0, 7525
Telemetry [candidate] (7.64 ms) : 0, 7640
ProfilingAgent [baseline] (123.371 ms) : 0, 123371
ProfilingAgent [candidate] (126.188 ms) : 0, 126188
Profiling [baseline] (123.396 ms) : 0, 123396
Profiling [candidate] (126.215 ms) : 0, 126215
Loading

Load

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
end_time 2024-01-24T15:27:45 2024-01-24T15:44:16
git_branch master smola/APPSEC-10243-process-parent-span
git_commit_date 1706108160 1706108693
git_commit_sha 3dc58c7 bf15270
release_version 1.29.0-SNAPSHOT~3dc58c7020 1.29.0-SNAPSHOT~bf15270539
start_time 2024-01-24T15:27:32 2024-01-24T15:44:03
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1706111310 1706111310
ci_job_id 417456764 417456764
ci_pipeline_id 27106629 27106629
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 10 metrics, 12 unstable metrics.

Request duration reports for insecure-bank 
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.29.0-SNAPSHOT~bf15270539, baseline=1.29.0-SNAPSHOT~3dc58c7020
    dateFormat X
    axisFormat %s
section baseline
no_agent (373.926 µs) : 354, 394
.   : milestone, 374,
iast (481.26 µs) : 461, 502
.   : milestone, 481,
iast_FULL (541.601 µs) : 521, 562
.   : milestone, 542,
iast_INACTIVE (445.55 µs) : 425, 466
.   : milestone, 446,
iast_TELEMETRY_OFF (482.542 µs) : 461, 504
.   : milestone, 483,
tracing (445.456 µs) : 425, 466
.   : milestone, 445,
section candidate
no_agent (365.492 µs) : 346, 385
.   : milestone, 365,
iast (478.005 µs) : 457, 499
.   : milestone, 478,
iast_FULL (542.558 µs) : 522, 563
.   : milestone, 543,
iast_INACTIVE (444.3 µs) : 424, 465
.   : milestone, 444,
iast_TELEMETRY_OFF (476.54 µs) : 456, 497
.   : milestone, 477,
tracing (448.757 µs) : 428, 469
.   : milestone, 449,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 373.926 µs [354.303 µs, 393.55 µs] -
iast 481.26 µs [460.604 µs, 501.917 µs] 107.334 µs (28.7%)
iast_FULL 541.601 µs [521.028 µs, 562.174 µs] 167.674 µs (44.8%)
iast_INACTIVE 445.55 µs [424.791 µs, 466.309 µs] 71.624 µs (19.2%)
iast_TELEMETRY_OFF 482.542 µs [461.367 µs, 503.717 µs] 108.616 µs (29.0%)
tracing 445.456 µs [424.809 µs, 466.104 µs] 71.53 µs (19.1%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 365.492 µs [345.808 µs, 385.176 µs] -
iast 478.005 µs [457.485 µs, 498.526 µs] 112.514 µs (30.8%)
iast_FULL 542.558 µs [522.054 µs, 563.061 µs] 177.066 µs (48.4%)
iast_INACTIVE 444.3 µs [423.678 µs, 464.923 µs] 78.808 µs (21.6%)
iast_TELEMETRY_OFF 476.54 µs [455.845 µs, 497.235 µs] 111.048 µs (30.4%)
tracing 448.757 µs [428.022 µs, 469.493 µs] 83.266 µs (22.8%)
Request duration reports for petclinic 
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.29.0-SNAPSHOT~bf15270539, baseline=1.29.0-SNAPSHOT~3dc58c7020
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.363 ms) : 1344, 1382
.   : milestone, 1363,
appsec (1.77 ms) : 1744, 1795
.   : milestone, 1770,
iast (1.533 ms) : 1509, 1558
.   : milestone, 1533,
profiling (1.515 ms) : 1490, 1540
.   : milestone, 1515,
tracing (1.506 ms) : 1482, 1530
.   : milestone, 1506,
section candidate
no_agent (1.358 ms) : 1339, 1377
.   : milestone, 1358,
appsec (1.772 ms) : 1747, 1797
.   : milestone, 1772,
iast (1.518 ms) : 1493, 1544
.   : milestone, 1518,
profiling (1.527 ms) : 1501, 1553
.   : milestone, 1527,
tracing (1.486 ms) : 1461, 1511
.   : milestone, 1486,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.363 ms [1.344 ms, 1.382 ms] -
appsec 1.77 ms [1.744 ms, 1.795 ms] 406.695 µs (29.8%)
iast 1.533 ms [1.509 ms, 1.558 ms] 170.273 µs (12.5%)
profiling 1.515 ms [1.49 ms, 1.54 ms] 152.125 µs (11.2%)
tracing 1.506 ms [1.482 ms, 1.53 ms] 142.81 µs (10.5%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.358 ms [1.339 ms, 1.377 ms] -
appsec 1.772 ms [1.747 ms, 1.797 ms] 413.687 µs (30.5%)
iast 1.518 ms [1.493 ms, 1.544 ms] 160.052 µs (11.8%)
profiling 1.527 ms [1.501 ms, 1.553 ms] 168.792 µs (12.4%)
tracing 1.486 ms [1.461 ms, 1.511 ms] 127.698 µs (9.4%)

@smola smola mentioned this pull request Jan 15, 2024
Merged
@smola smola force-pushed the smola/APPSEC-10243-process-parent-span branch from 728d35b to 2865560 Compare January 15, 2024 11:55
@smola smola changed the base branch from master to smola/processimpl-error-type January 15, 2024 11:56
@smola smola added the inst: others All other instrumentations label Jan 15, 2024
@smola smola requested review from a team, cataphract and ValentinZakharov January 15, 2024 11:56
@smola smola marked this pull request as ready for review January 15, 2024 11:56
@smola smola requested a review from a team as a code owner January 15, 2024 11:56
@smola smola requested review from ygree and am312 January 15, 2024 11:56
@smola smola force-pushed the smola/processimpl-error-type branch from 6944920 to 2f89971 Compare January 16, 2024 15:01
@smola smola force-pushed the smola/APPSEC-10243-process-parent-span branch from caffcd4 to 3fae52e Compare January 16, 2024 15:03
Base automatically changed from smola/processimpl-error-type to master January 16, 2024 15:34
@smola smola force-pushed the smola/APPSEC-10243-process-parent-span branch from 3fae52e to d7d17b6 Compare January 16, 2024 15:51
@cataphract
Copy link
Contributor

So what happens if the command keeps running after the parent finishes? Won't this delay the trace for a long time?

@smola
Copy link
Member Author

smola commented Jan 19, 2024

So what happens if the command keeps running after the parent finishes? Won't this delay the trace for a long time?

Unfortunately, it will. As far as I can tell, the same would happen today with HTTP client calls. This has not been an issue so far in Dotnet (available for longer) and Node. If this becomes a problem in practice, we might need to add a mechanism to close the subprocess span earlier.

@smola smola merged commit 1557ad1 into master Jan 25, 2024
130 checks passed
@smola smola deleted the smola/APPSEC-10243-process-parent-span branch January 25, 2024 13:23
@github-actions github-actions bot added this to the 1.29.0 milestone Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ValentinZakharov ValentinZakharov ValentinZakharov approved these changes

@cataphract cataphract Awaiting requested review from cataphract cataphract was automatically assigned from DataDog/asm-java

@ygree ygree Awaiting requested review from ygree ygree is a code owner automatically assigned from DataDog/apm-java

@am312 am312 Awaiting requested review from am312 am312 is a code owner automatically assigned from DataDog/apm-java

Assignees
No one assigned
Labels
inst: others All other instrumentations run-tests: all Run all tests
Projects
None yet
Milestone
1.29.0
Development

Successfully merging this pull request may close these issues.
3 participants
@smola @cataphract @ValentinZakharov