Verify validity of extracted `SpanID` for compound header extraction span links #8003

mhlidd · 2024-11-22T20:13:39Z

What Does This Do

Adds a check to ensure that Span Links are only created in compound header extraction when the SpanID is valid.

Motivation

This system-tests PR introduces a test to ensure that all libraries only create span links in distributed tracing header extraction when the SpanID is valid. This PR aims to implement this check in dd-trace-java and pass the test referenced.

Additional Notes

Contributor Checklist

Format the title according the contribution guidelines
Assign the type: and (comp: or inst:) labels in addition to any usefull labels
Squash your commits prior merging or merge using GitHub's Squash and merge
Don't use close, fix or any linking keywords when referencing an issue.
Use solves instead, and assign the PR milestone to the issue
Update the public documentation in case of new configuration flag or behavior

Jira ticket: [PROJ-IDENT]

pr-commenter · 2024-11-22T20:52:57Z

Benchmarks

Startup

Parameters

	Baseline	Candidate
baseline_or_candidate	baseline	candidate
git_branch	master	mhlidd/check_valid_span_id
git_commit_date	1732735962	1732736933
git_commit_sha	`207f770`	`f8cce3c`
release_version	1.44.0-SNAPSHOT~207f770623	1.43.0-SNAPSHOT~f8cce3c3ed

See matching parameters

	Baseline	Candidate
application	insecure-bank	insecure-bank
ci_job_date	1732739435	1732739435
ci_job_id	722389149	722389149
ci_pipeline_id	50080282	50080282
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 56 metrics, 7 unstable metrics.

Startup time reports for insecure-bank

gantt
    title insecure-bank - global startup overhead: candidate=1.43.0-SNAPSHOT~f8cce3c3ed, baseline=1.44.0-SNAPSHOT~207f770623

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.091 s) : 0, 1090848
Total [baseline] (8.673 s) : 0, 8673455
Agent [candidate] (1.088 s) : 0, 1088090
Total [candidate] (8.67 s) : 0, 8670490
section iast
Agent [baseline] (1.227 s) : 0, 1227217
Total [baseline] (9.271 s) : 0, 9270980
Agent [candidate] (1.217 s) : 0, 1217392
Total [candidate] (9.199 s) : 0, 9199235
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.218 s) : 0, 1218362
Total [baseline] (9.16 s) : 0, 9160221
Agent [candidate] (1.218 s) : 0, 1218019
Total [candidate] (9.161 s) : 0, 9161459
section iast_TELEMETRY_OFF
Agent [baseline] (1.214 s) : 0, 1213867
Total [baseline] (9.16 s) : 0, 9159858
Agent [candidate] (1.221 s) : 0, 1221070
Total [candidate] (9.171 s) : 0, 9171082

baseline results

Module	Variant	Duration	Δ tracing
Agent	tracing	1.091 s	-
Agent	iast	1.227 s	136.37 ms (12.5%)
Agent	iast_HARDCODED_SECRET_DISABLED	1.218 s	127.514 ms (11.7%)
Agent	iast_TELEMETRY_OFF	1.214 s	123.02 ms (11.3%)
Total	tracing	8.673 s	-
Total	iast	9.271 s	597.525 ms (6.9%)
Total	iast_HARDCODED_SECRET_DISABLED	9.16 s	486.766 ms (5.6%)
Total	iast_TELEMETRY_OFF	9.16 s	486.403 ms (5.6%)

candidate results

Module	Variant	Duration	Δ tracing
Agent	tracing	1.088 s	-
Agent	iast	1.217 s	129.301 ms (11.9%)
Agent	iast_HARDCODED_SECRET_DISABLED	1.218 s	129.928 ms (11.9%)
Agent	iast_TELEMETRY_OFF	1.221 s	132.979 ms (12.2%)
Total	tracing	8.67 s	-
Total	iast	9.199 s	528.745 ms (6.1%)
Total	iast_HARDCODED_SECRET_DISABLED	9.161 s	490.969 ms (5.7%)
Total	iast_TELEMETRY_OFF	9.171 s	500.592 ms (5.8%)

