-
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
Add Hikari Pool Name tag #7672
Add Hikari Pool Name tag #7672
Conversation
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 55 metrics, 8 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.41.0-SNAPSHOT~08ce028e57, baseline=1.41.0-SNAPSHOT~d6c734ceba
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.067 s) : 0, 1066657
Total [baseline] (8.546 s) : 0, 8545532
Agent [candidate] (1.075 s) : 0, 1075007
Total [candidate] (8.55 s) : 0, 8550494
section iast
Agent [baseline] (1.196 s) : 0, 1196096
Total [baseline] (9.06 s) : 0, 9059685
Agent [candidate] (1.204 s) : 0, 1204008
Total [candidate] (9.084 s) : 0, 9083858
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.194 s) : 0, 1194189
Total [baseline] (9.074 s) : 0, 9073855
Agent [candidate] (1.197 s) : 0, 1197268
Total [candidate] (9.093 s) : 0, 9092608
section iast_TELEMETRY_OFF
Agent [baseline] (1.191 s) : 0, 1190898
Total [baseline] (9.061 s) : 0, 9060694
Agent [candidate] (1.199 s) : 0, 1198653
Total [candidate] (9.066 s) : 0, 9065901
gantt
title insecure-bank - break down per module: candidate=1.41.0-SNAPSHOT~08ce028e57, baseline=1.41.0-SNAPSHOT~d6c734ceba
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (681.339 ms) : 0, 681339
BytebuddyAgent [candidate] (686.668 ms) : 0, 686668
GlobalTracer [baseline] (309.765 ms) : 0, 309765
GlobalTracer [candidate] (312.197 ms) : 0, 312197
AppSec [baseline] (53.815 ms) : 0, 53815
AppSec [candidate] (54.199 ms) : 0, 54199
Remote Config [baseline] (656.017 µs) : 0, 656
Remote Config [candidate] (665.657 µs) : 0, 666
Telemetry [baseline] (7.476 ms) : 0, 7476
Telemetry [candidate] (7.544 ms) : 0, 7544
section iast
BytebuddyAgent [baseline] (797.384 ms) : 0, 797384
BytebuddyAgent [candidate] (803.608 ms) : 0, 803608
GlobalTracer [baseline] (299.127 ms) : 0, 299127
GlobalTracer [candidate] (300.521 ms) : 0, 300521
AppSec [baseline] (55.304 ms) : 0, 55304
AppSec [candidate] (57.163 ms) : 0, 57163
IAST [baseline] (22.957 ms) : 0, 22957
IAST [candidate] (21.256 ms) : 0, 21256
Remote Config [baseline] (603.682 µs) : 0, 604
Remote Config [candidate] (601.335 µs) : 0, 601
Telemetry [baseline] (7.029 ms) : 0, 7029
Telemetry [candidate] (7.088 ms) : 0, 7088
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (795.953 ms) : 0, 795953
BytebuddyAgent [candidate] (797.885 ms) : 0, 797885
GlobalTracer [baseline] (298.412 ms) : 0, 298412
GlobalTracer [candidate] (299.472 ms) : 0, 299472
AppSec [baseline] (56.951 ms) : 0, 56951
AppSec [candidate] (53.528 ms) : 0, 53528
IAST [baseline] (21.422 ms) : 0, 21422
IAST [candidate] (25.051 ms) : 0, 25051
Remote Config [baseline] (618.9 µs) : 0, 619
Remote Config [candidate] (604.393 µs) : 0, 604
Telemetry [baseline] (7.196 ms) : 0, 7196
Telemetry [candidate] (7.015 ms) : 0, 7015
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (793.361 ms) : 0, 793361
BytebuddyAgent [candidate] (798.688 ms) : 0, 798688
GlobalTracer [baseline] (298.466 ms) : 0, 298466
GlobalTracer [candidate] (300.458 ms) : 0, 300458
AppSec [baseline] (54.582 ms) : 0, 54582
AppSec [candidate] (55.475 ms) : 0, 55475
IAST [baseline] (23.315 ms) : 0, 23315
IAST [candidate] (22.647 ms) : 0, 22647
Remote Config [baseline] (626.846 µs) : 0, 627
Remote Config [candidate] (613.744 µs) : 0, 614
Telemetry [baseline] (6.903 ms) : 0, 6903
Telemetry [candidate] (7.016 ms) : 0, 7016
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.41.0-SNAPSHOT~08ce028e57, baseline=1.41.0-SNAPSHOT~d6c734ceba
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.063 s) : 0, 1063458
Total [baseline] (10.382 s) : 0, 10382256
Agent [candidate] (1.067 s) : 0, 1066628
Total [candidate] (10.402 s) : 0, 10402279
section appsec
Agent [baseline] (1.199 s) : 0, 1198787
Total [baseline] (10.563 s) : 0, 10563111
Agent [candidate] (1.203 s) : 0, 1202546
Total [candidate] (10.58 s) : 0, 10580211
section iast
Agent [baseline] (1.194 s) : 0, 1194091
Total [baseline] (10.858 s) : 0, 10857630
Agent [candidate] (1.196 s) : 0, 1196236
Total [candidate] (10.808 s) : 0, 10808186
section profiling
Agent [baseline] (1.262 s) : 0, 1262461
Total [baseline] (10.579 s) : 0, 10578987
Agent [candidate] (1.267 s) : 0, 1266558
Total [candidate] (10.592 s) : 0, 10592276
gantt
title petclinic - break down per module: candidate=1.41.0-SNAPSHOT~08ce028e57, baseline=1.41.0-SNAPSHOT~d6c734ceba
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (679.205 ms) : 0, 679205
BytebuddyAgent [candidate] (681.089 ms) : 0, 681089
GlobalTracer [baseline] (308.799 ms) : 0, 308799
GlobalTracer [candidate] (310.039 ms) : 0, 310039
AppSec [baseline] (53.727 ms) : 0, 53727
AppSec [candidate] (53.701 ms) : 0, 53701
Remote Config [baseline] (660.866 µs) : 0, 661
Remote Config [candidate] (670.903 µs) : 0, 671
Telemetry [baseline] (7.488 ms) : 0, 7488
Telemetry [candidate] (7.513 ms) : 0, 7513
section appsec
BytebuddyAgent [baseline] (697.245 ms) : 0, 697245
BytebuddyAgent [candidate] (698.427 ms) : 0, 698427
GlobalTracer [baseline] (305.99 ms) : 0, 305990
GlobalTracer [candidate] (307.638 ms) : 0, 307638
AppSec [baseline] (161.816 ms) : 0, 161816
AppSec [candidate] (162.576 ms) : 0, 162576
Remote Config [baseline] (641.899 µs) : 0, 642
Remote Config [candidate] (631.989 µs) : 0, 632
Telemetry [baseline] (9.492 ms) : 0, 9492
Telemetry [candidate] (9.93 ms) : 0, 9930
IAST [baseline] (20.011 ms) : 0, 20011
IAST [candidate] (19.432 ms) : 0, 19432
section iast
BytebuddyAgent [baseline] (796.196 ms) : 0, 796196
BytebuddyAgent [candidate] (797.423 ms) : 0, 797423
GlobalTracer [baseline] (298.347 ms) : 0, 298347
GlobalTracer [candidate] (299.082 ms) : 0, 299082
AppSec [baseline] (55.534 ms) : 0, 55534
AppSec [candidate] (52.041 ms) : 0, 52041
Remote Config [baseline] (600.573 µs) : 0, 601
Remote Config [candidate] (593.89 µs) : 0, 594
Telemetry [baseline] (7.025 ms) : 0, 7025
Telemetry [candidate] (6.938 ms) : 0, 6938
IAST [baseline] (22.747 ms) : 0, 22747
IAST [candidate] (26.51 ms) : 0, 26510
section profiling
ProfilingAgent [baseline] (95.486 ms) : 0, 95486
ProfilingAgent [candidate] (96.781 ms) : 0, 96781
BytebuddyAgent [baseline] (674.298 ms) : 0, 674298
BytebuddyAgent [candidate] (675.491 ms) : 0, 675491
GlobalTracer [baseline] (391.836 ms) : 0, 391836
GlobalTracer [candidate] (393.169 ms) : 0, 393169
AppSec [baseline] (54.4 ms) : 0, 54400
AppSec [candidate] (54.428 ms) : 0, 54428
Remote Config [baseline] (650.649 µs) : 0, 651
Remote Config [candidate] (649.339 µs) : 0, 649
Telemetry [baseline] (7.367 ms) : 0, 7367
Telemetry [candidate] (7.421 ms) : 0, 7421
Profiling [baseline] (95.51 ms) : 0, 95510
Profiling [candidate] (96.805 ms) : 0, 96805
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 petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.41.0-SNAPSHOT~08ce028e57, baseline=1.41.0-SNAPSHOT~d6c734ceba
dateFormat X
axisFormat %s
section baseline
no_agent (1.347 ms) : 1327, 1366
. : milestone, 1347,
appsec (1.728 ms) : 1705, 1750
. : milestone, 1728,
appsec_no_iast (1.721 ms) : 1697, 1746
. : milestone, 1721,
iast (1.489 ms) : 1466, 1512
. : milestone, 1489,
profiling (1.494 ms) : 1469, 1518
. : milestone, 1494,
tracing (1.462 ms) : 1438, 1487
. : milestone, 1462,
section candidate
no_agent (1.351 ms) : 1332, 1370
. : milestone, 1351,
appsec (1.738 ms) : 1714, 1761
. : milestone, 1738,
appsec_no_iast (1.722 ms) : 1697, 1747
. : milestone, 1722,
iast (1.472 ms) : 1448, 1495
. : milestone, 1472,
profiling (1.492 ms) : 1470, 1515
. : milestone, 1492,
tracing (1.49 ms) : 1466, 1514
. : milestone, 1490,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.41.0-SNAPSHOT~08ce028e57, baseline=1.41.0-SNAPSHOT~d6c734ceba
dateFormat X
axisFormat %s
section baseline
no_agent (378.075 µs) : 358, 398
. : milestone, 378,
iast (491.855 µs) : 470, 513
. : milestone, 492,
iast_FULL (563.493 µs) : 542, 585
. : milestone, 563,
iast_GLOBAL (515.878 µs) : 493, 538
. : milestone, 516,
iast_HARDCODED_SECRET_DISABLED (495.573 µs) : 474, 517
. : milestone, 496,
iast_INACTIVE (453.695 µs) : 433, 475
. : milestone, 454,
iast_TELEMETRY_OFF (484.557 µs) : 463, 506
. : milestone, 485,
tracing (450.899 µs) : 430, 472
. : milestone, 451,
section candidate
no_agent (378.786 µs) : 359, 398
. : milestone, 379,
iast (484.395 µs) : 463, 506
. : milestone, 484,
iast_FULL (556.743 µs) : 535, 578
. : milestone, 557,
iast_GLOBAL (506.342 µs) : 485, 527
. : milestone, 506,
iast_HARDCODED_SECRET_DISABLED (491.812 µs) : 470, 514
. : milestone, 492,
iast_INACTIVE (454.183 µs) : 433, 476
. : milestone, 454,
iast_TELEMETRY_OFF (475.971 µs) : 455, 497
. : milestone, 476,
tracing (446.713 µs) : 426, 467
. : milestone, 447,
Dacapo |
mistake ping, messed up rebase 😅 |
Debugger benchmarksParameters
See matching parameters
SummaryFound 0 performance improvements and 5 performance regressions! Performance is the same for 0 metrics, 10 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 (329.678 µs) : 242, 417
. : milestone, 330,
basic (308.972 µs) : 272, 346
. : milestone, 309,
loop (10.359 ms) : 10286, 10431
. : milestone, 10359,
section candidate
noprobe (302.813 µs) : 260, 345
. : milestone, 303,
basic (298.203 µs) : 289, 307
. : milestone, 298,
loop (10.956 ms) : 10927, 10985
. : milestone, 10956,
|
Kafka / producer-benchmarkParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 3 metrics, 0 unstable metrics. See unchanged results
|
Kafka / consumer-benchmarkParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 3 metrics, 0 unstable metrics. See unchanged results
|
public static class HikariGetConnectionAdvice { | ||
@Advice.OnMethodEnter(suppress = Throwable.class) | ||
public static void start(@Advice.This final HikariDataSource ds) { | ||
if (activeSpan() == 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 won't work because it relies on the DatasourceInstrumentation
that's disabled by default. The test passes because injectSysConfig("dd.integration.jdbc-datasource.enabled", "true")
Also there are two advices kickin in on the same method enter and here we assume an execution order.
Also, in order to do a split-by-tag on the jdbc.query
spans, you should add this tag on the statement span and not on the connection span.
An approach might be here to store that pool information on the DBInfo
state that's stored when the connection is created. In fact DriverInstrumentation
already link java.sql.Connection
to DbInfo
. So here you can use the InstrumentationContext
to retrieve that dbInfo value and enrich it with the hikari pool. Then, you shall ensure that that the tag is added when the statement spans are created
…and JDBCDecorator tag setting
...agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/DBInfo.java
Outdated
Show resolved
Hide resolved
internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/Tags.java
Outdated
Show resolved
Hide resolved
} | ||
|
||
String hikariPoolname = ds.getPoolName(); | ||
DBInfo dbInfo = InstrumentationContext.get(Connection.class, DBInfo.class).get(unwrapped); |
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.
nit: check for null before using get()
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. Hikari 6 is not yet tested hence the lockfile should be bumped before merging
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
bb275ac
to
08ce028
Compare
What Does This Do
Adds instrumentation for Hikari Pool Name. If using a Hikari pool, adds a
db.pool.name
tag to span.Motivation
Helps with issue: #7152
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: [PROJ-IDENT]