Fix NPE on traceState() in DDSpanLink.java #6758

cecile75 · 2024-02-28T15:14:28Z

What Does This Do

Ensure tracestate from span links can't be null.

Motivation

NPE observed.

Additional Notes

Jira ticket: [APMS-11554]

nayeem-kamal

LGTM

pr-commenter · 2024-02-28T16:06:43Z

Benchmarks

Startup

Parameters

	Baseline	Candidate
baseline_or_candidate	baseline	candidate
git_branch	master	cecile/NPEtracestate
git_commit_date	1709209624	1709225377
git_commit_sha	`487a9e0`	`e97ee04`
release_version	1.31.0-SNAPSHOT~487a9e0583	1.31.0-SNAPSHOT~e97ee04072

See matching parameters

	Baseline	Candidate
application	insecure-bank	insecure-bank
ci_job_date	1709228348	1709228348
ci_job_id	447342960	447342960
ci_pipeline_id	29223007	29223007
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 41 metrics, 13 unstable metrics.

Startup time reports for petclinic

gantt
    title petclinic - global startup overhead: candidate=1.31.0-SNAPSHOT~e97ee04072, baseline=1.31.0-SNAPSHOT~487a9e0583

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.081 s) : 0, 1081493
Total [baseline] (9.197 s) : 0, 9196698
Agent [candidate] (1.082 s) : 0, 1081825
Total [candidate] (9.207 s) : 0, 9206910
section appsec
Agent [baseline] (1.187 s) : 0, 1186966
Total [baseline] (9.356 s) : 0, 9355845
Agent [candidate] (1.19 s) : 0, 1190099
Total [candidate] (9.376 s) : 0, 9375596
section iast
Agent [baseline] (1.21 s) : 0, 1209551
Total [baseline] (9.355 s) : 0, 9355176
Agent [candidate] (1.212 s) : 0, 1212472
Total [candidate] (9.409 s) : 0, 9409162
section profiling
Agent [baseline] (1.277 s) : 0, 1276862
Total [baseline] (9.39 s) : 0, 9389522
Agent [candidate] (1.276 s) : 0, 1275719
Total [candidate] (9.373 s) : 0, 9372803

baseline results

Module	Variant	Duration	Δ tracing
Agent	tracing	1.081 s	-
Agent	appsec	1.187 s	105.473 ms (9.8%)
Agent	iast	1.21 s	128.058 ms (11.8%)
Agent	profiling	1.277 s	195.368 ms (18.1%)
Total	tracing	9.197 s	-
Total	appsec	9.356 s	159.146 ms (1.7%)
Total	iast	9.355 s	158.477 ms (1.7%)
Total	profiling	9.39 s	192.824 ms (2.1%)

candidate results

Module	Variant	Duration	Δ tracing
Agent	tracing	1.082 s	-
Agent	appsec	1.19 s	108.274 ms (10.0%)
Agent	iast	1.212 s	130.647 ms (12.1%)
Agent	profiling	1.276 s	193.893 ms (17.9%)
Total	tracing	9.207 s	-
Total	appsec	9.376 s	168.686 ms (1.8%)
Total	iast	9.409 s	202.252 ms (2.2%)
Total	profiling	9.373 s	165.894 ms (1.8%)