gantt
    title insecure-bank - break down per module: candidate=1.43.0-SNAPSHOT~f8cce3c3ed, baseline=1.44.0-SNAPSHOT~207f770623

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (693.36 ms) : 0, 693360
BytebuddyAgent [candidate] (691.703 ms) : 0, 691703
GlobalTracer [baseline] (317.835 ms) : 0, 317835
GlobalTracer [candidate] (317.983 ms) : 0, 317983
AppSec [baseline] (54.641 ms) : 0, 54641
AppSec [candidate] (54.735 ms) : 0, 54735
Remote Config [baseline] (676.573 µs) : 0, 677
Remote Config [candidate] (687.48 µs) : 0, 687
Telemetry [baseline] (10.613 ms) : 0, 10613
Telemetry [candidate] (9.267 ms) : 0, 9267
section iast
BytebuddyAgent [baseline] (817.989 ms) : 0, 817989
BytebuddyAgent [candidate] (809.696 ms) : 0, 809696
GlobalTracer [baseline] (307.768 ms) : 0, 307768
GlobalTracer [candidate] (307.636 ms) : 0, 307636
AppSec [baseline] (56.564 ms) : 0, 56564
AppSec [candidate] (57.523 ms) : 0, 57523
IAST [baseline] (22.817 ms) : 0, 22817
IAST [candidate] (20.679 ms) : 0, 20679
Remote Config [baseline] (642.775 µs) : 0, 643
Remote Config [candidate] (615.856 µs) : 0, 616
Telemetry [baseline] (7.56 ms) : 0, 7560
Telemetry [candidate] (7.481 ms) : 0, 7481
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (811.393 ms) : 0, 811393
BytebuddyAgent [candidate] (809.427 ms) : 0, 809427
GlobalTracer [baseline] (305.954 ms) : 0, 305954
GlobalTracer [candidate] (307.613 ms) : 0, 307613
AppSec [baseline] (58.382 ms) : 0, 58382
AppSec [candidate] (58.087 ms) : 0, 58087
IAST [baseline] (20.736 ms) : 0, 20736
IAST [candidate] (20.914 ms) : 0, 20914
Remote Config [baseline] (635.384 µs) : 0, 635
Remote Config [candidate] (630.766 µs) : 0, 631
Telemetry [baseline] (7.519 ms) : 0, 7519
Telemetry [candidate] (7.59 ms) : 0, 7590
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (807.892 ms) : 0, 807892
BytebuddyAgent [candidate] (811.338 ms) : 0, 811338
GlobalTracer [baseline] (306.245 ms) : 0, 306245
GlobalTracer [candidate] (309.081 ms) : 0, 309081
AppSec [baseline] (57.792 ms) : 0, 57792
AppSec [candidate] (57.724 ms) : 0, 57724
IAST [baseline] (20.253 ms) : 0, 20253
IAST [candidate] (21.077 ms) : 0, 21077
Remote Config [baseline] (617.614 µs) : 0, 618
Remote Config [candidate] (616.859 µs) : 0, 617
Telemetry [baseline] (7.345 ms) : 0, 7345
Telemetry [candidate] (7.415 ms) : 0, 7415

Startup time reports for petclinic

gantt
    title petclinic - global startup overhead: candidate=1.43.0-SNAPSHOT~f8cce3c3ed, baseline=1.44.0-SNAPSHOT~207f770623

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.095 s) : 0, 1095339
Total [baseline] (10.455 s) : 0, 10455239
Agent [candidate] (1.098 s) : 0, 1098434
Total [candidate] (10.445 s) : 0, 10444660
section appsec
Agent [baseline] (1.225 s) : 0, 1224507
Total [baseline] (10.747 s) : 0, 10746614
Agent [candidate] (1.229 s) : 0, 1229093
Total [candidate] (10.728 s) : 0, 10728374
section iast
Agent [baseline] (1.219 s) : 0, 1218899
Total [baseline] (10.93 s) : 0, 10929611
Agent [candidate] (1.218 s) : 0, 1217740
Total [candidate] (10.943 s) : 0, 10943407
section profiling
Agent [baseline] (1.324 s) : 0, 1323844
Total [baseline] (10.841 s) : 0, 10840713
Agent [candidate] (1.32 s) : 0, 1319676
Total [candidate] (10.794 s) : 0, 10793802

baseline results

Module	Variant	Duration	Δ tracing
Agent	tracing	1.095 s	-
Agent	appsec	1.225 s	129.168 ms (11.8%)
Agent	iast	1.219 s	123.56 ms (11.3%)
Agent	profiling	1.324 s	228.505 ms (20.9%)
Total	tracing	10.455 s	-
Total	appsec	10.747 s	291.375 ms (2.8%)
Total	iast	10.93 s	474.372 ms (4.5%)
Total	profiling	10.841 s	385.474 ms (3.7%)

candidate results

Module	Variant	Duration	Δ tracing
Agent	tracing	1.098 s	-
Agent	appsec	1.229 s	130.659 ms (11.9%)
Agent	iast	1.218 s	119.306 ms (10.9%)
Agent	profiling	1.32 s	221.241 ms (20.1%)
Total	tracing	10.445 s	-
Total	appsec	10.728 s	283.714 ms (2.7%)
Total	iast	10.943 s	498.747 ms (4.8%)
Total	profiling	10.794 s	349.142 ms (3.3%)

