From c6d2de6b60d8fa8b97f5d71f03d86d6813c147e7 Mon Sep 17 00:00:00 2001 From: nayeem-kamal Date: Tue, 18 Mar 2025 16:52:33 -0400 Subject: [PATCH 01/14] added span events functionality and tests --- .../common/writer/DDSpanJsonAdapter.java | 1 + .../main/java/datadog/trace/core/DDSpan.java | 40 ++++++ .../writer/DDSpanJsonAdapterTest.groovy | 135 ++++++++++++++++++ .../datadog/trace/core/DDSpanTest.groovy | 88 ++++++++++++ .../instrumentation/api/DDSpanEvent.java | 71 +++++++++ 5 files changed, 335 insertions(+) create mode 100644 dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDSpanJsonAdapterTest.groovy create mode 100644 internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/DDSpanEvent.java diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/DDSpanJsonAdapter.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/DDSpanJsonAdapter.java index 93c6995d4e8..bff89574be4 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/DDSpanJsonAdapter.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/DDSpanJsonAdapter.java @@ -14,6 +14,7 @@ import java.util.Set; class DDSpanJsonAdapter extends JsonAdapter { + private static final String SPAN_EVENTS = "_dd.span_events"; private final boolean hexIds; DDSpanJsonAdapter(final boolean hexIds) { diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java index 0b576654936..16b043ef388 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java @@ -23,15 +23,18 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink; import datadog.trace.bootstrap.instrumentation.api.AttachableWrapper; +import datadog.trace.bootstrap.instrumentation.api.DDSpanEvent; import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities; import datadog.trace.bootstrap.instrumentation.api.Tags; import java.io.PrintWriter; import java.io.StringWriter; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLongFieldUpdater; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import javax.annotation.Nonnull; @@ -47,6 +50,7 @@ */ public class DDSpan implements AgentSpan, CoreSpan, AttachableWrapper { private static final Logger log = LoggerFactory.getLogger(DDSpan.class); + private static final String SPAN_EVENTS = "_dd.span_events"; static DDSpan create( final String instrumentationName, @@ -109,6 +113,8 @@ static DDSpan create( private final List links; + private final List events = new ArrayList<>(); + /** * Spans should be constructed using the builder, not by calling the constructor directly. * @@ -156,6 +162,17 @@ private void finishAndAddToTrace(final long durationNano) { @Override public void finish() { + if (events != null && !events.isEmpty()) { + StringBuilder eventsJson = new StringBuilder("["); + for (int i = 0; i < events.size(); i++) { + if (i > 0) { + eventsJson.append(","); + } + eventsJson.append(events.get(i).toJson()); + } + eventsJson.append("]"); + setTag(SPAN_EVENTS, eventsJson.toString()); + } if (!externalClock) { // no external clock was used, so we can rely on nano time finishAndAddToTrace(context.getTraceCollector().getCurrentTimeNano() - startTimeNano); @@ -856,4 +873,27 @@ public boolean isOutbound() { Object spanKind = context.getTag(Tags.SPAN_KIND); return Tags.SPAN_KIND_CLIENT.equals(spanKind) || Tags.SPAN_KIND_PRODUCER.equals(spanKind); } + + public AgentSpan addEvent(String name) { + return addEvent(name, null); + } + + public AgentSpan addEvent(String name, Map attributes) { + if (name != null) { + events.add(new DDSpanEvent(name, attributes)); + } + return this; + } + + public AgentSpan addEvent( + String name, Map attributes, long timestamp, TimeUnit unit) { + if (name != null) { + events.add(new DDSpanEvent(name, attributes, unit.toNanos(timestamp))); + } + return this; + } + + public List getEvents() { + return events; + } } diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDSpanJsonAdapterTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDSpanJsonAdapterTest.groovy new file mode 100644 index 00000000000..2335793327c --- /dev/null +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDSpanJsonAdapterTest.groovy @@ -0,0 +1,135 @@ +package datadog.trace.common.writer + +import com.squareup.moshi.Moshi +import com.squareup.moshi.Types +import datadog.trace.core.DDSpan +import datadog.trace.core.test.DDCoreSpecification + +class DDSpanJsonAdapterTest extends DDCoreSpecification { + def tracer = tracerBuilder().writer(new ListWriter()).build() + def adapter = new Moshi.Builder() + .add(DDSpanJsonAdapter.buildFactory(false)) + .build() + .adapter(Types.newParameterizedType(List, DDSpan)) + def genericAdapter = new Moshi.Builder().build().adapter(Object) + + def "test span event serialization"() { + setup: + def span = tracer.buildSpan("test").start() + def eventName = "test-event" + def attributes = ["key1": "value1", "key2": 123] + def timestamp = System.currentTimeMillis() + + when: "adding event with name and attributes" + span.addEvent(eventName, attributes, timestamp, java.util.concurrent.TimeUnit.MILLISECONDS) + span.finish() + def jsonStr = adapter.toJson([span]) + + then: "event is serialized correctly in meta section" + def actual = genericAdapter.fromJson(jsonStr) + def actualSpan = actual[0] + + // Verify basic span fields + actualSpan.service == span.getServiceName() + actualSpan.name == span.getOperationName() + actualSpan.resource == span.getResourceName() + actualSpan.trace_id == span.getTraceId().toLong() + actualSpan.span_id == span.getSpanId() + actualSpan.parent_id == span.getParentId() + actualSpan.start == span.getStartTime() + actualSpan.duration == span.getDurationNano() + actualSpan.error == span.getError() + actualSpan.type == span.getSpanType() + + // Verify span events + def actualEvents = actualSpan.meta["_dd.span_events"] + def expectedEvent = "[{\"time_unix_nano\":${java.util.concurrent.TimeUnit.MILLISECONDS.toNanos(timestamp)},\"name\":\"test-event\",\"attributes\":{\"key1\":\"value1\",\"key2\":123}}]" + actualEvents.toString() == expectedEvent.toString() + + cleanup: + tracer.close() + } + + def "test multiple span events serialization"() { + setup: + def span = tracer.buildSpan("test").start() + def timestamp1 = System.currentTimeMillis() + def timestamp2 = timestamp1 + 1000 + + when: "adding multiple events" + span.addEvent("event1", ["key1": "value1"], timestamp1, java.util.concurrent.TimeUnit.MILLISECONDS) + span.addEvent("event2", ["key2": "value2"], timestamp2, java.util.concurrent.TimeUnit.MILLISECONDS) + span.finish() + def jsonStr = adapter.toJson([span]) + + then: "events are serialized correctly in meta section" + def actual = genericAdapter.fromJson(jsonStr) + def actualSpan = actual[0] + + // Verify basic span fields + actualSpan.service == span.getServiceName() + actualSpan.name == span.getOperationName() + actualSpan.resource == span.getResourceName() + actualSpan.trace_id == span.getTraceId().toLong() + actualSpan.span_id == span.getSpanId() + actualSpan.parent_id == span.getParentId() + actualSpan.start == span.getStartTime() + actualSpan.duration == span.getDurationNano() + actualSpan.error == span.getError() + actualSpan.type == span.getSpanType() + + // Verify span events + def actualEvents = actualSpan.meta["_dd.span_events"] + + def expectedEvents = "[{\"time_unix_nano\":${java.util.concurrent.TimeUnit.MILLISECONDS.toNanos(timestamp1)},\"name\":\"event1\",\"attributes\":{\"key1\":\"value1\"}},{\"time_unix_nano\":${java.util.concurrent.TimeUnit.MILLISECONDS.toNanos(timestamp2)},\"name\":\"event2\",\"attributes\":{\"key2\":\"value2\"}}]" + + actualEvents.toString() == expectedEvents.toString() + + + cleanup: + tracer.close() + } + + def "test span events added directly as tag"() { + setup: + def span = tracer.buildSpan("test").start() + def eventsJson = """[ + { + "time_unix_nano": 1234567890000000, + "name": "manual-event", + "attributes": { + "foo": "bar", + "count": 42 + } + } + ]""" + + when: "adding events JSON directly as a tag" + span.setTag("_dd.span_events", eventsJson) + span.finish() + def jsonStr = adapter.toJson([span]) + + then: "events JSON is preserved exactly in meta section" + def actual = genericAdapter.fromJson(jsonStr) + def actualSpan = actual[0] + + // Verify basic span fields + actualSpan.service == span.getServiceName() + actualSpan.name == span.getOperationName() + actualSpan.resource == span.getResourceName() + actualSpan.trace_id == span.getTraceId().toLong() + actualSpan.span_id == span.getSpanId() + actualSpan.parent_id == span.getParentId() + actualSpan.start == span.getStartTime() + actualSpan.duration == span.getDurationNano() + actualSpan.error == span.getError() + actualSpan.type == span.getSpanType() + + // Verify span events + def actualEvents = actualSpan.meta["_dd.span_events"] + actualEvents.toString() == eventsJson + + cleanup: + tracer.close() + } +} \ No newline at end of file diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy index 8385a42d543..75242bf2ef3 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy @@ -461,4 +461,92 @@ class DDSpanTest extends DDCoreSpecification { then: span.isError() } + + def "span events functionality"() { + setup: + def span = tracer.buildSpan("test").start() + def eventName = "test-event" + def attributes = ["key1": "value1", "key2": 123] + def timestamp = System.currentTimeMillis() + + when: + span.addEvent(eventName) + def events = span.getEvents() + + then: + events.size() == 1 + events[0].name == eventName + events[0].attributes == null || events[0].attributes.isEmpty() + + when: + span = tracer.buildSpan("test").start() + span.addEvent(eventName, attributes) + events = span.getEvents() + + then: + events.size() == 1 + events[0].name == eventName + events[0].attributes == attributes + + when: + span = tracer.buildSpan("test").start() + span.addEvent(eventName, attributes, timestamp, TimeUnit.MILLISECONDS) + events = span.getEvents() + + then: + events.size() == 1 + events[0].name == eventName + events[0].attributes == attributes + events[0].timestampNanos == TimeUnit.MILLISECONDS.toNanos(timestamp) + + when: + span = tracer.buildSpan("test").start() + span.addEvent("event1") + span.addEvent("event2", ["key": "value"]) + span.addEvent("event3", null, timestamp, TimeUnit.MILLISECONDS) + events = span.getEvents() + + then: + events.size() == 3 + events[0].name == "event1" + events[1].name == "event2" + events[2].name == "event3" + events[0].attributes == null || events[0].attributes.isEmpty() + events[1].attributes == ["key": "value"] + events[2].attributes == null || events[2].attributes.isEmpty() + + when: + span = tracer.buildSpan("test").start() + span.addEvent(null) + span.addEvent(null, ["key": "value"]) + span.addEvent(null, null, timestamp, TimeUnit.MILLISECONDS) + events = span.getEvents() + + then: + events.isEmpty() + } + + def "span events added as tag are preserved"() { + setup: + def span = tracer.buildSpan("test").start() + def eventsJson = """[{ + "time_unix_nano": 1234567890000000, + "name": "manual-event", + "attributes": { + "foo": "bar", + "count": 42 + } + }]""" + + when: + span.setTag("_dd.span_events", eventsJson) + def events = span.getEvents() + + then: + span.getTag("_dd.span_events") == eventsJson + events.isEmpty() // The events list should be empty since we're bypassing addEvent() + + cleanup: + span.finish() + } } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/DDSpanEvent.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/DDSpanEvent.java new file mode 100644 index 00000000000..1ee168a7363 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/DDSpanEvent.java @@ -0,0 +1,71 @@ +package datadog.trace.bootstrap.instrumentation.api; + +import datadog.trace.api.time.SystemTimeSource; +import datadog.trace.api.time.TimeSource; +import java.util.Map; + +public class DDSpanEvent { + private static TimeSource timeSource = SystemTimeSource.INSTANCE; + private final String name; + private final Map attributes; + private final long timestampNanos; + + public DDSpanEvent(String name, Map attributes) { + this.name = name; + this.attributes = attributes; + this.timestampNanos = timeSource.getCurrentTimeNanos(); + } + + public DDSpanEvent(String name, Map attributes, long timestamp) { + this.name = name; + this.attributes = attributes; + this.timestampNanos = timestamp; + } + + public String getName() { + return name; + } + + public Map getAttributes() { + return attributes; + } + + public long getTimestampNanos() { + return timestampNanos; + } + + public static void setTimeSource(TimeSource source) { + timeSource = source; + } + + public String toJson() { + StringBuilder json = new StringBuilder(); + json.append("{\"time_unix_nano\":") + .append(timestampNanos) + .append(",\"name\":\"") + .append(name) + .append("\""); + + if (attributes != null && !attributes.isEmpty()) { + json.append(",\"attributes\":{"); + boolean first = true; + for (Map.Entry entry : attributes.entrySet()) { + if (!first) { + json.append(","); + } + json.append("\"").append(entry.getKey()).append("\":"); + Object value = entry.getValue(); + if (value instanceof String) { + json.append("\"").append(value).append("\""); + } else { + json.append(value); + } + first = false; + } + json.append("}"); + } + + json.append("}"); + return json.toString(); + } +} From 0bf493c9a528a25dafa554f2b0087fc099211411 Mon Sep 17 00:00:00 2001 From: nayeem-kamal Date: Tue, 18 Mar 2025 17:38:23 -0400 Subject: [PATCH 02/14] added additional testing for span events --- .../common/writer/DDSpanJsonAdapter.java | 1 - .../api/DDSpanEventTest.groovy | 142 ++++++++++++++++++ .../datadog/trace/core/DDSpanTest.groovy | 2 +- 3 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 dd-trace-core/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/DDSpanEventTest.groovy diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/DDSpanJsonAdapter.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/DDSpanJsonAdapter.java index bff89574be4..93c6995d4e8 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/DDSpanJsonAdapter.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/DDSpanJsonAdapter.java @@ -14,7 +14,6 @@ import java.util.Set; class DDSpanJsonAdapter extends JsonAdapter { - private static final String SPAN_EVENTS = "_dd.span_events"; private final boolean hexIds; DDSpanJsonAdapter(final boolean hexIds) { diff --git a/dd-trace-core/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/DDSpanEventTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/DDSpanEventTest.groovy new file mode 100644 index 00000000000..4dfdf4b7604 --- /dev/null +++ b/dd-trace-core/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/DDSpanEventTest.groovy @@ -0,0 +1,142 @@ +package datadog.trace.bootstrap.instrumentation.api + +import datadog.trace.api.time.SystemTimeSource +import datadog.trace.api.time.TimeSource +import datadog.trace.core.test.DDCoreSpecification +import spock.lang.Shared + +class DDSpanEventTest extends DDCoreSpecification { + @Shared + def mockTimeSource = Mock(TimeSource) + @Shared + def defaultTimestamp = 1234567890000000L + + def setup() { + mockTimeSource = Mock(TimeSource) // Create a fresh mock for each test + DDSpanEvent.setTimeSource(mockTimeSource) + } + + def cleanup() { + DDSpanEvent.setTimeSource(SystemTimeSource.INSTANCE) + } + + def "test event creation with current time"() { + given: + mockTimeSource.getCurrentTimeNanos() >> defaultTimestamp + def name = "test-event" + def attributes = ["key1": "value1", "key2": 123] + + when: + def event = new DDSpanEvent(name, attributes) + + then: + event.getName() == name + event.getAttributes() == attributes + event.getTimestampNanos() == defaultTimestamp + } + + def "test event creation with explicit timestamp"() { + given: + def timestamp = 1742232412103000000L + def name = "test-event" + def attributes = ["key1": "value1", "key2": 123] + + when: + def event = new DDSpanEvent(name, attributes, timestamp) + + then: + 0 * mockTimeSource.getCurrentTimeNanos() + event.getName() == name + event.getAttributes() == attributes + event.getTimestampNanos() == timestamp + } + + def "test event creation with null attributes"() { + given: + mockTimeSource.getCurrentTimeNanos() >> defaultTimestamp + def name = "test-event" + + when: + def event = new DDSpanEvent(name, null) + + then: + event.getName() == name + event.getAttributes() == null + event.getTimestampNanos() == defaultTimestamp + } + + def "test event creation with empty attributes"() { + given: + mockTimeSource.getCurrentTimeNanos() >> defaultTimestamp + def name = "test-event" + def attributes = [:] + + when: + def event = new DDSpanEvent(name, attributes) + + then: + event.getName() == name + event.getAttributes() == attributes + event.getTimestampNanos() == defaultTimestamp + } + + def "test toJson with different attribute types"() { + given: + def timestamp = 1742232412103000000L + def name = "test-event" + def attributes = [ + "string": "value", + "number": 42, + "boolean": true, + "null": null + ] + + when: + def event = new DDSpanEvent(name, attributes, timestamp) + def json = event.toJson() + + then: + json == """{"time_unix_nano":${timestamp},"name":"${name}","attributes":{"string":"value","number":42,"boolean":true,"null":null}}""" + } + + def "test toJson with null attributes"() { + given: + def timestamp = 1742232412103000000L + def name = "test-event" + + when: + def event = new DDSpanEvent(name, null, timestamp) + def json = event.toJson() + + then: + json == """{"time_unix_nano":${timestamp},"name":"${name}"}""" + } + + def "test toJson with empty attributes"() { + given: + def timestamp = 1742232412103000000L + def name = "test-event" + def attributes = [:] + + when: + def event = new DDSpanEvent(name, attributes, timestamp) + def json = event.toJson() + + then: + json == """{"time_unix_nano":${timestamp},"name":"${name}"}""" + } + + def "test time source change"() { + given: + def newTimeSource = Mock(TimeSource) + def timestamp = 1742232412103000000L + newTimeSource.getCurrentTimeNanos() >> timestamp + + when: + DDSpanEvent.setTimeSource(newTimeSource) + def event = new DDSpanEvent("test", [:]) + + then: + event.getTimestampNanos() == timestamp + } +} \ No newline at end of file diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy index 75242bf2ef3..552bc6b804b 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy @@ -462,7 +462,7 @@ class DDSpanTest extends DDCoreSpecification { span.isError() } - def "span events functionality"() { + def "span events are attached"() { setup: def span = tracer.buildSpan("test").start() def eventName = "test-event" From d896cbb2a7b9b6617c7855f4a1d34f36d4365dbf Mon Sep 17 00:00:00 2001 From: Nayeem Kamal Date: Tue, 18 Mar 2025 17:44:49 -0400 Subject: [PATCH 03/14] Apply suggestions from code review Co-authored-by: datadog-datadog-prod-us1[bot] <88084959+datadog-datadog-prod-us1[bot]@users.noreply.github.com> --- dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java | 2 +- .../trace/bootstrap/instrumentation/api/DDSpanEvent.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java index 16b043ef388..15c73647739 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java @@ -170,7 +170,7 @@ public void finish() { } eventsJson.append(events.get(i).toJson()); } - eventsJson.append("]"); + eventsJson.append(']'); setTag(SPAN_EVENTS, eventsJson.toString()); } if (!externalClock) { diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/DDSpanEvent.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/DDSpanEvent.java index 1ee168a7363..329b33e6c78 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/DDSpanEvent.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/DDSpanEvent.java @@ -65,7 +65,7 @@ public String toJson() { json.append("}"); } - json.append("}"); + json.append('}'); return json.toString(); } } From 1053c560144028ac7ab66be90eb71acff66e4ec5 Mon Sep 17 00:00:00 2001 From: Marco Costa Date: Mon, 24 Mar 2025 17:37:14 -0700 Subject: [PATCH 04/14] Add SpanNativeAttributes --- .../main/java/datadog/trace/core/DDSpan.java | 24 +- .../datadog/trace/core/DDSpanContext.java | 10 +- .../java/datadog/trace/core/DDSpanEvent.java | 291 +++++++++++++++ .../api/DDSpanEventTest.groovy | 142 -------- .../writer/DDSpanJsonAdapterTest.groovy | 67 +--- .../datadog/trace/core/DDSpanEventTest.groovy | 253 +++++++++++++ .../datadog/trace/core/DDSpanTest.groovy | 48 +-- .../instrumentation/api/DDSpanEvent.java | 71 ---- .../api/SpanNativeAttributes.java | 340 ++++++++++++++++++ .../api/SpanNativeAttributesTest.groovy | 221 ++++++++++++ .../trace/test/util/DDSpecification.groovy | 7 + 11 files changed, 1160 insertions(+), 314 deletions(-) create mode 100644 dd-trace-core/src/main/java/datadog/trace/core/DDSpanEvent.java delete mode 100644 dd-trace-core/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/DDSpanEventTest.groovy create mode 100644 dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanEventTest.groovy delete mode 100644 internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/DDSpanEvent.java create mode 100644 internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributes.java create mode 100644 internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java index 15c73647739..1889059d5e8 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java @@ -23,13 +23,12 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink; import datadog.trace.bootstrap.instrumentation.api.AttachableWrapper; -import datadog.trace.bootstrap.instrumentation.api.DDSpanEvent; import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities; +import datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes; import datadog.trace.bootstrap.instrumentation.api.Tags; import java.io.PrintWriter; import java.io.StringWriter; -import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; @@ -50,7 +49,6 @@ */ public class DDSpan implements AgentSpan, CoreSpan, AttachableWrapper { private static final Logger log = LoggerFactory.getLogger(DDSpan.class); - private static final String SPAN_EVENTS = "_dd.span_events"; static DDSpan create( final String instrumentationName, @@ -113,7 +111,7 @@ static DDSpan create( private final List links; - private final List events = new ArrayList<>(); + private final List events; /** * Spans should be constructed using the builder, not by calling the constructor directly. @@ -142,6 +140,7 @@ private DDSpan( } this.links = links == null ? new CopyOnWriteArrayList<>() : new CopyOnWriteArrayList<>(links); + this.events = new CopyOnWriteArrayList<>(); } public boolean isFinished() { @@ -162,17 +161,6 @@ private void finishAndAddToTrace(final long durationNano) { @Override public void finish() { - if (events != null && !events.isEmpty()) { - StringBuilder eventsJson = new StringBuilder("["); - for (int i = 0; i < events.size(); i++) { - if (i > 0) { - eventsJson.append(","); - } - eventsJson.append(events.get(i).toJson()); - } - eventsJson.append(']'); - setTag(SPAN_EVENTS, eventsJson.toString()); - } if (!externalClock) { // no external clock was used, so we can rely on nano time finishAndAddToTrace(context.getTraceCollector().getCurrentTimeNano() - startTimeNano); @@ -721,7 +709,7 @@ public CharSequence getType() { @Override public void processTagsAndBaggage(final MetadataConsumer consumer) { - context.processTagsAndBaggage(consumer, longRunningVersion, links); + context.processTagsAndBaggage(consumer, longRunningVersion, links, events); } @Override @@ -878,7 +866,7 @@ public AgentSpan addEvent(String name) { return addEvent(name, null); } - public AgentSpan addEvent(String name, Map attributes) { + public AgentSpan addEvent(String name, SpanNativeAttributes attributes) { if (name != null) { events.add(new DDSpanEvent(name, attributes)); } @@ -886,7 +874,7 @@ public AgentSpan addEvent(String name, Map attributes) { } public AgentSpan addEvent( - String name, Map attributes, long timestamp, TimeUnit unit) { + String name, SpanNativeAttributes attributes, long timestamp, TimeUnit unit) { if (name != null) { events.add(new DDSpanEvent(name, attributes, unit.toNanos(timestamp))); } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index bce7d8800f3..9e41300a64d 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -1,5 +1,6 @@ package datadog.trace.core; +import static datadog.trace.api.DDTags.SPAN_EVENTS; import static datadog.trace.api.DDTags.SPAN_LINKS; import static datadog.trace.api.cache.RadixTreeCache.HTTP_STATUSES; import static datadog.trace.bootstrap.instrumentation.api.ErrorPriorities.UNSET; @@ -845,7 +846,10 @@ public void setMetaStruct(final String field, final T value) { } public void processTagsAndBaggage( - final MetadataConsumer consumer, int longRunningVersion, List links) { + final MetadataConsumer consumer, + int longRunningVersion, + List links, + List events) { synchronized (unsafeTags) { // Tags Map tags = @@ -854,6 +858,10 @@ public void processTagsAndBaggage( if (linksTag != null) { tags.put(SPAN_LINKS, linksTag); } + String eventsTag = DDSpanEvent.toTag(events); + if (events != null && !events.isEmpty()) { + tags.put(SPAN_EVENTS, eventsTag); + } // Baggage Map baggageItemsWithPropagationTags; if (injectBaggageAsTags) { diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanEvent.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanEvent.java new file mode 100644 index 00000000000..5f1d8a38750 --- /dev/null +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanEvent.java @@ -0,0 +1,291 @@ +package datadog.trace.core; + +import com.squareup.moshi.FromJson; +import com.squareup.moshi.JsonAdapter; +import com.squareup.moshi.JsonReader; +import com.squareup.moshi.JsonWriter; +import com.squareup.moshi.Moshi; +import com.squareup.moshi.ToJson; +import datadog.trace.api.time.SystemTimeSource; +import datadog.trace.api.time.TimeSource; +import datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes; +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * A Span Event is a non-functional, instantaneous telemetry signal that happens during the + * execution of a Span. They have a `name`, which creates a semantic separation between events. They + * also have `attributes`, which are key-value pairs that can contain any arbitrary data about the + * event. The `attributes` are normally semantically defined based on the `name` of the event. This + * data model closely follows the OpenTelemetry specification. + * + * @see OpenTelemetry + * Span Events + */ +public class DDSpanEvent { + private static final Logger LOGGER = LoggerFactory.getLogger(DDSpanEvent.class); + private static final int TAG_MAX_LENGTH = 25_000; + private static TimeSource timeSource = SystemTimeSource.INSTANCE; + private final String name; + private final SpanNativeAttributes attributes; + private final long timestampNanos; + + public DDSpanEvent(String name, SpanNativeAttributes attributes) { + this(name, attributes, timeSource.getCurrentTimeNanos()); + } + + public DDSpanEvent(String name, SpanNativeAttributes attributes, long timestamp) { + this.name = name; + this.attributes = attributes; + this.timestampNanos = timestamp; + } + + /** + * The name of the span event. + * + * @return event name + */ + public String getName() { + return name; + } + + /** + * The attributes of the span event. + * + * @return event attributes + */ + public SpanNativeAttributes getAttributes() { + return attributes; + } + + /** + * The timestamp of the span event in nanoseconds. + * + * @return event timestamp + */ + public long getTimestampNanos() { + return timestampNanos; + } + + public static void setTimeSource(TimeSource source) { + timeSource = source; + } + + public String toJson() { + return getAdapter().toJson(this); + } + + /** + * Encode a span event collection into a Span tag value. + * + * @param events The span event collection to encode. + * @return The encoded tag value, {@code null} if no events. + */ + public static String toTag(List events) { + if (events == null || events.isEmpty()) { + return null; + } + // Manually encode as JSON array + StringBuilder builder = new StringBuilder("["); + int index = 0; + while (index < events.size()) { + String eventAsJson = events.get(index).toJson(); + int arrayCharsNeeded = index == 0 ? 1 : 2; // Closing bracket and comma separator if needed + if (eventAsJson.length() + builder.length() + arrayCharsNeeded >= TAG_MAX_LENGTH) { + // Do no more fit inside a span tag, stop adding span events + break; + } + if (index > 0) { + builder.append(','); + } + builder.append(eventAsJson); + index++; + } + // Notify of dropped events + while (index < events.size()) { + LOGGER.debug("Span tag full. Dropping span events {}", events.get(index)); + index++; + } + return builder.append(']').toString(); + } + + private static JsonAdapter getAdapter() { + return AdapterHolder.ADAPTER; + } + + private static class AdapterHolder { + static final JsonAdapter ADAPTER = createAdapter(); + + private static JsonAdapter createAdapter() { + Moshi moshi = new Moshi.Builder().add(new DDSpanEventAdapter()).build(); + return moshi.adapter(DDSpanEvent.class); + } + } + + /** Custom JSON adapter for {@link DDSpanEvent} objects. */ + private static class DDSpanEventAdapter extends JsonAdapter { + @FromJson + @Override + public DDSpanEvent fromJson(JsonReader reader) throws IOException { + reader.beginObject(); + String name = null; + SpanNativeAttributes attributes = null; + long timestampNanos = 0; + + while (reader.hasNext()) { + switch (reader.nextName()) { + case "name": + name = reader.nextString(); + break; + case "time_unix_nano": + timestampNanos = reader.nextLong(); + break; + case "attributes": + if (reader.peek() == JsonReader.Token.NULL) { + reader.nextNull(); + attributes = SpanNativeAttributes.EMPTY; + } else { + attributes = readAttributes(reader); + } + break; + default: + reader.skipValue(); + } + } + + reader.endObject(); + return new DDSpanEvent(name, attributes, timestampNanos); + } + + @ToJson + @Override + public void toJson(JsonWriter writer, DDSpanEvent value) throws IOException { + writer.beginObject(); + writer.name("time_unix_nano").value(value.timestampNanos); + writer.name("name").value(value.name); + + if (value.attributes != null && !value.attributes.isEmpty()) { + writer.name("attributes"); + writeAttributes(writer, value.attributes); + } + + writer.endObject(); + } + + private SpanNativeAttributes readAttributes(JsonReader reader) throws IOException { + Map, Object> attributes = new HashMap<>(); + reader.beginObject(); + while (reader.hasNext()) { + String key = reader.nextName(); + SpanNativeAttributes.AttributeKey attributeKey = + SpanNativeAttributes.AttributeKey.stringKey(key); + Object value = readValue(reader); + attributes.put(attributeKey, value); + } + reader.endObject(); + return new SpanNativeAttributes(attributes); + } + + private void writeAttributes(JsonWriter writer, SpanNativeAttributes attributes) + throws IOException { + writer.beginObject(); + for (Map.Entry, Object> entry : + attributes.data().entrySet()) { + writer.name(entry.getKey().getKey()); + writeValue(writer, entry.getKey(), entry.getValue()); + } + writer.endObject(); + } + + private Object readValue(JsonReader reader) throws IOException { + switch (reader.peek()) { + case STRING: + return reader.nextString(); + case NUMBER: + return reader.nextDouble(); + case BOOLEAN: + return reader.nextBoolean(); + case NULL: + return reader.nextNull(); + case BEGIN_ARRAY: + reader.beginArray(); + List array = new ArrayList<>(); + while (reader.hasNext()) { + array.add(readValue(reader)); + } + reader.endArray(); + return array; + case BEGIN_OBJECT: + reader.beginObject(); + Map object = new HashMap<>(); + while (reader.hasNext()) { + String key = reader.nextName(); + object.put(key, readValue(reader)); + } + reader.endObject(); + return object; + default: + throw new IOException("Unexpected token: " + reader.peek()); + } + } + + private void writeValue( + JsonWriter writer, SpanNativeAttributes.AttributeKey key, Object value) + throws IOException { + if (value == null) { + return; + } + + switch (key.getType()) { + case STRING: + writer.value((String) value); + break; + case BOOLEAN: + writer.value((Boolean) value); + break; + case LONG: + writer.value((Long) value); + break; + case DOUBLE: + writer.value((Double) value); + break; + case STRING_ARRAY: + writer.beginArray(); + for (String item : (List) value) { + writer.value(item); + } + writer.endArray(); + break; + case BOOLEAN_ARRAY: + writer.beginArray(); + for (Boolean item : (List) value) { + writer.value(item); + } + writer.endArray(); + break; + case LONG_ARRAY: + writer.beginArray(); + for (Long item : (List) value) { + writer.value(item); + } + writer.endArray(); + break; + case DOUBLE_ARRAY: + writer.beginArray(); + for (Double item : (List) value) { + writer.value(item); + } + writer.endArray(); + break; + default: + // Not a valid type + } + } + } +} diff --git a/dd-trace-core/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/DDSpanEventTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/DDSpanEventTest.groovy deleted file mode 100644 index 4dfdf4b7604..00000000000 --- a/dd-trace-core/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/DDSpanEventTest.groovy +++ /dev/null @@ -1,142 +0,0 @@ -package datadog.trace.bootstrap.instrumentation.api - -import datadog.trace.api.time.SystemTimeSource -import datadog.trace.api.time.TimeSource -import datadog.trace.core.test.DDCoreSpecification -import spock.lang.Shared - -class DDSpanEventTest extends DDCoreSpecification { - @Shared - def mockTimeSource = Mock(TimeSource) - @Shared - def defaultTimestamp = 1234567890000000L - - def setup() { - mockTimeSource = Mock(TimeSource) // Create a fresh mock for each test - DDSpanEvent.setTimeSource(mockTimeSource) - } - - def cleanup() { - DDSpanEvent.setTimeSource(SystemTimeSource.INSTANCE) - } - - def "test event creation with current time"() { - given: - mockTimeSource.getCurrentTimeNanos() >> defaultTimestamp - def name = "test-event" - def attributes = ["key1": "value1", "key2": 123] - - when: - def event = new DDSpanEvent(name, attributes) - - then: - event.getName() == name - event.getAttributes() == attributes - event.getTimestampNanos() == defaultTimestamp - } - - def "test event creation with explicit timestamp"() { - given: - def timestamp = 1742232412103000000L - def name = "test-event" - def attributes = ["key1": "value1", "key2": 123] - - when: - def event = new DDSpanEvent(name, attributes, timestamp) - - then: - 0 * mockTimeSource.getCurrentTimeNanos() - event.getName() == name - event.getAttributes() == attributes - event.getTimestampNanos() == timestamp - } - - def "test event creation with null attributes"() { - given: - mockTimeSource.getCurrentTimeNanos() >> defaultTimestamp - def name = "test-event" - - when: - def event = new DDSpanEvent(name, null) - - then: - event.getName() == name - event.getAttributes() == null - event.getTimestampNanos() == defaultTimestamp - } - - def "test event creation with empty attributes"() { - given: - mockTimeSource.getCurrentTimeNanos() >> defaultTimestamp - def name = "test-event" - def attributes = [:] - - when: - def event = new DDSpanEvent(name, attributes) - - then: - event.getName() == name - event.getAttributes() == attributes - event.getTimestampNanos() == defaultTimestamp - } - - def "test toJson with different attribute types"() { - given: - def timestamp = 1742232412103000000L - def name = "test-event" - def attributes = [ - "string": "value", - "number": 42, - "boolean": true, - "null": null - ] - - when: - def event = new DDSpanEvent(name, attributes, timestamp) - def json = event.toJson() - - then: - json == """{"time_unix_nano":${timestamp},"name":"${name}","attributes":{"string":"value","number":42,"boolean":true,"null":null}}""" - } - - def "test toJson with null attributes"() { - given: - def timestamp = 1742232412103000000L - def name = "test-event" - - when: - def event = new DDSpanEvent(name, null, timestamp) - def json = event.toJson() - - then: - json == """{"time_unix_nano":${timestamp},"name":"${name}"}""" - } - - def "test toJson with empty attributes"() { - given: - def timestamp = 1742232412103000000L - def name = "test-event" - def attributes = [:] - - when: - def event = new DDSpanEvent(name, attributes, timestamp) - def json = event.toJson() - - then: - json == """{"time_unix_nano":${timestamp},"name":"${name}"}""" - } - - def "test time source change"() { - given: - def newTimeSource = Mock(TimeSource) - def timestamp = 1742232412103000000L - newTimeSource.getCurrentTimeNanos() >> timestamp - - when: - DDSpanEvent.setTimeSource(newTimeSource) - def event = new DDSpanEvent("test", [:]) - - then: - event.getTimestampNanos() == timestamp - } -} \ No newline at end of file diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDSpanJsonAdapterTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDSpanJsonAdapterTest.groovy index 2335793327c..0d5e81d6f92 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDSpanJsonAdapterTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDSpanJsonAdapterTest.groovy @@ -4,6 +4,7 @@ import com.squareup.moshi.Moshi import com.squareup.moshi.Types import datadog.trace.core.DDSpan import datadog.trace.core.test.DDCoreSpecification +import datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes class DDSpanJsonAdapterTest extends DDCoreSpecification { def tracer = tracerBuilder().writer(new ListWriter()).build() @@ -17,7 +18,11 @@ class DDSpanJsonAdapterTest extends DDCoreSpecification { setup: def span = tracer.buildSpan("test").start() def eventName = "test-event" - def attributes = ["key1": "value1", "key2": 123] + def attributes = SpanNativeAttributes.builder() + .put("key1", "value1") + .put("key2", 123L) + .putDoubleArray("key3", [1.1d, 2.2d, 3.3d]) + .build() def timestamp = System.currentTimeMillis() when: "adding event with name and attributes" @@ -42,8 +47,8 @@ class DDSpanJsonAdapterTest extends DDCoreSpecification { actualSpan.type == span.getSpanType() // Verify span events - def actualEvents = actualSpan.meta["_dd.span_events"] - def expectedEvent = "[{\"time_unix_nano\":${java.util.concurrent.TimeUnit.MILLISECONDS.toNanos(timestamp)},\"name\":\"test-event\",\"attributes\":{\"key1\":\"value1\",\"key2\":123}}]" + 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]}}]" actualEvents.toString() == expectedEvent.toString() cleanup: @@ -57,8 +62,12 @@ class DDSpanJsonAdapterTest extends DDCoreSpecification { def timestamp2 = timestamp1 + 1000 when: "adding multiple events" - span.addEvent("event1", ["key1": "value1"], timestamp1, java.util.concurrent.TimeUnit.MILLISECONDS) - span.addEvent("event2", ["key2": "value2"], timestamp2, java.util.concurrent.TimeUnit.MILLISECONDS) + span.addEvent("event1", SpanNativeAttributes.builder() + .put("key1", "value1") + .build(), timestamp1, java.util.concurrent.TimeUnit.MILLISECONDS) + span.addEvent("event2", SpanNativeAttributes.builder() + .put("key2", "value2") + .build(), timestamp2, java.util.concurrent.TimeUnit.MILLISECONDS) span.finish() def jsonStr = adapter.toJson([span]) @@ -79,57 +88,13 @@ class DDSpanJsonAdapterTest extends DDCoreSpecification { actualSpan.type == span.getSpanType() // Verify span events - def actualEvents = actualSpan.meta["_dd.span_events"] + def actualEvents = actualSpan.meta["events"] def expectedEvents = "[{\"time_unix_nano\":${java.util.concurrent.TimeUnit.MILLISECONDS.toNanos(timestamp1)},\"name\":\"event1\",\"attributes\":{\"key1\":\"value1\"}},{\"time_unix_nano\":${java.util.concurrent.TimeUnit.MILLISECONDS.toNanos(timestamp2)},\"name\":\"event2\",\"attributes\":{\"key2\":\"value2\"}}]" actualEvents.toString() == expectedEvents.toString() - - cleanup: - tracer.close() - } - - def "test span events added directly as tag"() { - setup: - def span = tracer.buildSpan("test").start() - def eventsJson = """[ - { - "time_unix_nano": 1234567890000000, - "name": "manual-event", - "attributes": { - "foo": "bar", - "count": 42 - } - } - ]""" - - when: "adding events JSON directly as a tag" - span.setTag("_dd.span_events", eventsJson) - span.finish() - def jsonStr = adapter.toJson([span]) - - then: "events JSON is preserved exactly in meta section" - def actual = genericAdapter.fromJson(jsonStr) - def actualSpan = actual[0] - - // Verify basic span fields - actualSpan.service == span.getServiceName() - actualSpan.name == span.getOperationName() - actualSpan.resource == span.getResourceName() - actualSpan.trace_id == span.getTraceId().toLong() - actualSpan.span_id == span.getSpanId() - actualSpan.parent_id == span.getParentId() - actualSpan.start == span.getStartTime() - actualSpan.duration == span.getDurationNano() - actualSpan.error == span.getError() - actualSpan.type == span.getSpanType() - - // Verify span events - def actualEvents = actualSpan.meta["_dd.span_events"] - actualEvents.toString() == eventsJson - cleanup: tracer.close() } -} \ No newline at end of file +} diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanEventTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanEventTest.groovy new file mode 100644 index 00000000000..b893b271822 --- /dev/null +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanEventTest.groovy @@ -0,0 +1,253 @@ +package datadog.trace.core + +import datadog.trace.api.time.SystemTimeSource +import datadog.trace.api.time.TimeSource +import datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes +import datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey +import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.stringKey +import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.booleanKey +import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.longKey +import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.doubleKey +import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.stringArrayKey +import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.booleanArrayKey +import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.longArrayKey +import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.doubleArrayKey +import datadog.trace.core.test.DDCoreSpecification +import spock.lang.Shared + +class DDSpanEventTest extends DDCoreSpecification { + @Shared + def mockTimeSource = Mock(TimeSource) + @Shared + def defaultTimestamp = 1234567890000000L + + def setup() { + mockTimeSource = Mock(TimeSource) // Create a fresh mock for each test + DDSpanEvent.setTimeSource(mockTimeSource) + } + + def cleanup() { + DDSpanEvent.setTimeSource(SystemTimeSource.INSTANCE) + } + + def "test event creation with current time"() { + given: + mockTimeSource.getCurrentTimeNanos() >> defaultTimestamp + def name = "test-event" + def attributes = SpanNativeAttributes.builder() + .put("key1", "value1") + .put("key2", 123L) + .build() + + when: + def event = new DDSpanEvent(name, attributes) + + then: + event.getName() == name + event.getAttributes() == attributes + event.getTimestampNanos() == defaultTimestamp + } + + def "test event creation with explicit timestamp"() { + given: + def timestamp = 1742232412103000000L + def name = "test-event" + def attributes = SpanNativeAttributes.builder() + .put("key1", "value1") + .put("key2", 123L) + .build() + + when: + def event = new DDSpanEvent(name, attributes, timestamp) + + then: + 0 * mockTimeSource.getCurrentTimeNanos() + event.getName() == name + event.getAttributes() == attributes + event.getTimestampNanos() == timestamp + } + + def "test time source change"() { + given: + def newTimeSource = Mock(TimeSource) + def timestamp = 1742232412103000000L + newTimeSource.getCurrentTimeNanos() >> timestamp + + when: + DDSpanEvent.setTimeSource(newTimeSource) + def event = new DDSpanEvent("test", SpanNativeAttributes.EMPTY) + + then: + event.getTimestampNanos() == timestamp + } + + def "test event creation with null attributes"() { + given: + mockTimeSource.getCurrentTimeNanos() >> defaultTimestamp + def name = "test-event" + + when: + def event = new DDSpanEvent(name, null) + + then: + event.getName() == name + event.getAttributes() == null + event.getTimestampNanos() == defaultTimestamp + } + + def "test event creation with empty attributes"() { + given: + mockTimeSource.getCurrentTimeNanos() >> defaultTimestamp + def name = "test-event" + def attributes = SpanNativeAttributes.EMPTY + + when: + def event = new DDSpanEvent(name, attributes) + + then: + event.getName() == name + event.getAttributes() == attributes + event.getTimestampNanos() == defaultTimestamp + } + + def "test toJson with different attribute types"() { + given: + def timestamp = 1742232412103000000L + def name = "test-event" + def attributes = SpanNativeAttributes.builder() + .put("boolean", true) + .put("string", "value") + .put("int", 42L) + .put("long", 123L) + .build() + + when: + def event = new DDSpanEvent(name, attributes, timestamp) + def json = event.toJson() + + then: + json == """{"time_unix_nano":${timestamp},"name":"${name}","attributes":{"boolean":true,"string":"value","int":42,"long":123}}""" + } + + def "test toJson with array attribute types"() { + given: + def timestamp = 1742232412103000000L + def name = "test-event" + def attributes = SpanNativeAttributes.builder() + .putStringArray("strings", ["a", "b", "c"]) + .putBooleanArray("booleans", [true, false, true]) + .putLongArray("longs", [1L, 2L, 3L]) + .putDoubleArray("doubles", [1.1d, 2.2d, 3.3d]) + .build() + + when: + def event = new DDSpanEvent(name, attributes, timestamp) + def json = event.toJson() + + then: + json == """{"time_unix_nano":${timestamp},"name":"${name}","attributes":{"strings":["a","b","c"],"booleans":[true,false,true],"longs":[1,2,3],"doubles":[1.1,2.2,3.3]}}""" + } + + def "test toJson with null attributes"() { + given: + def timestamp = 1742232412103000000L + def name = "test-event" + + when: + def event = new DDSpanEvent(name, null, timestamp) + def json = event.toJson() + + then: + json == """{"time_unix_nano":${timestamp},"name":"${name}"}""" + } + + def "test toJson with empty attributes"() { + given: + def timestamp = 1742232412103000000L + def name = "test-event" + def attributes = SpanNativeAttributes.EMPTY + + when: + def event = new DDSpanEvent(name, attributes, timestamp) + def json = event.toJson() + + then: + json == """{"time_unix_nano":${timestamp},"name":"${name}"}""" + } + + def "test toJson with null values in attributes"() { + setup: + def eventName = "test-event" + def attributes = SpanNativeAttributes.builder() + // Test null values are skipped for all types + .put("nullString", null as String) + .put("nullBoolean", null as Boolean) + .put("nullLong", null as Long) + .put("nullDouble", null as Double) + .putStringArray("nullStringArray", null) + .putBooleanArray("nullBooleanArray", null) + .putLongArray("nullLongArray", null) + .putDoubleArray("nullDoubleArray", null) + .build() + def event = new DDSpanEvent(eventName, attributes) + + when: + def json = event.toJson() + + then: + def expectedJson = """{"time_unix_nano":${event.timestampNanos},"name":"test-event"}""" + json == expectedJson + } + + def "test toJson with empty arrays"() { + given: + def timestamp = 1742232412103000000L + def name = "test-event" + def attributes = SpanNativeAttributes.builder() + .putStringArray("strings", []) + .putBooleanArray("booleans", []) + .putLongArray("longs", []) + .putDoubleArray("doubles", []) + .build() + + when: + def event = new DDSpanEvent(name, attributes, timestamp) + def json = event.toJson() + + then: + json == """{"time_unix_nano":${timestamp},"name":"${name}","attributes":{"strings":[],"booleans":[],"longs":[],"doubles":[]}}""" + } + + def "test toTag with multiple events"() { + given: + def events = [ + new DDSpanEvent("event1", SpanNativeAttributes.builder().put("key1", "value1").build()), + new DDSpanEvent("event2", SpanNativeAttributes.builder().put("key2", "value2").build()) + ] + + when: + def tag = DDSpanEvent.toTag(events) + + then: + tag.contains("event1") + tag.contains("event2") + tag.contains("key1") + tag.contains("key2") + } + + def "test toTag with empty events list"() { + when: + def tag = DDSpanEvent.toTag([]) + + then: + tag == null + } + + def "test toTag with null events list"() { + when: + def tag = DDSpanEvent.toTag(null) + + then: + tag == null + } +} diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy index 552bc6b804b..f33ba7b1f7a 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy @@ -8,6 +8,7 @@ import datadog.trace.api.sampling.PrioritySampling import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext import datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopPathwayContext import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities +import datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes import datadog.trace.bootstrap.instrumentation.api.TagContext import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString import datadog.trace.common.sampling.RateByServiceTraceSampler @@ -462,11 +463,14 @@ class DDSpanTest extends DDCoreSpecification { span.isError() } - def "span events are attached"() { + def "span events are added"() { setup: def span = tracer.buildSpan("test").start() def eventName = "test-event" - def attributes = ["key1": "value1", "key2": 123] + def attributes = SpanNativeAttributes.builder() + .put("key1", "value1") + .put("key2", 123L) + .build() def timestamp = System.currentTimeMillis() when: @@ -502,7 +506,9 @@ class DDSpanTest extends DDCoreSpecification { when: span = tracer.buildSpan("test").start() span.addEvent("event1") - span.addEvent("event2", ["key": "value"]) + span.addEvent("event2", SpanNativeAttributes.builder() + .put("key", "value") + .build()) span.addEvent("event3", null, timestamp, TimeUnit.MILLISECONDS) events = span.getEvents() @@ -511,42 +517,22 @@ class DDSpanTest extends DDCoreSpecification { events[0].name == "event1" events[1].name == "event2" events[2].name == "event3" - events[0].attributes == null || events[0].attributes.isEmpty() - events[1].attributes == ["key": "value"] - events[2].attributes == null || events[2].attributes.isEmpty() + events[0].attributes == null + events[1].attributes == SpanNativeAttributes.builder() + .put("key", "value") + .build() + events[2].attributes == null when: span = tracer.buildSpan("test").start() span.addEvent(null) - span.addEvent(null, ["key": "value"]) + span.addEvent(null, SpanNativeAttributes.builder() + .put("key", "value") + .build()) span.addEvent(null, null, timestamp, TimeUnit.MILLISECONDS) events = span.getEvents() then: events.isEmpty() } - - def "span events added as tag are preserved"() { - setup: - def span = tracer.buildSpan("test").start() - def eventsJson = """[{ - "time_unix_nano": 1234567890000000, - "name": "manual-event", - "attributes": { - "foo": "bar", - "count": 42 - } - }]""" - - when: - span.setTag("_dd.span_events", eventsJson) - def events = span.getEvents() - - then: - span.getTag("_dd.span_events") == eventsJson - events.isEmpty() // The events list should be empty since we're bypassing addEvent() - - cleanup: - span.finish() - } } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/DDSpanEvent.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/DDSpanEvent.java deleted file mode 100644 index 329b33e6c78..00000000000 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/DDSpanEvent.java +++ /dev/null @@ -1,71 +0,0 @@ -package datadog.trace.bootstrap.instrumentation.api; - -import datadog.trace.api.time.SystemTimeSource; -import datadog.trace.api.time.TimeSource; -import java.util.Map; - -public class DDSpanEvent { - private static TimeSource timeSource = SystemTimeSource.INSTANCE; - private final String name; - private final Map attributes; - private final long timestampNanos; - - public DDSpanEvent(String name, Map attributes) { - this.name = name; - this.attributes = attributes; - this.timestampNanos = timeSource.getCurrentTimeNanos(); - } - - public DDSpanEvent(String name, Map attributes, long timestamp) { - this.name = name; - this.attributes = attributes; - this.timestampNanos = timestamp; - } - - public String getName() { - return name; - } - - public Map getAttributes() { - return attributes; - } - - public long getTimestampNanos() { - return timestampNanos; - } - - public static void setTimeSource(TimeSource source) { - timeSource = source; - } - - public String toJson() { - StringBuilder json = new StringBuilder(); - json.append("{\"time_unix_nano\":") - .append(timestampNanos) - .append(",\"name\":\"") - .append(name) - .append("\""); - - if (attributes != null && !attributes.isEmpty()) { - json.append(",\"attributes\":{"); - boolean first = true; - for (Map.Entry entry : attributes.entrySet()) { - if (!first) { - json.append(","); - } - json.append("\"").append(entry.getKey()).append("\":"); - Object value = entry.getValue(); - if (value instanceof String) { - json.append("\"").append(value).append("\""); - } else { - json.append(value); - } - first = false; - } - json.append("}"); - } - - json.append('}'); - return json.toString(); - } -} diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributes.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributes.java new file mode 100644 index 00000000000..499f15c1ae0 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributes.java @@ -0,0 +1,340 @@ +package datadog.trace.bootstrap.instrumentation.api; + +import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.booleanArrayKey; +import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.booleanKey; +import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.doubleArrayKey; +import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.doubleKey; +import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.longArrayKey; +import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.longKey; +import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.stringArrayKey; +import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.stringKey; +import static java.util.Objects.requireNonNull; + +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +/** + * Represents span attributes (used by {@link SpanLink} and {@link DDSpanEvent}). Unlike {@link + * SpanAttributes}, this class supports all valid types for attribute values, while {@link + * SpanAttributes} stringify all values. The data structure is modeled closely after the `Attribute` + * in OpenTelemetry. + * + * @see OpenTelemetry + * Attribute + */ +public class SpanNativeAttributes { + public static final SpanNativeAttributes EMPTY = new SpanNativeAttributes(Collections.emptyMap()); + + private final Map, Object> attributes; + + /* Use the {#builder} to create instances of this class */ + public SpanNativeAttributes(Map, Object> attributes) { + this.attributes = attributes; + } + + /** + * Gets a builder to create attributes. + * + * @return A builder to create attributes. + */ + public static Builder builder() { + return new Builder(); + } + + /** + * Returns the complete set of attributes. + * + * @return The attributes map. + */ + public Map, Object> data() { + return attributes; + } + + public boolean isEmpty() { + return this.attributes.isEmpty(); + } + + @Override + public String toString() { + return "SpanNativeAttributes{" + this.attributes + '}'; + } + + @Override + public boolean equals(Object o) { + if (o == this) { + return true; + } + if (o instanceof SpanNativeAttributes) { + SpanNativeAttributes that = (SpanNativeAttributes) o; + return this.attributes.equals(that.data()); + } + return false; + } + + /** Builder to ensure type-safe attribute creation. */ + public static class Builder { + private final Map, Object> attributes; + + protected Builder() { + this.attributes = new LinkedHashMap<>(); + } + + /** + * Adds an attribute key and non-null value. If the value is null, the attribute will be + * ignored. + * + * @param key a non-null attribute name + * @param value a non-null attribute value + * @return this builder + */ + public Builder put(String key, String value) { + requireNonNull(key, "key must not be null"); + if (value != null) { + this.attributes.put(stringKey(key), value); + } + return this; + } + + /** + * Adds an attribute key and non-null value. If the value is null, the attribute will be + * ignored. + * + * @param key a non-null attribute name + * @param value a non-null attribute value + * @return this builder + */ + public Builder put(String key, Boolean value) { + requireNonNull(key, "key must not be null"); + if (value != null) { + this.attributes.put(booleanKey(key), value); + } + return this; + } + + /** + * Adds an attribute key and non-null value. If the value is null, the attribute will be + * ignored. + * + * @param key a non-null attribute name + * @param value a non-null attribute value + * @return this builder + */ + public Builder put(String key, Long value) { + requireNonNull(key, "key must not be null"); + if (value != null) { + this.attributes.put(longKey(key), value); + } + return this; + } + + /** + * Adds an attribute key and non-null value. If the value is null, the attribute will be + * ignored. + * + * @param key a non-null attribute name + * @param value a non-null attribute value + * @return this builder + */ + public Builder put(String key, Double value) { + requireNonNull(key, "key must not be null"); + if (value != null) { + this.attributes.put(doubleKey(key), value); + } + return this; + } + + /** + * Adds an attribute key and non-null, homogeneous array of non-null values. If the array is + * null, the attribute will be ignored. + * + * @param key a non-null attribute name + * @param array a non-null array of non-null values + * @return this builder + */ + public Builder putStringArray(String key, List array) { + requireNonNull(key, "key must not be null"); + if (array != null) { + this.attributes.put(stringArrayKey(key), array); + } + return this; + } + + /** + * Adds an attribute key and non-null, homogeneous array of non-null values. If the array is + * null, the attribute will be ignored. + * + * @param key a non-null attribute name + * @param array a non-null array of non-null values + * @return this builder + */ + public Builder putBooleanArray(String key, List array) { + requireNonNull(key, "key must not be null"); + if (array != null) { + this.attributes.put(booleanArrayKey(key), array); + } + return this; + } + + /** + * Adds an attribute key and non-null, homogeneous array of non-null values. If the array is + * null, the attribute will be ignored. + * + * @param key a non-null attribute name + * @param array a non-null array of non-null values + * @return this builder + */ + public Builder putLongArray(String key, List array) { + requireNonNull(key, "key must not be null"); + if (array != null) { + this.attributes.put(longArrayKey(key), array); + } + return this; + } + + /** + * Adds an attribute key and non-null, homogeneous array of non-null values. + * + * @param key a non-null attribute name + * @param array a non-null array of non-null values + * @return this builder + */ + public Builder putDoubleArray(String key, List array) { + requireNonNull(key, "key must not be null"); + if (array != null) { + this.attributes.put(doubleArrayKey(key), array); + } + return this; + } + + public SpanNativeAttributes build() { + return new SpanNativeAttributes(this.attributes); + } + } + + /** All valid types for attribute values. */ + public enum AttributeType { + STRING, // String + BOOLEAN, // Boolean + LONG, // Long + DOUBLE, // Double + STRING_ARRAY, // List + BOOLEAN_ARRAY, // List + LONG_ARRAY, // List + DOUBLE_ARRAY // List + } + + /** + * Represents an attribute key. Can be cached and reused to prevent object creation on every + * attribute set. + * + * @param the respective types of {@link AttributeType} + */ + public interface AttributeKey extends Comparable> { + /** + * The non-null, unique key name. + * + * @return the key name + */ + String getKey(); + + /** + * The type of the attribute value. + * + * @return the type + */ + AttributeType getType(); + + static AttributeKey stringKey(String key) { + return create(key, AttributeType.STRING); + } + + static AttributeKey booleanKey(String key) { + return create(key, AttributeType.BOOLEAN); + } + + static AttributeKey longKey(String key) { + return create(key, AttributeType.LONG); + } + + static AttributeKey doubleKey(String key) { + return create(key, AttributeType.DOUBLE); + } + + static AttributeKey> stringArrayKey(String key) { + return create(key, AttributeType.STRING_ARRAY); + } + + static AttributeKey> booleanArrayKey(String key) { + return create(key, AttributeType.BOOLEAN_ARRAY); + } + + static AttributeKey> longArrayKey(String key) { + return create(key, AttributeType.LONG_ARRAY); + } + + static AttributeKey> doubleArrayKey(String key) { + return create(key, AttributeType.DOUBLE_ARRAY); + } + + /** Internal method to create an {@link AttributeKey}. */ + static AttributeKey create(String key, AttributeType type) { + return new AttributeKeyImpl<>(type, key); + } + + /** Keys are unique based on their key names only, regardless of the type. */ + @Override + default int compareTo(AttributeKey o) { + return getKey().compareTo(o.getKey()); + } + + class AttributeKeyImpl implements AttributeKey { + private final AttributeType type; + private final String key; + private final int hashCode; + + private AttributeKeyImpl(AttributeType type, String key) { + this.type = type; + this.key = key; + + // Two AttributeKeyImpl are equal if they have the same key, regardless of the type + this.hashCode = key.hashCode(); + } + + @Override + public String getKey() { + return key; + } + + @Override + public AttributeType getType() { + return type; + } + + // Two AttributeKeyImpl are equal if they have the same key, regardless of the type + @Override + public boolean equals(Object o) { + if (o == this) { + return true; + } + if (o instanceof AttributeKeyImpl) { + AttributeKeyImpl that = (AttributeKeyImpl) o; + return getKey().equals(that.getKey()); + } + return false; + } + + @Override + public int hashCode() { + return hashCode; + } + + @Override + public String toString() { + return key; + } + } + } +} diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy new file mode 100644 index 00000000000..6c350c8f45c --- /dev/null +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy @@ -0,0 +1,221 @@ +package datadog.trace.bootstrap.instrumentation.api + +import datadog.trace.test.util.DDSpecification +import spock.lang.Subject + +class SpanNativeAttributesTest extends DDSpecification { + + @Subject + SpanNativeAttributes attributes + + def "empty attributes"() { + when: + attributes = SpanNativeAttributes.EMPTY + + then: + attributes.isEmpty() + attributes.data().isEmpty() + } + + def "builder creates empty attributes"() { + when: + attributes = SpanNativeAttributes.builder().build() + + then: + attributes.isEmpty() + attributes.data().isEmpty() + } + + def "builder adds string attribute"() { + when: + attributes = SpanNativeAttributes.builder() + .put("key", "value") + .build() + + then: + !attributes.isEmpty() + attributes.data().size() == 1 + attributes.data().get(SpanNativeAttributes.AttributeKey.stringKey("key")) == "value" + } + + def "builder adds boolean attribute"() { + when: + attributes = SpanNativeAttributes.builder() + .put("key", true) + .build() + + then: + !attributes.isEmpty() + attributes.data().size() == 1 + attributes.data().get(SpanNativeAttributes.AttributeKey.booleanKey("key")) == true + } + + def "builder adds long attribute"() { + when: + attributes = SpanNativeAttributes.builder() + .put("key", 123L) + .build() + + then: + !attributes.isEmpty() + attributes.data().size() == 1 + attributes.data().get(SpanNativeAttributes.AttributeKey.longKey("key")) == 123L + } + + def "builder adds double attribute"() { + when: + attributes = SpanNativeAttributes.builder() + .put("key", 42.0) + .build() + + then: + !attributes.isEmpty() + attributes.data().size() == 1 + attributes.data().get(SpanNativeAttributes.AttributeKey.doubleKey("key")) == 42.0 + } + + def "builder adds string array attribute"() { + when: + attributes = SpanNativeAttributes.builder() + .putStringArray("key", ["a", "b"]) + .build() + + then: + !attributes.isEmpty() + attributes.data().size() == 1 + attributes.data().get(SpanNativeAttributes.AttributeKey.stringArrayKey("key")) == ["a", "b"] + } + + def "builder adds boolean array attribute"() { + when: + attributes = SpanNativeAttributes.builder() + .putBooleanArray("key", [true, false]) + .build() + + then: + !attributes.isEmpty() + attributes.data().size() == 1 + attributes.data().get(SpanNativeAttributes.AttributeKey.booleanArrayKey("key")) == [true, false] + } + + def "builder adds long array attribute"() { + when: + attributes = SpanNativeAttributes.builder() + .putLongArray("key", [1L, 2L]) + .build() + + then: + !attributes.isEmpty() + attributes.data().size() == 1 + attributes.data().get(SpanNativeAttributes.AttributeKey.longArrayKey("key")) == [1L, 2L] + } + + def "builder adds double array attribute"() { + when: + attributes = SpanNativeAttributes.builder() + .putDoubleArray("key", [1.1, 2.2]) + .build() + + then: + !attributes.isEmpty() + attributes.data().size() == 1 + attributes.data().get(SpanNativeAttributes.AttributeKey.doubleArrayKey("key")) == [1.1, 2.2] + } + + def "builder skips null values"() { + when: + attributes = SpanNativeAttributes.builder() + .put("key1", "value") + .put("key2", null) + .build() + + then: + !attributes.isEmpty() + attributes.data().size() == 1 + attributes.data().get(SpanNativeAttributes.AttributeKey.stringKey("key1")) == "value" + !attributes.data().containsKey(SpanNativeAttributes.AttributeKey.stringKey("key2")) + } + + def "builder requires non-null keys"() { + when: + SpanNativeAttributes.builder().put(null, "value") + + then: + thrown(NullPointerException) + } + + def "forEach iterates over all attributes"() { + setup: + def builder = SpanNativeAttributes.builder() + .put("key1", "value1") + .put("key2", 123L) + .put("key3", true) + attributes = builder.build() + def visited = [:] + + when: + attributes.forEach { key, value -> + visited[key.getKey()] = value + } + + then: + visited.size() == 3 + visited["key1"] == "value1" + visited["key2"] == 123L + visited["key3"] == true + } + + def "forEach handles null consumer"() { + setup: + attributes = SpanNativeAttributes.builder() + .put("key", "value") + .build() + + when: + attributes.forEach(null) + + then: + noExceptionThrown() + } + + def "equals compares attributes correctly"() { + setup: + def builder1 = SpanNativeAttributes.builder() + .put("key1", "value1") + .put("key2", 123L) + def builder2 = SpanNativeAttributes.builder() + .put("key1", "value1") + .put("key2", 123L) + def builder3 = SpanNativeAttributes.builder() + .put("key1", "value1") + .put("key2", 456L) + + when: + def attrs1 = builder1.build() + def attrs2 = builder2.build() + def attrs3 = builder3.build() + + then: + attrs1 == attrs2 + attrs1 != attrs3 + attrs1 != null + attrs1 != "not an attributes object" + } + + def "toString includes all attributes"() { + setup: + attributes = SpanNativeAttributes.builder() + .put("key1", "value1") + .put("key2", 123L) + .build() + + when: + def string = attributes.toString() + + then: + string.contains("key1") + string.contains("value1") + string.contains("key2") + string.contains("123") + } +} diff --git a/utils/test-utils/src/main/groovy/datadog/trace/test/util/DDSpecification.groovy b/utils/test-utils/src/main/groovy/datadog/trace/test/util/DDSpecification.groovy index 734f347e36d..118a7516120 100644 --- a/utils/test-utils/src/main/groovy/datadog/trace/test/util/DDSpecification.groovy +++ b/utils/test-utils/src/main/groovy/datadog/trace/test/util/DDSpecification.groovy @@ -90,6 +90,13 @@ abstract class DDSpecification extends Specification { void setupSpec() { assert !configModificationFailed: "Config class modification failed. Ensure all test classes extend DDSpecification" + + // TODO: REMOVE ME MARCO! + System.getenv().findAll { it.key.startsWith("DD_") }.each { env -> + System.clearProperty(env.key) + } + // TODO: REMOVE ME MARCO! + assert System.getenv().findAll { it.key.startsWith("DD_") }.isEmpty() assert systemPropertiesExceptAllowed().findAll { it.key.toString().startsWith("dd.") }.isEmpty() From 6a2ff8d8725d8cd40cab339b99ae5eb5b25a8bba Mon Sep 17 00:00:00 2001 From: Marco Costa Date: Tue, 25 Mar 2025 09:59:47 -0700 Subject: [PATCH 05/14] Remove unused methods --- .../java/datadog/trace/core/DDSpanEvent.java | 75 +------------------ 1 file changed, 1 insertion(+), 74 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanEvent.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanEvent.java index 5f1d8a38750..8c3ee685dbe 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanEvent.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanEvent.java @@ -133,34 +133,7 @@ private static class DDSpanEventAdapter extends JsonAdapter { @FromJson @Override public DDSpanEvent fromJson(JsonReader reader) throws IOException { - reader.beginObject(); - String name = null; - SpanNativeAttributes attributes = null; - long timestampNanos = 0; - - while (reader.hasNext()) { - switch (reader.nextName()) { - case "name": - name = reader.nextString(); - break; - case "time_unix_nano": - timestampNanos = reader.nextLong(); - break; - case "attributes": - if (reader.peek() == JsonReader.Token.NULL) { - reader.nextNull(); - attributes = SpanNativeAttributes.EMPTY; - } else { - attributes = readAttributes(reader); - } - break; - default: - reader.skipValue(); - } - } - - reader.endObject(); - return new DDSpanEvent(name, attributes, timestampNanos); + throw new UnsupportedOperationException("Deserialization is not implemented"); } @ToJson @@ -178,20 +151,6 @@ public void toJson(JsonWriter writer, DDSpanEvent value) throws IOException { writer.endObject(); } - private SpanNativeAttributes readAttributes(JsonReader reader) throws IOException { - Map, Object> attributes = new HashMap<>(); - reader.beginObject(); - while (reader.hasNext()) { - String key = reader.nextName(); - SpanNativeAttributes.AttributeKey attributeKey = - SpanNativeAttributes.AttributeKey.stringKey(key); - Object value = readValue(reader); - attributes.put(attributeKey, value); - } - reader.endObject(); - return new SpanNativeAttributes(attributes); - } - private void writeAttributes(JsonWriter writer, SpanNativeAttributes attributes) throws IOException { writer.beginObject(); @@ -203,38 +162,6 @@ private void writeAttributes(JsonWriter writer, SpanNativeAttributes attributes) writer.endObject(); } - private Object readValue(JsonReader reader) throws IOException { - switch (reader.peek()) { - case STRING: - return reader.nextString(); - case NUMBER: - return reader.nextDouble(); - case BOOLEAN: - return reader.nextBoolean(); - case NULL: - return reader.nextNull(); - case BEGIN_ARRAY: - reader.beginArray(); - List array = new ArrayList<>(); - while (reader.hasNext()) { - array.add(readValue(reader)); - } - reader.endArray(); - return array; - case BEGIN_OBJECT: - reader.beginObject(); - Map object = new HashMap<>(); - while (reader.hasNext()) { - String key = reader.nextName(); - object.put(key, readValue(reader)); - } - reader.endObject(); - return object; - default: - throw new IOException("Unexpected token: " + reader.peek()); - } - } - private void writeValue( JsonWriter writer, SpanNativeAttributes.AttributeKey key, Object value) throws IOException { From fd612660370d389f10fa247a7295f605b8b83fdd Mon Sep 17 00:00:00 2001 From: Marco Costa Date: Tue, 25 Mar 2025 10:02:14 -0700 Subject: [PATCH 06/14] Fix some tests --- .../java/datadog/trace/core/DDSpanEvent.java | 2 -- .../api/SpanNativeAttributesTest.groovy | 36 +------------------ 2 files changed, 1 insertion(+), 37 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanEvent.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanEvent.java index 8c3ee685dbe..307f3e554a3 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanEvent.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanEvent.java @@ -10,8 +10,6 @@ import datadog.trace.api.time.TimeSource; import datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes; import java.io.IOException; -import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; import org.slf4j.Logger; diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy index 6c350c8f45c..ccd95497f1d 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy @@ -126,7 +126,7 @@ class SpanNativeAttributesTest extends DDSpecification { when: attributes = SpanNativeAttributes.builder() .put("key1", "value") - .put("key2", null) + .put("key2", null as String) .build() then: @@ -144,40 +144,6 @@ class SpanNativeAttributesTest extends DDSpecification { thrown(NullPointerException) } - def "forEach iterates over all attributes"() { - setup: - def builder = SpanNativeAttributes.builder() - .put("key1", "value1") - .put("key2", 123L) - .put("key3", true) - attributes = builder.build() - def visited = [:] - - when: - attributes.forEach { key, value -> - visited[key.getKey()] = value - } - - then: - visited.size() == 3 - visited["key1"] == "value1" - visited["key2"] == 123L - visited["key3"] == true - } - - def "forEach handles null consumer"() { - setup: - attributes = SpanNativeAttributes.builder() - .put("key", "value") - .build() - - when: - attributes.forEach(null) - - then: - noExceptionThrown() - } - def "equals compares attributes correctly"() { setup: def builder1 = SpanNativeAttributes.builder() From f68d11a5df66ca7db9c7dcfa3b0e8354247a5677 Mon Sep 17 00:00:00 2001 From: Marco Costa Date: Tue, 25 Mar 2025 12:05:44 -0700 Subject: [PATCH 07/14] Improve testing --- .../api/SpanNativeAttributes.java | 37 ++++- .../api/SpanNativeAttributesTest.groovy | 144 +++++++++--------- 2 files changed, 105 insertions(+), 76 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributes.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributes.java index 499f15c1ae0..c0fc5e2b0b4 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributes.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributes.java @@ -86,6 +86,27 @@ protected Builder() { * Adds an attribute key and non-null value. If the value is null, the attribute will be * ignored. * + * It's recommended to use this method, as you can cache the {@link AttributeKey} and reuse it, + * avoiding object creation on every attribute added. + * + * @param key a non-null attribute name + * @param value a non-null attribute value + * @return this builder + */ + public Builder put(AttributeKey key, T value) { + requireNonNull(key, "key must not be null"); + if (value != null) { + this.attributes.put(key, value); + } + return this; + } + + /** + * Adds an attribute key and non-null value. If the value is null, the attribute will be + * ignored. + * + * Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object creation. + * * @param key a non-null attribute name * @param value a non-null attribute value * @return this builder @@ -102,6 +123,8 @@ public Builder put(String key, String value) { * Adds an attribute key and non-null value. If the value is null, the attribute will be * ignored. * + * Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object creation. + * * @param key a non-null attribute name * @param value a non-null attribute value * @return this builder @@ -118,6 +141,8 @@ public Builder put(String key, Boolean value) { * Adds an attribute key and non-null value. If the value is null, the attribute will be * ignored. * + * Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object creation. + * * @param key a non-null attribute name * @param value a non-null attribute value * @return this builder @@ -134,6 +159,8 @@ public Builder put(String key, Long value) { * Adds an attribute key and non-null value. If the value is null, the attribute will be * ignored. * + * Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object creation. + * * @param key a non-null attribute name * @param value a non-null attribute value * @return this builder @@ -150,6 +177,8 @@ public Builder put(String key, Double value) { * Adds an attribute key and non-null, homogeneous array of non-null values. If the array is * null, the attribute will be ignored. * + * Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object creation. + * * @param key a non-null attribute name * @param array a non-null array of non-null values * @return this builder @@ -166,6 +195,8 @@ public Builder putStringArray(String key, List array) { * Adds an attribute key and non-null, homogeneous array of non-null values. If the array is * null, the attribute will be ignored. * + * Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object creation. + * * @param key a non-null attribute name * @param array a non-null array of non-null values * @return this builder @@ -182,6 +213,8 @@ public Builder putBooleanArray(String key, List array) { * Adds an attribute key and non-null, homogeneous array of non-null values. If the array is * null, the attribute will be ignored. * + * Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object creation. + * * @param key a non-null attribute name * @param array a non-null array of non-null values * @return this builder @@ -197,6 +230,8 @@ public Builder putLongArray(String key, List array) { /** * Adds an attribute key and non-null, homogeneous array of non-null values. * + * Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object creation. + * * @param key a non-null attribute name * @param array a non-null array of non-null values * @return this builder @@ -333,7 +368,7 @@ public int hashCode() { @Override public String toString() { - return key; + return "AttributeKey{" + key + ", " + type + "}"; } } } diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy index ccd95497f1d..2ef2736154d 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy @@ -26,11 +26,11 @@ class SpanNativeAttributesTest extends DDSpecification { attributes.data().isEmpty() } - def "builder adds string attribute"() { + def "builder adds AttributeKey attribute"() { when: attributes = SpanNativeAttributes.builder() - .put("key", "value") - .build() + .put(SpanNativeAttributes.AttributeKey.stringKey("key"), "value") + .build() then: !attributes.isEmpty() @@ -38,110 +38,87 @@ class SpanNativeAttributesTest extends DDSpecification { attributes.data().get(SpanNativeAttributes.AttributeKey.stringKey("key")) == "value" } - def "builder adds boolean attribute"() { - when: - attributes = SpanNativeAttributes.builder() - .put("key", true) - .build() - - then: - !attributes.isEmpty() - attributes.data().size() == 1 - attributes.data().get(SpanNativeAttributes.AttributeKey.booleanKey("key")) == true - } - - def "builder adds long attribute"() { + def "builder adds typed attribute"() { when: attributes = SpanNativeAttributes.builder() - .put("key", 123L) + ."${method}"("key", value) .build() then: !attributes.isEmpty() attributes.data().size() == 1 - attributes.data().get(SpanNativeAttributes.AttributeKey.longKey("key")) == 123L + attributes.data().get(SpanNativeAttributes.AttributeKey."${keyType}"("key")) == value + + where: + method | keyType | value + 'put' | 'stringKey' | 'value' + 'put' | 'booleanKey' | true + 'put' | 'longKey' | 123L + 'put' | 'doubleKey' | 42.0d + 'putStringArray' | 'stringArrayKey' | ['a', 'b'] + 'putBooleanArray'| 'booleanArrayKey'| [true, false] + 'putLongArray' | 'longArrayKey' | [1L, 2L] + 'putDoubleArray' | 'doubleArrayKey' | [1.1d, 2.2d] } - def "builder adds double attribute"() { - when: - attributes = SpanNativeAttributes.builder() - .put("key", 42.0) - .build() - - then: - !attributes.isEmpty() - attributes.data().size() == 1 - attributes.data().get(SpanNativeAttributes.AttributeKey.doubleKey("key")) == 42.0 - } - - def "builder adds string array attribute"() { + def "builder skips null values"() { when: attributes = SpanNativeAttributes.builder() - .putStringArray("key", ["a", "b"]) + .put("key1", "value") + .put("key2", null as String) .build() then: !attributes.isEmpty() attributes.data().size() == 1 - attributes.data().get(SpanNativeAttributes.AttributeKey.stringArrayKey("key")) == ["a", "b"] + attributes.data().get(SpanNativeAttributes.AttributeKey.stringKey("key1")) == "value" + !attributes.data().containsKey(SpanNativeAttributes.AttributeKey.stringKey("key2")) } - def "builder adds boolean array attribute"() { + def "builder requires non-null keys for AttributeKey"() { when: - attributes = SpanNativeAttributes.builder() - .putBooleanArray("key", [true, false]) - .build() + SpanNativeAttributes.builder().put(null as SpanNativeAttributes.AttributeKey, "value") then: - !attributes.isEmpty() - attributes.data().size() == 1 - attributes.data().get(SpanNativeAttributes.AttributeKey.booleanArrayKey("key")) == [true, false] + thrown(NullPointerException) } - def "builder adds long array attribute"() { + def "builder requires non-null keys for all attribute types"() { when: - attributes = SpanNativeAttributes.builder() - .putLongArray("key", [1L, 2L]) - .build() + SpanNativeAttributes.builder()."${method}"(null, value) then: - !attributes.isEmpty() - attributes.data().size() == 1 - attributes.data().get(SpanNativeAttributes.AttributeKey.longArrayKey("key")) == [1L, 2L] - } - - def "builder adds double array attribute"() { - when: - attributes = SpanNativeAttributes.builder() - .putDoubleArray("key", [1.1, 2.2]) - .build() + thrown(NullPointerException) - then: - !attributes.isEmpty() - attributes.data().size() == 1 - attributes.data().get(SpanNativeAttributes.AttributeKey.doubleArrayKey("key")) == [1.1, 2.2] + where: + method | value + 'put' | 'string value' + 'put' | true + 'put' | 123L + 'put' | 42.0d + 'putStringArray' | ['a', 'b'] + 'putBooleanArray' | [true, false] + 'putLongArray' | [1L, 2L] + 'putDoubleArray' | [1.1d, 2.2d] } - def "builder skips null values"() { + def "builder skips null values for all attribute types"() { when: attributes = SpanNativeAttributes.builder() - .put("key1", "value") - .put("key2", null as String) + .put(SpanNativeAttributes.AttributeKey.stringKey("string"), null as String) + .put('string', null as String) + .put('boolean', null as Boolean) + .put('long', null as Long) + .put('double', null as Double) + .putStringArray('stringArray', null) + .putBooleanArray('booleanArray', null) + .putLongArray('longArray', null) + .putDoubleArray('doubleArray', null) .build() then: - !attributes.isEmpty() - attributes.data().size() == 1 - attributes.data().get(SpanNativeAttributes.AttributeKey.stringKey("key1")) == "value" - !attributes.data().containsKey(SpanNativeAttributes.AttributeKey.stringKey("key2")) - } - - def "builder requires non-null keys"() { - when: - SpanNativeAttributes.builder().put(null, "value") - - then: - thrown(NullPointerException) + attributes.isEmpty() + attributes.data().isEmpty() } def "equals compares attributes correctly"() { @@ -164,8 +141,25 @@ class SpanNativeAttributesTest extends DDSpecification { then: attrs1 == attrs2 attrs1 != attrs3 - attrs1 != null - attrs1 != "not an attributes object" + attrs2 != attrs3 + } + + def "AttributeKey has the correct values"() { + when: + def key = SpanNativeAttributes.AttributeKey.stringKey("key") + + then: + key.getKey() == "key" + key.getType() == SpanNativeAttributes.AttributeType.STRING + key.toString() == "AttributeKey{key, STRING}" + + key.compareTo(SpanNativeAttributes.AttributeKey.stringKey("key")) == 0 + key.equals(SpanNativeAttributes.AttributeKey.stringKey("key")) + key.hashCode() == SpanNativeAttributes.AttributeKey.stringKey("key").hashCode() + + key.equals(SpanNativeAttributes.AttributeKey.stringKey("other")) == false + key.compareTo(SpanNativeAttributes.AttributeKey.stringKey("other")) != 0 + key.hashCode() != SpanNativeAttributes.AttributeKey.stringKey("other").hashCode() } def "toString includes all attributes"() { From d99b87ce213811c72806c78c0cb7059effa8bf73 Mon Sep 17 00:00:00 2001 From: Marco Costa Date: Tue, 25 Mar 2025 12:14:31 -0700 Subject: [PATCH 08/14] Lint --- .../api/SpanNativeAttributes.java | 30 ++++++++++++------- .../api/SpanNativeAttributesTest.groovy | 6 ++-- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributes.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributes.java index c0fc5e2b0b4..7fda7bd0fd5 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributes.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributes.java @@ -86,14 +86,14 @@ protected Builder() { * Adds an attribute key and non-null value. If the value is null, the attribute will be * ignored. * - * It's recommended to use this method, as you can cache the {@link AttributeKey} and reuse it, - * avoiding object creation on every attribute added. + *

It's recommended to use this method, as you can cache the {@link AttributeKey} and reuse + * it, avoiding object creation on every attribute added. * * @param key a non-null attribute name * @param value a non-null attribute value * @return this builder */ - public Builder put(AttributeKey key, T value) { + public Builder put(AttributeKey key, T value) { requireNonNull(key, "key must not be null"); if (value != null) { this.attributes.put(key, value); @@ -105,7 +105,8 @@ public Builder put(AttributeKey key, T value) { * Adds an attribute key and non-null value. If the value is null, the attribute will be * ignored. * - * Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object creation. + *

Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object + * creation. * * @param key a non-null attribute name * @param value a non-null attribute value @@ -123,7 +124,8 @@ public Builder put(String key, String value) { * Adds an attribute key and non-null value. If the value is null, the attribute will be * ignored. * - * Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object creation. + *

Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object + * creation. * * @param key a non-null attribute name * @param value a non-null attribute value @@ -141,7 +143,8 @@ public Builder put(String key, Boolean value) { * Adds an attribute key and non-null value. If the value is null, the attribute will be * ignored. * - * Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object creation. + *

Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object + * creation. * * @param key a non-null attribute name * @param value a non-null attribute value @@ -159,7 +162,8 @@ public Builder put(String key, Long value) { * Adds an attribute key and non-null value. If the value is null, the attribute will be * ignored. * - * Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object creation. + *

Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object + * creation. * * @param key a non-null attribute name * @param value a non-null attribute value @@ -177,7 +181,8 @@ public Builder put(String key, Double value) { * Adds an attribute key and non-null, homogeneous array of non-null values. If the array is * null, the attribute will be ignored. * - * Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object creation. + *

Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object + * creation. * * @param key a non-null attribute name * @param array a non-null array of non-null values @@ -195,7 +200,8 @@ public Builder putStringArray(String key, List array) { * Adds an attribute key and non-null, homogeneous array of non-null values. If the array is * null, the attribute will be ignored. * - * Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object creation. + *

Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object + * creation. * * @param key a non-null attribute name * @param array a non-null array of non-null values @@ -213,7 +219,8 @@ public Builder putBooleanArray(String key, List array) { * Adds an attribute key and non-null, homogeneous array of non-null values. If the array is * null, the attribute will be ignored. * - * Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object creation. + *

Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object + * creation. * * @param key a non-null attribute name * @param array a non-null array of non-null values @@ -230,7 +237,8 @@ public Builder putLongArray(String key, List array) { /** * Adds an attribute key and non-null, homogeneous array of non-null values. * - * Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object creation. + *

Note: Use {@link #put(AttributeKey, Object)} instead if possible, to avoid object + * creation. * * @param key a non-null attribute name * @param array a non-null array of non-null values diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy index 2ef2736154d..eb3256ca5ae 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy @@ -29,8 +29,8 @@ class SpanNativeAttributesTest extends DDSpecification { def "builder adds AttributeKey attribute"() { when: attributes = SpanNativeAttributes.builder() - .put(SpanNativeAttributes.AttributeKey.stringKey("key"), "value") - .build() + .put(SpanNativeAttributes.AttributeKey.stringKey("key"), "value") + .build() then: !attributes.isEmpty() @@ -152,7 +152,7 @@ class SpanNativeAttributesTest extends DDSpecification { key.getKey() == "key" key.getType() == SpanNativeAttributes.AttributeType.STRING key.toString() == "AttributeKey{key, STRING}" - + key.compareTo(SpanNativeAttributes.AttributeKey.stringKey("key")) == 0 key.equals(SpanNativeAttributes.AttributeKey.stringKey("key")) key.hashCode() == SpanNativeAttributes.AttributeKey.stringKey("key").hashCode() From a10ae0e7541e8e8dde6ef2e023a91b867f5445b3 Mon Sep 17 00:00:00 2001 From: Marco Costa Date: Tue, 25 Mar 2025 14:03:58 -0700 Subject: [PATCH 09/14] More tests --- .../api/SpanNativeAttributes.java | 2 +- .../api/SpanNativeAttributesTest.groovy | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributes.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributes.java index 7fda7bd0fd5..88208a608f7 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributes.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributes.java @@ -31,7 +31,7 @@ public class SpanNativeAttributes { private final Map, Object> attributes; /* Use the {#builder} to create instances of this class */ - public SpanNativeAttributes(Map, Object> attributes) { + protected SpanNativeAttributes(Map, Object> attributes) { this.attributes = attributes; } diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy index eb3256ca5ae..749c1ffedfb 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/instrumentation/api/SpanNativeAttributesTest.groovy @@ -142,6 +142,8 @@ class SpanNativeAttributesTest extends DDSpecification { attrs1 == attrs2 attrs1 != attrs3 attrs2 != attrs3 + attrs1 != "different type" + attrs1.equals attrs1 // `==` does not reach the class implementation of equals } def "AttributeKey has the correct values"() { @@ -153,13 +155,23 @@ class SpanNativeAttributesTest extends DDSpecification { key.getType() == SpanNativeAttributes.AttributeType.STRING key.toString() == "AttributeKey{key, STRING}" + key.equals key + key.compareTo(key) == 0 + key.compareTo(SpanNativeAttributes.AttributeKey.stringKey("key")) == 0 key.equals(SpanNativeAttributes.AttributeKey.stringKey("key")) key.hashCode() == SpanNativeAttributes.AttributeKey.stringKey("key").hashCode() - key.equals(SpanNativeAttributes.AttributeKey.stringKey("other")) == false - key.compareTo(SpanNativeAttributes.AttributeKey.stringKey("other")) != 0 - key.hashCode() != SpanNativeAttributes.AttributeKey.stringKey("other").hashCode() + key.equals(SpanNativeAttributes.AttributeKey.stringKey("aaa")) == false + key.compareTo(SpanNativeAttributes.AttributeKey.stringKey("aaa")) > 0 + key.hashCode() != SpanNativeAttributes.AttributeKey.stringKey("aaa").hashCode() + + key.equals(SpanNativeAttributes.AttributeKey.stringKey("zzz")) == false + key.compareTo(SpanNativeAttributes.AttributeKey.stringKey("zzz")) < 0 + key.hashCode() != SpanNativeAttributes.AttributeKey.stringKey("zzz").hashCode() + + key.equals("different type") == false + key.hashCode() != "different type".hashCode() } def "toString includes all attributes"() { From 572faccd2a5b33b9d6cfaa7c72c8c8a308ae3e4f Mon Sep 17 00:00:00 2001 From: nayeem-kamal Date: Wed, 26 Mar 2025 08:31:11 -0400 Subject: [PATCH 10/14] removed unused imports --- .../groovy/datadog/trace/core/DDSpanEventTest.groovy | 9 --------- 1 file changed, 9 deletions(-) diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanEventTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanEventTest.groovy index b893b271822..ea27ba09280 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanEventTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanEventTest.groovy @@ -3,15 +3,6 @@ package datadog.trace.core import datadog.trace.api.time.SystemTimeSource import datadog.trace.api.time.TimeSource import datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes -import datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey -import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.stringKey -import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.booleanKey -import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.longKey -import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.doubleKey -import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.stringArrayKey -import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.booleanArrayKey -import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.longArrayKey -import static datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes.AttributeKey.doubleArrayKey import datadog.trace.core.test.DDCoreSpecification import spock.lang.Shared From 4a2d5c52cb86af78a554017e90345ab7b613e9cb Mon Sep 17 00:00:00 2001 From: Marco Costa Date: Thu, 27 Mar 2025 10:29:45 -0700 Subject: [PATCH 11/14] Revert this local hack --- .../groovy/datadog/trace/test/util/DDSpecification.groovy | 7 ------- 1 file changed, 7 deletions(-) diff --git a/utils/test-utils/src/main/groovy/datadog/trace/test/util/DDSpecification.groovy b/utils/test-utils/src/main/groovy/datadog/trace/test/util/DDSpecification.groovy index 118a7516120..734f347e36d 100644 --- a/utils/test-utils/src/main/groovy/datadog/trace/test/util/DDSpecification.groovy +++ b/utils/test-utils/src/main/groovy/datadog/trace/test/util/DDSpecification.groovy @@ -90,13 +90,6 @@ abstract class DDSpecification extends Specification { void setupSpec() { assert !configModificationFailed: "Config class modification failed. Ensure all test classes extend DDSpecification" - - // TODO: REMOVE ME MARCO! - System.getenv().findAll { it.key.startsWith("DD_") }.each { env -> - System.clearProperty(env.key) - } - // TODO: REMOVE ME MARCO! - assert System.getenv().findAll { it.key.startsWith("DD_") }.isEmpty() assert systemPropertiesExceptAllowed().findAll { it.key.toString().startsWith("dd.") }.isEmpty() From 351dc1ff4de6030f55d18bac2e739146d07e11c7 Mon Sep 17 00:00:00 2001 From: nayeem-kamal Date: Wed, 16 Apr 2025 15:46:58 -0400 Subject: [PATCH 12/14] added span events for graphql --- .../ExecutionInstrumentationContext.java | 64 +++++++++++++++++++ .../datadog/trace/api/ConfigDefaults.java | 2 + .../config/TraceInstrumentationConfig.java | 2 + .../main/java/datadog/trace/api/Config.java | 31 +++++++++ .../instrumentation/api/AgentSpan.java | 4 ++ .../instrumentation/api/ImmutableSpan.java | 8 +++ .../instrumentation/api/NoopSpan.java | 8 +++ 7 files changed, 119 insertions(+) diff --git a/dd-java-agent/instrumentation/graphql-java/graphql-java-common/src/main/java/datadog/trace/instrumentation/graphqljava/ExecutionInstrumentationContext.java b/dd-java-agent/instrumentation/graphql-java/graphql-java-common/src/main/java/datadog/trace/instrumentation/graphqljava/ExecutionInstrumentationContext.java index df9ffebd692..c3474f5a0a5 100644 --- a/dd-java-agent/instrumentation/graphql-java/graphql-java-common/src/main/java/datadog/trace/instrumentation/graphqljava/ExecutionInstrumentationContext.java +++ b/dd-java-agent/instrumentation/graphql-java/graphql-java-common/src/main/java/datadog/trace/instrumentation/graphqljava/ExecutionInstrumentationContext.java @@ -2,14 +2,19 @@ import static datadog.trace.instrumentation.graphqljava.GraphQLDecorator.DECORATE; +import datadog.trace.api.Config; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes; import graphql.ExecutionResult; import graphql.GraphQLError; import graphql.execution.instrumentation.SimpleInstrumentationContext; import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; public class ExecutionInstrumentationContext extends SimpleInstrumentationContext { private final State state; + private static final List errorExtensions = Config.get().getTraceGraphqlErrorExtensions(); public ExecutionInstrumentationContext(State state) { this.state = state; @@ -30,6 +35,65 @@ public void onCompleted(ExecutionResult result, Throwable t) { } requestSpan.setErrorMessage(error); requestSpan.setError(true); + + // Add span events for each GraphQL error + for (GraphQLError graphQLError : errors) { + SpanNativeAttributes.Builder attributes = + SpanNativeAttributes.builder().put("message", graphQLError.getMessage()); + + // Add locations if available + if (graphQLError.getLocations() != null && !graphQLError.getLocations().isEmpty()) { + List locationStrings = + graphQLError.getLocations().stream() + .map(loc -> loc.getLine() + ":" + loc.getColumn()) + .collect(Collectors.toList()); + attributes.putStringArray("locations", locationStrings); + } + + // Add path if available + if (graphQLError.getPath() != null && !graphQLError.getPath().isEmpty()) { + List pathStrings = + graphQLError.getPath().stream().map(Object::toString).collect(Collectors.toList()); + attributes.putStringArray("path", pathStrings); + } + + // Add extensions if available + Map extensions = graphQLError.getExtensions(); + if (extensions != null && !extensions.isEmpty()) { + + for (String extensionKey : errorExtensions) { + if (extensions.containsKey(extensionKey)) { + Object value = extensions.get(extensionKey); + if (value != null) { + if (value instanceof Number) { + if (value instanceof Long) { + attributes.put("extensions." + extensionKey, (Long) value); + } else if (value instanceof Double) { + attributes.put("extensions." + extensionKey, (Double) value); + } else { + attributes.put("extensions." + extensionKey, value.toString()); + } + } else if (value instanceof Boolean) { + attributes.put("extensions." + extensionKey, (Boolean) value); + } else if (value instanceof List) { + List list = (List) value; + if (!list.isEmpty() && list.get(0) instanceof String) { + attributes.putStringArray( + "extensions." + extensionKey, + list.stream().map(Object::toString).collect(Collectors.toList())); + } else { + attributes.put("extensions." + extensionKey, value.toString()); + } + } else { + attributes.put("extensions." + extensionKey, value.toString()); + } + } + } + } + } + + requestSpan.addEvent("dd.graphql.query.error", attributes.build()); + } } } requestSpan.setTag("graphql.source", state.getQuery()); diff --git a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java index 5e620d344e1..e7a21f9e895 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java @@ -342,5 +342,7 @@ public final class ConfigDefaults { "$.Credentials.SessionToken", "$.InventoryConfigurationList[*].Destination.S3BucketDestination.Encryption.SSEKMS.KeyId"); + static final String DEFAULT_TRACE_GRAPHQL_ERROR_EXTENSIONS = " , , ,"; + private ConfigDefaults() {} } diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java index 687f68aa494..f90aad6d624 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java @@ -176,5 +176,7 @@ public final class TraceInstrumentationConfig { public static final String SQS_BODY_PROPAGATION_ENABLED = "trace.sqs.body.propagation.enabled"; public static final String ADD_SPAN_POINTERS = "add.span.pointers"; + public static final String TRACE_GRAPHQL_ERROR_EXTENSIONS = "trace.graphql.error.extensions"; + private TraceInstrumentationConfig() {} } diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 1f311b576d2..323ee9cad3a 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -554,6 +554,7 @@ public static String getHostName() { private final String agentlessLogSubmissionLevel; private final String agentlessLogSubmissionUrl; private final String agentlessLogSubmissionProduct; + private final List traceGraphqlErrorExtensions; private final Set cloudPayloadTaggingServices; @Nullable private final List cloudRequestPayloadTagging; @@ -2005,9 +2006,32 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) this.jdkSocketEnabled = configProvider.getBoolean(JDK_SOCKET_ENABLED, true); + this.traceGraphqlErrorExtensions = + parseGraphqlErrorExtensions( + configProvider.getString( + TRACE_GRAPHQL_ERROR_EXTENSIONS, DEFAULT_TRACE_GRAPHQL_ERROR_EXTENSIONS)); + log.debug("New instance: {}", this); } + private static List parseGraphqlErrorExtensions(String config) { + if (config == null || config.trim().isEmpty()) { + return Collections.emptyList(); + } + + List patterns = new LinkedList<>(); + String[] parts = config.split(","); + + for (String part : parts) { + String trimmed = part.trim(); + if (!trimmed.isEmpty()) { + patterns.add(trimmed); + } + } + + return patterns; + } + /** * Converts a list of packages in Jacoco exclusion format ({@code * my.package.*,my.other.package.*}) to list of package prefixes suitable for use with ASM ({@code @@ -4184,6 +4208,10 @@ public int getCloudPayloadTaggingMaxTags() { return cloudPayloadTaggingMaxTags; } + public List getTraceGraphqlErrorExtensions() { + return traceGraphqlErrorExtensions; + } + private Set getSettingsSetFromEnvironment( String name, Function mapper, boolean splitOnWS) { final String value = configProvider.getString(name, ""); @@ -4860,6 +4888,9 @@ public String toString() { + cloudRequestPayloadTagging + ", cloudResponsePayloadTagging=" + cloudResponsePayloadTagging + + ", traceGraphqlErrorExtensions='" + + traceGraphqlErrorExtensions + + '\'' + '}'; } } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java index 412ac7c85cc..04a9bce13ed 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java @@ -188,6 +188,10 @@ default boolean isValid() { boolean isOutbound(); + public AgentSpan addEvent(String name); + + public AgentSpan addEvent(String name, SpanNativeAttributes attributes); + default AgentSpan asAgentSpan() { return this; } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ImmutableSpan.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ImmutableSpan.java index 620d51b57a1..878bc6de6ec 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ImmutableSpan.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ImmutableSpan.java @@ -185,6 +185,14 @@ public AgentSpan setMetaStruct(String field, Object value) { return this; } + public AgentSpan addEvent(String name) { + return this; + } + + public AgentSpan addEvent(String name, SpanNativeAttributes attributes) { + return this; + } + @Override public void setRequestBlockingAction(RequestBlockingAction rba) {} diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/NoopSpan.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/NoopSpan.java index 5d476c39b20..cae9714309a 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/NoopSpan.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/NoopSpan.java @@ -140,6 +140,14 @@ public boolean isOutbound() { return false; } + public AgentSpan addEvent(String name) { + return this; + } + + public AgentSpan addEvent(String name, SpanNativeAttributes attributes) { + return this; + } + @Override public boolean isRequiresPostProcessing() { return false; From 52100f72e4f0d09c5255e3f16237592003dd9712 Mon Sep 17 00:00:00 2001 From: nayeem-kamal Date: Tue, 22 Apr 2025 14:51:14 -0400 Subject: [PATCH 13/14] added tests --- .../src/test/groovy/GraphQLTest.groovy | 184 +++++++++++++++++- .../ExecutionInstrumentationContext.java | 59 +----- .../graphqljava/GraphQLDecorator.java | 66 +++++++ 3 files changed, 249 insertions(+), 60 deletions(-) diff --git a/dd-java-agent/instrumentation/graphql-java/graphql-java-14.0/src/test/groovy/GraphQLTest.groovy b/dd-java-agent/instrumentation/graphql-java/graphql-java-14.0/src/test/groovy/GraphQLTest.groovy index 62dc90d324b..882c5fe0b7a 100644 --- a/dd-java-agent/instrumentation/graphql-java/graphql-java-14.0/src/test/groovy/GraphQLTest.groovy +++ b/dd-java-agent/instrumentation/graphql-java/graphql-java-14.0/src/test/groovy/GraphQLTest.groovy @@ -360,7 +360,30 @@ abstract class GraphQLTest extends VersionedNamingTestBase { "error.message" { it.contains("Field 'title' in type 'Book' is undefined") } "error.message" { it.contains("(and 1 more errors)") } defaultTags() + events { + event { + eventName "error" + attributes { + "error.type" "graphql.validation.ValidationError" + "error.message" "Field 'title' in type 'Book' is undefined" + "error.stack" String + "graphql.error.path" null + } + } + event { + eventName "error" + attributes { + "error.type" "graphql.validation.ValidationError" + "error.message" "Field 'color' in type 'Book' is undefined" + "error.stack" String + "graphql.error.path" null + } + } + } + + } + } span { operationName "graphql.validation" @@ -416,9 +439,20 @@ abstract class GraphQLTest extends VersionedNamingTestBase { "$Tags.COMPONENT" "graphql-java" "graphql.source" query "graphql.operation.name" null - "error.message" { it.toLowerCase().startsWith("invalid syntax") } + "error.message" "Invalid Syntax : offending token ')' at line 2 column 30" defaultTags() } + events { + event { + eventName "error" + attributes { + "error.type" "graphql.parser.InvalidSyntaxException" + "error.message" "Invalid Syntax : offending token ')' at line 2 column 30" + "error.stack" String + "graphql.error.path" null + } + } + } } span { operationName "graphql.parsing" @@ -430,7 +464,7 @@ abstract class GraphQLTest extends VersionedNamingTestBase { tags { "$Tags.COMPONENT" "graphql-java" "error.type" "graphql.parser.InvalidSyntaxException" - "error.message" { it.toLowerCase().startsWith("invalid syntax") } + "error.message" "Invalid Syntax : offending token ')' at line 2 column 30" "error.stack" String defaultTags() } @@ -648,6 +682,152 @@ abstract class GraphQLTest extends VersionedNamingTestBase { } } } + + def "query fetch error with span events"() { + setup: + def query = 'query findBookById {\n' + + ' bookById(id: "book-1") {\n' + + ' id #test\n' + + ' cover\n' + // throws an exception when fetched + ' year\n' + // also throws an exception + ' }\n' + + '}' + def expectedQuery = 'query findBookById {\n' + + ' bookById(id: {String}) {\n' + + ' id\n' + + ' cover\n' + + ' year\n' + + ' }\n' + + '}\n' + ExecutionResult result = graphql.execute(query) + + expect: + !result.getErrors().isEmpty() + + assertTraces(1) { + trace(7) { + span { + operationName operation() + resourceName "findBookById" + spanType DDSpanTypes.GRAPHQL + errored true + measured true + parent() + tags { + "$Tags.COMPONENT" "graphql-java" + "graphql.source" expectedQuery + "graphql.operation.name" "findBookById" + "error.message" { it.contains("Exception while fetching data (/bookById/cover) : TEST") } + defaultTags() + } + events { + event { + eventName "error" + attributes { + "error.type" "java.lang.IllegalStateException" + "error.message" "TEST" + "error.stack" String + "graphql.error.path" "/bookById/cover" + } + } + event { + eventName "error" + attributes { + "error.type" "java.lang.IllegalStateException" + "error.message" "TEST" + "error.stack" String + "graphql.error.path" "/bookById/year" + } + } + } + } + span { + operationName "graphql.field" + resourceName "Book.cover" + childOf(span(0)) + spanType DDSpanTypes.GRAPHQL + errored true + measured true + tags { + "$Tags.COMPONENT" "graphql-java" + "graphql.type" "String" + "graphql.coordinates" "Book.cover" + "error.type" "java.lang.IllegalStateException" + "error.message" "TEST" + "error.stack" String + defaultTags() + } + } + span { + operationName "Book.year" + resourceName "Book.year" + childOf(span(0)) + spanType DDSpanTypes.GRAPHQL + errored true + measured true + tags { + "$Tags.COMPONENT" "graphql-java" + "graphql.type" "Int" + "graphql.coordinates" "Book.year" + "error.type" "java.lang.IllegalStateException" + "error.message" "TEST" + "error.stack" String + defaultTags() + } + } + span { + operationName "graphql.field" + resourceName "Query.bookById" + childOf(span(0)) + spanType DDSpanTypes.GRAPHQL + errored false + measured true + tags { + "$Tags.COMPONENT" "graphql-java" + "graphql.type" "Book" + "graphql.coordinates" "Query.bookById" + defaultTags() + } + } + span { + operationName "getBookById" + resourceName "book" + childOf(span(3)) + spanType null + errored false + measured false + tags { + "$Tags.COMPONENT" "trace" + defaultTags() + } + } + span { + operationName "graphql.validation" + resourceName "graphql.validation" + childOf(span(0)) + spanType DDSpanTypes.GRAPHQL + errored false + measured true + tags { + "$Tags.COMPONENT" "graphql-java" + defaultTags() + } + } + span { + operationName "graphql.parsing" + resourceName "graphql.parsing" + childOf(span(0)) + spanType DDSpanTypes.GRAPHQL + errored false + measured true + tags { + "$Tags.COMPONENT" "graphql-java" + defaultTags() + } + } + } + } + } } @Flaky diff --git a/dd-java-agent/instrumentation/graphql-java/graphql-java-common/src/main/java/datadog/trace/instrumentation/graphqljava/ExecutionInstrumentationContext.java b/dd-java-agent/instrumentation/graphql-java/graphql-java-common/src/main/java/datadog/trace/instrumentation/graphqljava/ExecutionInstrumentationContext.java index c3474f5a0a5..9156784397e 100644 --- a/dd-java-agent/instrumentation/graphql-java/graphql-java-common/src/main/java/datadog/trace/instrumentation/graphqljava/ExecutionInstrumentationContext.java +++ b/dd-java-agent/instrumentation/graphql-java/graphql-java-common/src/main/java/datadog/trace/instrumentation/graphqljava/ExecutionInstrumentationContext.java @@ -4,13 +4,10 @@ import datadog.trace.api.Config; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; -import datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes; import graphql.ExecutionResult; import graphql.GraphQLError; import graphql.execution.instrumentation.SimpleInstrumentationContext; import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; public class ExecutionInstrumentationContext extends SimpleInstrumentationContext { private final State state; @@ -38,61 +35,7 @@ public void onCompleted(ExecutionResult result, Throwable t) { // Add span events for each GraphQL error for (GraphQLError graphQLError : errors) { - SpanNativeAttributes.Builder attributes = - SpanNativeAttributes.builder().put("message", graphQLError.getMessage()); - - // Add locations if available - if (graphQLError.getLocations() != null && !graphQLError.getLocations().isEmpty()) { - List locationStrings = - graphQLError.getLocations().stream() - .map(loc -> loc.getLine() + ":" + loc.getColumn()) - .collect(Collectors.toList()); - attributes.putStringArray("locations", locationStrings); - } - - // Add path if available - if (graphQLError.getPath() != null && !graphQLError.getPath().isEmpty()) { - List pathStrings = - graphQLError.getPath().stream().map(Object::toString).collect(Collectors.toList()); - attributes.putStringArray("path", pathStrings); - } - - // Add extensions if available - Map extensions = graphQLError.getExtensions(); - if (extensions != null && !extensions.isEmpty()) { - - for (String extensionKey : errorExtensions) { - if (extensions.containsKey(extensionKey)) { - Object value = extensions.get(extensionKey); - if (value != null) { - if (value instanceof Number) { - if (value instanceof Long) { - attributes.put("extensions." + extensionKey, (Long) value); - } else if (value instanceof Double) { - attributes.put("extensions." + extensionKey, (Double) value); - } else { - attributes.put("extensions." + extensionKey, value.toString()); - } - } else if (value instanceof Boolean) { - attributes.put("extensions." + extensionKey, (Boolean) value); - } else if (value instanceof List) { - List list = (List) value; - if (!list.isEmpty() && list.get(0) instanceof String) { - attributes.putStringArray( - "extensions." + extensionKey, - list.stream().map(Object::toString).collect(Collectors.toList())); - } else { - attributes.put("extensions." + extensionKey, value.toString()); - } - } else { - attributes.put("extensions." + extensionKey, value.toString()); - } - } - } - } - } - - requestSpan.addEvent("dd.graphql.query.error", attributes.build()); + DECORATE.errorSpanEvent(requestSpan, graphQLError); } } } diff --git a/dd-java-agent/instrumentation/graphql-java/graphql-java-common/src/main/java/datadog/trace/instrumentation/graphqljava/GraphQLDecorator.java b/dd-java-agent/instrumentation/graphql-java/graphql-java-common/src/main/java/datadog/trace/instrumentation/graphqljava/GraphQLDecorator.java index 7981de23037..140da5222ef 100644 --- a/dd-java-agent/instrumentation/graphql-java/graphql-java-common/src/main/java/datadog/trace/instrumentation/graphqljava/GraphQLDecorator.java +++ b/dd-java-agent/instrumentation/graphql-java/graphql-java-common/src/main/java/datadog/trace/instrumentation/graphqljava/GraphQLDecorator.java @@ -2,6 +2,7 @@ import static datadog.trace.api.gateway.Events.EVENTS; +import datadog.trace.api.Config; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; @@ -11,8 +12,10 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import datadog.trace.bootstrap.instrumentation.api.InternalSpanTypes; +import datadog.trace.bootstrap.instrumentation.api.SpanNativeAttributes; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.bootstrap.instrumentation.decorator.BaseDecorator; +import graphql.GraphQLError; import graphql.execution.ExecutionContext; import graphql.language.Argument; import graphql.language.Field; @@ -20,8 +23,10 @@ import graphql.language.StringValue; import graphql.language.Value; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.function.BiFunction; +import java.util.stream.Collectors; public class GraphQLDecorator extends BaseDecorator { public static final GraphQLDecorator DECORATE = new GraphQLDecorator(); @@ -32,6 +37,7 @@ public class GraphQLDecorator extends BaseDecorator { public static final CharSequence GRAPHQL_VALIDATION = UTF8BytesString.create("graphql.validation"); public static final CharSequence GRAPHQL_JAVA = UTF8BytesString.create("graphql-java"); + private static final List errorExtensions = Config.get().getTraceGraphqlErrorExtensions(); // Extract this to allow for easier testing protected AgentTracer.TracerAPI tracer() { @@ -106,4 +112,64 @@ public AgentSpan onRequest(final AgentSpan span, final ExecutionContext context) return span; } + + public AgentSpan errorSpanEvent(AgentSpan requestSpan, GraphQLError graphQLError) { + SpanNativeAttributes.Builder attributes = + SpanNativeAttributes.builder().put("message", graphQLError.getMessage()); + + // Add locations if available + if (graphQLError.getLocations() != null && !graphQLError.getLocations().isEmpty()) { + List locationStrings = + graphQLError.getLocations().stream() + .map(loc -> loc.getLine() + ":" + loc.getColumn()) + .collect(Collectors.toList()); + attributes.putStringArray("locations", locationStrings); + } + + // Add path if available + if (graphQLError.getPath() != null && !graphQLError.getPath().isEmpty()) { + List pathStrings = + graphQLError.getPath().stream().map(Object::toString).collect(Collectors.toList()); + attributes.putStringArray("path", pathStrings); + } + + // Add extensions if available + Map extensions = graphQLError.getExtensions(); + if (extensions != null && !extensions.isEmpty()) { + + for (String extensionKey : errorExtensions) { + if (extensions.containsKey(extensionKey)) { + Object value = extensions.get(extensionKey); + if (value != null) { + if (value instanceof Number) { + if (value instanceof Long) { + attributes.put("extensions." + extensionKey, (Long) value); + } else if (value instanceof Double) { + attributes.put("extensions." + extensionKey, (Double) value); + } else { + attributes.put("extensions." + extensionKey, value.toString()); + } + } else if (value instanceof Boolean) { + attributes.put("extensions." + extensionKey, (Boolean) value); + } else if (value instanceof List) { + List list = (List) value; + if (!list.isEmpty() && list.get(0) instanceof String) { + attributes.putStringArray( + "extensions." + extensionKey, + list.stream().map(Object::toString).collect(Collectors.toList())); + } else { + attributes.put("extensions." + extensionKey, value.toString()); + } + } else { + attributes.put("extensions." + extensionKey, value.toString()); + } + } + } + } + } + + requestSpan.addEvent("dd.graphql.query.error", attributes.build()); + + return requestSpan; + } } From 175e6e4b57742b79b0e78431e25e12701f6d9776 Mon Sep 17 00:00:00 2001 From: nayeem-kamal Date: Mon, 28 Apr 2025 15:10:43 -0400 Subject: [PATCH 14/14] fixed tests --- .../src/test/groovy/GraphQLTest.groovy | 224 ++++-------------- 1 file changed, 41 insertions(+), 183 deletions(-) diff --git a/dd-java-agent/instrumentation/graphql-java/graphql-java-14.0/src/test/groovy/GraphQLTest.groovy b/dd-java-agent/instrumentation/graphql-java/graphql-java-14.0/src/test/groovy/GraphQLTest.groovy index 882c5fe0b7a..2f733b64133 100644 --- a/dd-java-agent/instrumentation/graphql-java/graphql-java-14.0/src/test/groovy/GraphQLTest.groovy +++ b/dd-java-agent/instrumentation/graphql-java/graphql-java-14.0/src/test/groovy/GraphQLTest.groovy @@ -78,7 +78,7 @@ abstract class GraphQLTest extends VersionedNamingTestBase { .type(newTypeWiring("Book").dataFetcher("year", new DataFetcher>() { @Override CompletionStage get(DataFetchingEnvironment environment) throws Exception { - return CompletableFuture.completedStage(2015) + return CompletableFuture.completedFuture(2015) } })) .build() @@ -359,31 +359,25 @@ abstract class GraphQLTest extends VersionedNamingTestBase { "graphql.operation.name" null "error.message" { it.contains("Field 'title' in type 'Book' is undefined") } "error.message" { it.contains("(and 1 more errors)") } - defaultTags() - events { - event { - eventName "error" - attributes { - "error.type" "graphql.validation.ValidationError" - "error.message" "Field 'title' in type 'Book' is undefined" - "error.stack" String - "graphql.error.path" null - } - } - event { - eventName "error" - attributes { - "error.type" "graphql.validation.ValidationError" - "error.message" "Field 'color' in type 'Book' is undefined" - "error.stack" String - "graphql.error.path" null - } - } + "events" { + def events = new groovy.json.JsonSlurper().parseText(it) as List + events.size() == 2 + def event1 = events[0] + event1.name == "dd.graphql.query.error" + event1.time_unix_nano instanceof Long + def attrs1 = event1.attributes + attrs1.message == "Validation error of type FieldUndefined: Field 'title' in type 'Book' is undefined @ 'bookById/title'" + attrs1.locations == ["4:5"] + + def event2 = events[1] + event2.name == "dd.graphql.query.error" + event2.time_unix_nano instanceof Long + def attrs2 = event2.attributes + attrs2.message == "Validation error of type FieldUndefined: Field 'color' in type 'Book' is undefined @ 'bookById/color'" + attrs2.locations == ["5:5"] } - - + defaultTags() } - } span { operationName "graphql.validation" @@ -439,19 +433,18 @@ abstract class GraphQLTest extends VersionedNamingTestBase { "$Tags.COMPONENT" "graphql-java" "graphql.source" query "graphql.operation.name" null - "error.message" "Invalid Syntax : offending token ')' at line 2 column 30" - defaultTags() - } - events { - event { - eventName "error" - attributes { - "error.type" "graphql.parser.InvalidSyntaxException" - "error.message" "Invalid Syntax : offending token ')' at line 2 column 30" - "error.stack" String - "graphql.error.path" null - } + "error.message" { it.toLowerCase().startsWith("invalid syntax") } + "events" { + def events = new groovy.json.JsonSlurper().parseText(it) as List + events.size() == 1 + def event = events[0] + event.name == "dd.graphql.query.error" + event.time_unix_nano instanceof Long + def attrs = event.attributes + attrs.message == "Invalid Syntax : offending token ')' at line 2 column 25" + attrs.locations == ["2:25"] } + defaultTags() } } span { @@ -464,7 +457,7 @@ abstract class GraphQLTest extends VersionedNamingTestBase { tags { "$Tags.COMPONENT" "graphql-java" "error.type" "graphql.parser.InvalidSyntaxException" - "error.message" "Invalid Syntax : offending token ')' at line 2 column 30" + "error.message" { it.toLowerCase().startsWith("invalid syntax") } "error.stack" String defaultTags() } @@ -506,6 +499,17 @@ abstract class GraphQLTest extends VersionedNamingTestBase { "graphql.source" expectedQuery "graphql.operation.name" "findBookById" "error.message" "Exception while fetching data (/bookById/cover) : TEST" + "events" { + def events = new groovy.json.JsonSlurper().parseText(it) as List + events.size() == 1 + def event = events[0] + event.name == "dd.graphql.query.error" + event.time_unix_nano instanceof Long + def attrs = event.attributes + attrs.message == "Exception while fetching data (/bookById/cover) : TEST" + attrs.locations == ["4:5"] + attrs.path == ["bookById", "cover"] + } defaultTags() } } @@ -682,152 +686,6 @@ abstract class GraphQLTest extends VersionedNamingTestBase { } } } - - def "query fetch error with span events"() { - setup: - def query = 'query findBookById {\n' + - ' bookById(id: "book-1") {\n' + - ' id #test\n' + - ' cover\n' + // throws an exception when fetched - ' year\n' + // also throws an exception - ' }\n' + - '}' - def expectedQuery = 'query findBookById {\n' + - ' bookById(id: {String}) {\n' + - ' id\n' + - ' cover\n' + - ' year\n' + - ' }\n' + - '}\n' - ExecutionResult result = graphql.execute(query) - - expect: - !result.getErrors().isEmpty() - - assertTraces(1) { - trace(7) { - span { - operationName operation() - resourceName "findBookById" - spanType DDSpanTypes.GRAPHQL - errored true - measured true - parent() - tags { - "$Tags.COMPONENT" "graphql-java" - "graphql.source" expectedQuery - "graphql.operation.name" "findBookById" - "error.message" { it.contains("Exception while fetching data (/bookById/cover) : TEST") } - defaultTags() - } - events { - event { - eventName "error" - attributes { - "error.type" "java.lang.IllegalStateException" - "error.message" "TEST" - "error.stack" String - "graphql.error.path" "/bookById/cover" - } - } - event { - eventName "error" - attributes { - "error.type" "java.lang.IllegalStateException" - "error.message" "TEST" - "error.stack" String - "graphql.error.path" "/bookById/year" - } - } - } - } - span { - operationName "graphql.field" - resourceName "Book.cover" - childOf(span(0)) - spanType DDSpanTypes.GRAPHQL - errored true - measured true - tags { - "$Tags.COMPONENT" "graphql-java" - "graphql.type" "String" - "graphql.coordinates" "Book.cover" - "error.type" "java.lang.IllegalStateException" - "error.message" "TEST" - "error.stack" String - defaultTags() - } - } - span { - operationName "Book.year" - resourceName "Book.year" - childOf(span(0)) - spanType DDSpanTypes.GRAPHQL - errored true - measured true - tags { - "$Tags.COMPONENT" "graphql-java" - "graphql.type" "Int" - "graphql.coordinates" "Book.year" - "error.type" "java.lang.IllegalStateException" - "error.message" "TEST" - "error.stack" String - defaultTags() - } - } - span { - operationName "graphql.field" - resourceName "Query.bookById" - childOf(span(0)) - spanType DDSpanTypes.GRAPHQL - errored false - measured true - tags { - "$Tags.COMPONENT" "graphql-java" - "graphql.type" "Book" - "graphql.coordinates" "Query.bookById" - defaultTags() - } - } - span { - operationName "getBookById" - resourceName "book" - childOf(span(3)) - spanType null - errored false - measured false - tags { - "$Tags.COMPONENT" "trace" - defaultTags() - } - } - span { - operationName "graphql.validation" - resourceName "graphql.validation" - childOf(span(0)) - spanType DDSpanTypes.GRAPHQL - errored false - measured true - tags { - "$Tags.COMPONENT" "graphql-java" - defaultTags() - } - } - span { - operationName "graphql.parsing" - resourceName "graphql.parsing" - childOf(span(0)) - spanType DDSpanTypes.GRAPHQL - errored false - measured true - tags { - "$Tags.COMPONENT" "graphql-java" - defaultTags() - } - } - } - } - } } @Flaky