gantt
    title petclinic - break down per module: candidate=1.31.0-SNAPSHOT~e97ee04072, baseline=1.31.0-SNAPSHOT~487a9e0583

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (689.259 ms) : 0, 689259
BytebuddyAgent [candidate] (689.176 ms) : 0, 689176
GlobalTracer [baseline] (297.803 ms) : 0, 297803
GlobalTracer [candidate] (298.34 ms) : 0, 298340
AppSec [baseline] (51.441 ms) : 0, 51441
AppSec [candidate] (51.379 ms) : 0, 51379
Remote Config [baseline] (720.663 µs) : 0, 721
Remote Config [candidate] (714.175 µs) : 0, 714
Telemetry [baseline] (7.866 ms) : 0, 7866
Telemetry [candidate] (7.851 ms) : 0, 7851
section appsec
BytebuddyAgent [baseline] (693.468 ms) : 0, 693468
BytebuddyAgent [candidate] (694.686 ms) : 0, 694686
GlobalTracer [baseline] (299.462 ms) : 0, 299462
GlobalTracer [candidate] (300.879 ms) : 0, 300879
AppSec [baseline] (151.813 ms) : 0, 151813
AppSec [candidate] (152.3 ms) : 0, 152300
Remote Config [baseline] (642.864 µs) : 0, 643
Remote Config [candidate] (646.183 µs) : 0, 646
Telemetry [baseline] (7.021 ms) : 0, 7021
Telemetry [candidate] (7.017 ms) : 0, 7017
section iast
BytebuddyAgent [baseline] (802.691 ms) : 0, 802691
BytebuddyAgent [candidate] (804.964 ms) : 0, 804964
GlobalTracer [baseline] (289.108 ms) : 0, 289108
GlobalTracer [candidate] (290.213 ms) : 0, 290213
AppSec [baseline] (51.827 ms) : 0, 51827
AppSec [candidate] (52.707 ms) : 0, 52707
Remote Config [baseline] (596.231 µs) : 0, 596
Remote Config [candidate] (585.556 µs) : 0, 586
Telemetry [baseline] (7.47 ms) : 0, 7470
Telemetry [candidate] (6.688 ms) : 0, 6688
IAST [baseline] (23.346 ms) : 0, 23346
IAST [candidate] (22.823 ms) : 0, 22823
section profiling
ProfilingAgent [baseline] (91.588 ms) : 0, 91588
ProfilingAgent [candidate] (92.524 ms) : 0, 92524
BytebuddyAgent [baseline] (683.882 ms) : 0, 683882
BytebuddyAgent [candidate] (681.141 ms) : 0, 681141
GlobalTracer [baseline] (379.98 ms) : 0, 379980
GlobalTracer [candidate] (381.671 ms) : 0, 381671
AppSec [baseline] (52.956 ms) : 0, 52956
AppSec [candidate] (53.025 ms) : 0, 53025
Remote Config [baseline] (782.855 µs) : 0, 783
Remote Config [candidate] (779.199 µs) : 0, 779
Telemetry [baseline] (10.93 ms) : 0, 10930
Telemetry [candidate] (10.266 ms) : 0, 10266
Profiling [baseline] (91.612 ms) : 0, 91612
Profiling [candidate] (92.548 ms) : 0, 92548

Load

Parameters

	Baseline	Candidate
baseline_or_candidate	baseline	candidate
end_time	2024-02-29T17:15:29	2024-02-29T17:34:07
git_branch	master	cecile/NPEtracestate
git_commit_date	1709209624	1709225377
git_commit_sha	`487a9e0`	`e97ee04`
release_version	1.31.0-SNAPSHOT~487a9e0583	1.31.0-SNAPSHOT~e97ee04072
start_time	2024-02-29T17:15:16	2024-02-29T17:33:53

See matching parameters

	Baseline	Candidate
application	insecure-bank	insecure-bank
ci_job_date	1709228348	1709228348
ci_job_id	447342960	447342960
ci_pipeline_id	29223007	29223007
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 12 metrics, 14 unstable metrics.

Request duration reports for petclinic

gantt
    title petclinic - request duration [CI 0.99] : candidate=1.31.0-SNAPSHOT~e97ee04072, baseline=1.31.0-SNAPSHOT~487a9e0583
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.364 ms) : 1344, 1383
.   : milestone, 1364,
appsec (1.783 ms) : 1759, 1807
.   : milestone, 1783,
iast (1.509 ms) : 1485, 1532
.   : milestone, 1509,
profiling (1.525 ms) : 1501, 1548
.   : milestone, 1525,
tracing (1.502 ms) : 1478, 1526
.   : milestone, 1502,
section candidate
no_agent (1.364 ms) : 1345, 1383
.   : milestone, 1364,
appsec (1.807 ms) : 1784, 1830
.   : milestone, 1807,
iast (1.553 ms) : 1530, 1576
.   : milestone, 1553,
profiling (1.512 ms) : 1489, 1536
.   : milestone, 1512,
tracing (1.515 ms) : 1492, 1539
.   : milestone, 1515,