gantt
    title petclinic - break down per module: candidate=1.43.0-SNAPSHOT~f8cce3c3ed, baseline=1.44.0-SNAPSHOT~207f770623

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (696.668 ms) : 0, 696668
BytebuddyAgent [candidate] (698.052 ms) : 0, 698052
GlobalTracer [baseline] (318.906 ms) : 0, 318906
GlobalTracer [candidate] (320.589 ms) : 0, 320589
AppSec [baseline] (54.782 ms) : 0, 54782
AppSec [candidate] (55.293 ms) : 0, 55293
Remote Config [baseline] (676.104 µs) : 0, 676
Remote Config [candidate] (697.007 µs) : 0, 697
Telemetry [baseline] (10.564 ms) : 0, 10564
Telemetry [candidate] (9.994 ms) : 0, 9994
section appsec
BytebuddyAgent [baseline] (710.917 ms) : 0, 710917
BytebuddyAgent [candidate] (714.25 ms) : 0, 714250
GlobalTracer [baseline] (314.584 ms) : 0, 314584
GlobalTracer [candidate] (315.185 ms) : 0, 315185
AppSec [baseline] (166.269 ms) : 0, 166269
AppSec [candidate] (166.802 ms) : 0, 166802
Remote Config [baseline] (642.028 µs) : 0, 642
Remote Config [candidate] (649.196 µs) : 0, 649
Telemetry [baseline] (8.529 ms) : 0, 8529
Telemetry [candidate] (8.197 ms) : 0, 8197
IAST [baseline] (19.659 ms) : 0, 19659
IAST [candidate] (20.426 ms) : 0, 20426
section iast
BytebuddyAgent [baseline] (811.47 ms) : 0, 811470
BytebuddyAgent [candidate] (809.638 ms) : 0, 809638
GlobalTracer [baseline] (306.219 ms) : 0, 306219
GlobalTracer [candidate] (307.207 ms) : 0, 307207
AppSec [baseline] (58.43 ms) : 0, 58430
AppSec [candidate] (58.037 ms) : 0, 58037
Remote Config [baseline] (623.563 µs) : 0, 624
Remote Config [candidate] (641.928 µs) : 0, 642
Telemetry [baseline] (7.538 ms) : 0, 7538
Telemetry [candidate] (7.58 ms) : 0, 7580
IAST [baseline] (20.913 ms) : 0, 20913
IAST [candidate] (20.911 ms) : 0, 20911
section profiling
BytebuddyAgent [baseline] (691.725 ms) : 0, 691725
BytebuddyAgent [candidate] (687.957 ms) : 0, 687957
GlobalTracer [baseline] (435.44 ms) : 0, 435440
GlobalTracer [candidate] (437.156 ms) : 0, 437156
AppSec [baseline] (53.979 ms) : 0, 53979
AppSec [candidate] (53.908 ms) : 0, 53908
Remote Config [baseline] (666.921 µs) : 0, 667
Remote Config [candidate] (656.524 µs) : 0, 657
Telemetry [baseline] (7.823 ms) : 0, 7823
Telemetry [candidate] (7.8 ms) : 0, 7800
ProfilingAgent [baseline] (94.874 ms) : 0, 94874
ProfilingAgent [candidate] (93.121 ms) : 0, 93121
Profiling [baseline] (94.898 ms) : 0, 94898
Profiling [candidate] (93.145 ms) : 0, 93145

Load

Parameters

	Baseline	Candidate
baseline_or_candidate	baseline	candidate
end_time	2024-11-27T20:00:29	2024-11-27T20:07:28
git_branch	master	mhlidd/check_valid_span_id
git_commit_date	1732735962	1732736933
git_commit_sha	`207f770`	`f8cce3c`
release_version	1.44.0-SNAPSHOT~207f770623	1.43.0-SNAPSHOT~f8cce3c3ed
start_time	2024-11-27T20:00:15	2024-11-27T20:07:15

See matching parameters

	Baseline	Candidate
application	insecure-bank	insecure-bank
ci_job_date	1732738402	1732738402
ci_job_id	722389150	722389150
ci_pipeline_id	50080282	50080282
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 11 metrics, 17 unstable metrics.

Request duration reports for petclinic

