Consider spans with exception spanEvents as errors #30064

Guillaume-Barrier · 2024-10-11T15:37:32Z

What does this PR do?

This PR adds support for spans with OpenTelemetry exception span events to be considered as error spans so that trace chunks that contain them can go through the ErrorSampler.

Motivation

Error Tracking will support OpenTelemetry exception span events as issues. The sampler should not drop spans that don't have an error status but do have exception span events.

https://datadoghq.atlassian.net/browse/ERRORT-4722

Error Tracking will support OpenTelemetry exception span events as issues. The sampler should not drop spans that don't have an error status but do have exception span events.

bits-bot · 2024-10-11T15:37:37Z

All committers have signed the CLA.

pr-commenter · 2024-10-11T15:48:54Z

Regression Detector

Regression Detector Results

Run ID: 44d4f6eb-7a64-4aa4-bc22-f8a8cfcbc77d Metrics dashboard Target profiles

Baseline: ff5a62b
Comparison: 2d590bb

Performance changes are noted in the perf column of each table:

✅ = significantly better comparison variant performance
❌ = significantly worse comparison variant performance
➖ = no significant change in performance

No significant changes in experiment optimization goals

Confidence level: 90.00%
Effect size tolerance: |Δ mean %| ≥ 5.00%

There were no significant changes in experiment optimization goals at this confidence level and effect size tolerance.

Fine details of change detection per experiment

perf	experiment	goal	Δ mean %	Δ mean % CI	trials	links
➖	file_tree	memory utilization	+1.07	[+0.95, +1.20]	1	Logs
➖	pycheck_lots_of_tags	% cpu utilization	+0.88	[-1.68, +3.43]	1	Logs
➖	idle	memory utilization	+0.36	[+0.32, +0.41]	1	Logs bounds checks dashboard
➖	file_to_blackhole_1000ms_latency	egress throughput	+0.19	[-0.29, +0.68]	1	Logs
➖	file_to_blackhole_500ms_latency	egress throughput	+0.09	[-0.16, +0.34]	1	Logs
➖	uds_dogstatsd_to_api_cpu	% cpu utilization	+0.04	[-0.69, +0.78]	1	Logs
➖	file_to_blackhole_300ms_latency	egress throughput	+0.01	[-0.16, +0.19]	1	Logs
➖	file_to_blackhole_100ms_latency	egress throughput	+0.01	[-0.22, +0.23]	1	Logs
➖	tcp_dd_logs_filter_exclude	ingress throughput	-0.00	[-0.01, +0.01]	1	Logs
➖	uds_dogstatsd_to_api	ingress throughput	-0.01	[-0.10, +0.07]	1	Logs
➖	tcp_syslog_to_blackhole	ingress throughput	-0.02	[-0.07, +0.04]	1	Logs
➖	file_to_blackhole_0ms_latency	egress throughput	-0.02	[-0.36, +0.31]	1	Logs
➖	otel_to_otel_logs	ingress throughput	-0.37	[-1.18, +0.43]	1	Logs
➖	basic_py_check	% cpu utilization	-0.39	[-3.10, +2.32]	1	Logs
➖	idle_all_features	memory utilization	-1.12	[-1.23, -1.02]	1	Logs bounds checks dashboard

Bounds Checks

perf	experiment	bounds_check_name	replicates_passed
✅	file_to_blackhole_0ms_latency	memory_usage	10/10
✅	file_to_blackhole_1000ms_latency	memory_usage	10/10
✅	file_to_blackhole_100ms_latency	memory_usage	10/10
✅	file_to_blackhole_300ms_latency	memory_usage	10/10
✅	file_to_blackhole_500ms_latency	memory_usage	10/10
✅	idle	memory_usage	10/10
✅	idle_all_features	memory_usage	10/10

Explanation

A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".

For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:

Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.
Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.
Its configuration does not mark it "erratic".

…or-sampler

There is now a second method to convert otlp spans to dd spans. Adding the has_exception tag in this method as well.

agent-platform-auto-pr · 2024-10-14T09:34:13Z

Test changes on VM

Use this command from test-infra-definitions to manually test this PR changes on a VM:

inv create-vm --pipeline-id=46589357 --os-family=ubuntu

Note: This applies to commit 2d590bb

pkg/trace/agent/agent_test.go

Removing the _dd.span_events.has_exception tag requires to loop through the whole chunk which is not ideal. Also: - tag is hidden (although visible in devtools) - indication of why the chunk was sampled if there is no error span - error sampler can be disabled so no clean up And while it is there, this tag could also be used at processing to detect exception span events without even unmarshalling.

releasenotes/notes/support-exception-span-events-in-error-sampler-c9f4803b5c18705d.yaml

…ler-c9f4803b5c18705d.yaml Co-authored-by: Iñigo López de Heredia <[email protected]>

marcotc · 2024-10-15T16:41:41Z

pkg/trace/agent/agent.go