baseline results

Variant	Request duration [CI 0.99]	Δ no_agent
no_agent	1.364 ms [1.344 ms, 1.383 ms]	-
appsec	1.783 ms [1.759 ms, 1.807 ms]	418.976 µs (30.7%)
iast	1.509 ms [1.485 ms, 1.532 ms]	145.046 µs (10.6%)
profiling	1.525 ms [1.501 ms, 1.548 ms]	161.118 µs (11.8%)
tracing	1.502 ms [1.478 ms, 1.526 ms]	138.276 µs (10.1%)

candidate results

Variant	Request duration [CI 0.99]	Δ no_agent
no_agent	1.364 ms [1.345 ms, 1.383 ms]	-
appsec	1.807 ms [1.784 ms, 1.83 ms]	442.682 µs (32.5%)
iast	1.553 ms [1.53 ms, 1.576 ms]	189.13 µs (13.9%)
profiling	1.512 ms [1.489 ms, 1.536 ms]	148.122 µs (10.9%)
tracing	1.515 ms [1.492 ms, 1.539 ms]	151.295 µs (11.1%)

Request duration reports for insecure-bank

gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.31.0-SNAPSHOT~e97ee04072, baseline=1.31.0-SNAPSHOT~487a9e0583
    dateFormat X
    axisFormat %s
section baseline
no_agent (370.413 µs) : 351, 390
.   : milestone, 370,
iast (472.519 µs) : 452, 493
.   : milestone, 473,
iast_FULL (544.507 µs) : 524, 565
.   : milestone, 545,
iast_GLOBAL (497.427 µs) : 477, 517
.   : milestone, 497,
iast_HARDCODED_SECRET_DISABLED (479.873 µs) : 459, 501
.   : milestone, 480,
iast_INACTIVE (455.14 µs) : 435, 475
.   : milestone, 455,
iast_TELEMETRY_OFF (470.009 µs) : 450, 491
.   : milestone, 470,
tracing (448.62 µs) : 428, 469
.   : milestone, 449,
section candidate
no_agent (369.067 µs) : 349, 389
.   : milestone, 369,
iast (479.263 µs) : 458, 501
.   : milestone, 479,
iast_FULL (542.807 µs) : 522, 563
.   : milestone, 543,
iast_GLOBAL (498.156 µs) : 477, 519
.   : milestone, 498,
iast_HARDCODED_SECRET_DISABLED (473.988 µs) : 454, 494
.   : milestone, 474,
iast_INACTIVE (457.538 µs) : 436, 479
.   : milestone, 458,
iast_TELEMETRY_OFF (472.468 µs) : 452, 493
.   : milestone, 472,
tracing (445.961 µs) : 426, 466
.   : milestone, 446,

baseline results

Variant	Request duration [CI 0.99]	Δ no_agent
no_agent	370.413 µs [350.653 µs, 390.173 µs]	-
iast	472.519 µs [452.253 µs, 492.784 µs]	102.106 µs (27.6%)
iast_FULL	544.507 µs [523.735 µs, 565.279 µs]	174.094 µs (47.0%)
iast_GLOBAL	497.427 µs [477.368 µs, 517.487 µs]	127.015 µs (34.3%)
iast_HARDCODED_SECRET_DISABLED	479.873 µs [459.052 µs, 500.694 µs]	109.46 µs (29.6%)
iast_INACTIVE	455.14 µs [434.928 µs, 475.351 µs]	84.727 µs (22.9%)
iast_TELEMETRY_OFF	470.009 µs [449.507 µs, 490.511 µs]	99.596 µs (26.9%)
tracing	448.62 µs [428.087 µs, 469.153 µs]	78.207 µs (21.1%)

candidate results

