Fix NPE in user events tracking (#5732) by smola · Pull Request #5757 · DataDog/dd-trace-java

smola · 2023-08-23T07:50:35Z

What Does This Do

Fixed SpringSecurity NullPointerException when user creation happens outside request.
That exception used to be caught and logged to debug level.

Motivation

Additional Notes

Backport of #5732

* Fixed SpringSecurity NullPointerException when user creation happens outside request. * Spotless

pr-commenter · 2023-08-23T08:35:07Z

Benchmarks

Startup

Parameters

	Baseline	Candidate
commit	1.20.0-SNAPSHOT~3a4d10997a	1.20.0-SNAPSHOT~023d122058
config	baseline	candidate

See matching parameters

	Baseline	Candidate
application	insecure-bank	insecure-bank
module	Agent	Agent
parent	None	None
variant	appsec	appsec

Summary

Found 6 performance improvements and 2 performance regressions! Performance is the same for 54 cases.

scenario	Δ mean execution_time	candidate mean execution_time	baseline mean execution_time
scenario:insecure-bank:appsec:Telemetry	better [-670.780µs; -456.263µs] or [-11.625%; -7.907%]	5.207ms	5.770ms
scenario:insecure-bank:iast:Remote Config	worse [+76.279µs; +128.239µs] or [+13.960%; +23.469%]	648.675µs	546.415µs
scenario:insecure-bank:profiling:Telemetry	better [-864.181µs; -618.587µs] or [-14.354%; -10.275%]	5.279ms	6.020ms
scenario:insecure-bank:tracing:Telemetry	better [-708.525µs; -529.772µs] or [-11.846%; -8.858%]	5.362ms	5.981ms
scenario:petclinic:appsec:Telemetry	better [-627.828µs; -554.552µs] or [-10.971%; -9.690%]	5.132ms	5.723ms
scenario:petclinic:iast:Remote Config	worse [+69.941µs; +111.474µs] or [+12.879%; +20.527%]	633.759µs	543.052µs
scenario:petclinic:profiling:Telemetry	better [-738.199µs; -560.542µs] or [-12.414%; -9.427%]	5.297ms	5.946ms
scenario:petclinic:tracing:Telemetry	better [-680.923µs; -387.064µs] or [-11.366%; -6.461%]	5.457ms	5.991ms

Startup time reports for insecure-bank

gantt
    title insecure-bank - global startup overhead: candidate=1.20.0-SNAPSHOT~023d122058, baseline=1.20.0-SNAPSHOT~3a4d10997a

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (987.608 ms) : 0, 987608
Total [baseline] (8.631 s) : 0, 8631173
Agent [candidate] (984.894 ms) : 0, 984894
Total [candidate] (8.63 s) : 0, 8630107
section appsec
Agent [baseline] (1.072 s) : 0, 1072236
Total [baseline] (8.718 s) : 0, 8717742
Agent [candidate] (1.084 s) : 0, 1083877
Total [candidate] (8.774 s) : 0, 8773678
section iast
Agent [baseline] (1.093 s) : 0, 1092505
Total [baseline] (9.162 s) : 0, 9162068
Agent [candidate] (1.091 s) : 0, 1091242
Total [candidate] (9.191 s) : 0, 9191316
section profiling
Agent [baseline] (1.163 s) : 0, 1163197
Total [baseline] (8.919 s) : 0, 8918548
Agent [candidate] (1.15 s) : 0, 1149952
Total [candidate] (8.916 s) : 0, 8915647

baseline results

Module	Variant	Duration	Δ tracing
Agent	tracing	987.608 ms	-
Agent	appsec	1.072 s	84.628 ms (8.6%)
Agent	iast	1.093 s	104.897 ms (10.6%)
Agent	profiling	1.163 s	175.59 ms (17.8%)
Total	tracing	8.631 s	-
Total	appsec	8.718 s	86.569 ms (1.0%)
Total	iast	9.162 s	530.896 ms (6.2%)
Total	profiling	8.919 s	287.375 ms (3.3%)

candidate results

Module	Variant	Duration	Δ tracing
Agent	tracing	984.894 ms	-
Agent	appsec	1.084 s	98.983 ms (10.1%)
Agent	iast	1.091 s	106.349 ms (10.8%)
Agent	profiling	1.15 s	165.058 ms (16.8%)
Total	tracing	8.63 s	-
Total	appsec	8.774 s	143.571 ms (1.7%)
Total	iast	9.191 s	561.209 ms (6.5%)
Total	profiling	8.916 s	285.54 ms (3.3%)

gantt
    title insecure-bank - break down per module: candidate=1.20.0-SNAPSHOT~023d122058, baseline=1.20.0-SNAPSHOT~3a4d10997a

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (612.226 ms) : 0, 612226
BytebuddyAgent [candidate] (610.628 ms) : 0, 610628
GlobalTracer [baseline] (284.516 ms) : 0, 284516
GlobalTracer [candidate] (283.642 ms) : 0, 283642
AppSec [baseline] (49.831 ms) : 0, 49831
AppSec [candidate] (50.191 ms) : 0, 50191
Remote Config [baseline] (659.363 µs) : 0, 659
Remote Config [candidate] (666.572 µs) : 0, 667
Telemetry [baseline] (5.981 ms) : 0, 5981
Telemetry [candidate] (5.362 ms) : 0, 5362
section appsec
BytebuddyAgent [baseline] (612.002 ms) : 0, 612002
BytebuddyAgent [candidate] (619.366 ms) : 0, 619366
GlobalTracer [baseline] (284.645 ms) : 0, 284645
GlobalTracer [candidate] (288.213 ms) : 0, 288213
AppSec [baseline] (134.821 ms) : 0, 134821
AppSec [candidate] (135.747 ms) : 0, 135747
Remote Config [baseline] (724.465 µs) : 0, 724
Remote Config [candidate] (732.659 µs) : 0, 733
Telemetry [baseline] (5.77 ms) : 0, 5770
Telemetry [candidate] (5.207 ms) : 0, 5207
section iast
BytebuddyAgent [baseline] (720.142 ms) : 0, 720142
BytebuddyAgent [candidate] (716.799 ms) : 0, 716799
GlobalTracer [baseline] (270.849 ms) : 0, 270849
GlobalTracer [candidate] (273.009 ms) : 0, 273009
AppSec [baseline] (47.174 ms) : 0, 47174
AppSec [candidate] (47.442 ms) : 0, 47442
Remote Config [baseline] (546.415 µs) : 0, 546
Remote Config [candidate] (648.675 µs) : 0, 649
Telemetry [baseline] (5.674 ms) : 0, 5674
Telemetry [candidate] (5.623 ms) : 0, 5623
IAST [baseline] (13.791 ms) : 0, 13792
IAST [candidate] (13.362 ms) : 0, 13362
section profiling
ProfilingAgent [baseline] (76.809 ms) : 0, 76809
ProfilingAgent [candidate] (74.382 ms) : 0, 74382
BytebuddyAgent [baseline] (629.806 ms) : 0, 629806
BytebuddyAgent [candidate] (622.544 ms) : 0, 622544
GlobalTracer [baseline] (347.378 ms) : 0, 347378
GlobalTracer [candidate] (345.672 ms) : 0, 345672
AppSec [baseline] (49.203 ms) : 0, 49203
AppSec [candidate] (48.681 ms) : 0, 48681
Remote Config [baseline] (657.643 µs) : 0, 658
Remote Config [candidate] (650.901 µs) : 0, 651
Telemetry [baseline] (6.02 ms) : 0, 6020
Telemetry [candidate] (5.279 ms) : 0, 5279
Profiling [baseline] (76.833 ms) : 0, 76833
Profiling [candidate] (74.407 ms) : 0, 74407

Startup time reports for petclinic

gantt
    title petclinic - global startup overhead: candidate=1.20.0-SNAPSHOT~023d122058, baseline=1.20.0-SNAPSHOT~3a4d10997a

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (988.093 ms) : 0, 988093
Total [baseline] (9.169 s) : 0, 9168600
Agent [candidate] (999.312 ms) : 0, 999312
Total [candidate] (9.172 s) : 0, 9171684
section appsec
Agent [baseline] (1.071 s) : 0, 1071036
Total [baseline] (9.254 s) : 0, 9254032
Agent [candidate] (1.069 s) : 0, 1068532
Total [candidate] (9.244 s) : 0, 9243754
section iast
Agent [baseline] (1.09 s) : 0, 1089741
Total [baseline] (9.408 s) : 0, 9407823
Agent [candidate] (1.093 s) : 0, 1093328
Total [candidate] (9.426 s) : 0, 9426355
section profiling
Agent [baseline] (1.155 s) : 0, 1154547
Total [baseline] (9.463 s) : 0, 9462995
Agent [candidate] (1.152 s) : 0, 1151811
Total [candidate] (9.46 s) : 0, 9459973

baseline results

Module	Variant	Duration	Δ tracing
Agent	tracing	988.093 ms	-
Agent	appsec	1.071 s	82.944 ms (8.4%)
Agent	iast	1.09 s	101.649 ms (10.3%)
Agent	profiling	1.155 s	166.454 ms (16.8%)
Total	tracing	9.169 s	-
Total	appsec	9.254 s	85.431 ms (0.9%)
Total	iast	9.408 s	239.223 ms (2.6%)
Total	profiling	9.463 s	294.395 ms (3.2%)

candidate results

Module	Variant	Duration	Δ tracing
Agent	tracing	999.312 ms	-
Agent	appsec	1.069 s	69.22 ms (6.9%)
Agent	iast	1.093 s	94.016 ms (9.4%)
Agent	profiling	1.152 s	152.499 ms (15.3%)
Total	tracing	9.172 s	-
Total	appsec	9.244 s	72.071 ms (0.8%)
Total	iast	9.426 s	254.671 ms (2.8%)
Total	profiling	9.46 s	288.29 ms (3.1%)

gantt
    title petclinic - break down per module: candidate=1.20.0-SNAPSHOT~023d122058, baseline=1.20.0-SNAPSHOT~3a4d10997a

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (612.781 ms) : 0, 612781
BytebuddyAgent [candidate] (619.367 ms) : 0, 619367
GlobalTracer [baseline] (283.992 ms) : 0, 283992
GlobalTracer [candidate] (288.333 ms) : 0, 288333
AppSec [baseline] (50.343 ms) : 0, 50343
AppSec [candidate] (50.722 ms) : 0, 50722
Remote Config [baseline] (663.429 µs) : 0, 663
Remote Config [candidate] (673.552 µs) : 0, 674
Telemetry [baseline] (5.991 ms) : 0, 5991
Telemetry [candidate] (5.457 ms) : 0, 5457
section appsec
BytebuddyAgent [baseline] (611.93 ms) : 0, 611930
BytebuddyAgent [candidate] (609.414 ms) : 0, 609414
GlobalTracer [baseline] (283.817 ms) : 0, 283817
GlobalTracer [candidate] (283.772 ms) : 0, 283772
AppSec [baseline] (134.521 ms) : 0, 134521
AppSec [candidate] (135.299 ms) : 0, 135299
Remote Config [baseline] (722.132 µs) : 0, 722
Remote Config [candidate] (720.625 µs) : 0, 721
Telemetry [baseline] (5.723 ms) : 0, 5723
Telemetry [candidate] (5.132 ms) : 0, 5132
section iast
BytebuddyAgent [baseline] (717.916 ms) : 0, 717916
BytebuddyAgent [candidate] (718.653 ms) : 0, 718653
GlobalTracer [baseline] (270.397 ms) : 0, 270397
GlobalTracer [candidate] (273.388 ms) : 0, 273388
AppSec [baseline] (47.142 ms) : 0, 47142
AppSec [candidate] (47.29 ms) : 0, 47290
Remote Config [baseline] (543.052 µs) : 0, 543
Remote Config [candidate] (633.759 µs) : 0, 634
Telemetry [baseline] (5.659 ms) : 0, 5659
Telemetry [candidate] (5.476 ms) : 0, 5476
IAST [baseline] (13.717 ms) : 0, 13717
IAST [candidate] (13.301 ms) : 0, 13301
section profiling
ProfilingAgent [baseline] (77.037 ms) : 0, 77037
ProfilingAgent [candidate] (75.691 ms) : 0, 75691
BytebuddyAgent [baseline] (624.098 ms) : 0, 624098
BytebuddyAgent [candidate] (622.523 ms) : 0, 622523
GlobalTracer [baseline] (345.109 ms) : 0, 345109
GlobalTracer [candidate] (345.89 ms) : 0, 345890
AppSec [baseline] (48.864 ms) : 0, 48864
AppSec [candidate] (48.993 ms) : 0, 48993
Remote Config [baseline] (646.228 µs) : 0, 646
Remote Config [candidate] (641.444 µs) : 0, 641
Telemetry [baseline] (5.946 ms) : 0, 5946
Telemetry [candidate] (5.297 ms) : 0, 5297
Profiling [baseline] (77.062 ms) : 0, 77062
Profiling [candidate] (75.715 ms) : 0, 75715

Load

Parameters

	Baseline	Candidate
commit	1.20.0-SNAPSHOT~3a4d10997a	1.20.0-SNAPSHOT~023d122058
config	baseline	candidate
end_time	2023-08-23T08:12:18	2023-08-23T08:30:22
start_time	2023-08-23T08:12:05	2023-08-23T08:30:10

See matching parameters

	Baseline	Candidate
application	insecure-bank	insecure-bank
variant	iast	iast

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 22 cases.

cataphract · 2023-08-23T12:34:26Z

  @Override
  public TraceSegment getTraceSegment() {
-    AgentSpan.Context ctx = activeSpan().context();
+    AgentSpan activeSpan = activeSpan();


This method leaves me a bit uncomfortable, because it behaves differently from RequestContext#getTraceSegment, which never returns null. There is a TraceSegment.Noop that can be used here, but at the cost doing unnecessary work if there's no active span. The question is whether there being no active span is a frequent enough occurrence, that it's working risking NPEs

@cataphract Agreed. Since this was the only instance where we had no active spans. However, given that this is a backport PR, I would prefer to move on with this option, which was already merged and tested in master.

I'll open a separate PR to master to follow this up.

On the other hand, the CoreTracer#getTraceSegment usage is pretty limited. All current usages consider null.

Fixed NPE in User Events Tracking (#5732)

023d122

* Fixed SpringSecurity NullPointerException when user creation happens outside request. * Spotless

smola added type: bug fix Bug fix comp: asm waf Application Security Management (WAF) labels Aug 23, 2023

smola requested review from a team and ValentinZakharov August 23, 2023 07:50

smola requested a review from a team as a code owner August 23, 2023 07:50

smola requested a review from dougqh August 23, 2023 07:50

smola changed the title ~~Fixed NPE in User Events Tracking (#5732)~~ Fix NPE in User Events Tracking (#5732) Aug 23, 2023

cataphract reviewed Aug 23, 2023

View reviewed changes

smola requested a review from cataphract August 23, 2023 13:09

smola changed the title ~~Fix NPE in User Events Tracking (#5732)~~ Fix NPE in user events tracking (#5732) Aug 23, 2023

cataphract approved these changes Aug 23, 2023

View reviewed changes

smola merged commit b3eaa8a into release/v1.19.x Aug 23, 2023

smola deleted the smola/5732-cherry-pick branch August 23, 2023 14:24

smola added this to the 1.19.3 milestone Aug 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix NPE in user events tracking (#5732)#5757

Fix NPE in user events tracking (#5732)#5757
smola merged 1 commit into
release/v1.19.xfrom
smola/5732-cherry-pick

smola commented Aug 23, 2023 •

edited

Loading

Uh oh!

pr-commenter Bot commented Aug 23, 2023

Uh oh!

cataphract Aug 23, 2023

Uh oh!

smola Aug 23, 2023

Uh oh!

smola Aug 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

smola commented Aug 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Does This Do

Motivation

Additional Notes

Uh oh!

pr-commenter Bot commented Aug 23, 2023

Benchmarks

Startup

Parameters

Summary

Load

Parameters

Summary

Uh oh!

cataphract Aug 23, 2023

Choose a reason for hiding this comment

Uh oh!

smola Aug 23, 2023

Choose a reason for hiding this comment

Uh oh!

smola Aug 23, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

smola commented Aug 23, 2023 •

edited

Loading