gantt
    title petclinic - request duration [CI 0.99] : candidate=1.43.0-SNAPSHOT~f8cce3c3ed, baseline=1.44.0-SNAPSHOT~207f770623
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.366 ms) : 1347, 1385
.   : milestone, 1366,
appsec (1.743 ms) : 1719, 1768
.   : milestone, 1743,
appsec_no_iast (1.741 ms) : 1716, 1767
.   : milestone, 1741,
iast (1.499 ms) : 1476, 1522
.   : milestone, 1499,
profiling (1.527 ms) : 1504, 1551
.   : milestone, 1527,
tracing (1.496 ms) : 1471, 1521
.   : milestone, 1496,
section candidate
no_agent (1.355 ms) : 1335, 1374
.   : milestone, 1355,
appsec (1.767 ms) : 1741, 1792
.   : milestone, 1767,
appsec_no_iast (1.737 ms) : 1712, 1762
.   : milestone, 1737,
iast (1.487 ms) : 1464, 1510
.   : milestone, 1487,
profiling (1.529 ms) : 1505, 1554
.   : milestone, 1529,
tracing (1.482 ms) : 1457, 1507
.   : milestone, 1482,

baseline results

Variant	Request duration [CI 0.99]	Δ no_agent
no_agent	1.366 ms [1.347 ms, 1.385 ms]	-
appsec	1.743 ms [1.719 ms, 1.768 ms]	377.777 µs (27.7%)
appsec_no_iast	1.741 ms [1.716 ms, 1.767 ms]	375.714 µs (27.5%)
iast	1.499 ms [1.476 ms, 1.522 ms]	133.613 µs (9.8%)
profiling	1.527 ms [1.504 ms, 1.551 ms]	161.7 µs (11.8%)
tracing	1.496 ms [1.471 ms, 1.521 ms]	130.181 µs (9.5%)

candidate results

Variant	Request duration [CI 0.99]	Δ no_agent
no_agent	1.355 ms [1.335 ms, 1.374 ms]	-
appsec	1.767 ms [1.741 ms, 1.792 ms]	411.788 µs (30.4%)
appsec_no_iast	1.737 ms [1.712 ms, 1.762 ms]	382.423 µs (28.2%)
iast	1.487 ms [1.464 ms, 1.51 ms]	132.285 µs (9.8%)
profiling	1.529 ms [1.505 ms, 1.554 ms]	174.633 µs (12.9%)
tracing	1.482 ms [1.457 ms, 1.507 ms]	127.199 µs (9.4%)

Request duration reports for insecure-bank

gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.43.0-SNAPSHOT~f8cce3c3ed, baseline=1.44.0-SNAPSHOT~207f770623
    dateFormat X
    axisFormat %s
section baseline
no_agent (372.382 µs) : 353, 392
.   : milestone, 372,
iast (494.573 µs) : 473, 516
.   : milestone, 495,
iast_FULL (657.93 µs) : 636, 679
.   : milestone, 658,
iast_GLOBAL (527.288 µs) : 505, 549
.   : milestone, 527,
iast_HARDCODED_SECRET_DISABLED (496.177 µs) : 475, 518
.   : milestone, 496,
iast_INACTIVE (459.507 µs) : 438, 481
.   : milestone, 460,
iast_TELEMETRY_OFF (486.376 µs) : 465, 508
.   : milestone, 486,
tracing (451.975 µs) : 431, 473
.   : milestone, 452,
section candidate
no_agent (379.694 µs) : 358, 402
.   : milestone, 380,
iast (503.928 µs) : 482, 526
.   : milestone, 504,
iast_FULL (661.702 µs) : 640, 684
.   : milestone, 662,
iast_GLOBAL (532.069 µs) : 509, 555
.   : milestone, 532,
iast_HARDCODED_SECRET_DISABLED (497.324 µs) : 475, 519
.   : milestone, 497,
iast_INACTIVE (459.31 µs) : 438, 481
.   : milestone, 459,
iast_TELEMETRY_OFF (482.516 µs) : 461, 504
.   : milestone, 483,
tracing (456.206 µs) : 435, 477
.   : milestone, 456,

baseline results

Variant	Request duration [CI 0.99]	Δ no_agent
no_agent	372.382 µs [352.745 µs, 392.019 µs]	-
iast	494.573 µs [473.108 µs, 516.038 µs]	122.191 µs (32.8%)
iast_FULL	657.93 µs [636.379 µs, 679.48 µs]	285.548 µs (76.7%)
iast_GLOBAL	527.288 µs [505.305 µs, 549.271 µs]	154.906 µs (41.6%)
iast_HARDCODED_SECRET_DISABLED	496.177 µs [474.561 µs, 517.793 µs]	123.795 µs (33.2%)
iast_INACTIVE	459.507 µs [438.098 µs, 480.915 µs]	87.125 µs (23.4%)
iast_TELEMETRY_OFF	486.376 µs [464.648 µs, 508.103 µs]	113.994 µs (30.6%)
tracing	451.975 µs [430.646 µs, 473.303 µs]	79.593 µs (21.4%)