Variant	Request duration [CI 0.99]	Δ no_agent
no_agent	369.067 µs [348.954 µs, 389.181 µs]	-
iast	479.263 µs [457.811 µs, 500.715 µs]	110.196 µs (29.9%)
iast_FULL	542.807 µs [522.345 µs, 563.268 µs]	173.739 µs (47.1%)
iast_GLOBAL	498.156 µs [477.16 µs, 519.152 µs]	129.089 µs (35.0%)
iast_HARDCODED_SECRET_DISABLED	473.988 µs [453.623 µs, 494.353 µs]	104.92 µs (28.4%)
iast_INACTIVE	457.538 µs [436.252 µs, 478.823 µs]	88.47 µs (24.0%)
iast_TELEMETRY_OFF	472.468 µs [451.788 µs, 493.148 µs]	103.401 µs (28.0%)
tracing	445.961 µs [425.56 µs, 466.362 µs]	76.894 µs (20.8%)

PerfectSlayer

Before merging, I would like to check why there is null value in the first place.

PerfectSlayer

Hey, I would rather see the constructor changed in order to prevent any further issue.

To do this, you would need to check if traceState is null in SpanLink constructor and set an empty string "" if it is.
Similarly, you can check for null attributes and set it to EMPTY from SpanLinkAttributes.

The main idea is, if we fix the usage of the variable, we will need to fix everywhere is is (and will be) used. Whereas if you fix it where is it set, you fix it once for all (as the variable is final, once set it won’t change).

PerfectSlayer · 2024-03-01T08:39:00Z

Thanks for the fix! 💪 I update the PR description to match the new changes.

Fix NPE toSpanLinkJson

9fecbe0

cecile75 requested a review from a team as a code owner February 28, 2024 15:14

cecile75 requested review from ygree and nayeem-kamal February 28, 2024 15:14

cecile75 mentioned this pull request Feb 28, 2024

Fix NPE on traceState() in DDSpanLink.java #6753

Closed

nayeem-kamal approved these changes Feb 28, 2024

View reviewed changes

PerfectSlayer added type: bug comp: remote config Configuration at Runtime labels Feb 29, 2024

PerfectSlayer reviewed Feb 29, 2024

View reviewed changes

PerfectSlayer requested changes Feb 29, 2024

View reviewed changes

Cecile Terpin added 2 commits February 29, 2024 17:09

Merge branch 'master' into cecile/NPEtracestate

a53ee7d

move the fix to the constructor directly

e97ee04

cecile75 requested a review from PerfectSlayer February 29, 2024 16:51

PerfectSlayer approved these changes Mar 1, 2024

View reviewed changes

cecile75 merged commit 2ada17d into master Mar 1, 2024
79 checks passed

cecile75 deleted the cecile/NPEtracestate branch March 1, 2024 09:16

github-actions bot added this to the 1.31.0 milestone Mar 1, 2024

PerfectSlayer mentioned this pull request Mar 1, 2024

Ensure span links trace ids and attributes can't be null #6766

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix NPE on traceState() in DDSpanLink.java #6758

Fix NPE on traceState() in DDSpanLink.java #6758

cecile75 commented Feb 28, 2024 •

edited by PerfectSlayer

Loading

nayeem-kamal left a comment

pr-commenter bot commented Feb 28, 2024 •

edited

Loading

PerfectSlayer left a comment

PerfectSlayer left a comment •

edited

Loading

PerfectSlayer commented Mar 1, 2024

Fix NPE on traceState() in DDSpanLink.java #6758

Fix NPE on traceState() in DDSpanLink.java #6758

Conversation

cecile75 commented Feb 28, 2024 • edited by PerfectSlayer Loading

What Does This Do

Motivation

Additional Notes

nayeem-kamal left a comment

Choose a reason for hiding this comment

pr-commenter bot commented Feb 28, 2024 • edited Loading

Benchmarks

Startup

Parameters

Summary

Load

Parameters

Summary

PerfectSlayer left a comment

Choose a reason for hiding this comment

PerfectSlayer left a comment • edited Loading

Choose a reason for hiding this comment

PerfectSlayer commented Mar 1, 2024

cecile75 commented Feb 28, 2024 •

edited by PerfectSlayer

Loading

pr-commenter bot commented Feb 28, 2024 •

edited

Loading

PerfectSlayer left a comment •

edited

Loading