Skip to content

Add "data" property to Span #1235

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.

Sign up for GitHub

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

Closed
maciejwalkowiak wants to merge 4 commits into from
Closed

Add "data" property to Span #1235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e61efe2
Add "data" property to Span
maciejwalkowiak Feb 8, 2021
96eb13b
Polish.
maciejwalkowiak Feb 9, 2021
1f8790b
Polish tests.
maciejwalkowiak Feb 9, 2021
47ea0c8
Merge branch 'main' into gh-1233
bruno-garcia Feb 10, 2021
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# vNext

* Enchancement: Improve EventProcessor nullability annotations (#1229).
* Enchancement: Add "data" property to Span (#1235).
* Fix: Disable Gson HTML escaping

# 4.1.0
Expand Down
14 changes: 3 additions & 11 deletions sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,6 @@ public static void main(String[] args) throws InterruptedException {
// Performance configuration options
// Set what percentage of traces should be collected
options.setTracesSampleRate(1.0); // set 0.5 to send 50% of traces

// Determine traces sample rate based on the sampling context
options.setTracesSampler(
context -> {
// only 10% of transactions with "/product" prefix will be collected
if (!context.getTransactionContext().getName().startsWith("/products")) {
return 0.1;
} else {
return 0.5;
}
});
});

Sentry.addBreadcrumb(
Expand Down Expand Up @@ -139,8 +128,11 @@ public static void main(String[] args) throws InterruptedException {
// Transactions collect execution time of the piece of code that's executed between the start
// and finish of transaction.
ITransaction transaction = Sentry.startTransaction("transaction name");
transaction.setData("data-key", "data-value");
Sentry.configureScope(scope -> scope.setTransaction(transaction));
// Transactions can contain one or more Spans
ISpan outerSpan = transaction.startChild("child");
outerSpan.setData("span-data-key", "span-data-value");
Thread.sleep(100);
// Spans create a tree structure. Each span can have one ore more spans inside.
ISpan innerSpan = outerSpan.startChild("jdbc", "select * from product where id = :id");
Expand Down
18 changes: 14 additions & 4 deletions sentry/api/sentry.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,12 +282,14 @@ public abstract interface class io/sentry/ISerializer {
public abstract interface class io/sentry/ISpan {
public abstract fun finish ()V
public abstract fun finish (Lio/sentry/SpanStatus;)V
public abstract fun getData (Ljava/lang/String;)Ljava/lang/Object;
public abstract fun getDescription ()Ljava/lang/String;
public abstract fun getOperation ()Ljava/lang/String;
public abstract fun getSpanContext ()Lio/sentry/SpanContext;
public abstract fun getStatus ()Lio/sentry/SpanStatus;
public abstract fun getThrowable ()Ljava/lang/Throwable;
public abstract fun isFinished ()Z
public abstract fun setData (Ljava/lang/String;Ljava/lang/Object;)V
public abstract fun setDescription (Ljava/lang/String;)V
public abstract fun setOperation (Ljava/lang/String;)V
public abstract fun setStatus (Lio/sentry/SpanStatus;)V
Expand Down Expand Up @@ -348,13 +350,15 @@ public final class io/sentry/NoOpLogger : io/sentry/ILogger {
public final class io/sentry/NoOpSpan : io/sentry/ISpan {
public fun finish ()V
public fun finish (Lio/sentry/SpanStatus;)V
public fun getData (Ljava/lang/String;)Ljava/lang/Object;
public fun getDescription ()Ljava/lang/String;
public static fun getInstance ()Lio/sentry/NoOpSpan;
public fun getOperation ()Ljava/lang/String;
public fun getSpanContext ()Lio/sentry/SpanContext;
public fun getStatus ()Lio/sentry/SpanStatus;
public fun getThrowable ()Ljava/lang/Throwable;
public fun isFinished ()Z
public fun setData (Ljava/lang/String;Ljava/lang/Object;)V
public fun setDescription (Ljava/lang/String;)V
public fun setOperation (Ljava/lang/String;)V
public fun setStatus (Lio/sentry/SpanStatus;)V
Expand All @@ -369,6 +373,7 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction {
public fun finish ()V
public fun finish (Lio/sentry/SpanStatus;)V
public fun getContexts ()Lio/sentry/protocol/Contexts;
public fun getData (Ljava/lang/String;)Ljava/lang/Object;
public fun getDescription ()Ljava/lang/String;
public fun getEventId ()Lio/sentry/protocol/SentryId;
public static fun getInstance ()Lio/sentry/NoOpTransaction;
Expand All @@ -383,6 +388,7 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction {
public fun getTransaction ()Ljava/lang/String;
public fun isFinished ()Z
public fun isSampled ()Ljava/lang/Boolean;
public fun setData (Ljava/lang/String;Ljava/lang/Object;)V
public fun setDescription (Ljava/lang/String;)V
public fun setName (Ljava/lang/String;)V
public fun setOperation (Ljava/lang/String;)V
Expand Down Expand Up @@ -547,15 +553,19 @@ public abstract class io/sentry/SentryBaseEvent {
public fun getContexts ()Lio/sentry/protocol/Contexts;
public fun getEnvironment ()Ljava/lang/String;
public fun getEventId ()Lio/sentry/protocol/SentryId;
public fun getExtra (Ljava/lang/String;)Ljava/lang/Object;
public fun getOriginThrowable ()Ljava/lang/Throwable;
public fun getRelease ()Ljava/lang/String;
public fun getRequest ()Lio/sentry/protocol/Request;
public fun getSdk ()Lio/sentry/protocol/SdkVersion;
public fun getTag (Ljava/lang/String;)Ljava/lang/String;
public fun getThrowable ()Ljava/lang/Throwable;
public fun removeExtra (Ljava/lang/String;)V
public fun removeTag (Ljava/lang/String;)V
public fun setEnvironment (Ljava/lang/String;)V
public fun setEventId (Lio/sentry/protocol/SentryId;)V
public fun setExtra (Ljava/lang/String;Ljava/lang/Object;)V
public fun setExtras (Ljava/util/Map;)V
public fun setRelease (Ljava/lang/String;)V
public fun setRequest (Lio/sentry/protocol/Request;)V
public fun setSdk (Lio/sentry/protocol/SdkVersion;)V
Expand Down Expand Up @@ -638,7 +648,6 @@ public final class io/sentry/SentryEvent : io/sentry/SentryBaseEvent, io/sentry/
public fun getDebugMeta ()Lio/sentry/protocol/DebugMeta;
public fun getDist ()Ljava/lang/String;
public fun getExceptions ()Ljava/util/List;
public fun getExtra (Ljava/lang/String;)Ljava/lang/Object;
public fun getFingerprints ()Ljava/util/List;
public fun getLevel ()Lio/sentry/SentryLevel;
public fun getLogger ()Ljava/lang/String;
Expand All @@ -653,14 +662,11 @@ public final class io/sentry/SentryEvent : io/sentry/SentryBaseEvent, io/sentry/
public fun getUser ()Lio/sentry/protocol/User;
public fun isCrashed ()Z
public fun isErrored ()Z
public fun removeExtra (Ljava/lang/String;)V
public fun removeModule (Ljava/lang/String;)V
public fun setBreadcrumbs (Ljava/util/List;)V
public fun setDebugMeta (Lio/sentry/protocol/DebugMeta;)V
public fun setDist (Ljava/lang/String;)V
public fun setExceptions (Ljava/util/List;)V
public fun setExtra (Ljava/lang/String;Ljava/lang/Object;)V
public fun setExtras (Ljava/util/Map;)V
public fun setFingerprints (Ljava/util/List;)V
public fun setLevel (Lio/sentry/SentryLevel;)V
public fun setLogger (Ljava/lang/String;)V
Expand Down Expand Up @@ -842,6 +848,7 @@ public final class io/sentry/SentryTransaction : io/sentry/SentryBaseEvent, io/s
public fun <init> (Ljava/lang/String;Lio/sentry/SpanContext;Lio/sentry/IHub;)V
public fun finish ()V
public fun finish (Lio/sentry/SpanStatus;)V
public fun getData (Ljava/lang/String;)Ljava/lang/Object;
public fun getDescription ()Ljava/lang/String;
public fun getLatestActiveSpan ()Lio/sentry/Span;
public fun getName ()Ljava/lang/String;
Expand All @@ -852,6 +859,7 @@ public final class io/sentry/SentryTransaction : io/sentry/SentryBaseEvent, io/s
public fun getTransaction ()Ljava/lang/String;
public fun isFinished ()Z
public fun isSampled ()Ljava/lang/Boolean;
public fun setData (Ljava/lang/String;Ljava/lang/Object;)V
public fun setDescription (Ljava/lang/String;)V
public fun setName (Ljava/lang/String;)V
public fun setOperation (Ljava/lang/String;)V
Expand Down Expand Up @@ -911,11 +919,13 @@ public final class io/sentry/ShutdownHookIntegration : io/sentry/Integration {
public final class io/sentry/Span : io/sentry/SpanContext, io/sentry/ISpan {
public fun finish ()V
public fun finish (Lio/sentry/SpanStatus;)V
public fun getData (Ljava/lang/String;)Ljava/lang/Object;
public fun getSpanContext ()Lio/sentry/SpanContext;
public fun getStartTimestamp ()Ljava/util/Date;
public fun getThrowable ()Ljava/lang/Throwable;
public fun getTimestamp ()Ljava/util/Date;
public fun isFinished ()Z
public fun setData (Ljava/lang/String;Ljava/lang/Object;)V
public fun setThrowable (Ljava/lang/Throwable;)V
public fun startChild (Ljava/lang/String;)Lio/sentry/ISpan;
public fun startChild (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ISpan;
Expand Down
11 changes: 11 additions & 0 deletions sentry/src/main/java/io/sentry/ISpan.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,17 @@ public interface ISpan {
*/
void setTag(@NotNull String key, @NotNull String value);

/**
* Set extra data on span on transaction
*
* @param key the data key
* @param value the data value
*/
void setData(@NotNull String key, @NotNull Object value);

@Nullable
Object getData(@NotNull String key);

/**
* Returns if span has finished.
*
Expand Down
8 changes: 8 additions & 0 deletions sentry/src/main/java/io/sentry/NoOpSpan.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@ public void setThrowable(@Nullable Throwable throwable) {}
@Override
public void setTag(@NotNull String key, @NotNull String value) {}

@Override
public void setData(@NotNull String key, @NotNull Object value) {}

@Override
public @Nullable Object getData(@NotNull String key) {
return null;
}

@Override
public boolean isFinished() {
return false;
Expand Down
8 changes: 8 additions & 0 deletions sentry/src/main/java/io/sentry/NoOpTransaction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,4 +130,12 @@ public void setThrowable(@Nullable Throwable throwable) {}

@Override
public void setTag(@NotNull String key, @NotNull String value) {}

@Override
public void setData(@NotNull String key, @NotNull Object value) {}

@Override
public @Nullable Object getData(@NotNull String key) {
return null;
}
}
38 changes: 38 additions & 0 deletions sentry/src/main/java/io/sentry/SentryBaseEvent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import io.sentry.protocol.SentryId;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

Expand Down Expand Up @@ -59,6 +60,13 @@ public abstract class SentryBaseEvent {
*/
private String environment;

/**
* Arbitrary extra information set by the user.
*
* <p>```json { "extra": { "my_key": 1, "some_other_value": "foo bar" } }```
*/
private final Map<String, Object> extra = new ConcurrentHashMap<>();

/** The captured Throwable */
protected transient @Nullable Throwable throwable;

Expand Down Expand Up @@ -174,4 +182,34 @@ public String getEnvironment() {
public void setEnvironment(String environment) {
this.environment = environment;
}

@NotNull
Map<String, Object> getExtras() {
return extra;
}

/**
* Copies extras from the map given by parameter to extras on the event. If key already exists,
* its value gets overwritten.
*
* @param extra - the extras
*/
public void setExtras(final @NotNull Map<String, Object> extra) {
final Map<String, Object> copy = new HashMap<>(extra);
for (Map.Entry<String, Object> entry : copy.entrySet()) {
this.extra.put(entry.getKey(), entry.getValue());
}
Comment thread
maciejwalkowiak marked this conversation as resolved.
}

public void setExtra(final @NotNull String key, final @NotNull Object value) {
extra.put(key, value);
}

public void removeExtra(final @NotNull String key) {
extra.remove(key);
}

public @Nullable Object getExtra(final @NotNull String key) {
return extra.get(key);
}
}
35 changes: 0 additions & 35 deletions sentry/src/main/java/io/sentry/SentryEvent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,6 @@ public final class SentryEvent extends SentryBaseEvent implements IUnknownProper
/** List of breadcrumbs recorded before this event. */
private List<Breadcrumb> breadcrumbs;

/**
* Arbitrary extra information set by the user.
*
* <p>```json { "extra": { "my_key": 1, "some_other_value": "foo bar" } }```
*/
private Map<String, Object> extra;

private Map<String, Object> unknown;
/**
* Name and versions of all installed modules/packages/dependencies in the current
Expand Down Expand Up @@ -261,34 +254,6 @@ public void addBreadcrumb(final @Nullable String message) {
this.addBreadcrumb(new Breadcrumb(message));
}

Map<String, Object> getExtras() {
return extra;
}

public void setExtras(Map<String, Object> extra) {
this.extra = extra;
}

public void setExtra(String key, Object value) {
if (extra == null) {
extra = new HashMap<>();
}
extra.put(key, value);
}

public void removeExtra(@NotNull String key) {
if (extra != null) {
extra.remove(key);
}
}

public @Nullable Object getExtra(final @NotNull String key) {
if (extra != null) {
return extra.get(key);
}
return null;
}

@ApiStatus.Internal
@Override
public void acceptUnknownProperties(Map<String, Object> unknown) {
Expand Down
11 changes: 11 additions & 0 deletions sentry/src/main/java/io/sentry/SentryTransaction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public final class SentryTransaction extends SentryBaseEvent implements ITransac

/** A list of spans within this transaction. Can be empty. */
private final @NotNull List<Span> spans = new CopyOnWriteArrayList<>();

/**
* A hub this transaction is attached to. Marked as transient to be ignored during JSON
* serialization.
Expand Down Expand Up @@ -204,6 +205,16 @@ public void setDescription(@Nullable String description) {
return this.context;
}

@Override
public void setData(@NotNull String key, @NotNull Object value) {

@bruno-garcia bruno-garcia Feb 9, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it worth adding setData if event has setExtra already?
The other SDKs where transactions is just an event (without the separation we did here) will have just setExtra in the docs and samples?

@marandaneto marandaneto Feb 9, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

getsentry/sentry-cocoa#932 (comment)

@bruno-garcia bruno-garcia Feb 9, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Made a comment there. I think adding stuff to transaction such as that field isn't the way to go since this isn't what the protocol has. It'll be confusing to have both Extra and Data.

We have to remember that in reality Transaction is an Event, we just turned things around here to simplify (not sure it did at this point) things by not having all the error API/pipeline that worked for errors also work for Transctions.

@maciejwalkowiak maciejwalkowiak Feb 9, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It had to be added here because ITransaction extends ISpan. Docs also say that each transaction is a span, that's why we have such design. Now I believe it would be just better if we make this statement true again and add data to transactions on the Sentry backend.

@bruno-garcia bruno-garcia Feb 9, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe that's the problem. Having transation inherit from Span.

data will definitely not be added on the backend.

Sentry and its SDKs: Transction is event with type=transaction.

We decided to create a class for it and split some fields. Maybe we just don't inherit from Span?
Composition only in this case.

@maciejwalkowiak maciejwalkowiak Feb 9, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can think about it but it will probably change the design quite a bit. RIght now, when you get the current span to create a child span, you don't need to worry if it's a transaction or if it's a span - you just get an ISpan that lets you create children.

@bruno-garcia bruno-garcia Feb 9, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should discuss this because we'll need to get to a design where spans can be created alone.
Transaction is a container to capture a set of spans (and is a Sentry event) where a Span is not, and one day could be ingested individually (rather in batches) but outside of a Transaction.

Friday we can discuss this if not earlier. I'll see if @mitsuhiko can join us.

@marandaneto marandaneto Feb 10, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is part of our agenda for tomorrow

@marandaneto marandaneto Feb 15, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See #1233 (comment)

setExtra(key, value);
}

@Override
public @Nullable Object getData(@NotNull String key) {
return getExtra(key);
}

/**
* Sets transaction status.
*
Expand Down
15 changes: 15 additions & 0 deletions sentry/src/main/java/io/sentry/Span.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import io.sentry.protocol.SentryId;
import io.sentry.util.Objects;
import java.util.Date;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

Expand All @@ -13,6 +15,9 @@ public final class Span extends SpanContext implements ISpan {
/** The moment in time when span has ended. */
private @Nullable Date timestamp;

/** Arbitrary data associated with the Span. */
private final Map<String, Object> data = new ConcurrentHashMap<>();

/**
* A transaction this span is attached to. Marked as transient to be ignored during JSON
* serialization.
Expand Down Expand Up @@ -78,6 +83,16 @@ public void finish(@Nullable SpanStatus status) {
return this;
}

@Override
public void setData(@NotNull String key, @NotNull Object value) {
data.put(key, value);
}

@Override
public @Nullable Object getData(@NotNull String key) {
return data.get(key);
}

@Override
public boolean isFinished() {
return this.timestamp != null;
Expand Down
Loading