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.

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

Fix for MySQL multi-host internally_generated connection urls #6853

Merged
merged 1 commit into from Mar 28, 2024

Conversation

ygree
Copy link
Contributor

@ygree ygree commented Mar 27, 2024

What Does This Do

Fixes a reported problem when the instrumentation failed to parse MySQL multi-host connection URLs such as "jdbc:mysql:loadbalance://**internally_generated**1711497942886**"

Uses an original connection URL to extract database information and attaches it to the underlying connection object so that it can be properly propagated and used with and without the split-by-instance setting.

Motivation

Provide a fix for a reported bug to properly extract and store database connection information from the MySQL multi-host connection URL and prevent a parsing exception.

Additional Notes

Jira ticket: APMS-11717

@ygree ygree added type: bug inst: jdbc JDBC instrumentation labels Mar 27, 2024
@ygree ygree self-assigned this Mar 27, 2024
@ygree ygree requested a review from a team as a code owner March 27, 2024 23:08
@ygree ygree requested review from mcculls and am312 March 27, 2024 23:08
@pr-commenter
Copy link

pr-commenter bot commented Mar 27, 2024

Benchmarks

Startup

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master ygree/fix-mysql-multi-host-connection
git_commit_date 1711573590 1711586440
git_commit_sha 722243d f987356
release_version 1.32.0-SNAPSHOT~722243d370 1.31.0-SNAPSHOT~f987356cbd
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1711589866 1711589866
ci_job_id 471679363 471679363
ci_pipeline_id 30984553 30984553
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
module Agent Agent
parent None None
variant iast iast

Summary

Found 0 performance improvements and 7 performance regressions! Performance is the same for 36 metrics, 11 unstable metrics.

