-
Notifications
You must be signed in to change notification settings - Fork 292
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.
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
Implement the DebuggerProbe #7588
Conversation
dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/DebuggerProbe.java
Show resolved
Hide resolved
dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/Configuration.java
Show resolved
Hide resolved
dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/DebuggerProbe.java
Show resolved
Hide resolved
.../agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerProductChangesListener.java
Show resolved
Hide resolved
dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/DebuggerProbe.java
Show resolved
Hide resolved
dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/Configuration.java
Show resolved
Hide resolved
dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/DebuggerProbe.java
Show resolved
Hide resolved
.../agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerProductChangesListener.java
Show resolved
Hide resolved
I don't know where that submodule came or how to get rid of it if anyone has any advice. do I just delete that folder locally and repush to my branch? |
...ent/agent-debugger/src/main/java/com/datadog/debugger/codeorigin/CodeOriginProbeManager.java
Outdated
Show resolved
Hide resolved
...ent/agent-debugger/src/main/java/com/datadog/debugger/codeorigin/CodeOriginProbeManager.java
Outdated
Show resolved
Hide resolved
...ent/agent-debugger/src/main/java/com/datadog/debugger/codeorigin/CodeOriginProbeManager.java
Outdated
Show resolved
Hide resolved
...ent/agent-debugger/src/main/java/com/datadog/debugger/codeorigin/CodeOriginProbeManager.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/CodeOriginProbe.java
Show resolved
Hide resolved
dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/CodeOriginProbe.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/DebuggerProbe.java
Show resolved
Hide resolved
...gger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/spanorigin/CodeOriginInfo.java
Show resolved
Hide resolved
...ent/agent-debugger/src/main/java/com/datadog/debugger/codeorigin/CodeOriginProbeManager.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/CodeOriginProbe.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/CodeOriginProbe.java
Show resolved
Hide resolved
dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/CodeOriginProbe.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/probe/DebuggerProbeTest.java
Outdated
Show resolved
Hide resolved
Debugger benchmarksParameters
See matching parameters
SummaryFound 3 performance improvements and 0 performance regressions! Performance is the same for 3 metrics, 9 unstable metrics.
See unchanged results
Request duration reports for reportsgantt
title reports - request duration [CI 0.99] : candidate=None, baseline=None
dateFormat X
axisFormat %s
section baseline
noprobe (317.208 µs) : 251, 384
. : milestone, 317,
basic (298.518 µs) : 280, 317
. : milestone, 299,
loop (10.562 ms) : 10498, 10627
. : milestone, 10562,
section candidate
noprobe (320.504 µs) : 253, 388
. : milestone, 321,
basic (303.96 µs) : 287, 321
. : milestone, 304,
loop (10.315 ms) : 10272, 10358
. : milestone, 10315,
|
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 48 metrics, 15 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.40.0-SNAPSHOT~de6b0c4d70, baseline=1.40.0-SNAPSHOT~04db2f0bfb
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.064 s) : 0, 1064240
Total [baseline] (8.514 s) : 0, 8514326
Agent [candidate] (1.071 s) : 0, 1070610
Total [candidate] (8.524 s) : 0, 8523868
section iast
Agent [baseline] (1.186 s) : 0, 1185801
Total [baseline] (8.964 s) : 0, 8963995
Agent [candidate] (1.189 s) : 0, 1188604
Total [candidate] (8.973 s) : 0, 8972966
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.195 s) : 0, 1195360
Total [baseline] (8.943 s) : 0, 8943393
Agent [candidate] (1.186 s) : 0, 1185792
Total [candidate] (8.946 s) : 0, 8945860
section iast_TELEMETRY_OFF
Agent [baseline] (1.184 s) : 0, 1184184
Total [baseline] (8.954 s) : 0, 8954332
Agent [candidate] (1.186 s) : 0, 1185502
Total [candidate] (8.961 s) : 0, 8961044
gantt
title insecure-bank - break down per module: candidate=1.40.0-SNAPSHOT~de6b0c4d70, baseline=1.40.0-SNAPSHOT~04db2f0bfb
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (679.567 ms) : 0, 679567
BytebuddyAgent [candidate] (684.061 ms) : 0, 684061
GlobalTracer [baseline] (311.122 ms) : 0, 311122
GlobalTracer [candidate] (312.352 ms) : 0, 312352
AppSec [baseline] (51.633 ms) : 0, 51633
AppSec [candidate] (52.099 ms) : 0, 52099
Remote Config [baseline] (715.966 µs) : 0, 716
Remote Config [candidate] (728.01 µs) : 0, 728
Telemetry [baseline] (7.519 ms) : 0, 7519
Telemetry [candidate] (7.596 ms) : 0, 7596
section iast
BytebuddyAgent [baseline] (789.488 ms) : 0, 789488
BytebuddyAgent [candidate] (790.922 ms) : 0, 790922
GlobalTracer [baseline] (298.347 ms) : 0, 298347
GlobalTracer [candidate] (299.192 ms) : 0, 299192
AppSec [baseline] (53.538 ms) : 0, 53538
AppSec [candidate] (52.343 ms) : 0, 52343
IAST [baseline] (21.985 ms) : 0, 21985
IAST [candidate] (24.433 ms) : 0, 24433
Remote Config [baseline] (1.383 ms) : 0, 1383
Remote Config [candidate] (667.719 µs) : 0, 668
Telemetry [baseline] (7.433 ms) : 0, 7433
Telemetry [candidate] (7.355 ms) : 0, 7355
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (795.899 ms) : 0, 795899
BytebuddyAgent [candidate] (789.31 ms) : 0, 789310
GlobalTracer [baseline] (300.338 ms) : 0, 300338
GlobalTracer [candidate] (298.686 ms) : 0, 298686
AppSec [baseline] (51.136 ms) : 0, 51136
AppSec [candidate] (53.298 ms) : 0, 53298
IAST [baseline] (24.686 ms) : 0, 24686
IAST [candidate] (22.054 ms) : 0, 22054
Remote Config [baseline] (614.042 µs) : 0, 614
Remote Config [candidate] (614.034 µs) : 0, 614
Telemetry [baseline] (8.94 ms) : 0, 8940
Telemetry [candidate] (8.186 ms) : 0, 8186
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (787.153 ms) : 0, 787153
BytebuddyAgent [candidate] (787.944 ms) : 0, 787944
GlobalTracer [baseline] (299.255 ms) : 0, 299255
GlobalTracer [candidate] (299.319 ms) : 0, 299319
AppSec [baseline] (52.243 ms) : 0, 52243
AppSec [candidate] (53.294 ms) : 0, 53294
IAST [baseline] (22.417 ms) : 0, 22417
IAST [candidate] (22.51 ms) : 0, 22510
Remote Config [baseline] (609.336 µs) : 0, 609
Remote Config [candidate] (677.656 µs) : 0, 678
Telemetry [baseline] (8.834 ms) : 0, 8834
Telemetry [candidate] (8.04 ms) : 0, 8040
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.40.0-SNAPSHOT~de6b0c4d70, baseline=1.40.0-SNAPSHOT~04db2f0bfb
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.062 s) : 0, 1062442
Total [baseline] (10.388 s) : 0, 10387813
Agent [candidate] (1.064 s) : 0, 1064072
Total [candidate] (10.42 s) : 0, 10419818
section appsec
Agent [baseline] (1.2 s) : 0, 1199759
Total [baseline] (10.622 s) : 0, 10621865
Agent [candidate] (1.198 s) : 0, 1197531
Total [candidate] (10.565 s) : 0, 10565299
section iast
Agent [baseline] (1.187 s) : 0, 1187035
Total [baseline] (10.85 s) : 0, 10850052
Agent [candidate] (1.2 s) : 0, 1200326
Total [candidate] (10.797 s) : 0, 10796713
section profiling
Agent [baseline] (1.257 s) : 0, 1256822
Total [baseline] (10.563 s) : 0, 10563072
Agent [candidate] (1.263 s) : 0, 1262585
Total [candidate] (10.623 s) : 0, 10622697
gantt
title petclinic - break down per module: candidate=1.40.0-SNAPSHOT~de6b0c4d70, baseline=1.40.0-SNAPSHOT~04db2f0bfb
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (678.583 ms) : 0, 678583
BytebuddyAgent [candidate] (679.526 ms) : 0, 679526
GlobalTracer [baseline] (310.257 ms) : 0, 310257
GlobalTracer [candidate] (310.768 ms) : 0, 310768
AppSec [baseline] (51.795 ms) : 0, 51795
AppSec [candidate] (51.808 ms) : 0, 51808
Remote Config [baseline] (719.99 µs) : 0, 720
Remote Config [candidate] (727.022 µs) : 0, 727
Telemetry [baseline] (7.491 ms) : 0, 7491
Telemetry [candidate] (7.565 ms) : 0, 7565
section appsec
BytebuddyAgent [baseline] (701.586 ms) : 0, 701586
BytebuddyAgent [candidate] (699.942 ms) : 0, 699942
GlobalTracer [baseline] (304.305 ms) : 0, 304305
GlobalTracer [candidate] (303.843 ms) : 0, 303843
AppSec [baseline] (161.131 ms) : 0, 161131
AppSec [candidate] (161.94 ms) : 0, 161940
IAST [baseline] (19.531 ms) : 0, 19531
IAST [candidate] (19.732 ms) : 0, 19732
Remote Config [baseline] (643.564 µs) : 0, 644
Remote Config [candidate] (646.803 µs) : 0, 647
Telemetry [baseline] (8.661 ms) : 0, 8661
Telemetry [candidate] (7.598 ms) : 0, 7598
section iast
BytebuddyAgent [baseline] (789.941 ms) : 0, 789941
BytebuddyAgent [candidate] (800.186 ms) : 0, 800186
GlobalTracer [baseline] (298.836 ms) : 0, 298836
GlobalTracer [candidate] (301.235 ms) : 0, 301235
AppSec [baseline] (53.131 ms) : 0, 53131
AppSec [candidate] (54.042 ms) : 0, 54042
IAST [baseline] (22.667 ms) : 0, 22667
IAST [candidate] (23.013 ms) : 0, 23013
Remote Config [baseline] (1.421 ms) : 0, 1421
Remote Config [candidate] (616.787 µs) : 0, 617
Telemetry [baseline] (7.4 ms) : 0, 7400
Telemetry [candidate] (7.42 ms) : 0, 7420
section profiling
BytebuddyAgent [baseline] (670.758 ms) : 0, 670758
BytebuddyAgent [candidate] (673.859 ms) : 0, 673859
GlobalTracer [baseline] (391.796 ms) : 0, 391796
GlobalTracer [candidate] (394.002 ms) : 0, 394002
AppSec [baseline] (52.366 ms) : 0, 52366
AppSec [candidate] (52.285 ms) : 0, 52285
Remote Config [baseline] (708.435 µs) : 0, 708
Remote Config [candidate] (713.55 µs) : 0, 714
Telemetry [baseline] (7.444 ms) : 0, 7444
Telemetry [candidate] (7.445 ms) : 0, 7445
ProfilingAgent [baseline] (95.859 ms) : 0, 95859
ProfilingAgent [candidate] (96.073 ms) : 0, 96073
Profiling [baseline] (95.883 ms) : 0, 95883
Profiling [candidate] (96.097 ms) : 0, 96097
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 17 unstable metrics. Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.40.0-SNAPSHOT~de6b0c4d70, baseline=1.40.0-SNAPSHOT~04db2f0bfb
dateFormat X
axisFormat %s
section baseline
no_agent (1.351 ms) : 1332, 1371
. : milestone, 1351,
appsec (1.726 ms) : 1703, 1749
. : milestone, 1726,
appsec_no_iast (1.733 ms) : 1707, 1758
. : milestone, 1733,
iast (1.478 ms) : 1455, 1501
. : milestone, 1478,
profiling (1.533 ms) : 1508, 1558
. : milestone, 1533,
tracing (1.481 ms) : 1457, 1504
. : milestone, 1481,
section candidate
no_agent (1.35 ms) : 1330, 1370
. : milestone, 1350,
appsec (1.712 ms) : 1688, 1737
. : milestone, 1712,
appsec_no_iast (1.756 ms) : 1732, 1779
. : milestone, 1756,
iast (1.494 ms) : 1471, 1517
. : milestone, 1494,
profiling (1.536 ms) : 1511, 1560
. : milestone, 1536,
tracing (1.482 ms) : 1458, 1506
. : milestone, 1482,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.40.0-SNAPSHOT~de6b0c4d70, baseline=1.40.0-SNAPSHOT~04db2f0bfb
dateFormat X
axisFormat %s
section baseline
no_agent (378.4 µs) : 359, 398
. : milestone, 378,
iast (496.806 µs) : 475, 518
. : milestone, 497,
iast_FULL (553.354 µs) : 532, 575
. : milestone, 553,
iast_GLOBAL (512.814 µs) : 491, 534
. : milestone, 513,
iast_HARDCODED_SECRET_DISABLED (487.66 µs) : 466, 509
. : milestone, 488,
iast_INACTIVE (448.533 µs) : 428, 469
. : milestone, 449,
iast_TELEMETRY_OFF (475.815 µs) : 453, 499
. : milestone, 476,
tracing (443.821 µs) : 423, 464
. : milestone, 444,
section candidate
no_agent (372.545 µs) : 353, 392
. : milestone, 373,
iast (496.7 µs) : 474, 519
. : milestone, 497,
iast_FULL (554.299 µs) : 533, 576
. : milestone, 554,
iast_GLOBAL (510.746 µs) : 489, 532
. : milestone, 511,
iast_HARDCODED_SECRET_DISABLED (493.262 µs) : 471, 515
. : milestone, 493,
iast_INACTIVE (453.921 µs) : 433, 475
. : milestone, 454,
iast_TELEMETRY_OFF (480.804 µs) : 458, 504
. : milestone, 481,
tracing (441.981 µs) : 422, 462
. : milestone, 442,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 1 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.40.0-SNAPSHOT~de6b0c4d70, baseline=1.40.0-SNAPSHOT~04db2f0bfb
dateFormat X
axisFormat %s
section baseline
no_agent (1.466 ms) : 1454, 1477
. : milestone, 1466,
appsec (2.299 ms) : 2258, 2339
. : milestone, 2299,
iast (2.062 ms) : 2013, 2112
. : milestone, 2062,
iast_GLOBAL (2.112 ms) : 2061, 2163
. : milestone, 2112,
profiling (2.194 ms) : 2046, 2342
. : milestone, 2194,
tracing (1.906 ms) : 1868, 1945
. : milestone, 1906,
section candidate
no_agent (1.472 ms) : 1461, 1484
. : milestone, 1472,
appsec (2.3 ms) : 2260, 2341
. : milestone, 2300,
iast (2.036 ms) : 1988, 2084
. : milestone, 2036,
iast_GLOBAL (2.106 ms) : 2056, 2155
. : milestone, 2106,
profiling (1.922 ms) : 1881, 1963
. : milestone, 1922,
tracing (1.909 ms) : 1871, 1948
. : milestone, 1909,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.40.0-SNAPSHOT~de6b0c4d70, baseline=1.40.0-SNAPSHOT~04db2f0bfb
dateFormat X
axisFormat %s
section baseline
no_agent (15.075 s) : 15075000, 15075000
. : milestone, 15075000,
appsec (15.617 s) : 15617000, 15617000
. : milestone, 15617000,
iast (18.878 s) : 18878000, 18878000
. : milestone, 18878000,
iast_GLOBAL (17.881 s) : 17881000, 17881000
. : milestone, 17881000,
profiling (16.177 s) : 16177000, 16177000
. : milestone, 16177000,
tracing (14.779 s) : 14779000, 14779000
. : milestone, 14779000,
section candidate
no_agent (15.217 s) : 15217000, 15217000
. : milestone, 15217000,
appsec (15.325 s) : 15325000, 15325000
. : milestone, 15325000,
iast (18.982 s) : 18982000, 18982000
. : milestone, 18982000,
iast_GLOBAL (18.204 s) : 18204000, 18204000
. : milestone, 18204000,
profiling (15.444 s) : 15444000, 15444000
. : milestone, 15444000,
tracing (15.316 s) : 15316000, 15316000
. : milestone, 15316000,
|
add some tests some clean up
stop walking the stack at the first hit since that's all we use anyway.
|
||
import datadog.trace.bootstrap.debugger.DebuggerContext.CodeOriginRecorder; | ||
|
||
public class DefaultCodeOriginRecorder implements CodeOriginRecorder { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I think this class is useless: we could implement CodeOriginRecorder in CodeOriginPRobeManager
there is no added value here.
Could be done in a next PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i 100% agree. this is all an outgrowth of mimicking the exception probe stuff but has felt overly convoluted for a while now. I'd really love to clean it up but have been waiting to get it working first. it's high on my list to clean up post-PR.
if (localRootSpan.getTag(getId()) != null) { | ||
span = (AgentSpan) localRootSpan.getTag(getId()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Object probeTag = localRootSpan.getTag(getId());
if (probeTag != null) {
span = (AgentSpan) probeTag;
}
Can be done later
@Override | ||
public InstrumentationResult.Status instrument( | ||
MethodInfo methodInfo, List<DiagnosticMessage> diagnostics, List<ProbeId> probeIds) { | ||
return new CapturedContextInstrumentor(this, methodInfo, diagnostics, probeIds, false, null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not the best way to do it. It's overkill for just a callback on evaluate.
If we add condition that requires to evaluate the context then yes it makes sense. otherwise I think we need a dedicated instrumentor that avoid capturing for nothing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i was just pondering this issue, actually. we should talk this one through and aim for a future iteration to streamline that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM , my last major concern is about instrumentor
What Does This Do
Implement the DebuggerProbe as defined in the RFC as well as some related changes from the Code Origin for Spans RFC
Motivation
Additional Notes
Contributor Checklist
type:
and (comp:
orinst:
) labels in addition to any usefull labelsclose
,fix
or any linking keywords when referencing an issue.Use
solves
instead, and assign the PR milestone to the issueJira ticket: DEBUG-2719