+}
+
+func spanContainsExceptionSpanEvent(span *pb.Span) bool {
+	if hasExceptionSpanEvents, ok := span.Meta["_dd.span_events.has_exception"]; ok && hasExceptionSpanEvents == "true" {


When will this tag be set?
When:

any exception is present, or

When there's an exception with exception.escaped true, or

A different logic.

I ask because if the logic is consistent, it's best to implement it here, instead of letting every library implement individually.

Hi Marco! tag is set for any exception (see otlp.go)

dineshg13

Small nit: Looks good otherwise

dineshg13 · 2024-10-15T18:50:33Z

pkg/trace/api/otlp.go

@@ -517,6 +517,12 @@ func (o *OTLPReceiver) convertSpan(rattr map[string]string, lib pcommon.Instrume
 	if in.Events().Len() > 0 {
 		transform.SetMetaOTLP(span, "events", transform.MarshalEvents(in.Events()))
 	}
+	for i := range in.Events().Len() {
+		if in.Events().At(i).Name() == "exception" {


should we do contains?

from what I can see in the otel specs, the name of an exception span event should always be "exception", is there any reason not to use the built-in string comparison operator ==?

Guillaume-Barrier · 2024-10-18T11:33:48Z

Closing in favor of #30065 because exception span events should only be taken into consideration for Error Tracking Backend Standalone (subject of the other PR).

Consider spans with exception spanEvents as errors

42def98

Error Tracking will support OpenTelemetry exception span events as issues. The sampler should not drop spans that don't have an error status but do have exception span events.

github-actions bot added team/agent-apm trace-agent team/opentelemetry OpenTelemetry team labels Oct 11, 2024

Guillaume-Barrier and others added 2 commits October 14, 2024 10:48

Merge branch 'main' into guillaume.barrier/support-span-events-in-err…

182e2de

…or-sampler

Add tag in receive span v2

8a5fddf

There is now a second method to convert otlp spans to dd spans. Adding the has_exception tag in this method as well.

Guillaume-Barrier marked this pull request as ready for review October 14, 2024 11:18

Guillaume-Barrier requested review from a team as code owners October 14, 2024 11:18

Guillaume-Barrier requested a review from dineshg13 October 14, 2024 11:18

dineshg13 reviewed Oct 14, 2024

View reviewed changes

pkg/trace/agent/agent_test.go Outdated Show resolved Hide resolved

Guillaume-Barrier force-pushed the guillaume.barrier/support-span-events-in-error-sampler branch from 1488f96 to d815c6c Compare October 14, 2024 15:53

urseberry approved these changes Oct 14, 2024

View reviewed changes

ichinaski approved these changes Oct 15, 2024

View reviewed changes

releasenotes/notes/support-exception-span-events-in-error-sampler-c9f4803b5c18705d.yaml Outdated Show resolved Hide resolved

Guillaume-Barrier and others added 2 commits October 15, 2024 09:54

Update releasenotes/notes/support-exception-span-events-in-error-samp…

71d6b82

…ler-c9f4803b5c18705d.yaml Co-authored-by: Iñigo López de Heredia <[email protected]>

Fix release note

2d590bb

Guillaume-Barrier requested a review from dineshg13 October 15, 2024 13:49

marcotc reviewed Oct 15, 2024

View reviewed changes

dineshg13 approved these changes Oct 15, 2024

View reviewed changes

Guillaume-Barrier closed this Oct 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consider spans with exception spanEvents as errors #30064

Consider spans with exception spanEvents as errors #30064

Guillaume-Barrier commented Oct 11, 2024 •

edited

Loading

bits-bot commented Oct 11, 2024 •

edited

Loading

pr-commenter bot commented Oct 11, 2024 •

edited by cit-pr-commenter bot

Loading

Fine details of change detection per experiment

Explanation

agent-platform-auto-pr bot commented Oct 14, 2024 •

edited

Loading

marcotc Oct 15, 2024

Guillaume-Barrier Oct 16, 2024

dineshg13 left a comment

dineshg13 Oct 15, 2024

Guillaume-Barrier Oct 16, 2024 •

edited

Loading

Guillaume-Barrier commented Oct 18, 2024

Consider spans with exception spanEvents as errors #30064

Consider spans with exception spanEvents as errors #30064

Conversation

Guillaume-Barrier commented Oct 11, 2024 • edited Loading

What does this PR do?

Motivation

bits-bot commented Oct 11, 2024 • edited Loading

pr-commenter bot commented Oct 11, 2024 • edited by cit-pr-commenter bot Loading

Regression Detector

Regression Detector Results

No significant changes in experiment optimization goals

Fine details of change detection per experiment

Bounds Checks

Explanation

agent-platform-auto-pr bot commented Oct 14, 2024 • edited Loading

Test changes on VM

marcotc Oct 15, 2024

Choose a reason for hiding this comment

Guillaume-Barrier Oct 16, 2024

Choose a reason for hiding this comment

dineshg13 left a comment

Choose a reason for hiding this comment

dineshg13 Oct 15, 2024

Choose a reason for hiding this comment

Guillaume-Barrier Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Guillaume-Barrier commented Oct 18, 2024

Guillaume-Barrier commented Oct 11, 2024 •

edited

Loading

bits-bot commented Oct 11, 2024 •

edited

Loading

pr-commenter bot commented Oct 11, 2024 •

edited by cit-pr-commenter bot

Loading

agent-platform-auto-pr bot commented Oct 14, 2024 •

edited

Loading

Guillaume-Barrier Oct 16, 2024 •

edited

Loading