candidate results

Variant	Request duration [CI 0.99]	Δ no_agent
no_agent	379.694 µs [357.717 µs, 401.672 µs]	-
iast	503.928 µs [482.303 µs, 525.554 µs]	124.234 µs (32.7%)
iast_FULL	661.702 µs [639.801 µs, 683.603 µs]	282.008 µs (74.3%)
iast_GLOBAL	532.069 µs [508.743 µs, 555.395 µs]	152.375 µs (40.1%)
iast_HARDCODED_SECRET_DISABLED	497.324 µs [475.482 µs, 519.165 µs]	117.629 µs (31.0%)
iast_INACTIVE	459.31 µs [437.547 µs, 481.073 µs]	79.616 µs (21.0%)
iast_TELEMETRY_OFF	482.516 µs [461.218 µs, 503.814 µs]	102.822 µs (27.1%)
tracing	456.206 µs [434.994 µs, 477.418 µs]	76.512 µs (20.2%)

Dacapo

PerfectSlayer · 2024-11-25T08:55:16Z

dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java

+            } else if (extractedContext.getSpanId()
+                != 0) { // Check that SpanID of secondary extracted context is valid


I would rather check that ContextInterpreter implementations are properly implemented by returning only fullContext to true when context is complete (so it will be an ExtractedContext) rather than adding tests on every access.

PerfectSlayer

Looks good! 👍
One minor change about adding one test case to the tests.

dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogHttpExtractorTest.groovy

PerfectSlayer · 2024-12-04T12:39:14Z

Last comment resolved and CI is happy, you're good to merge 👍

mhlidd · 2024-12-05T16:04:03Z

After deliberation with the team, the consensus is to leave it as is in order to preserve synthetics

verify extracted SpanID is valid before creating span link

38c4abd

mhlidd changed the title ~~verify extracted SpanID is valid before creating span link~~ Verify validity of extracted SpanID for compound header extraction span links Nov 22, 2024

spotless

1af74f6

PerfectSlayer reviewed Nov 25, 2024

View reviewed changes

mhlidd added 6 commits November 25, 2024 15:32

adding check in contextinterpreter

c2c4c3e

updating unit tests

46c023d

trying addHeader

b752a18

fixing final tests

477d25a

replacing span-id with parent-id

1074154

adding span id to one more test

f8cce3c

mhlidd marked this pull request as ready for review November 27, 2024 21:22

mhlidd requested review from a team as code owners November 27, 2024 21:22

mhlidd requested review from mcculls and PerfectSlayer November 27, 2024 21:22

PerfectSlayer approved these changes Dec 3, 2024

View reviewed changes

dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogHttpExtractorTest.groovy Show resolved Hide resolved

adding default test case

478e2e4

mhlidd closed this Dec 5, 2024

mhlidd deleted the mhlidd/check_valid_span_id branch December 5, 2024 16:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Verify validity of extracted `SpanID` for compound header extraction span links #8003

Verify validity of extracted `SpanID` for compound header extraction span links #8003

mhlidd commented Nov 22, 2024 •

edited

Loading

pr-commenter bot commented Nov 22, 2024 •

edited

Loading

PerfectSlayer Nov 25, 2024

PerfectSlayer left a comment

PerfectSlayer commented Dec 4, 2024

mhlidd commented Dec 5, 2024

		} else if (extractedContext.getSpanId()
		!= 0) { // Check that SpanID of secondary extracted context is valid

Verify validity of extracted SpanID for compound header extraction span links #8003

Verify validity of extracted SpanID for compound header extraction span links #8003

Conversation

mhlidd commented Nov 22, 2024 • edited Loading

What Does This Do

Motivation

Additional Notes

Contributor Checklist

pr-commenter bot commented Nov 22, 2024 • edited Loading

Benchmarks

Startup

Parameters

Summary

Load

Parameters

Summary

Dacapo

PerfectSlayer Nov 25, 2024

Choose a reason for hiding this comment

PerfectSlayer left a comment

Choose a reason for hiding this comment

PerfectSlayer commented Dec 4, 2024

mhlidd commented Dec 5, 2024

Verify validity of extracted `SpanID` for compound header extraction span links #8003

Verify validity of extracted `SpanID` for compound header extraction span links #8003

mhlidd commented Nov 22, 2024 •

edited

Loading

pr-commenter bot commented Nov 22, 2024 •

edited

Loading