scenario Δ mean execution_time candidate mean execution_time baseline mean execution_time
scenario:startup:insecure-bank:iast:Remote Config worse
[+15.109µs; +69.107µs] or [+2.629%; +12.027%]
616.727µs 574.619µs
scenario:startup:insecure-bank:iast_TELEMETRY_OFF:Remote Config worse
[+26.071µs; +54.228µs] or [+4.564%; +9.494%]
611.349µs 571.199µs
scenario:startup:insecure-bank:tracing:AppSec worse
[+1.494ms; +1.952ms] or [+3.021%; +3.948%]
51.167ms 49.444ms
scenario:startup:insecure-bank:tracing:Remote Config worse
[+27.424µs; +60.821µs] or [+4.203%; +9.321%]
696.612µs 652.489µs
scenario:startup:petclinic:profiling:AppSec worse
[+1.724ms; +3.135ms] or [+3.422%; +6.222%]
52.808ms 50.379ms
scenario:startup:petclinic:tracing:AppSec worse
[+1.390ms; +2.491ms] or [+2.819%; +5.049%]
51.265ms 49.325ms
scenario:startup:petclinic:tracing:Remote Config worse
[+40.269µs; +71.848µs] or [+6.104%; +10.892%]
715.726µs 659.668µs
Startup time reports for petclinic
gantt
    title petclinic - global startup overhead: candidate=1.31.0-SNAPSHOT~f987356cbd, baseline=1.32.0-SNAPSHOT~722243d370

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.075 s) : 0, 1074850
Total [baseline] (10.41 s) : 0, 10409545
Agent [candidate] (1.074 s) : 0, 1074261
Total [candidate] (10.475 s) : 0, 10475277
section appsec
Agent [baseline] (1.199 s) : 0, 1198795
Total [baseline] (10.591 s) : 0, 10591170
Agent [candidate] (1.176 s) : 0, 1175750
Total [candidate] (10.446 s) : 0, 10445670
section iast
Agent [baseline] (1.198 s) : 0, 1198137
Total [baseline] (10.791 s) : 0, 10791422
Agent [candidate] (1.2 s) : 0, 1200164
Total [candidate] (10.806 s) : 0, 10806279
section profiling
Agent [baseline] (1.275 s) : 0, 1274521
Total [baseline] (10.638 s) : 0, 10638161
Agent [candidate] (1.276 s) : 0, 1275591
Total [candidate] (10.636 s) : 0, 10636146
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.075 s -
Agent appsec 1.199 s 123.944 ms (11.5%)
Agent iast 1.198 s 123.286 ms (11.5%)
Agent profiling 1.275 s 199.671 ms (18.6%)
Total tracing 10.41 s -
Total appsec 10.591 s 181.625 ms (1.7%)
Total iast 10.791 s 381.876 ms (3.7%)
Total profiling 10.638 s 228.615 ms (2.2%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.074 s -
Agent appsec 1.176 s 101.489 ms (9.4%)
Agent iast 1.2 s 125.904 ms (11.7%)
Agent profiling 1.276 s 201.331 ms (18.7%)
Total tracing 10.475 s -
Total appsec 10.446 s -29.606 ms (-0.3%)
Total iast 10.806 s 331.003 ms (3.2%)
Total profiling 10.636 s 160.87 ms (1.5%)
gantt
    title petclinic - break down per module: candidate=1.31.0-SNAPSHOT~f987356cbd, baseline=1.32.0-SNAPSHOT~722243d370

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (685.603 ms) : 0, 685603
BytebuddyAgent [candidate] (682.806 ms) : 0, 682806
GlobalTracer [baseline] (297.215 ms) : 0, 297215
GlobalTracer [candidate] (297.503 ms) : 0, 297503
AppSec [baseline] (49.325 ms) : 0, 49325
AppSec [candidate] (51.265 ms) : 0, 51265
Remote Config [baseline] (659.668 µs) : 0, 660
Remote Config [candidate] (715.726 µs) : 0, 716
Telemetry [baseline] (7.524 ms) : 0, 7524
Telemetry [candidate] (7.757 ms) : 0, 7757
section appsec
BytebuddyAgent [baseline] (696.754 ms) : 0, 696754
BytebuddyAgent [candidate] (684.773 ms) : 0, 684773
GlobalTracer [baseline] (291.48 ms) : 0, 291480
GlobalTracer [candidate] (297.902 ms) : 0, 297902
AppSec [baseline] (149.795 ms) : 0, 149795
AppSec [candidate] (151.202 ms) : 0, 151202
Remote Config [baseline] (611.834 µs) : 0, 612
Remote Config [candidate] (634.791 µs) : 0, 635
Telemetry [baseline] (6.819 ms) : 0, 6819
Telemetry [candidate] (6.913 ms) : 0, 6913
IAST [baseline] (18.899 ms) : 0, 18899
section iast
BytebuddyAgent [baseline] (794.498 ms) : 0, 794498
BytebuddyAgent [candidate] (793.418 ms) : 0, 793418
GlobalTracer [baseline] (287.89 ms) : 0, 287890
GlobalTracer [candidate] (288.948 ms) : 0, 288948
AppSec [baseline] (49.975 ms) : 0, 49975
AppSec [candidate] (55.823 ms) : 0, 55823
Remote Config [baseline] (570.326 µs) : 0, 570
Remote Config [candidate] (606.482 µs) : 0, 606
Telemetry [baseline] (7.319 ms) : 0, 7319
Telemetry [candidate] (8.103 ms) : 0, 8103
IAST [baseline] (23.569 ms) : 0, 23569
IAST [candidate] (18.914 ms) : 0, 18914
section profiling
BytebuddyAgent [baseline] (681.586 ms) : 0, 681586
BytebuddyAgent [candidate] (678.696 ms) : 0, 678696
GlobalTracer [baseline] (381.884 ms) : 0, 381884
GlobalTracer [candidate] (383.254 ms) : 0, 383254
AppSec [baseline] (50.379 ms) : 0, 50379
AppSec [candidate] (52.808 ms) : 0, 52808
Remote Config [baseline] (708.289 µs) : 0, 708
Remote Config [candidate] (758.053 µs) : 0, 758
Telemetry [baseline] (7.488 ms) : 0, 7488
Telemetry [candidate] (9.707 ms) : 0, 9707
ProfilingAgent [baseline] (95.819 ms) : 0, 95819
ProfilingAgent [candidate] (94.081 ms) : 0, 94081
Profiling [baseline] (95.842 ms) : 0, 95842
Profiling [candidate] (94.104 ms) : 0, 94104

Load

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
end_time 2024-03-28T01:13:09 2024-03-28T01:32:41
git_branch master ygree/fix-mysql-multi-host-connection
git_commit_date 1711573590 1711586440
git_commit_sha 722243d f987356
release_version 1.32.0-SNAPSHOT~722243d370 1.31.0-SNAPSHOT~f987356cbd
start_time 2024-03-28T01:12:56 2024-03-28T01:32:28
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1711589866 1711589866
ci_job_id 471679363 471679363
ci_pipeline_id 30984553 30984553
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
variant iast iast

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 14 unstable metrics.

Request duration reports for petclinic
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.31.0-SNAPSHOT~f987356cbd, baseline=1.32.0-SNAPSHOT~722243d370
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.347 ms) : 1328, 1366
.   : milestone, 1347,
appsec (1.705 ms) : 1680, 1730
.   : milestone, 1705,
iast (1.496 ms) : 1473, 1519
.   : milestone, 1496,
profiling (1.505 ms) : 1480, 1531
.   : milestone, 1505,
tracing (1.483 ms) : 1460, 1507
.   : milestone, 1483,
section candidate
no_agent (1.334 ms) : 1315, 1353
.   : milestone, 1334,
appsec (1.736 ms) : 1711, 1761
.   : milestone, 1736,
iast (1.508 ms) : 1485, 1532
.   : milestone, 1508,
profiling (1.497 ms) : 1472, 1521
.   : milestone, 1497,
tracing (1.5 ms) : 1476, 1524
.   : milestone, 1500,
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.347 ms [1.328 ms, 1.366 ms] -
appsec 1.705 ms [1.68 ms, 1.73 ms] 357.481 µs (26.5%)
iast 1.496 ms [1.473 ms, 1.519 ms] 148.747 µs (11.0%)
profiling 1.505 ms [1.48 ms, 1.531 ms] 158.069 µs (11.7%)
tracing 1.483 ms [1.46 ms, 1.507 ms] 135.815 µs (10.1%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.334 ms [1.315 ms, 1.353 ms] -
appsec 1.736 ms [1.711 ms, 1.761 ms] 401.821 µs (30.1%)
iast 1.508 ms [1.485 ms, 1.532 ms] 174.103 µs (13.0%)
profiling 1.497 ms [1.472 ms, 1.521 ms] 162.676 µs (12.2%)
tracing 1.5 ms [1.476 ms, 1.524 ms] 165.506 µs (12.4%)
Request duration reports for insecure-bank
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.31.0-SNAPSHOT~f987356cbd, baseline=1.32.0-SNAPSHOT~722243d370
    dateFormat X
    axisFormat %s
section baseline
no_agent (365.777 µs) : 346, 386
.   : milestone, 366,
iast (474.379 µs) : 454, 495
.   : milestone, 474,
iast_FULL (543.228 µs) : 522, 564
.   : milestone, 543,
iast_GLOBAL (498.843 µs) : 478, 520
.   : milestone, 499,
iast_HARDCODED_SECRET_DISABLED (477.938 µs) : 457, 498
.   : milestone, 478,
iast_INACTIVE (454.18 µs) : 433, 475
.   : milestone, 454,
iast_TELEMETRY_OFF (473.197 µs) : 452, 494
.   : milestone, 473,
tracing (446.163 µs) : 425, 467
.   : milestone, 446,
section candidate
no_agent (363.855 µs) : 344, 384
.   : milestone, 364,
iast (482.941 µs) : 462, 504
.   : milestone, 483,
iast_FULL (534.09 µs) : 513, 555
.   : milestone, 534,
iast_GLOBAL (497.384 µs) : 476, 519
.   : milestone, 497,
iast_HARDCODED_SECRET_DISABLED (476.246 µs) : 455, 497
.   : milestone, 476,
iast_INACTIVE (447.899 µs) : 427, 469
.   : milestone, 448,
iast_TELEMETRY_OFF (474.719 µs) : 453, 496
.   : milestone, 475,
tracing (444.203 µs) : 423, 465
.   : milestone, 444,
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 365.777 µs [345.892 µs, 385.662 µs] -
iast 474.379 µs [453.825 µs, 494.933 µs] 108.602 µs (29.7%)
iast_FULL 543.228 µs [522.44 µs, 564.017 µs] 177.452 µs (48.5%)
iast_GLOBAL 498.843 µs [478.086 µs, 519.601 µs] 133.067 µs (36.4%)
iast_HARDCODED_SECRET_DISABLED 477.938 µs [457.434 µs, 498.442 µs] 112.161 µs (30.7%)
iast_INACTIVE 454.18 µs [433.247 µs, 475.113 µs] 88.403 µs (24.2%)
iast_TELEMETRY_OFF 473.197 µs [452.33 µs, 494.064 µs] 107.42 µs (29.4%)
tracing 446.163 µs [425.491 µs, 466.836 µs] 80.387 µs (22.0%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 363.855 µs [343.822 µs, 383.888 µs] -
iast 482.941 µs [461.863 µs, 504.019 µs] 119.086 µs (32.7%)
iast_FULL 534.09 µs [513.46 µs, 554.72 µs] 170.235 µs (46.8%)
iast_GLOBAL 497.384 µs [476.173 µs, 518.596 µs] 133.53 µs (36.7%)
iast_HARDCODED_SECRET_DISABLED 476.246 µs [455.154 µs, 497.338 µs] 112.391 µs (30.9%)
iast_INACTIVE 447.899 µs [427.031 µs, 468.766 µs] 84.044 µs (23.1%)
iast_TELEMETRY_OFF 474.719 µs [453.469 µs, 495.968 µs] 110.864 µs (30.5%)
tracing 444.203 µs [423.495 µs, 464.91 µs] 80.348 µs (22.1%)

@ygree ygree force-pushed the ygree/fix-mysql-multi-host-connection branch from d804bcf to f987356 Compare March 28, 2024 00:41
@am312
Copy link
Contributor

am312 commented Mar 28, 2024

This might be good to eventually include in a broader solution covering the variety of unsupported connection URLs I discussed here:
https://docs.google.com/document/d/1m5unhxc4GWwcTXr1za1pPoOQexSlTyWZrXzOqcwWho0/edit?usp=sharing

@@ -824,7 +830,7 @@ public static DBInfo parse(String connectionUrl, final Properties props) {
}
return GENERIC_URL_LIKE.doParse(connectionUrl, parsedProps).build();
} catch (final Exception e) {
ExceptionLogger.LOGGER.debug("Error parsing URL", e);
ExceptionLogger.LOGGER.debug("Error parsing URL {}", jdbcUrl, e);
Copy link
Contributor

Choose a reason for hiding this comment

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

The full URL could contain sensitive details - can we merge the fix first and discuss this logging change separately?

Copy link
Contributor

@mcculls mcculls left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd leave the logging change to a separate PR/discussion

@ygree
Copy link
Contributor Author

ygree commented Mar 28, 2024

This might be good to eventually include in a broader solution covering the variety of unsupported connection URLs I discussed here: https://docs.google.com/document/d/1m5unhxc4GWwcTXr1za1pPoOQexSlTyWZrXzOqcwWho0/edit?usp=sharing

This change doesn't seem to be relevant to the broader initiative, since it's not about supporting a different connection URL, but about properly handling connection proxy objects.

@ygree ygree force-pushed the ygree/fix-mysql-multi-host-connection branch from f987356 to 0613d39 Compare March 28, 2024 16:45
@ygree ygree force-pushed the ygree/fix-mysql-multi-host-connection branch from 0613d39 to 283ffad Compare March 28, 2024 16:47
@ygree ygree enabled auto-merge (squash) March 28, 2024 17:21
@ygree ygree merged commit 56c42ad into master Mar 28, 2024
76 of 77 checks passed
@ygree ygree deleted the ygree/fix-mysql-multi-host-connection branch March 28, 2024 17:24
@github-actions github-actions bot added this to the 1.32.0 milestone Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inst: jdbc JDBC instrumentation type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants