Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support addEvent Otel API #7408

Merged
merged 40 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
3de8679
Initial span events implementation + tests
mtoffl01 Aug 9, 2024
d654056
use DDTags.SPAN_EVENTS constant for setting tag
mtoffl01 Aug 9, 2024
f2bb9ac
added more tests and used moshi for json encoding
mtoffl01 Aug 12, 2024
f8bc1db
remove unused method toString on SpanEvent instance
mtoffl01 Aug 12, 2024
fae43bc
Started work on recordException
mtoffl01 Aug 13, 2024
761ad85
Removed moshi dependency
mtoffl01 Aug 13, 2024
add0ef0
Merge branch 'master' into mtoff/span-events
mtoffl01 Aug 14, 2024
ea4d8df
Fixed case where custom attributes should override default excpeption…
mtoffl01 Aug 15, 2024
00e47a5
Finishing up record exception unit tests
mtoffl01 Aug 15, 2024
80b8adf
Added annotations for clarity
mtoffl01 Aug 15, 2024
4e84b96
nits: reverted little changes that were superfluous
mtoffl01 Aug 15, 2024
35dd084
nits: reverted little changes that were superfluous
mtoffl01 Aug 15, 2024
a7626bb
Include new helperClassNames
mtoffl01 Aug 16, 2024
c611567
Added support for not-stringifying span events attributes, while main…
mtoffl01 Aug 16, 2024
8e353f7
fixing attributes manipulation for arrays
mtoffl01 Aug 16, 2024
90bc619
Fixed json parsing for array Attributes values
mtoffl01 Aug 26, 2024
14db2ad
Added better javadocs to functions
mtoffl01 Aug 26, 2024
565e43d
Fix exception.escaped and timesource for SpanEvent
mtoffl01 Sep 3, 2024
7655446
Fix exception.escaped and timesource for SpanEvent
mtoffl01 Sep 3, 2024
3123288
Update internal-api/src/main/java/datadog/trace/bootstrap/instrumenta…
mtoffl01 Sep 3, 2024
b4ae914
Update internal-api/src/main/java/datadog/trace/bootstrap/instrumenta…
mtoffl01 Sep 3, 2024
a2026b2
Update javadoc
mtoffl01 Sep 3, 2024
6690d3f
merge w upstream
mtoffl01 Sep 3, 2024
ccca4f3
fix nits
mtoffl01 Sep 6, 2024
0f04383
change SpanEvent method to private-package access scope
mtoffl01 Sep 6, 2024
9efd98e
Changed span event Attributes to a separate class
mtoffl01 Sep 10, 2024
87a7698
Fix error stack formatting for exception.stacktrace tag
mtoffl01 Sep 11, 2024
e6d69db
Remove superfluous import
mtoffl01 Sep 11, 2024
4d9b456
Optimized defaultExceptionAttributes
mtoffl01 Sep 11, 2024
f8721e4
Reverted all previous changes to spanlinks
mtoffl01 Sep 11, 2024
c34e139
Remove missed changes to spanlinks files
mtoffl01 Sep 11, 2024
41692b6
Merge branch 'master' into mtoff/span-events
mtoffl01 Sep 11, 2024
6d6cbc3
Moved exception attributes processing into spanevent class
mtoffl01 Sep 12, 2024
71d4b69
Fix error meta behavior with record exception
mtoffl01 Sep 12, 2024
684170a
Update comments and fix little nits
mtoffl01 Sep 12, 2024
af71579
Improve javadoc
mtoffl01 Sep 12, 2024
5950ade
javadoc improvement
mtoffl01 Sep 12, 2024
4051d56
feat(otel): Improve convention handling
PerfectSlayer Sep 16, 2024
e143d28
feat(otel): Simplify time API usage
PerfectSlayer Sep 16, 2024
3af7937
feat(otel): Improve tests
PerfectSlayer Sep 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,18 @@
import static java.lang.Boolean.parseBoolean;
import static java.util.Locale.ROOT;

import datadog.trace.api.DDTags;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.SpanAttributes;
import datadog.trace.bootstrap.instrumentation.api.Tags;
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.api.trace.SpanKind;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -125,6 +130,13 @@ public static void applyNamingConvention(AgentSpan span) {
}
}

public static void setEventsAsTag(AgentSpan span, List<SpanEvent> events) {
if (events == null || events.isEmpty()) {
return;
}
span.setTag(DDTags.SPAN_EVENTS, SpanEvent.toTag(events));
}

