-
Notifications
You must be signed in to change notification settings - Fork 278
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
Full mode for SQL Server #7186
base: master
Are you sure you want to change the base?
Full mode for SQL Server #7186
Conversation
...entation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java
Outdated
Show resolved
Hide resolved
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 petclinicgantt
title petclinic - global startup overhead: candidate=1.36.0-SNAPSHOT~59fca7845d, baseline=1.38.0-SNAPSHOT~0e5e2749b0
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.074 s) : 0, 1074499
Total [baseline] (10.375 s) : 0, 10374799
Agent [candidate] (1.064 s) : 0, 1064037
Total [candidate] (10.375 s) : 0, 10375095
section appsec
Agent [baseline] (1.183 s) : 0, 1183476
Total [baseline] (10.464 s) : 0, 10463508
Agent [candidate] (1.193 s) : 0, 1192539
Total [candidate] (10.474 s) : 0, 10474220
section iast
Agent [baseline] (1.179 s) : 0, 1178769
Total [baseline] (10.67 s) : 0, 10670042
Agent [candidate] (1.169 s) : 0, 1169092
Total [candidate] (10.705 s) : 0, 10704620
section profiling
Agent [baseline] (1.268 s) : 0, 1268322
Total [baseline] (10.55 s) : 0, 10550381
Agent [candidate] (1.261 s) : 0, 1260603
Total [candidate] (10.595 s) : 0, 10595275
gantt
title petclinic - break down per module: candidate=1.36.0-SNAPSHOT~59fca7845d, baseline=1.38.0-SNAPSHOT~0e5e2749b0
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (672.676 ms) : 0, 672676
BytebuddyAgent [candidate] (666.219 ms) : 0, 666219
GlobalTracer [baseline] (308.216 ms) : 0, 308216
GlobalTracer [candidate] (305.191 ms) : 0, 305191
AppSec [baseline] (50.332 ms) : 0, 50332
AppSec [candidate] (49.935 ms) : 0, 49935
Remote Config [baseline] (673.625 µs) : 0, 674
Remote Config [candidate] (734.856 µs) : 0, 735
Telemetry [baseline] (7.698 ms) : 0, 7698
Telemetry [candidate] (7.49 ms) : 0, 7490
section appsec
BytebuddyAgent [baseline] (676.721 ms) : 0, 676721
BytebuddyAgent [candidate] (681.906 ms) : 0, 681906
GlobalTracer [baseline] (299.499 ms) : 0, 299499
GlobalTracer [candidate] (300.382 ms) : 0, 300382
AppSec [baseline] (153.67 ms) : 0, 153670
AppSec [candidate] (154.539 ms) : 0, 154539
Remote Config [baseline] (623.864 µs) : 0, 624
Remote Config [candidate] (653.981 µs) : 0, 654
Telemetry [baseline] (8.353 ms) : 0, 8353
Telemetry [candidate] (9.432 ms) : 0, 9432
IAST [baseline] (19.658 ms) : 0, 19658
IAST [candidate] (21.565 ms) : 0, 21565
section iast
BytebuddyAgent [baseline] (785.344 ms) : 0, 785344
BytebuddyAgent [candidate] (779.096 ms) : 0, 779096
GlobalTracer [baseline] (297.463 ms) : 0, 297463
GlobalTracer [candidate] (293.745 ms) : 0, 293745
AppSec [baseline] (47.53 ms) : 0, 47530
AppSec [candidate] (47.044 ms) : 0, 47044
Remote Config [baseline] (585.714 µs) : 0, 586
Remote Config [candidate] (605.933 µs) : 0, 606
Telemetry [baseline] (8.538 ms) : 0, 8538
Telemetry [candidate] (8.641 ms) : 0, 8641
IAST [baseline] (25.675 ms) : 0, 25675
IAST [candidate] (26.695 ms) : 0, 26695
section profiling
BytebuddyAgent [baseline] (665.47 ms) : 0, 665470
BytebuddyAgent [candidate] (661.124 ms) : 0, 661124
GlobalTracer [baseline] (388.836 ms) : 0, 388836
GlobalTracer [candidate] (387.438 ms) : 0, 387438
AppSec [baseline] (51.791 ms) : 0, 51791
AppSec [candidate] (51.239 ms) : 0, 51239
Remote Config [baseline] (659.49 µs) : 0, 659
Remote Config [candidate] (671.314 µs) : 0, 671
Telemetry [baseline] (7.334 ms) : 0, 7334
Telemetry [candidate] (7.362 ms) : 0, 7362
ProfilingAgent [baseline] (96.752 ms) : 0, 96752
ProfilingAgent [candidate] (95.778 ms) : 0, 95778
Profiling [baseline] (96.778 ms) : 0, 96778
Profiling [candidate] (95.802 ms) : 0, 95802
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.36.0-SNAPSHOT~59fca7845d, baseline=1.38.0-SNAPSHOT~0e5e2749b0
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.063 s) : 0, 1063158
Total [baseline] (8.507 s) : 0, 8507077
Agent [candidate] (1.061 s) : 0, 1061132
Total [candidate] (8.49 s) : 0, 8490476
section iast
Agent [baseline] (1.17 s) : 0, 1169677
Total [baseline] (8.98 s) : 0, 8979562
Agent [candidate] (1.175 s) : 0, 1175330
Total [candidate] (8.992 s) : 0, 8992302
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.172 s) : 0, 1172452
Total [baseline] (8.919 s) : 0, 8918800
Agent [candidate] (1.176 s) : 0, 1175779
Total [candidate] (8.928 s) : 0, 8927620
section iast_TELEMETRY_OFF
Agent [baseline] (1.167 s) : 0, 1167181
Total [baseline] (8.925 s) : 0, 8925144
Agent [candidate] (1.18 s) : 0, 1179559
Total [candidate] (8.981 s) : 0, 8980704
gantt
title insecure-bank - break down per module: candidate=1.36.0-SNAPSHOT~59fca7845d, baseline=1.38.0-SNAPSHOT~0e5e2749b0
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (665.189 ms) : 0, 665189
BytebuddyAgent [candidate] (664.505 ms) : 0, 664505
GlobalTracer [baseline] (304.999 ms) : 0, 304999
GlobalTracer [candidate] (304.14 ms) : 0, 304140
AppSec [baseline] (50.151 ms) : 0, 50151
AppSec [candidate] (49.766 ms) : 0, 49766
Remote Config [baseline] (675.368 µs) : 0, 675
Remote Config [candidate] (770.171 µs) : 0, 770
Telemetry [baseline] (7.635 ms) : 0, 7635
Telemetry [candidate] (7.613 ms) : 0, 7613
section iast
BytebuddyAgent [baseline] (779.771 ms) : 0, 779771
BytebuddyAgent [candidate] (783.908 ms) : 0, 783908
GlobalTracer [baseline] (295.45 ms) : 0, 295450
GlobalTracer [candidate] (295.909 ms) : 0, 295909
AppSec [baseline] (48.099 ms) : 0, 48099
AppSec [candidate] (47.231 ms) : 0, 47231
IAST [baseline] (25.318 ms) : 0, 25318
IAST [candidate] (27.346 ms) : 0, 27346
Remote Config [baseline] (576.111 µs) : 0, 576
Remote Config [candidate] (598.809 µs) : 0, 599
Telemetry [baseline] (6.938 ms) : 0, 6938
Telemetry [candidate] (7.016 ms) : 0, 7016
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (780.69 ms) : 0, 780690
BytebuddyAgent [candidate] (783.901 ms) : 0, 783901
GlobalTracer [baseline] (295.904 ms) : 0, 295904
GlobalTracer [candidate] (296.034 ms) : 0, 296034
AppSec [baseline] (47.972 ms) : 0, 47972
AppSec [candidate] (47.202 ms) : 0, 47202
IAST [baseline] (26.013 ms) : 0, 26013
IAST [candidate] (26.771 ms) : 0, 26771
Remote Config [baseline] (562.823 µs) : 0, 563
Remote Config [candidate] (641.982 µs) : 0, 642
Telemetry [baseline] (7.71 ms) : 0, 7710
Telemetry [candidate] (7.821 ms) : 0, 7821
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (776.327 ms) : 0, 776327
BytebuddyAgent [candidate] (785.98 ms) : 0, 785980
GlobalTracer [baseline] (294.518 ms) : 0, 294518
GlobalTracer [candidate] (297.198 ms) : 0, 297198
AppSec [baseline] (47.108 ms) : 0, 47108
AppSec [candidate] (47.535 ms) : 0, 47535
IAST [baseline] (28.309 ms) : 0, 28309
IAST [candidate] (26.559 ms) : 0, 26559
Remote Config [baseline] (574.184 µs) : 0, 574
Remote Config [candidate] (595.138 µs) : 0, 595
Telemetry [baseline] (6.78 ms) : 0, 6780
Telemetry [candidate] (8.204 ms) : 0, 8204
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 16 unstable metrics. Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.36.0-SNAPSHOT~59fca7845d, baseline=1.38.0-SNAPSHOT~0e5e2749b0
dateFormat X
axisFormat %s
section baseline
no_agent (375.019 µs) : 353, 397
. : milestone, 375,
iast (486.205 µs) : 465, 508
. : milestone, 486,
iast_FULL (546.707 µs) : 526, 568
. : milestone, 547,
iast_GLOBAL (508.78 µs) : 487, 530
. : milestone, 509,
iast_HARDCODED_SECRET_DISABLED (479.959 µs) : 459, 501
. : milestone, 480,
iast_INACTIVE (456.469 µs) : 435, 478
. : milestone, 456,
iast_TELEMETRY_OFF (473.943 µs) : 452, 496
. : milestone, 474,
tracing (443.236 µs) : 423, 464
. : milestone, 443,
section candidate
no_agent (367.971 µs) : 348, 388
. : milestone, 368,
iast (480.902 µs) : 460, 502
. : milestone, 481,
iast_FULL (553.5 µs) : 532, 575
. : milestone, 554,
iast_GLOBAL (509.578 µs) : 488, 531
. : milestone, 510,
iast_HARDCODED_SECRET_DISABLED (480.594 µs) : 460, 501
. : milestone, 481,
iast_INACTIVE (458.877 µs) : 437, 481
. : milestone, 459,
iast_TELEMETRY_OFF (471.615 µs) : 451, 492
. : milestone, 472,
tracing (441.742 µs) : 421, 462
. : milestone, 442,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.36.0-SNAPSHOT~59fca7845d, baseline=1.38.0-SNAPSHOT~0e5e2749b0
dateFormat X
axisFormat %s
section baseline
no_agent (1.354 ms) : 1335, 1374
. : milestone, 1354,
appsec (1.716 ms) : 1692, 1740
. : milestone, 1716,
appsec_no_iast (1.713 ms) : 1687, 1738
. : milestone, 1713,
iast (1.477 ms) : 1455, 1500
. : milestone, 1477,
profiling (1.511 ms) : 1486, 1537
. : milestone, 1511,
tracing (1.461 ms) : 1437, 1486
. : milestone, 1461,
section candidate
no_agent (1.365 ms) : 1345, 1386
. : milestone, 1365,
appsec (1.736 ms) : 1712, 1760
. : milestone, 1736,
appsec_no_iast (1.724 ms) : 1700, 1748
. : milestone, 1724,
iast (1.485 ms) : 1463, 1507
. : milestone, 1485,
profiling (1.494 ms) : 1469, 1518
. : milestone, 1494,
tracing (1.467 ms) : 1443, 1492
. : milestone, 1467,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.36.0-SNAPSHOT~59fca7845d, baseline=1.38.0-SNAPSHOT~0e5e2749b0
dateFormat X
axisFormat %s
section baseline
no_agent (1.467 ms) : 1455, 1478
. : milestone, 1467,
appsec (2.249 ms) : 2214, 2285
. : milestone, 2249,
iast (1.968 ms) : 1926, 2010
. : milestone, 1968,
iast_GLOBAL (2.015 ms) : 1973, 2057
. : milestone, 2015,
profiling (1.874 ms) : 1840, 1909
. : milestone, 1874,
tracing (1.855 ms) : 1822, 1888
. : milestone, 1855,
section candidate
no_agent (1.467 ms) : 1456, 1479
. : milestone, 1467,
appsec (2.217 ms) : 2184, 2250
. : milestone, 2217,
iast (1.971 ms) : 1931, 2012
. : milestone, 1971,
iast_GLOBAL (2.003 ms) : 1964, 2043
. : milestone, 2003,
profiling (1.842 ms) : 1811, 1874
. : milestone, 1842,
tracing (1.829 ms) : 1799, 1860
. : milestone, 1829,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.36.0-SNAPSHOT~59fca7845d, baseline=1.38.0-SNAPSHOT~0e5e2749b0
dateFormat X
axisFormat %s
section baseline
no_agent (15.006 s) : 15006000, 15006000
. : milestone, 15006000,
appsec (15.347 s) : 15347000, 15347000
. : milestone, 15347000,
iast (18.666 s) : 18666000, 18666000
. : milestone, 18666000,
iast_GLOBAL (17.804 s) : 17804000, 17804000
. : milestone, 17804000,
profiling (15.439 s) : 15439000, 15439000
. : milestone, 15439000,
tracing (14.858 s) : 14858000, 14858000
. : milestone, 14858000,
section candidate
no_agent (15.288 s) : 15288000, 15288000
. : milestone, 15288000,
appsec (14.958 s) : 14958000, 14958000
. : milestone, 14958000,
iast (18.892 s) : 18892000, 18892000
. : milestone, 18892000,
iast_GLOBAL (17.734 s) : 17734000, 17734000
. : milestone, 17734000,
profiling (15.352 s) : 15352000, 15352000
. : milestone, 15352000,
tracing (14.93 s) : 14930000, 14930000
. : milestone, 14930000,
|
4d0aa82
to
87f47d8
Compare
...c/src/main/java/datadog/trace/instrumentation/jdbc/SQLServerPreparedStatementExcecution.java
Outdated
Show resolved
Hide resolved
...c/src/main/java/datadog/trace/instrumentation/jdbc/SQLServerPreparedStatementExcecution.java
Outdated
Show resolved
Hide resolved
...entation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java
Outdated
Show resolved
Hide resolved
fbd65dd
to
e8008cf
Compare
...c/main/java/datadog/trace/instrumentation/jdbc/AbstractPreparedStatementInstrumentation.java
Outdated
Show resolved
Hide resolved
...ent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java
Show resolved
Hide resolved
...entation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java
Outdated
Show resolved
Hide resolved
...ent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java
Outdated
Show resolved
Hide resolved
...c/main/java/datadog/trace/instrumentation/jdbc/AbstractPreparedStatementInstrumentation.java
Show resolved
Hide resolved
instrumentationStatement.execute(instrumentationSql); | ||
instrumentationStatement.close(); | ||
} catch (SQLException e) { | ||
log.warn( |
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 would avoid to set it as warn. Instead I'd would log as debug and rate limited
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.
Converted to debug
now. What technique do you recommend for rate limiting?
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.
There is something that's doing it datadog.trace.relocate.api.RatelimitedLogger
but it works for the warn level. It can be perhaps changed to rate limit debugging also. My reason suggesting to avoid warn as level is because it might be interpret by the customer as an issue of their application while it's a message internal of our tracer. Also, an application might do sql query very often. If there is an issue, logging each time will plummet the performances a lot
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 changed the level to debug
. Do you think I should introduce the rate limiter for the debug
level as well?
...ent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java
Show resolved
Hide resolved
statement.executeQuery(query) | ||
|
||
then: | ||
assert statement.sql == "/*${serviceInjection}*/ ${query}" |
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.
Would it be possible to add assertions for the spans created as well?
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.
span_id
is created in setContextInfo and then started in StatementAdvisor. I don't know how to intercept the call and get the value in any of those places. Unlike in other DBs, span_id/trace_id
aren't in the comment. Any ideas I could explore?
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 thinking at using trace assert to test that the set context span is created in the expected order and carries tags you need. A bit like other tests are doing:
dd-trace-java/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTestBase.groovy
Line 181 in db1f4c3
assertTraces(1) { |
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.
The tests you pointed to in your previous message seem to run queries on local databases of type h2
, derby
, etc. We don't have a SQL Server running locally. Consequently, if we want to make the same end-to-end test with SQL Server, we would need to integrate spinning up a SQL Server docker container in the CI. Do you think that's a good way to proceed?
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.
there is no need to spin a docker container explicitly on the CI. I can suggest using testcontainers to automatically spin a container during the lifetime of the test. (here is an example using postgres:
Line 154 in 95a52cc
postgres = new PostgreSQLContainer("postgres:11.1") |
5ede329
to
16c3197
Compare
What Does This Do
Add full APM/DBM mode for SQL Server.
Motivation
A large demand.
Additional Notes
Jira ticket: [PROJ-IDENT]