-
Notifications
You must be signed in to change notification settings - Fork 293
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 pending traces report in tracer flares #8053
base: master
Are you sure you want to change the base?
Conversation
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 51 metrics, 12 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.46.0-SNAPSHOT~b443ddd358, baseline=1.46.0-SNAPSHOT~65e472f132
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.042 s) : 0, 1041995
Total [baseline] (10.482 s) : 0, 10481904
Agent [candidate] (1.047 s) : 0, 1046822
Total [candidate] (10.498 s) : 0, 10497634
section appsec
Agent [baseline] (1.18 s) : 0, 1180388
Total [baseline] (10.747 s) : 0, 10746902
Agent [candidate] (1.179 s) : 0, 1179456
Total [candidate] (10.762 s) : 0, 10761507
section iast
Agent [baseline] (1.172 s) : 0, 1171739
Total [baseline] (10.946 s) : 0, 10946270
Agent [candidate] (1.169 s) : 0, 1168777
Total [candidate] (10.96 s) : 0, 10960477
section profiling
Agent [baseline] (1.257 s) : 0, 1256880
Total [baseline] (10.853 s) : 0, 10852856
Agent [candidate] (1.259 s) : 0, 1258766
Total [candidate] (10.856 s) : 0, 10855557
gantt
title petclinic - break down per module: candidate=1.46.0-SNAPSHOT~b443ddd358, baseline=1.46.0-SNAPSHOT~65e472f132
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (718.301 ms) : 0, 718301
BytebuddyAgent [candidate] (720.332 ms) : 0, 720332
GlobalTracer [baseline] (240.575 ms) : 0, 240575
GlobalTracer [candidate] (242.468 ms) : 0, 242468
AppSec [baseline] (55.572 ms) : 0, 55572
AppSec [candidate] (55.637 ms) : 0, 55637
Remote Config [baseline] (709.726 µs) : 0, 710
Remote Config [candidate] (729.315 µs) : 0, 729
Telemetry [baseline] (11.55 ms) : 0, 11550
Telemetry [candidate] (12.199 ms) : 0, 12199
section appsec
BytebuddyAgent [baseline] (732.244 ms) : 0, 732244
BytebuddyAgent [candidate] (730.637 ms) : 0, 730637
GlobalTracer [baseline] (237.561 ms) : 0, 237561
GlobalTracer [candidate] (237.214 ms) : 0, 237214
AppSec [baseline] (175.739 ms) : 0, 175739
AppSec [candidate] (176.407 ms) : 0, 176407
Remote Config [baseline] (653.006 µs) : 0, 653
Remote Config [candidate] (656.382 µs) : 0, 656
Telemetry [baseline] (8.291 ms) : 0, 8291
Telemetry [candidate] (8.648 ms) : 0, 8648
IAST [baseline] (21.401 ms) : 0, 21401
IAST [candidate] (21.385 ms) : 0, 21385
section iast
BytebuddyAgent [baseline] (836.119 ms) : 0, 836119
BytebuddyAgent [candidate] (833.454 ms) : 0, 833454
GlobalTracer [baseline] (231.028 ms) : 0, 231028
GlobalTracer [candidate] (231.004 ms) : 0, 231004
AppSec [baseline] (54.042 ms) : 0, 54042
AppSec [candidate] (54.025 ms) : 0, 54025
Remote Config [baseline] (602.63 µs) : 0, 603
Remote Config [candidate] (611.59 µs) : 0, 612
Telemetry [baseline] (8.738 ms) : 0, 8738
Telemetry [candidate] (8.768 ms) : 0, 8768
IAST [baseline] (25.946 ms) : 0, 25946
IAST [candidate] (25.735 ms) : 0, 25735
section profiling
BytebuddyAgent [baseline] (704.412 ms) : 0, 704412
BytebuddyAgent [candidate] (706.129 ms) : 0, 706129
GlobalTracer [baseline] (349.951 ms) : 0, 349951
GlobalTracer [candidate] (351.402 ms) : 0, 351402
AppSec [baseline] (55.339 ms) : 0, 55339
AppSec [candidate] (54.132 ms) : 0, 54132
Remote Config [baseline] (700.984 µs) : 0, 701
Remote Config [candidate] (691.909 µs) : 0, 692
Telemetry [baseline] (8.968 ms) : 0, 8968
Telemetry [candidate] (8.952 ms) : 0, 8952
ProfilingAgent [baseline] (95.286 ms) : 0, 95286
ProfilingAgent [candidate] (95.26 ms) : 0, 95260
Profiling [baseline] (95.309 ms) : 0, 95309
Profiling [candidate] (95.284 ms) : 0, 95284
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.46.0-SNAPSHOT~b443ddd358, baseline=1.46.0-SNAPSHOT~65e472f132
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.045 s) : 0, 1045103
Total [baseline] (8.64 s) : 0, 8639625
Agent [candidate] (1.047 s) : 0, 1046631
Total [candidate] (8.651 s) : 0, 8650927
section iast
Agent [baseline] (1.168 s) : 0, 1167753
Total [baseline] (9.213 s) : 0, 9212517
Agent [candidate] (1.169 s) : 0, 1169341
Total [candidate] (9.22 s) : 0, 9219630
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.185 s) : 0, 1185268
Total [baseline] (9.238 s) : 0, 9238493
Agent [candidate] (1.176 s) : 0, 1176384
Total [candidate] (9.228 s) : 0, 9228245
section iast_TELEMETRY_OFF
Agent [baseline] (1.166 s) : 0, 1165926
Total [baseline] (9.248 s) : 0, 9248155
Agent [candidate] (1.164 s) : 0, 1164436
Total [candidate] (9.277 s) : 0, 9277065
gantt
title insecure-bank - break down per module: candidate=1.46.0-SNAPSHOT~b443ddd358, baseline=1.46.0-SNAPSHOT~65e472f132
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (717.574 ms) : 0, 717574
BytebuddyAgent [candidate] (719.786 ms) : 0, 719786
GlobalTracer [baseline] (241.96 ms) : 0, 241960
GlobalTracer [candidate] (242.222 ms) : 0, 242222
AppSec [baseline] (55.726 ms) : 0, 55726
AppSec [candidate] (55.767 ms) : 0, 55767
Remote Config [baseline] (1.461 ms) : 0, 1461
Remote Config [candidate] (721.451 µs) : 0, 721
Telemetry [baseline] (13.152 ms) : 0, 13152
Telemetry [candidate] (12.856 ms) : 0, 12856
section iast
BytebuddyAgent [baseline] (832.946 ms) : 0, 832946
BytebuddyAgent [candidate] (833.78 ms) : 0, 833780
GlobalTracer [baseline] (230.775 ms) : 0, 230775
GlobalTracer [candidate] (231.563 ms) : 0, 231563
IAST [baseline] (24.877 ms) : 0, 24877
IAST [candidate] (24.115 ms) : 0, 24115
AppSec [baseline] (54.574 ms) : 0, 54574
AppSec [candidate] (55.352 ms) : 0, 55352
Remote Config [baseline] (610.574 µs) : 0, 611
Remote Config [candidate] (611.917 µs) : 0, 612
Telemetry [baseline] (8.743 ms) : 0, 8743
Telemetry [candidate] (8.624 ms) : 0, 8624
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (846.615 ms) : 0, 846615
BytebuddyAgent [candidate] (839.629 ms) : 0, 839629
GlobalTracer [baseline] (233.106 ms) : 0, 233106
GlobalTracer [candidate] (232.589 ms) : 0, 232589
IAST [baseline] (24.618 ms) : 0, 24618
IAST [candidate] (28.136 ms) : 0, 28136
AppSec [baseline] (56.007 ms) : 0, 56007
AppSec [candidate] (51.353 ms) : 0, 51353
Remote Config [baseline] (634.657 µs) : 0, 635
Remote Config [candidate] (612.754 µs) : 0, 613
Telemetry [baseline] (8.789 ms) : 0, 8789
Telemetry [candidate] (8.704 ms) : 0, 8704
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (831.684 ms) : 0, 831684
BytebuddyAgent [candidate] (830.259 ms) : 0, 830259
GlobalTracer [baseline] (230.25 ms) : 0, 230250
GlobalTracer [candidate] (230.843 ms) : 0, 230843
IAST [baseline] (25.486 ms) : 0, 25486
IAST [candidate] (26.045 ms) : 0, 26045
AppSec [baseline] (53.957 ms) : 0, 53957
AppSec [candidate] (52.94 ms) : 0, 52940
Remote Config [baseline] (613.925 µs) : 0, 614
Remote Config [candidate] (610.595 µs) : 0, 611
Telemetry [baseline] (8.678 ms) : 0, 8678
Telemetry [candidate] (8.556 ms) : 0, 8556
LoadParameters
See matching parameters
SummaryFound 1 performance improvements and 0 performance regressions! Performance is the same for 10 metrics, 17 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.46.0-SNAPSHOT~b443ddd358, baseline=1.46.0-SNAPSHOT~65e472f132
dateFormat X
axisFormat %s
section baseline
no_agent (1.357 ms) : 1337, 1377
. : milestone, 1357,
appsec (1.739 ms) : 1716, 1763
. : milestone, 1739,
appsec_no_iast (1.748 ms) : 1725, 1771
. : milestone, 1748,
iast (1.497 ms) : 1474, 1521
. : milestone, 1497,
profiling (1.579 ms) : 1554, 1604
. : milestone, 1579,
tracing (1.47 ms) : 1445, 1495
. : milestone, 1470,
section candidate
no_agent (1.348 ms) : 1329, 1367
. : milestone, 1348,
appsec (1.753 ms) : 1729, 1776
. : milestone, 1753,
appsec_no_iast (1.75 ms) : 1726, 1774
. : milestone, 1750,
iast (1.507 ms) : 1484, 1531
. : milestone, 1507,
profiling (1.512 ms) : 1486, 1537
. : milestone, 1512,
tracing (1.496 ms) : 1471, 1521
. : milestone, 1496,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.46.0-SNAPSHOT~b443ddd358, baseline=1.46.0-SNAPSHOT~65e472f132
dateFormat X
axisFormat %s
section baseline
no_agent (373.103 µs) : 353, 393
. : milestone, 373,
iast (505.228 µs) : 482, 528
. : milestone, 505,
iast_FULL (738.71 µs) : 716, 762
. : milestone, 739,
iast_GLOBAL (556.739 µs) : 533, 580
. : milestone, 557,
iast_HARDCODED_SECRET_DISABLED (501.118 µs) : 478, 524
. : milestone, 501,
iast_INACTIVE (452.292 µs) : 431, 474
. : milestone, 452,
iast_TELEMETRY_OFF (498.23 µs) : 475, 522
. : milestone, 498,
tracing (454.269 µs) : 433, 476
. : milestone, 454,
section candidate
no_agent (379.618 µs) : 360, 400
. : milestone, 380,
iast (513.334 µs) : 490, 537
. : milestone, 513,
iast_FULL (738.599 µs) : 717, 760
. : milestone, 739,
iast_GLOBAL (557.494 µs) : 533, 582
. : milestone, 557,
iast_HARDCODED_SECRET_DISABLED (502.657 µs) : 481, 524
. : milestone, 503,
iast_INACTIVE (451.697 µs) : 431, 473
. : milestone, 452,
iast_TELEMETRY_OFF (489.075 µs) : 466, 512
. : milestone, 489,
tracing (443.336 µs) : 423, 464
. : milestone, 443,
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.46.0-SNAPSHOT~b443ddd358, baseline=1.46.0-SNAPSHOT~65e472f132
dateFormat X
axisFormat %s
section baseline
no_agent (1.465 ms) : 1453, 1476
. : milestone, 1465,
appsec (2.342 ms) : 2299, 2386
. : milestone, 2342,
iast (2.106 ms) : 2051, 2161
. : milestone, 2106,
iast_GLOBAL (2.149 ms) : 2094, 2205
. : milestone, 2149,
profiling (1.99 ms) : 1945, 2034
. : milestone, 1990,
tracing (1.942 ms) : 1899, 1984
. : milestone, 1942,
section candidate
no_agent (1.47 ms) : 1458, 1482
. : milestone, 1470,
appsec (2.335 ms) : 2292, 2378
. : milestone, 2335,
iast (2.095 ms) : 2041, 2150
. : milestone, 2095,
iast_GLOBAL (2.144 ms) : 2089, 2200
. : milestone, 2144,
profiling (1.98 ms) : 1936, 2025
. : milestone, 1980,
tracing (1.928 ms) : 1886, 1970
. : milestone, 1928,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.46.0-SNAPSHOT~b443ddd358, baseline=1.46.0-SNAPSHOT~65e472f132
dateFormat X
axisFormat %s
section baseline
no_agent (15.166 s) : 15166000, 15166000
. : milestone, 15166000,
appsec (14.992 s) : 14992000, 14992000
. : milestone, 14992000,
iast (18.39 s) : 18390000, 18390000
. : milestone, 18390000,
iast_GLOBAL (17.922 s) : 17922000, 17922000
. : milestone, 17922000,
profiling (15.142 s) : 15142000, 15142000
. : milestone, 15142000,
tracing (14.728 s) : 14728000, 14728000
. : milestone, 14728000,
section candidate
no_agent (15.452 s) : 15452000, 15452000
. : milestone, 15452000,
appsec (14.989 s) : 14989000, 14989000
. : milestone, 14989000,
iast (18.635 s) : 18635000, 18635000
. : milestone, 18635000,
iast_GLOBAL (17.978 s) : 17978000, 17978000
. : milestone, 17978000,
profiling (15.0 s) : 15000000, 15000000
. : milestone, 15000000,
tracing (15.167 s) : 15167000, 15167000
. : milestone, 15167000,
|
dd-trace-core/src/main/java/datadog/trace/core/flare/TracerFlareService.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/util/TracerDump.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/util/TracerDump.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/util/TracerDump.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/util/TracerDump.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/PendingTrace.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/util/TracerDump.java
Outdated
Show resolved
Hide resolved
Have you considered tackling this another way, by adding the reporting to |
4be2999
to
3e4955a
Compare
dd-trace-core/src/main/java/datadog/trace/core/PendingTraceBuffer.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/PendingTraceBuffer.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/PendingTraceBuffer.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/PendingTraceBuffer.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/PendingTraceBuffer.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/PendingTraceBuffer.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/PendingTraceBuffer.java
Outdated
Show resolved
Hide resolved
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.
Have drain and fill together feels way better! 🙌
Now, about using the collected elements:
- You can declare your comparator as a
private final Comparator<List<Span>> TRACE_BY_START_DATE
field so its meaning will be clear and it will only be allocated once - You will need to figure out how to filter, limit, and map your elements. I would recommend having a look at the Java
Stream
API - Now you get most of the part in place, try building some tests / triggering your code. You should be able to find some NPE by yourself 😉
dd-trace-core/src/main/java/datadog/trace/core/PendingTraceBuffer.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/PendingTraceBuffer.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/PendingTraceBuffer.java
Outdated
Show resolved
Hide resolved
5119ac7
to
3a8b13c
Compare
|
||
public class TraceDumpWriter implements Writer { | ||
|
||
private StringBuilder dumpText; |
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.
|
||
public class TraceDumpJsonExporter { | ||
|
||
private StringBuilder dumpText; |
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.
dd-trace-core/src/main/java/datadog/trace/core/PendingTraceBuffer.java
Outdated
Show resolved
Hide resolved
4f6a0db
to
9786fb5
Compare
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
cc @mcculls if you would like to check it before it gets merged
dd-trace-core/src/main/java/datadog/trace/core/PendingTraceBuffer.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/common/writer/TraceDumpJsonExporter.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/PendingTraceBuffer.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/PendingTraceBuffer.java
Outdated
Show resolved
Hide resolved
new Moshi.Builder() | ||
.add(DDSpanJsonAdapter.buildFactory(false)) | ||
.build() | ||
.adapter(Types.newParameterizedType(List.class, DDSpan.class)); |
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.
Any reason we don't use Collection.class
here, which would then let us use public void write(final Collection<DDSpan> trace) {
or if you want to keep it a bit more specific public void write(final Deque<DDSpan> trace) {
and avoid the need to copy span data into a temporary ArrayList
just to fit the expected interface?
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.
Initially it was because public void write(final List<DDSpan> trace)
has to be implemented regardless to override the writer
interface that declares the method. I'm thinking I could override the method with a dummy implementation and overload a new public void write(final Deque<DDSpan> trace) {
that has the actual implementation instead.
dd-trace-core/src/main/java/datadog/trace/core/PendingTraceBuffer.java
Outdated
Show resolved
Hide resolved
private static final class DumpDrain | ||
implements MessagePassingQueue.Consumer<Element>, MessagePassingQueue.Supplier<Element> { | ||
private static final DumpDrain DUMP_DRAIN = new DumpDrain(); | ||
private static final List<Element> DATA = new ArrayList<>(); |
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.
Can you make DATA
an instance field?
You could then introduce a method called List<Element> collectTraces()
which removes non-pending-trace elements, sorts it by oldest trace, then returns the data. You could also add a reset
method to clear the recorded data, which avoids the need to expose the internal collection - although note that would not compact the underlying array.
You might want to consider making the data
field volatile
- the collectTraces
method could then do the following:
- store the current
data
reference in a local variable - assign a
new ArrayList()
todata
- ready for the next dump request - return the local variable, i.e. the old
ArrayList
with the trace data
The main benefit this would bring is that there's a clear line between the writing and reading sides at the point we assign a new collection to data
. It would also allow the data
collection to be compacted, because we assign it a fresh ArrayList
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'm not fully sure I understand the point of making data
volatile. From what I understand, it essentially allows us to "clear" the data
array in collectTraces()
by assigning it to a temp variable (whose memory will be cleared once the code exits the scope of the for-loop that calls the method, and instantiate a clean ArrayList for future traces to be stored until the next dump. Is this understanding correct?
.add(DDSpanJsonAdapter.buildFactory(false)) | ||
.build() | ||
.adapter(Types.newParameterizedType(Collection.class, DDSpan.class)); | ||
private StringBuilder dumpText; |
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.
What Does This Do
APMAPI-906
Creates a TracerFlare Dump in order to dump traces that have been captured but not yet sent to the agent. This PR introduces a new
DUMP_DRAIN
element in order to properly extract the pending traces and sends such traces to aJsonExporter
in order to write them into theTracerFlare
as Json. These traces will be displayed from oldest first and are currently limited to 50 traces in a dump.Motivation
This PR will help with solving customer issues where traces are captured but not sent to the agent because the root span is never finished/written.
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: APMAPI-906