Conversation
EntryIterator and EntryChangeIterator are arguably redundant
Fixed bug TagValueConversions.toBoolean Could cause LegacyTagMap.EntryReader to produce incorrect answers to some queries For simplicity, now treating Byte and Short as Integer. That will make calling code doing primitive handling simpler. Fleshing out tests -- more tests to come
Coverage for byte, short, float, and double
Adding methods to AgentSpan / DDSpan that take TagMap.Entry/Reader objects directly This will enable TagMap.Entry reuse which can reduce memory allocation/GC pressure
Methods are intended to be used to create TagMap.Entry objects for repeatedly used values Overloads are provided for all the supported types to be easier for developers not familiar with TagMap internals. Internally, TagMap still uses the more explicit new<X>Entry methods.
Removing statics that were previously moved to TagValueConversions
- tests exposed missing TagMap.Entry.create for boolean - added explanatory strings to some asserts
…aDog/dd-trace-java into dougqh/fdirect-apis-for-tagmap-entry
| return delegate.setTag(key, value) | ||
| } | ||
|
|
||
| @Override |
There was a problem hiding this comment.
Most of these changes are from #10472 which I plan to merge first
See GitInfo and GitMetadataTraceInterceptor for the "real" changes here
| this.tag = tag; | ||
| this.commit = commit; | ||
|
|
||
| // GitInfo is reused across many traces, so create entries once and reuse them (see addTags) |
There was a problem hiding this comment.
Despite the rest of this class being coded defensively for null - as far as I could tell from GitMetadataTraceInterceptor, repositoryURL and commit.getSha() are never null.
That said, I'm happy to add defensive here and in addTags if so desired.
There was a problem hiding this comment.
In Test Optimization we've seen some edge cases in which commit.getSha() could be null (seems much more unlikely for repositoryURL). We usually have to point them to set the git info through environment variables in those cases. Will the TagMap.Entry.create call have issues with a null object?
There was a problem hiding this comment.
Good to know.
No, TagMap.Entry.create doesn't support having a null value.
In AgentSpan / DDSpan setting a null tag is translated into a removal.
TagMap does have EntryRemoval for that purpose, but EntryRemoval-s are never stored in TagMap directly just in TagMap.Ledger-s.
I think the solution is just to guard against the null-s. I just wanted to clarify whether or not null-s were expected in the Git related code.
There was a problem hiding this comment.
In another PR, I've tweaked TagMap.Entry.create to allow null & empty String to return a null TagMap.Entry.
And now, AgentSpan / DDSpan ignore being called with a null TagMap.Entry.
So that now allows null to be handled simply in the calling code, I'll update this PR once the other one is merged.
There was a problem hiding this comment.
Done. Thanks for the review. You were right that commit sha can be null.
| return commit; | ||
| } | ||
|
|
||
| public void addTags(AgentSpan span) { |
There was a problem hiding this comment.
I think eventually I'd like introduce an interface for objects that add tags to span or TagMap.
Since this is the first change of this sort, I decided to share this as is to solicit thoughts on the matter.
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 63 metrics, 8 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.59.0-SNAPSHOT~b5701ef310, baseline=1.60.0-SNAPSHOT~13b60019b9
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.066 s) : 0, 1066349
Total [baseline] (10.807 s) : 0, 10806680
Agent [candidate] (1.065 s) : 0, 1064982
Total [candidate] (10.857 s) : 0, 10856856
section appsec
Agent [baseline] (1.239 s) : 0, 1238742
Total [baseline] (10.955 s) : 0, 10955189
Agent [candidate] (1.24 s) : 0, 1239543
Total [candidate] (11.022 s) : 0, 11021608
section iast
Agent [baseline] (1.233 s) : 0, 1233270
Total [baseline] (11.202 s) : 0, 11202335
Agent [candidate] (1.234 s) : 0, 1233988
Total [candidate] (11.231 s) : 0, 11230915
section profiling
Agent [baseline] (1.19 s) : 0, 1190397
Total [baseline] (11.101 s) : 0, 11101008
Agent [candidate] (1.192 s) : 0, 1192029
Total [candidate] (10.972 s) : 0, 10972133
gantt
title petclinic - break down per module: candidate=1.59.0-SNAPSHOT~b5701ef310, baseline=1.60.0-SNAPSHOT~13b60019b9
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.181 ms) : 0, 1181
crashtracking [candidate] (1.178 ms) : 0, 1178
BytebuddyAgent [baseline] (628.651 ms) : 0, 628651
BytebuddyAgent [candidate] (629.047 ms) : 0, 629047
AgentMeter [baseline] (28.877 ms) : 0, 28877
AgentMeter [candidate] (28.907 ms) : 0, 28907
GlobalTracer [baseline] (257.987 ms) : 0, 257987
GlobalTracer [candidate] (257.989 ms) : 0, 257989
AppSec [baseline] (32.922 ms) : 0, 32922
AppSec [candidate] (32.857 ms) : 0, 32857
Debugger [baseline] (63.42 ms) : 0, 63420
Debugger [candidate] (61.354 ms) : 0, 61354
Remote Config [baseline] (605.059 µs) : 0, 605
Remote Config [candidate] (630.092 µs) : 0, 630
Telemetry [baseline] (12.154 ms) : 0, 12154
Telemetry [candidate] (13.135 ms) : 0, 13135
Flare Poller [baseline] (5.383 ms) : 0, 5383
Flare Poller [candidate] (4.574 ms) : 0, 4574
section appsec
crashtracking [baseline] (1.178 ms) : 0, 1178
crashtracking [candidate] (1.178 ms) : 0, 1178
BytebuddyAgent [baseline] (657.661 ms) : 0, 657661
BytebuddyAgent [candidate] (658.921 ms) : 0, 658921
AgentMeter [baseline] (11.857 ms) : 0, 11857
AgentMeter [candidate] (11.931 ms) : 0, 11931
GlobalTracer [baseline] (258.66 ms) : 0, 258660
GlobalTracer [candidate] (259.053 ms) : 0, 259053
IAST [baseline] (25.365 ms) : 0, 25365
IAST [candidate] (25.141 ms) : 0, 25141
AppSec [baseline] (167.857 ms) : 0, 167857
AppSec [candidate] (167.252 ms) : 0, 167252
Debugger [baseline] (67.358 ms) : 0, 67358
Debugger [candidate] (67.409 ms) : 0, 67409
Remote Config [baseline] (686.547 µs) : 0, 687
Remote Config [candidate] (654.946 µs) : 0, 655
Telemetry [baseline] (9.174 ms) : 0, 9174
Telemetry [candidate] (9.118 ms) : 0, 9118
Flare Poller [baseline] (3.594 ms) : 0, 3594
Flare Poller [candidate] (3.637 ms) : 0, 3637
section iast
crashtracking [baseline] (1.176 ms) : 0, 1176
crashtracking [candidate] (1.176 ms) : 0, 1176
BytebuddyAgent [baseline] (796.942 ms) : 0, 796942
BytebuddyAgent [candidate] (797.517 ms) : 0, 797517
AgentMeter [baseline] (11.276 ms) : 0, 11276
AgentMeter [candidate] (11.285 ms) : 0, 11285
GlobalTracer [baseline] (247.941 ms) : 0, 247941
GlobalTracer [candidate] (248.913 ms) : 0, 248913
IAST [baseline] (27.054 ms) : 0, 27054
IAST [candidate] (27.042 ms) : 0, 27042
AppSec [baseline] (32.692 ms) : 0, 32692
AppSec [candidate] (34.657 ms) : 0, 34657
Debugger [baseline] (68.364 ms) : 0, 68364
Debugger [candidate] (65.566 ms) : 0, 65566
Remote Config [baseline] (536.875 µs) : 0, 537
Remote Config [candidate] (526.168 µs) : 0, 526
Telemetry [baseline] (8.682 ms) : 0, 8682
Telemetry [candidate] (8.579 ms) : 0, 8579
Flare Poller [baseline] (3.426 ms) : 0, 3426
Flare Poller [candidate] (3.421 ms) : 0, 3421
section profiling
crashtracking [baseline] (1.223 ms) : 0, 1223
crashtracking [candidate] (1.208 ms) : 0, 1208
BytebuddyAgent [baseline] (682.619 ms) : 0, 682619
BytebuddyAgent [candidate] (683.722 ms) : 0, 683722
AgentMeter [baseline] (8.742 ms) : 0, 8742
AgentMeter [candidate] (8.763 ms) : 0, 8763
GlobalTracer [baseline] (216.165 ms) : 0, 216165
GlobalTracer [candidate] (216.415 ms) : 0, 216415
AppSec [baseline] (32.291 ms) : 0, 32291
AppSec [candidate] (32.417 ms) : 0, 32417
Debugger [baseline] (67.354 ms) : 0, 67354
Debugger [candidate] (67.508 ms) : 0, 67508
Remote Config [baseline] (591.835 µs) : 0, 592
Remote Config [candidate] (601.241 µs) : 0, 601
Telemetry [baseline] (8.843 ms) : 0, 8843
Telemetry [candidate] (8.737 ms) : 0, 8737
Flare Poller [baseline] (3.734 ms) : 0, 3734
Flare Poller [candidate] (3.715 ms) : 0, 3715
ProfilingAgent [baseline] (98.923 ms) : 0, 98923
ProfilingAgent [candidate] (99.088 ms) : 0, 99088
Profiling [baseline] (99.489 ms) : 0, 99489
Profiling [candidate] (99.655 ms) : 0, 99655
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.59.0-SNAPSHOT~b5701ef310, baseline=1.60.0-SNAPSHOT~13b60019b9
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.072 s) : 0, 1071972
Total [baseline] (8.776 s) : 0, 8775870
Agent [candidate] (1.066 s) : 0, 1065563
Total [candidate] (8.74 s) : 0, 8740271
section iast
Agent [baseline] (1.231 s) : 0, 1231067
Total [baseline] (9.367 s) : 0, 9366512
Agent [candidate] (1.24 s) : 0, 1239965
Total [candidate] (9.422 s) : 0, 9421661
gantt
title insecure-bank - break down per module: candidate=1.59.0-SNAPSHOT~b5701ef310, baseline=1.60.0-SNAPSHOT~13b60019b9
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.195 ms) : 0, 1195
crashtracking [candidate] (1.173 ms) : 0, 1173
BytebuddyAgent [baseline] (633.73 ms) : 0, 633730
BytebuddyAgent [candidate] (628.758 ms) : 0, 628758
AgentMeter [baseline] (29.15 ms) : 0, 29150
AgentMeter [candidate] (29.157 ms) : 0, 29157
GlobalTracer [baseline] (259.941 ms) : 0, 259941
GlobalTracer [candidate] (258.79 ms) : 0, 258790
AppSec [baseline] (33.153 ms) : 0, 33153
AppSec [candidate] (32.96 ms) : 0, 32960
Debugger [baseline] (60.92 ms) : 0, 60920
Debugger [candidate] (59.608 ms) : 0, 59608
Remote Config [baseline] (622.519 µs) : 0, 623
Remote Config [candidate] (620.74 µs) : 0, 621
Telemetry [baseline] (12.343 ms) : 0, 12343
Telemetry [candidate] (13.879 ms) : 0, 13879
Flare Poller [baseline] (5.378 ms) : 0, 5378
Flare Poller [candidate] (5.38 ms) : 0, 5380
section iast
crashtracking [baseline] (1.184 ms) : 0, 1184
crashtracking [candidate] (1.195 ms) : 0, 1195
BytebuddyAgent [baseline] (795.887 ms) : 0, 795887
BytebuddyAgent [candidate] (802.307 ms) : 0, 802307
AgentMeter [baseline] (11.266 ms) : 0, 11266
AgentMeter [candidate] (11.327 ms) : 0, 11327
GlobalTracer [baseline] (247.822 ms) : 0, 247822
GlobalTracer [candidate] (249.272 ms) : 0, 249272
IAST [baseline] (27.06 ms) : 0, 27060
IAST [candidate] (27.374 ms) : 0, 27374
AppSec [baseline] (33.037 ms) : 0, 33037
AppSec [candidate] (32.293 ms) : 0, 32293
Debugger [baseline] (66.755 ms) : 0, 66755
Debugger [candidate] (68.092 ms) : 0, 68092
Remote Config [baseline] (545.635 µs) : 0, 546
Remote Config [candidate] (540.154 µs) : 0, 540
Telemetry [baseline] (8.766 ms) : 0, 8766
Telemetry [candidate] (8.726 ms) : 0, 8726
Flare Poller [baseline] (3.527 ms) : 0, 3527
Flare Poller [candidate] (3.49 ms) : 0, 3490
LoadParameters
See matching parameters
SummaryFound 5 performance improvements and 2 performance regressions! Performance is the same for 13 metrics, 16 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.59.0-SNAPSHOT~b5701ef310, baseline=1.60.0-SNAPSHOT~13b60019b9
dateFormat X
axisFormat %s
section baseline
no_agent (1.286 ms) : 1274, 1297
. : milestone, 1286,
iast (3.371 ms) : 3323, 3419
. : milestone, 3371,
iast_FULL (6.261 ms) : 6196, 6326
. : milestone, 6261,
iast_GLOBAL (3.625 ms) : 3564, 3686
. : milestone, 3625,
profiling (2.594 ms) : 2568, 2619
. : milestone, 2594,
tracing (1.899 ms) : 1884, 1915
. : milestone, 1899,
section candidate
no_agent (1.271 ms) : 1260, 1282
. : milestone, 1271,
iast (3.474 ms) : 3424, 3524
. : milestone, 3474,
iast_FULL (6.324 ms) : 6259, 6389
. : milestone, 6324,
iast_GLOBAL (3.734 ms) : 3670, 3799
. : milestone, 3734,
profiling (2.199 ms) : 2180, 2218
. : milestone, 2199,
tracing (1.917 ms) : 1901, 1933
. : milestone, 1917,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.59.0-SNAPSHOT~b5701ef310, baseline=1.60.0-SNAPSHOT~13b60019b9
dateFormat X
axisFormat %s
section baseline
no_agent (18.7 ms) : 18506, 18893
. : milestone, 18700,
appsec (19.018 ms) : 18821, 19216
. : milestone, 19018,
code_origins (19.214 ms) : 19014, 19414
. : milestone, 19214,
iast (18.243 ms) : 18057, 18429
. : milestone, 18243,
profiling (20.253 ms) : 20046, 20460
. : milestone, 20253,
tracing (19.283 ms) : 19086, 19479
. : milestone, 19283,
section candidate
no_agent (17.765 ms) : 17583, 17947
. : milestone, 17765,
appsec (19.129 ms) : 18932, 19326
. : milestone, 19129,
code_origins (18.043 ms) : 17865, 18222
. : milestone, 18043,
iast (18.345 ms) : 18161, 18529
. : milestone, 18345,
profiling (19.599 ms) : 19397, 19800
. : milestone, 19599,
tracing (18.228 ms) : 18047, 18409
. : milestone, 18228,
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.59.0-SNAPSHOT~b5701ef310, baseline=1.60.0-SNAPSHOT~13b60019b9
dateFormat X
axisFormat %s
section baseline
no_agent (1.474 ms) : 1463, 1486
. : milestone, 1474,
appsec (3.729 ms) : 3514, 3944
. : milestone, 3729,
iast (2.253 ms) : 2184, 2322
. : milestone, 2253,
iast_GLOBAL (2.307 ms) : 2237, 2377
. : milestone, 2307,
profiling (2.094 ms) : 2039, 2149
. : milestone, 2094,
tracing (2.064 ms) : 2010, 2117
. : milestone, 2064,
section candidate
no_agent (1.473 ms) : 1461, 1484
. : milestone, 1473,
appsec (3.763 ms) : 3543, 3984
. : milestone, 3763,
iast (2.258 ms) : 2188, 2327
. : milestone, 2258,
iast_GLOBAL (2.289 ms) : 2220, 2359
. : milestone, 2289,
profiling (2.084 ms) : 2029, 2139
. : milestone, 2084,
tracing (2.074 ms) : 2020, 2128
. : milestone, 2074,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.59.0-SNAPSHOT~b5701ef310, baseline=1.60.0-SNAPSHOT~13b60019b9
dateFormat X
axisFormat %s
section baseline
no_agent (15.601 s) : 15601000, 15601000
. : milestone, 15601000,
appsec (14.985 s) : 14985000, 14985000
. : milestone, 14985000,
iast (18.312 s) : 18312000, 18312000
. : milestone, 18312000,
iast_GLOBAL (17.808 s) : 17808000, 17808000
. : milestone, 17808000,
profiling (15.032 s) : 15032000, 15032000
. : milestone, 15032000,
tracing (14.831 s) : 14831000, 14831000
. : milestone, 14831000,
section candidate
no_agent (14.77 s) : 14770000, 14770000
. : milestone, 14770000,
appsec (15.017 s) : 15017000, 15017000
. : milestone, 15017000,
iast (18.166 s) : 18166000, 18166000
. : milestone, 18166000,
iast_GLOBAL (17.931 s) : 17931000, 17931000
. : milestone, 17931000,
profiling (14.93 s) : 14930000, 14930000
. : milestone, 14930000,
tracing (14.842 s) : 14842000, 14842000
. : milestone, 14842000,
|
| this.tag = tag; | ||
| this.commit = commit; | ||
|
|
||
| // GitInfo is reused across many traces, so create entries once and reuse them (see addTags) |
There was a problem hiding this comment.
In Test Optimization we've seen some edge cases in which commit.getSha() could be null (seems much more unlikely for repositoryURL). We usually have to point them to set the git info through environment variables in those cases. Will the TagMap.Entry.create call have issues with a null object?
Co-authored-by: Alexey Kuznetsov <alexey.kuznetsov@datadoghq.com>
Using DisplayName to improve readability instead of separating prefix / suffix with underscore
- setMetric not guards against null - TagMap.create now allows null - and returns null in response The intention is to let null values benignly flow through TagMap.Entry.create into AgentSpan.setTag / setMetric
Fixing oversight of creating newAnyEntry rather newObjectEntry in create(tag, CharSequence)
…aDog/dd-trace-java into dougqh/fdirect-apis-for-tagmap-entry
The null handling here now relies on nulls benignly passing through TagMap.Entry.create and AgentSpan.setTag(TagMap.Entry)
Apparently in some places a null Commit is passed instead of Commit.NOOP
…ace-java into dougqh/gitinfo-entry-reuse
What Does This Do
Reuses TagMap.Entry objects for git repository url & commit sha
Motivation
Reduces allocation & garbage collection - results in better throughput under high load / when memory is scarce
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any useful labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]