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

Set SpanContext on SentryTransaction to avoid potential NPE. #1173

Merged
merged 4 commits into from
Jan 18, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

* Enchancement: Support SENTRY_TRACES_SAMPLE_RATE conf. via env variables (#1171)
* Enhancement: Pass request to CustomSamplingContext in Spring integration (#1172)
* Ref: Set SpanContext on SentryTransaction to avoid potential NPE (#1173)

# 4.0.0-alpha.3

Expand Down
33 changes: 20 additions & 13 deletions sentry/src/main/java/io/sentry/SentryTransaction.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ public final class SentryTransaction extends SentryBaseEvent implements ITransac
* A hub this transaction is attached to. Marked as transient to be ignored during JSON
* serialization.
*/
private @NotNull final transient IHub hub;
private final transient @NotNull IHub hub;

private final transient @NotNull SpanContext context;
maciejwalkowiak marked this conversation as resolved.
Show resolved Hide resolved

/** The {@code type} property is required in JSON payload sent to Sentry. */
@SuppressWarnings("UnusedVariable")
Expand All @@ -47,17 +49,16 @@ public final class SentryTransaction extends SentryBaseEvent implements ITransac
* Creates transaction with name and contexts.
*
* @param name - transaction name
* @param contexts - transaction contexts
* @param context - transaction contexts
* @param hub - the hub
*/
@TestOnly
public SentryTransaction(
final @NotNull String name, final @NotNull SpanContext contexts, final @NotNull IHub hub) {
Objects.requireNonNull(contexts, "contexts are required");
final @NotNull String name, final @NotNull SpanContext context, final @NotNull IHub hub) {
this.transaction = Objects.requireNonNull(name, "name is required");
this.startTimestamp = DateUtils.getCurrentDateTime();
this.hub = Objects.requireNonNull(hub, "hub is required");
this.getContexts().setTrace(contexts);
this.context = Objects.requireNonNull(context, "contexts is required");
}

/**
Expand Down Expand Up @@ -137,17 +138,17 @@ public void setName(final @NotNull String name) {

@NotNull
SpanId getSpanId() {
return getContexts().getTrace().getSpanId();
return this.context.getSpanId();
}

@NotNull
SentryId getTraceId() {
return getContexts().getTrace().getTraceId();
return this.context.getTraceId();
}

@Override
public @Nullable Boolean isSampled() {
return getContexts().getTrace().getSampled();
return this.context.getSampled();
}

@Override
Expand All @@ -156,6 +157,7 @@ public void finish() {
if (this.throwable != null) {
hub.setSpanContext(this.throwable, this.getSpanContext());
}
this.getContexts().setTrace(this.context);
maciejwalkowiak marked this conversation as resolved.
Show resolved Hide resolved
this.hub.captureTransaction(this, null);
}

Expand All @@ -166,7 +168,7 @@ public void finish() {
*/
@Override
public void setOperation(@Nullable String op) {
this.getContexts().getTrace().setOperation(op);
this.context.setOperation(op);
}

/**
Expand All @@ -176,17 +178,17 @@ public void setOperation(@Nullable String op) {
*/
@Override
public void setDescription(@Nullable String description) {
this.getContexts().getTrace().setDescription(description);
this.context.setDescription(description);
}

@Override
public @Nullable String getDescription() {
return this.getContexts().getTrace().getDescription();
return this.context.getDescription();
}

@Override
public @NotNull SpanContext getSpanContext() {
return this.getContexts().getTrace();
return this.context;
}

/**
Expand All @@ -196,7 +198,7 @@ public void setDescription(@Nullable String description) {
*/
@Override
public void setStatus(@Nullable SpanStatus spanStatus) {
this.getContexts().getTrace().setStatus(spanStatus);
this.context.setStatus(spanStatus);
}

/**
Expand Down Expand Up @@ -239,4 +241,9 @@ Date getTimestamp() {
}
return null;
}

@NotNull
SpanContext getContext() {
maciejwalkowiak marked this conversation as resolved.
Show resolved Hide resolved
return context;
}
}
1 change: 1 addition & 0 deletions sentry/src/test/java/io/sentry/GsonSerializerTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ class GsonSerializerTest {
trace.status = SpanStatus.OK
trace.setTag("myTag", "myValue")
val transaction = SentryTransaction("transaction-name", trace, mock())
transaction.finish()

val stringWriter = StringWriter()
fixture.serializer.serialize(transaction, stringWriter)
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/test/java/io/sentry/HubTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -1045,7 +1045,7 @@ class HubTest {

val transaction = hub.startTransaction(contexts)
assertTrue(transaction is SentryTransaction)
assertEquals(contexts, transaction.contexts.trace)
assertEquals(contexts, transaction.context)
}

@Test
Expand Down
1 change: 1 addition & 0 deletions sentry/src/test/java/io/sentry/SentryClientTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,7 @@ class SentryClientTest {
val scope = createScope()
val transaction = SentryTransaction("name")
scope.setTransaction(transaction)
transaction.finish()
sut.captureEvent(event, scope)
assertNotNull(event.contexts.trace)
assertEquals(transaction.contexts.trace, event.contexts.trace)
Expand Down
5 changes: 4 additions & 1 deletion sentry/src/test/java/io/sentry/SentryTransactionTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -133,20 +133,23 @@ class SentryTransactionTest {
fun `setting op sets op on TraceContext`() {
val transaction = SentryTransaction("name")
transaction.setOperation("op")
transaction.finish()
assertEquals("op", transaction.contexts.trace!!.operation)
}

@Test
fun `setting description sets description on TraceContext`() {
val transaction = SentryTransaction("name")
transaction.setDescription("desc")
transaction.description = "desc"
transaction.finish()
assertEquals("desc", transaction.contexts.trace!!.description)
}

@Test
fun `setting status sets status on TraceContext`() {
val transaction = SentryTransaction("name")
transaction.setStatus(SpanStatus.ALREADY_EXISTS)
transaction.finish()
assertEquals(SpanStatus.ALREADY_EXISTS, transaction.contexts.trace!!.status)
}

Expand Down