From 67252d3b3f72d519410491a4fa863c30eba51034 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Fri, 15 Jan 2021 16:02:08 +0100 Subject: [PATCH 1/3] Set SpanContext on SentryTransaction to avoid potential NPE. --- .../java/io/sentry/SentryTransaction.java | 27 ++++++++++++------- .../test/java/io/sentry/GsonSerializerTest.kt | 1 + sentry/src/test/java/io/sentry/HubTest.kt | 2 +- .../test/java/io/sentry/SentryClientTest.kt | 1 + .../java/io/sentry/SentryTransactionTest.kt | 5 +++- 5 files changed, 24 insertions(+), 12 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryTransaction.java b/sentry/src/main/java/io/sentry/SentryTransaction.java index 240c1770b6..9ed16188fb 100644 --- a/sentry/src/main/java/io/sentry/SentryTransaction.java +++ b/sentry/src/main/java/io/sentry/SentryTransaction.java @@ -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; /** The {@code type} property is required in JSON payload sent to Sentry. */ @SuppressWarnings("UnusedVariable") @@ -57,7 +59,7 @@ public SentryTransaction( 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 = contexts; } /** @@ -137,17 +139,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 @@ -156,6 +158,7 @@ public void finish() { if (this.throwable != null) { hub.setSpanContext(this.throwable, this.getSpanContext()); } + this.getContexts().setTrace(this.context); this.hub.captureTransaction(this, null); } @@ -166,7 +169,7 @@ public void finish() { */ @Override public void setOperation(@Nullable String op) { - this.getContexts().getTrace().setOperation(op); + this.context.setOperation(op); } /** @@ -176,17 +179,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; } /** @@ -196,7 +199,7 @@ public void setDescription(@Nullable String description) { */ @Override public void setStatus(@Nullable SpanStatus spanStatus) { - this.getContexts().getTrace().setStatus(spanStatus); + this.context.setStatus(spanStatus); } /** @@ -239,4 +242,8 @@ Date getTimestamp() { } return null; } + + SpanContext getContext() { + return context; + } } diff --git a/sentry/src/test/java/io/sentry/GsonSerializerTest.kt b/sentry/src/test/java/io/sentry/GsonSerializerTest.kt index 4de1227651..af925981e5 100644 --- a/sentry/src/test/java/io/sentry/GsonSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/GsonSerializerTest.kt @@ -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) diff --git a/sentry/src/test/java/io/sentry/HubTest.kt b/sentry/src/test/java/io/sentry/HubTest.kt index 165a3248c7..c633d72d79 100644 --- a/sentry/src/test/java/io/sentry/HubTest.kt +++ b/sentry/src/test/java/io/sentry/HubTest.kt @@ -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 diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 8c3bf44980..27a6ea6757 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -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) diff --git a/sentry/src/test/java/io/sentry/SentryTransactionTest.kt b/sentry/src/test/java/io/sentry/SentryTransactionTest.kt index b83f9426a9..68e5900360 100644 --- a/sentry/src/test/java/io/sentry/SentryTransactionTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTransactionTest.kt @@ -133,13 +133,15 @@ 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) } @@ -147,6 +149,7 @@ class SentryTransactionTest { 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) } From ba18c6260deed53c07154f9e445764f83d760d6f Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Mon, 18 Jan 2021 16:37:05 +0100 Subject: [PATCH 2/3] Code review fixes. --- sentry/src/main/java/io/sentry/SentryTransaction.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryTransaction.java b/sentry/src/main/java/io/sentry/SentryTransaction.java index 9ed16188fb..ac5db8a398 100644 --- a/sentry/src/main/java/io/sentry/SentryTransaction.java +++ b/sentry/src/main/java/io/sentry/SentryTransaction.java @@ -49,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.context = contexts; + this.context = Objects.requireNonNull(context, "contexts is required"); } /** @@ -243,6 +242,7 @@ Date getTimestamp() { return null; } + @NotNull SpanContext getContext() { return context; } From 80fe1691df95851cccb79758a9302c435b847f93 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Mon, 18 Jan 2021 16:38:24 +0100 Subject: [PATCH 3/3] Changelog. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c921d102e6..d491f15eb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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