private static String computeOperationName(AgentSpan span) {
Object spanKingTag = span.getTag(SPAN_KIND);
SpanKind spanKind =
Expand Down Expand Up @@ -228,11 +240,12 @@ private static String getStringAttribute(AgentSpan span, String key) {
return (String) tag;
}

public static AgentSpan.Attributes convertAttributes(Attributes attributes) {
if (attributes.isEmpty()) {
public static AgentSpan.Attributes convertAttributes(
Attributes attributes, SpanAttributes.Builder.Format format) {
if (attributes == null || attributes.isEmpty()) {
return SpanAttributes.EMPTY;
}
SpanAttributes.Builder builder = SpanAttributes.builder();
SpanAttributes.Builder builder = SpanAttributes.builder(format);
attributes.forEach(
(attributeKey, value) -> {
String key = attributeKey.getKey();
Expand Down Expand Up @@ -269,4 +282,49 @@ public static AgentSpan.Attributes convertAttributes(Attributes attributes) {
});
return builder.build();
}

/**
* Generate Attributes about the exception using a default list + the additionalAttributes
* provided by the user
*
* <p>If the same key exists in defaultAttributes and additionalAttributes, the latter always
* wins.
*
* @param exception The Throwable from which to build default attributes
* @param additionalAttributes Attributes provided by the user
* @return Attributes collection that combines defaultAttributes and additionalAttributes
*/
public static Attributes processExceptionAttributes(
PerfectSlayer marked this conversation as resolved.
Show resolved Hide resolved
Throwable exception, Attributes additionalAttributes) {
// "exception.escaped" should be true if exception will "escape" the scope of the span. I'm not
// sure how to check that from within here. And it doesn't even look like Otel dynamically sets
// this field:
// https://github.com/open-telemetry/opentelemetry-java/blob/v1.41.0/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/ImmutableExceptionEventData.java#L22
boolean escaped = false;
mtoffl01 marked this conversation as resolved.
Show resolved Hide resolved
Map<String, Object> defaultAttributes =
new HashMap<String, Object>() {
{
put("exception.message", exception.getMessage());
put("exception.type", exception.getClass().getName());
put("exception.escaped", escaped);
put("exception.stacktrace", Arrays.toString(exception.getStackTrace()));
}
};
// Create an AttributesBuilder with the additionalAttributes provided
AttributesBuilder attrsBuilder = additionalAttributes.toBuilder();
for (String key : defaultAttributes.keySet()) {
// Add defaultAttributes onto the builder iff an equivalent key was not provided in
// additionalAttributes
if (additionalAttributes.get(AttributeKey.stringKey(key)) == null) {
// naive implementation, as the default attributes values re only strings and booleans
Object value = defaultAttributes.get(key);
if (value instanceof String) {
attrsBuilder.put(key, (String) defaultAttributes.get(key));
} else if (value instanceof Boolean) {
attrsBuilder.put(key, (Boolean) defaultAttributes.get(key));
}
}
}
return attrsBuilder.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import static datadog.opentelemetry.shim.trace.OtelConventions.applyNamingConvention;
import static datadog.opentelemetry.shim.trace.OtelConventions.applyReservedAttribute;
import static datadog.opentelemetry.shim.trace.OtelConventions.processExceptionAttributes;
import static datadog.opentelemetry.shim.trace.OtelConventions.setEventsAsTag;
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
import static io.opentelemetry.api.trace.StatusCode.ERROR;
import static io.opentelemetry.api.trace.StatusCode.OK;
Expand All @@ -18,6 +20,7 @@
import io.opentelemetry.api.trace.StatusCode;
import io.opentelemetry.api.trace.TraceFlags;
import io.opentelemetry.api.trace.TraceState;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;
import javax.annotation.ParametersAreNonnullByDefault;
Expand All @@ -27,6 +30,7 @@ public class OtelSpan implements Span {
private final AgentSpan delegate;
private StatusCode statusCode;
private boolean recording;
private List<SpanEvent> events;

public OtelSpan(AgentSpan delegate) {
this.delegate = delegate;
Expand Down Expand Up @@ -71,13 +75,23 @@ public <T> Span setAttribute(AttributeKey<T> key, T value) {

@Override
public Span addEvent(String name, Attributes attributes) {
// Not supported
if (this.recording) {
if (this.events == null || this.events.isEmpty()) {
mtoffl01 marked this conversation as resolved.
Show resolved Hide resolved
this.events = new ArrayList<>();
}
this.events.add(new SpanEvent(name, attributes));
}
return this;
}

@Override
public Span addEvent(String name, Attributes attributes, long timestamp, TimeUnit unit) {
// Not supported
if (this.recording) {
if (this.events == null || this.events.isEmpty()) {
mtoffl01 marked this conversation as resolved.
Show resolved Hide resolved
this.events = new ArrayList<>();
}
this.events.add(new SpanEvent(name, attributes, timestamp, unit));
}
return this;
}

Expand All @@ -100,7 +114,11 @@ public Span setStatus(StatusCode statusCode, String description) {
@Override
public Span recordException(Throwable exception, Attributes additionalAttributes) {
if (this.recording) {
// Store exception as span tags as span events are not supported yet
if (this.events == null || this.events.isEmpty()) {
mtoffl01 marked this conversation as resolved.
Show resolved Hide resolved
this.events = new ArrayList<>();
}
this.events.add(
new SpanEvent("exception", processExceptionAttributes(exception, additionalAttributes)));
this.delegate.addThrowable(exception, ErrorPriorities.UNSET);
}
return this;
Expand All @@ -118,13 +136,15 @@ public Span updateName(String name) {
public void end() {
this.recording = false;
applyNamingConvention(this.delegate);
setEventsAsTag(this.delegate, this.events);
this.delegate.finish();
}

@Override
public void end(long timestamp, TimeUnit unit) {
this.recording = false;
applyNamingConvention(this.delegate);
setEventsAsTag(this.delegate, this.events);
this.delegate.finish(unit.toMicros(timestamp));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import datadog.opentelemetry.shim.context.propagation.TraceStateHelper;
import datadog.trace.api.DDSpanId;
import datadog.trace.api.DDTraceId;
import datadog.trace.bootstrap.instrumentation.api.SpanAttributes.Builder.Format;
import datadog.trace.bootstrap.instrumentation.api.SpanLink;
import io.opentelemetry.api.trace.SpanContext;

Expand All @@ -19,6 +20,6 @@ public OtelSpanLink(SpanContext spanContext, io.opentelemetry.api.common.Attribu
DDSpanId.fromHex(spanContext.getSpanId()),
spanContext.isSampled() ? SAMPLED_FLAG : DEFAULT_FLAGS,
TraceStateHelper.encodeHeader(spanContext.getTraceState()),
convertAttributes(attributes));
convertAttributes(attributes, Format.LINKS));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package datadog.opentelemetry.shim.trace;

import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.SpanAttributes;
import edu.umd.cs.findbugs.annotations.NonNull;
import io.opentelemetry.api.common.Attributes;
import java.util.List;
import java.util.concurrent.TimeUnit;

public class SpanEvent {
mtoffl01 marked this conversation as resolved.
Show resolved Hide resolved

private final long timestamp;
private final String name;
private final AgentSpan.Attributes attributes;

public SpanEvent(String name, Attributes attributes) {
this.name = name;
this.attributes =
OtelConventions.convertAttributes(attributes, SpanAttributes.Builder.Format.EVENTS);
this.timestamp = timeNano();
}

public SpanEvent(String name, Attributes attributes, long timestamp, TimeUnit unit) {
this.name = name;
this.attributes =
OtelConventions.convertAttributes(attributes, SpanAttributes.Builder.Format.EVENTS);
this.timestamp = timeNano(timestamp, unit);
}

private static long timeNano() {
return System.nanoTime();
}

private static long timeNano(long timestamp, TimeUnit unit) {
return TimeUnit.NANOSECONDS.convert(timestamp, unit);
}

public String toString() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why toString() is using a JSON encoding instead of a default Java object representation?

Copy link
Contributor Author

@mtoffl01 mtoffl01 Sep 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To ensure that the final String product contains nested quotes in the expected places. Like the keys need to have quotes around them, and only String values need to have quotes around them. E.g,
"{\"name\":\"Mikayla\",\"age\":27}".
Also, there is all the specific Attributes manipulation to do, so just using the native toString() method wasn't sufficient (in my experience testing it?).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To ensure that the final String product contains nested quotes in the expected places

It sounds like you are trying to enforce business logic related to the events serialisation into the human readable String representation of the object instance, used for debugging. There is no "expected" quotes for debug logging.

About your example, I would expect to have "{Makayla, 27yo}". That's way more readable for debugging and not tied to an existing format (so people won't try to rely and on it and parse it).

If you need a method to return the JSON-encoded instance, simply create a toJson() method and leave the toString() for debugging instead.

StringBuilder builder =
new StringBuilder(
"{\"time_unix_nano\":" + this.timestamp + ",\"name\":\"" + this.name + "\"");
if (!this.attributes.isEmpty()) {
builder
.append(",\"attributes\":")
.append(SpanAttributes.JSONParser.toJson(this.attributes.asMap()));
}
return builder.append("}").toString();
}

@NonNull
public static String toTag(List<SpanEvent> events) {
mtoffl01 marked this conversation as resolved.
Show resolved Hide resolved
// TODO: Do we want to enforce a maximum tag size, like TAG_MAX_LENGTH in DDSpanLink?
mtoffl01 marked this conversation as resolved.
Show resolved Hide resolved
StringBuilder builder = new StringBuilder("[");
for (int i = 0; i < events.size(); i++) {
if (i > 0) {
builder.append(",");
mtoffl01 marked this conversation as resolved.
Show resolved Hide resolved
}
builder.append(events.get(i).toString());
}
return builder.append("]").toString();
mtoffl01 marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public String[] helperClassNames() {
"datadog.opentelemetry.shim.trace.OtelExtractedContext",
"datadog.opentelemetry.shim.trace.OtelConventions",
"datadog.opentelemetry.shim.trace.OtelConventions$1",
"datadog.opentelemetry.shim.trace.OtelConventions$2",
"datadog.opentelemetry.shim.trace.OtelSpan",
"datadog.opentelemetry.shim.trace.OtelSpan$1",
"datadog.opentelemetry.shim.trace.OtelSpan$NoopSpan",
Expand All @@ -77,6 +78,7 @@ public String[] helperClassNames() {
"datadog.opentelemetry.shim.trace.OtelTracer",
"datadog.opentelemetry.shim.trace.OtelTracerBuilder",
"datadog.opentelemetry.shim.trace.OtelTracerProvider",
"datadog.opentelemetry.shim.trace.SpanEvent",
};
}

Expand Down
Loading
Loading