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

Change SymDB config #6120

Merged
merged 2 commits into from
Oct 31, 2023
Merged

Change SymDB config #6120

merged 2 commits into from
Oct 31, 2023

Conversation

jpbempel
Copy link
Member

@jpbempel jpbempel commented Oct 31, 2023 •
edited by jira bot
Loading

What Does This Do

Replace DD_DYNAMIC_INSTRUMENTATION_SYMBOL_ENABLED
to DD_SYMBOL_DATABASE_UPLOAD_ENABLED
Add DD_INTERNAL_FORCE_SYMBOL_DATABASE_UPLOAD for testing

Motivation

Additional Notes

Jira ticket: DEBUG-1898

@jpbempel
Change SymDB config
7045199 
Replace DD_DYNAMIC_INSTRUMENTATION_SYMBOL_ENABLED
to DD_SYMBOL_DATABASE_UPLOAD_ENABLED
Add DD_INTERNAL_FORCE_SYMBOL_DATABASE_UPLOAD for testing
@jpbempel jpbempel added the comp: debugger Dynamic Instrumentation label Oct 31, 2023
@jpbempel jpbempel requested review from a team as code owners October 31, 2023 08:57
@jpbempel jpbempel requested review from cimi, mcculls, ygree and ojung and removed request for a team October 31, 2023 08:57
ojung
ojung reviewed Oct 31, 2023
View reviewed changes
dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java Outdated
@@ -92,7 +92,7 @@ public static synchronized void run(
} else {
log.debug("No configuration poller available from SharedCommunicationObjects");
}
if (config.getDebuggerSymbolEnabled()) {
if (config.isDebuggerSymbolEnabled() || config.isDebuggerSymbolForceUpload()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The force upload should only take effect if symbol.database.upload.enabled is true also. We need a switch that decides based on isDebuggerSymbolForceUpload wether to upload symbols or stay dormant.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly for now, the force upload is redundant with enabled. So from an outside point of view (system tests) it will work as expected.
We have introduced the config token for now, but once we have the enabled-based mechanism then we can adapt the code to do what is intended.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'm saying also is that if isDebuggerSymbolEnabled is true, it should not automatically upload. Only if also isDebuggerSymbolForceUpload is enabled. We don't want the library to automatically upload. Only in two cases we want an upload:

  1. Enablement indicates that we should upload
  2. Force upload is set to true

@pr-commenter
Copy link

pr-commenter bot commented Oct 31, 2023 •
edited
Loading

Benchmarks

Startup

Parameters

Baseline Candidate
commit 1.23.0-SNAPSHOT~17c097d2c0 1.23.0-SNAPSHOT~99d0a0d642
config baseline candidate
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
module Agent Agent
parent None None
variant iast iast

Summary

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

scenario Δ mean execution_time candidate mean execution_time baseline mean execution_time
scenario:insecure-bank:iast:Telemetry worse
[+0.244ms; +6.328ms] or [+2.858%; +74.160%]		 11.820ms 8.534ms
scenario:petclinic:profiling:ProfilingAgent worse
[+16.176ms; +19.390ms] or [+13.328%; +15.977%]		 139.147ms 121.364ms
scenario:petclinic:profiling:Profiling worse
[+16.174ms; +19.390ms] or [+13.322%; +15.970%]		 139.195ms 121.413ms
Startup time reports for insecure-bank 
gantt
    title insecure-bank - global startup overhead: candidate=1.23.0-SNAPSHOT~99d0a0d642, baseline=1.23.0-SNAPSHOT~17c097d2c0

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.55 s) : 0, 1549765
Total [baseline] (12.207 s) : 0, 12207030
Agent [candidate] (1.546 s) : 0, 1546001
Total [candidate] (12.192 s) : 0, 12191923
section iast
Agent [baseline] (1.702 s) : 0, 1702380
Total [baseline] (12.945 s) : 0, 12944795
Agent [candidate] (1.708 s) : 0, 1708377
Total [candidate] (13.029 s) : 0, 13029343
section iast_TELEMETRY_OFF
Agent [baseline] (1.711 s) : 0, 1710792
Total [baseline] (12.958 s) : 0, 12957566
Agent [candidate] (1.702 s) : 0, 1702241
Total [candidate] (12.905 s) : 0, 12905097
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.55 s -
Agent iast 1.702 s 152.615 ms (9.8%)
Agent iast_TELEMETRY_OFF 1.711 s 161.027 ms (10.4%)
Total tracing 12.207 s -
Total iast 12.945 s 737.765 ms (6.0%)
Total iast_TELEMETRY_OFF 12.958 s 750.537 ms (6.1%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.546 s -
Agent iast 1.708 s 162.377 ms (10.5%)
Agent iast_TELEMETRY_OFF 1.702 s 156.24 ms (10.1%)
Total tracing 12.192 s -
Total iast 13.029 s 837.42 ms (6.9%)
Total iast_TELEMETRY_OFF 12.905 s 713.174 ms (5.8%) 
gantt
    title insecure-bank - break down per module: candidate=1.23.0-SNAPSHOT~99d0a0d642, baseline=1.23.0-SNAPSHOT~17c097d2c0

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (966.879 ms) : 0, 966879
BytebuddyAgent [candidate] (965.285 ms) : 0, 965285
GlobalTracer [baseline] (442.251 ms) : 0, 442251
GlobalTracer [candidate] (440.466 ms) : 0, 440466
AppSec [baseline] (70.98 ms) : 0, 70980
AppSec [candidate] (70.902 ms) : 0, 70902
Remote Config [baseline] (855.58 µs) : 0, 856
Remote Config [candidate] (840.032 µs) : 0, 840
Telemetry [baseline] (8.038 ms) : 0, 8038
Telemetry [candidate] (8.036 ms) : 0, 8036
section iast
BytebuddyAgent [baseline] (1.14 s) : 0, 1139739
BytebuddyAgent [candidate] (1.143 s) : 0, 1142549
GlobalTracer [baseline] (406.118 ms) : 0, 406118
GlobalTracer [candidate] (406.62 ms) : 0, 406620
AppSec [baseline] (66.71 ms) : 0, 66710
AppSec [candidate] (67.755 ms) : 0, 67755
IAST [baseline] (20.203 ms) : 0, 20203
IAST [candidate] (18.595 ms) : 0, 18595
Remote Config [baseline] (736.341 µs) : 0, 736
Remote Config [candidate] (718.148 µs) : 0, 718
Telemetry [baseline] (8.534 ms) : 0, 8534
Telemetry [candidate] (11.82 ms) : 0, 11820
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (1.143 s) : 0, 1142844
BytebuddyAgent [candidate] (1.136 s) : 0, 1135620
GlobalTracer [baseline] (411.407 ms) : 0, 411407
GlobalTracer [candidate] (409.272 ms) : 0, 409272
AppSec [baseline] (67.442 ms) : 0, 67442
AppSec [candidate] (66.974 ms) : 0, 66974
IAST [baseline] (17.631 ms) : 0, 17631
IAST [candidate] (19.753 ms) : 0, 19753
Remote Config [baseline] (694.316 µs) : 0, 694
Remote Config [candidate] (944.477 µs) : 0, 944
Telemetry [baseline] (9.894 ms) : 0, 9894
Telemetry [candidate] (9.209 ms) : 0, 9209
Loading
Startup time reports for petclinic 
gantt
    title petclinic - global startup overhead: candidate=1.23.0-SNAPSHOT~99d0a0d642, baseline=1.23.0-SNAPSHOT~17c097d2c0

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.548 s) : 0, 1548285
Total [baseline] (12.792 s) : 0, 12792415
Agent [candidate] (1.547 s) : 0, 1546638
Total [candidate] (12.766 s) : 0, 12766384
section appsec
Agent [baseline] (1.673 s) : 0, 1673202
Total [baseline] (12.811 s) : 0, 12811218
Agent [candidate] (1.676 s) : 0, 1675892
Total [candidate] (12.767 s) : 0, 12767419
section iast
Agent [baseline] (1.701 s) : 0, 1701063
Total [baseline] (13.212 s) : 0, 13212297
Agent [candidate] (1.709 s) : 0, 1709010
Total [candidate] (13.274 s) : 0, 13273849
section profiling
Agent [baseline] (1.818 s) : 0, 1817711
Total [baseline] (13.187 s) : 0, 13186908
Agent [candidate] (1.834 s) : 0, 1833508
Total [candidate] (13.125 s) : 0, 13124561
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.548 s -
Agent appsec 1.673 s 124.917 ms (8.1%)
Agent iast 1.701 s 152.778 ms (9.9%)
Agent profiling 1.818 s 269.425 ms (17.4%)
Total tracing 12.792 s -
Total appsec 12.811 s 18.803 ms (0.1%)
Total iast 13.212 s 419.882 ms (3.3%)
Total profiling 13.187 s 394.493 ms (3.1%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.547 s -
Agent appsec 1.676 s 129.253 ms (8.4%)
Agent iast 1.709 s 162.372 ms (10.5%)
Agent profiling 1.834 s 286.87 ms (18.5%)
Total tracing 12.766 s -
Total appsec 12.767 s 1.035 ms (0.0%)
Total iast 13.274 s 507.465 ms (4.0%)
Total profiling 13.125 s 358.177 ms (2.8%) 
gantt
    title petclinic - break down per module: candidate=1.23.0-SNAPSHOT~99d0a0d642, baseline=1.23.0-SNAPSHOT~17c097d2c0

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (964.32 ms) : 0, 964320
BytebuddyAgent [candidate] (965.164 ms) : 0, 965164
GlobalTracer [baseline] (443.2 ms) : 0, 443200
GlobalTracer [candidate] (440.629 ms) : 0, 440629
AppSec [baseline] (71.427 ms) : 0, 71427
AppSec [candidate] (71.278 ms) : 0, 71278
Remote Config [baseline] (871.664 µs) : 0, 872
Remote Config [candidate] (856.941 µs) : 0, 857
Telemetry [baseline] (8.14 ms) : 0, 8140
Telemetry [candidate] (8.015 ms) : 0, 8015
section appsec
BytebuddyAgent [baseline] (960.011 ms) : 0, 960011
BytebuddyAgent [candidate] (961.337 ms) : 0, 961337
GlobalTracer [baseline] (437.79 ms) : 0, 437790
GlobalTracer [candidate] (439.003 ms) : 0, 439003
AppSec [baseline] (206.844 ms) : 0, 206844
AppSec [candidate] (206.777 ms) : 0, 206777
Remote Config [baseline] (803.705 µs) : 0, 804
Remote Config [candidate] (803.373 µs) : 0, 803
Telemetry [baseline] (7.618 ms) : 0, 7618
Telemetry [candidate] (7.587 ms) : 0, 7587
section iast
BytebuddyAgent [baseline] (1.139 s) : 0, 1138815
BytebuddyAgent [candidate] (1.144 s) : 0, 1143602
GlobalTracer [baseline] (405.345 ms) : 0, 405345
GlobalTracer [candidate] (407.342 ms) : 0, 407342
AppSec [baseline] (68.506 ms) : 0, 68506
AppSec [candidate] (68.753 ms) : 0, 68753
Remote Config [baseline] (744.259 µs) : 0, 744
Remote Config [candidate] (744.217 µs) : 0, 744
Telemetry [baseline] (9.345 ms) : 0, 9345
Telemetry [candidate] (8.589 ms) : 0, 8589
IAST [baseline] (18.074 ms) : 0, 18074
IAST [candidate] (19.531 ms) : 0, 19531
section profiling
BytebuddyAgent [baseline] (980.148 ms) : 0, 980148
BytebuddyAgent [candidate] (981.312 ms) : 0, 981312
GlobalTracer [baseline] (546.97 ms) : 0, 546970
GlobalTracer [candidate] (544.638 ms) : 0, 544638
AppSec [baseline] (72.335 ms) : 0, 72335
AppSec [candidate] (71.417 ms) : 0, 71417
Remote Config [baseline] (820.398 µs) : 0, 820
Remote Config [candidate] (812.372 µs) : 0, 812
Telemetry [baseline] (8.047 ms) : 0, 8047
Telemetry [candidate] (7.943 ms) : 0, 7943
ProfilingAgent [baseline] (121.364 ms) : 0, 121364
ProfilingAgent [candidate] (139.147 ms) : 0, 139147
Profiling [baseline] (121.413 ms) : 0, 121413
Profiling [candidate] (139.195 ms) : 0, 139195
Loading

Load

Parameters

Baseline Candidate
commit 1.23.0-SNAPSHOT~17c097d2c0 1.23.0-SNAPSHOT~99d0a0d642
config baseline candidate
end_time 2023-10-31T11:10:40 2023-10-31T11:32:55
start_time 2023-10-31T11:10:25 2023-10-31T11:32:40
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.

Request duration reports for insecure-bank 
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.23.0-SNAPSHOT~99d0a0d642, baseline=1.23.0-SNAPSHOT~17c097d2c0
    dateFormat X
    axisFormat %s
section baseline
no_agent (440.507 µs) : 411, 470
.   : milestone, 441,
iast (560.336 µs) : 530, 591
.   : milestone, 560,
iast_FULL (653.453 µs) : 622, 685
.   : milestone, 653,
iast_INACTIVE (530.409 µs) : 500, 561
.   : milestone, 530,
iast_TELEMETRY_OFF (554.397 µs) : 524, 585
.   : milestone, 554,
tracing (522.78 µs) : 492, 554
.   : milestone, 523,
section candidate
no_agent (443.256 µs) : 414, 473
.   : milestone, 443,
iast (556.253 µs) : 526, 586
.   : milestone, 556,
iast_FULL (654.419 µs) : 622, 687
.   : milestone, 654,
iast_INACTIVE (529.412 µs) : 499, 560
.   : milestone, 529,
iast_TELEMETRY_OFF (550.293 µs) : 519, 581
.   : milestone, 550,
tracing (517.43 µs) : 488, 547
.   : milestone, 517,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 440.507 µs [411.158 µs, 469.856 µs] -
iast 560.336 µs [530.1 µs, 590.571 µs] 119.829 µs (27.2%)
iast_FULL 653.453 µs [622.263 µs, 684.643 µs] 212.946 µs (48.3%)
iast_INACTIVE 530.409 µs [500.079 µs, 560.739 µs] 89.902 µs (20.4%)
iast_TELEMETRY_OFF 554.397 µs [523.641 µs, 585.154 µs] 113.891 µs (25.9%)
tracing 522.78 µs [491.848 µs, 553.713 µs] 82.273 µs (18.7%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 443.256 µs [413.771 µs, 472.741 µs] -
iast 556.253 µs [526.065 µs, 586.441 µs] 112.997 µs (25.5%)
iast_FULL 654.419 µs [622.269 µs, 686.57 µs] 211.163 µs (47.6%)
iast_INACTIVE 529.412 µs [499.124 µs, 559.701 µs] 86.156 µs (19.4%)
iast_TELEMETRY_OFF 550.293 µs [519.396 µs, 581.19 µs] 107.037 µs (24.1%)
tracing 517.43 µs [487.598 µs, 547.261 µs] 74.174 µs (16.7%)
Request duration reports for petclinic 
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.23.0-SNAPSHOT~99d0a0d642, baseline=1.23.0-SNAPSHOT~17c097d2c0
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.679 ms) : 1655, 1703
.   : milestone, 1679,
appsec (2.129 ms) : 2095, 2164
.   : milestone, 2129,
iast (1.823 ms) : 1791, 1855
.   : milestone, 1823,
profiling (1.855 ms) : 1823, 1888
.   : milestone, 1855,
tracing (1.803 ms) : 1770, 1836
.   : milestone, 1803,
section candidate
no_agent (1.699 ms) : 1673, 1724
.   : milestone, 1699,
appsec (2.145 ms) : 2113, 2176
.   : milestone, 2145,
iast (1.843 ms) : 1810, 1876
.   : milestone, 1843,
profiling (1.841 ms) : 1808, 1874
.   : milestone, 1841,
tracing (1.811 ms) : 1778, 1844
.   : milestone, 1811,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.679 ms [1.655 ms, 1.703 ms] -
appsec 2.129 ms [2.095 ms, 2.164 ms] 450.478 µs (26.8%)
iast 1.823 ms [1.791 ms, 1.855 ms] 144.418 µs (8.6%)
profiling 1.855 ms [1.823 ms, 1.888 ms] 176.164 µs (10.5%)
tracing 1.803 ms [1.77 ms, 1.836 ms] 124.377 µs (7.4%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.699 ms [1.673 ms, 1.724 ms] -
appsec 2.145 ms [2.113 ms, 2.176 ms] 446.079 µs (26.3%)
iast 1.843 ms [1.81 ms, 1.876 ms] 144.89 µs (8.5%)
profiling 1.841 ms [1.808 ms, 1.874 ms] 142.229 µs (8.4%)
tracing 1.811 ms [1.778 ms, 1.844 ms] 112.284 µs (6.6%)

@jpbempel jpbempel merged commit 205e504 into master Oct 31, 2023
67 of 69 checks passed
@jpbempel jpbempel deleted the jpbempel/rename-symdb-config branch October 31, 2023 14:11
@github-actions github-actions bot added this to the 1.23.0 milestone Oct 31, 2023
@jpbempel jpbempel added the tag: no release notes Changes to exclude from release notes label Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ojung ojung ojung approved these changes

@cimi cimi Awaiting requested review from cimi cimi is a code owner automatically assigned from DataDog/debugger-java

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

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

Assignees
No one assigned
Labels
comp: debugger Dynamic Instrumentation tag: no release notes Changes to exclude from release notes
Projects
None yet
Milestone
1.23.0
Development

Successfully merging this pull request may close these issues.
2 participants
@jpbempel @ojung