-
Notifications
You must be signed in to change notification settings - Fork 300
Added span events to the DD Trace API #8585
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
base: master
Are you sure you want to change the base?
Conversation
internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/DDSpanEvent.java
Outdated
Show resolved
Hide resolved
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 65 metrics, 6 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.48.0-SNAPSHOT~4a2d5c52cb, baseline=1.48.0-SNAPSHOT~60f8046e47
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.062 s) : 0, 1062402
Total [baseline] (8.697 s) : 0, 8697176
Agent [candidate] (1.052 s) : 0, 1052346
Total [candidate] (8.652 s) : 0, 8651788
section iast
Agent [baseline] (1.176 s) : 0, 1175920
Total [baseline] (9.205 s) : 0, 9204919
Agent [candidate] (1.176 s) : 0, 1175718
Total [candidate] (9.22 s) : 0, 9220228
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.179 s) : 0, 1179165
Total [baseline] (9.187 s) : 0, 9187403
Agent [candidate] (1.18 s) : 0, 1180262
Total [candidate] (9.247 s) : 0, 9246938
section iast_TELEMETRY_OFF
Agent [baseline] (1.174 s) : 0, 1174246
Total [baseline] (9.274 s) : 0, 9273966
Agent [candidate] (1.175 s) : 0, 1175133
Total [candidate] (9.244 s) : 0, 9244271
gantt
title insecure-bank - break down per module: candidate=1.48.0-SNAPSHOT~4a2d5c52cb, baseline=1.48.0-SNAPSHOT~60f8046e47
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (727.443 ms) : 0, 727443
BytebuddyAgent [candidate] (720.736 ms) : 0, 720736
GlobalTracer [baseline] (242.838 ms) : 0, 242838
GlobalTracer [candidate] (241.184 ms) : 0, 241184
AppSec [baseline] (55.495 ms) : 0, 55495
AppSec [candidate] (54.763 ms) : 0, 54763
Debugger [baseline] (5.167 ms) : 0, 5167
Debugger [candidate] (4.386 ms) : 0, 4386
Remote Config [baseline] (729.932 µs) : 0, 730
Remote Config [candidate] (702.892 µs) : 0, 703
Telemetry [baseline] (14.389 ms) : 0, 14389
Telemetry [candidate] (14.454 ms) : 0, 14454
section iast
BytebuddyAgent [baseline] (837.769 ms) : 0, 837769
BytebuddyAgent [candidate] (837.645 ms) : 0, 837645
GlobalTracer [baseline] (230.225 ms) : 0, 230225
GlobalTracer [candidate] (230.32 ms) : 0, 230320
IAST [baseline] (22.603 ms) : 0, 22603
IAST [candidate] (22.796 ms) : 0, 22796
AppSec [baseline] (55.954 ms) : 0, 55954
AppSec [candidate] (55.646 ms) : 0, 55646
Debugger [baseline] (4.154 ms) : 0, 4154
Debugger [candidate] (4.132 ms) : 0, 4132
Remote Config [baseline] (590.163 µs) : 0, 590
Remote Config [candidate] (592.596 µs) : 0, 593
Telemetry [baseline] (8.669 ms) : 0, 8669
Telemetry [candidate] (8.663 ms) : 0, 8663
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (839.598 ms) : 0, 839598
BytebuddyAgent [candidate] (839.49 ms) : 0, 839490
GlobalTracer [baseline] (231.129 ms) : 0, 231129
GlobalTracer [candidate] (232.087 ms) : 0, 232087
IAST [baseline] (23.767 ms) : 0, 23767
IAST [candidate] (23.005 ms) : 0, 23005
AppSec [baseline] (55.094 ms) : 0, 55094
AppSec [candidate] (56.085 ms) : 0, 56085
Debugger [baseline] (4.174 ms) : 0, 4174
Debugger [candidate] (4.177 ms) : 0, 4177
Remote Config [baseline] (600.886 µs) : 0, 601
Remote Config [candidate] (600.516 µs) : 0, 601
Telemetry [baseline] (8.806 ms) : 0, 8806
Telemetry [candidate] (8.832 ms) : 0, 8832
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (835.812 ms) : 0, 835812
BytebuddyAgent [candidate] (836.697 ms) : 0, 836697
GlobalTracer [baseline] (230.394 ms) : 0, 230394
GlobalTracer [candidate] (230.587 ms) : 0, 230587
IAST [baseline] (22.391 ms) : 0, 22391
IAST [candidate] (22.536 ms) : 0, 22536
AppSec [baseline] (56.327 ms) : 0, 56327
AppSec [candidate] (55.969 ms) : 0, 55969
Debugger [baseline] (4.131 ms) : 0, 4131
Debugger [candidate] (4.13 ms) : 0, 4130
Remote Config [baseline] (611.594 µs) : 0, 612
Remote Config [candidate] (600.988 µs) : 0, 601
Telemetry [baseline] (8.608 ms) : 0, 8608
Telemetry [candidate] (8.605 ms) : 0, 8605
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.48.0-SNAPSHOT~4a2d5c52cb, baseline=1.48.0-SNAPSHOT~60f8046e47
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.046 s) : 0, 1046437
Total [baseline] (10.453 s) : 0, 10452612
Agent [candidate] (1.054 s) : 0, 1053940
Total [candidate] (10.428 s) : 0, 10428331
section appsec
Agent [baseline] (1.191 s) : 0, 1190540
Total [baseline] (10.719 s) : 0, 10718949
Agent [candidate] (1.189 s) : 0, 1189401
Total [candidate] (10.738 s) : 0, 10738494
section iast
Agent [baseline] (1.18 s) : 0, 1180427
Total [baseline] (11.043 s) : 0, 11042710
Agent [candidate] (1.177 s) : 0, 1177241
Total [candidate] (10.958 s) : 0, 10957573
section profiling
Agent [baseline] (1.275 s) : 0, 1274549
Total [baseline] (10.927 s) : 0, 10926509
Agent [candidate] (1.272 s) : 0, 1271785
Total [candidate] (10.84 s) : 0, 10839715
gantt
title petclinic - break down per module: candidate=1.48.0-SNAPSHOT~4a2d5c52cb, baseline=1.48.0-SNAPSHOT~60f8046e47
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (718.321 ms) : 0, 718321
BytebuddyAgent [candidate] (723.433 ms) : 0, 723433
GlobalTracer [baseline] (240.282 ms) : 0, 240282
GlobalTracer [candidate] (242.056 ms) : 0, 242056
AppSec [baseline] (54.648 ms) : 0, 54648
AppSec [candidate] (55.042 ms) : 0, 55042
Debugger [baseline] (5.116 ms) : 0, 5116
Debugger [candidate] (4.46 ms) : 0, 4460
Remote Config [baseline] (702.603 µs) : 0, 703
Remote Config [candidate] (732.945 µs) : 0, 733
Telemetry [baseline] (11.44 ms) : 0, 11440
Telemetry [candidate] (12.101 ms) : 0, 12101
section appsec
BytebuddyAgent [baseline] (737.19 ms) : 0, 737190
BytebuddyAgent [candidate] (736.808 ms) : 0, 736808
GlobalTracer [baseline] (236.8 ms) : 0, 236800
GlobalTracer [candidate] (236.595 ms) : 0, 236595
AppSec [baseline] (176.549 ms) : 0, 176549
AppSec [candidate] (176.103 ms) : 0, 176103
Debugger [baseline] (4.262 ms) : 0, 4262
Debugger [candidate] (4.315 ms) : 0, 4315
Remote Config [baseline] (643.424 µs) : 0, 643
Remote Config [candidate] (647.106 µs) : 0, 647
Telemetry [baseline] (8.143 ms) : 0, 8143
Telemetry [candidate] (8.18 ms) : 0, 8180
IAST [baseline] (21.791 ms) : 0, 21791
IAST [candidate] (21.532 ms) : 0, 21532
section iast
BytebuddyAgent [baseline] (840.15 ms) : 0, 840150
BytebuddyAgent [candidate] (838.524 ms) : 0, 838524
GlobalTracer [baseline] (231.134 ms) : 0, 231134
GlobalTracer [candidate] (230.654 ms) : 0, 230654
AppSec [baseline] (56.362 ms) : 0, 56362
AppSec [candidate] (55.894 ms) : 0, 55894
Debugger [baseline] (4.199 ms) : 0, 4199
Debugger [candidate] (4.119 ms) : 0, 4119
Remote Config [baseline] (597.628 µs) : 0, 598
Remote Config [candidate] (600.172 µs) : 0, 600
Telemetry [baseline] (8.836 ms) : 0, 8836
Telemetry [candidate] (8.704 ms) : 0, 8704
IAST [baseline] (22.988 ms) : 0, 22988
IAST [candidate] (22.737 ms) : 0, 22737
section profiling
BytebuddyAgent [baseline] (710.698 ms) : 0, 710698
BytebuddyAgent [candidate] (709.081 ms) : 0, 709081
GlobalTracer [baseline] (351.249 ms) : 0, 351249
GlobalTracer [candidate] (351.64 ms) : 0, 351640
AppSec [baseline] (54.862 ms) : 0, 54862
AppSec [candidate] (53.785 ms) : 0, 53785
Debugger [baseline] (4.34 ms) : 0, 4340
Debugger [candidate] (4.293 ms) : 0, 4293
Remote Config [baseline] (698.611 µs) : 0, 699
Remote Config [candidate] (688.44 µs) : 0, 688
Telemetry [baseline] (9.042 ms) : 0, 9042
Telemetry [candidate] (8.912 ms) : 0, 8912
ProfilingAgent [baseline] (101.974 ms) : 0, 101974
ProfilingAgent [candidate] (101.819 ms) : 0, 101819
Profiling [baseline] (102.0 ms) : 0, 102000
Profiling [candidate] (101.845 ms) : 0, 101845
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 13 metrics, 17 unstable metrics. Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.48.0-SNAPSHOT~4a2d5c52cb, baseline=1.48.0-SNAPSHOT~60f8046e47
dateFormat X
axisFormat %s
section baseline
no_agent (380.732 µs) : 360, 401
. : milestone, 381,
iast (509.68 µs) : 488, 531
. : milestone, 510,
iast_FULL (733.477 µs) : 711, 756
. : milestone, 733,
iast_GLOBAL (564.6 µs) : 541, 588
. : milestone, 565,
iast_HARDCODED_SECRET_DISABLED (511.124 µs) : 489, 533
. : milestone, 511,
iast_INACTIVE (470.584 µs) : 449, 492
. : milestone, 471,
iast_TELEMETRY_OFF (497.939 µs) : 476, 519
. : milestone, 498,
tracing (459.846 µs) : 438, 481
. : milestone, 460,
section candidate
no_agent (382.065 µs) : 362, 402
. : milestone, 382,
iast (510.085 µs) : 488, 532
. : milestone, 510,
iast_FULL (727.779 µs) : 706, 750
. : milestone, 728,
iast_GLOBAL (558.261 µs) : 537, 580
. : milestone, 558,
iast_HARDCODED_SECRET_DISABLED (515.123 µs) : 493, 537
. : milestone, 515,
iast_INACTIVE (469.004 µs) : 447, 491
. : milestone, 469,
iast_TELEMETRY_OFF (505.29 µs) : 483, 527
. : milestone, 505,
tracing (462.84 µs) : 441, 484
. : milestone, 463,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.48.0-SNAPSHOT~4a2d5c52cb, baseline=1.48.0-SNAPSHOT~60f8046e47
dateFormat X
axisFormat %s
section baseline
no_agent (1.35 ms) : 1331, 1370
. : milestone, 1350,
appsec (1.717 ms) : 1694, 1741
. : milestone, 1717,
appsec_no_iast (1.74 ms) : 1717, 1764
. : milestone, 1740,
code_origins (1.681 ms) : 1655, 1707
. : milestone, 1681,
iast (1.51 ms) : 1486, 1535
. : milestone, 1510,
profiling (1.508 ms) : 1484, 1532
. : milestone, 1508,
tracing (1.503 ms) : 1478, 1527
. : milestone, 1503,
section candidate
no_agent (1.353 ms) : 1334, 1373
. : milestone, 1353,
appsec (1.728 ms) : 1704, 1752
. : milestone, 1728,
appsec_no_iast (1.729 ms) : 1705, 1753
. : milestone, 1729,
code_origins (1.675 ms) : 1649, 1701
. : milestone, 1675,
iast (1.518 ms) : 1495, 1542
. : milestone, 1518,
profiling (1.521 ms) : 1498, 1543
. : milestone, 1521,
tracing (1.51 ms) : 1485, 1534
. : milestone, 1510,
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.48.0-SNAPSHOT~4a2d5c52cb, baseline=1.48.0-SNAPSHOT~60f8046e47
dateFormat X
axisFormat %s
section baseline
no_agent (1.475 ms) : 1464, 1487
. : milestone, 1475,
appsec (2.342 ms) : 2298, 2385
. : milestone, 2342,
iast (2.11 ms) : 2054, 2165
. : milestone, 2110,
iast_GLOBAL (2.159 ms) : 2103, 2214
. : milestone, 2159,
profiling (1.979 ms) : 1934, 2023
. : milestone, 1979,
tracing (1.956 ms) : 1914, 1999
. : milestone, 1956,
section candidate
no_agent (1.469 ms) : 1458, 1481
. : milestone, 1469,
appsec (2.341 ms) : 2297, 2385
. : milestone, 2341,
iast (2.12 ms) : 2064, 2175
. : milestone, 2120,
iast_GLOBAL (2.154 ms) : 2098, 2209
. : milestone, 2154,
profiling (1.969 ms) : 1925, 2013
. : milestone, 1969,
tracing (1.959 ms) : 1916, 2001
. : milestone, 1959,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.48.0-SNAPSHOT~4a2d5c52cb, baseline=1.48.0-SNAPSHOT~60f8046e47
dateFormat X
axisFormat %s
section baseline
no_agent (14.956 s) : 14956000, 14956000
. : milestone, 14956000,
appsec (14.974 s) : 14974000, 14974000
. : milestone, 14974000,
iast (18.767 s) : 18767000, 18767000
. : milestone, 18767000,
iast_GLOBAL (17.821 s) : 17821000, 17821000
. : milestone, 17821000,
profiling (14.942 s) : 14942000, 14942000
. : milestone, 14942000,
tracing (15.054 s) : 15054000, 15054000
. : milestone, 15054000,
section candidate
no_agent (15.339 s) : 15339000, 15339000
. : milestone, 15339000,
appsec (14.935 s) : 14935000, 14935000
. : milestone, 14935000,
iast (18.442 s) : 18442000, 18442000
. : milestone, 18442000,
iast_GLOBAL (17.746 s) : 17746000, 17746000
. : milestone, 17746000,
profiling (15.198 s) : 15198000, 15198000
. : milestone, 15198000,
tracing (15.092 s) : 15092000, 15092000
. : milestone, 15092000,
|
Co-authored-by: datadog-datadog-prod-us1[bot] <88084959+datadog-datadog-prod-us1[bot]@users.noreply.github.com>
} | ||
first = false; | ||
} | ||
json.append("}"); |
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.
🟠 Code Quality Violation
json.append("}"); | |
json.append('}'); |
Do not append a string literal that contains only a single char (...read more)
Do not append a string with single character to a StringBuffer
. Instead, append a character type. This is better practice and results in better performance. While this is negligible if the operation occurs once, it has performance impact if done at scale.
Learn More
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
utils/test-utils/src/main/groovy/datadog/trace/test/util/DDSpecification.groovy
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.
I'm posting the first part of my notes because I didn't have time to complete the review all at once.
* data model closely follows the OpenTelemetry specification. | ||
* | ||
* @see <a | ||
* href="https://github.com/open-telemetry/opentelemetry.io/blob/2b007bc89daf60fe72e25a11f7e7d21887faf4ae/content/en/docs/concepts/signals/traces.md#span-events">OpenTelemetry |
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.
You may want to use this shorter link instead https://opentelemetry.io/docs/concepts/signals/traces/#span-links
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 DDSpanEvent { | ||
private static final Logger LOGGER = LoggerFactory.getLogger(DDSpanEvent.class); | ||
private static final int TAG_MAX_LENGTH = 25_000; |
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.
It might be worth commenting on where this limit comes from.
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 AgentSpan addEvent(String name, SpanNativeAttributes attributes) { | ||
if (name != null) { | ||
events.add(new DDSpanEvent(name, attributes)); |
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.
How does an event without a timestamp appear in the UI?
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.
With the current timestamp.
return null; | ||
} | ||
// Manually encode as JSON array | ||
StringBuilder builder = new StringBuilder("["); |
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.
As a safer option to work with, you could just use com.squareup.moshi.JsonWriter
. As a bonus, no adapter or intermediate to string conversion is required.
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.
Awesome recommendation!
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 a new JSON component for this in the codebase.
|
||
// Verify span events | ||
def actualEvents = actualSpan.meta["events"] | ||
def expectedEvent = "[{\"time_unix_nano\":${java.util.concurrent.TimeUnit.MILLISECONDS.toNanos(timestamp)},\"name\":\"test-event\",\"attributes\":{\"key1\":\"value1\",\"key2\":123,\"key3\":[1.1,2.2,3.3]}}]" |
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.
Use single quotes to avoid escaping.
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.
👍
|
||
// Verify span events | ||
def actualEvents = actualSpan.meta["events"] | ||
def expectedEvent = "[{\"time_unix_nano\":${java.util.concurrent.TimeUnit.MILLISECONDS.toNanos(timestamp)},\"name\":\"test-event\",\"attributes\":{\"key1\":\"value1\",\"key2\":123,\"key3\":[1.1,2.2,3.3]}}]" |
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.
No need to use fully qualified name.
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.
👍
def actual = genericAdapter.fromJson(jsonStr) | ||
def actualSpan = actual[0] | ||
|
||
// Verify basic span fields |
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 test looks like an AI generated with unnecessary noise and duplicated code.
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 had a hard time getting the test coverage reporter to be happy (it kept saying it was 50% coverage, when the tests pretty much covered everything), so I added more tests than I thought were needed, and I agree some of it seems like too much.
I'll review these and see if can remove the noisy ones.
@@ -136,6 +140,7 @@ private DDSpan( | |||
} | |||
|
|||
this.links = links == null ? new CopyOnWriteArrayList<>() : new CopyOnWriteArrayList<>(links); | |||
this.events = new CopyOnWriteArrayList<>(); |
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.
We should rather not allocate the COW list by default for performance reason
What Does This Do
Adds the
addEvent
method toDDSpan
, to allow the creation of Span Events from integrations. This will be use shortly to add Span Events for the GraphQL integration.Today, it's only possible to create Span Events through the OpenTelemetry span.
Motivation
We want to record multiple query errors coming from a single GraphQL span. Our choice of implementation will be to emit multiple span events, one per GraphQL query error. These are slightly different from regular Span errors, so they will have custom fields in their query error span event.
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: AIDM-511