Skip to content

Commit

Permalink
Fix: set scope on transaction (#1409)
Browse files Browse the repository at this point in the history
  • Loading branch information
maciejwalkowiak authored Apr 19, 2021
1 parent 75a8742 commit 838067f
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 51 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* Fix: use connection and read timeouts in ApacheHttpClient based transport (#1397)
* Ref: Refactor converting HttpServletRequest to Sentry Request in Spring integration (#1387)
* Fix: handle network errors in SentrySpanClientHttpRequestInterceptor (#1407)
* Fix: set scope on transaction (#1409)

# 4.4.0-alpha.2

Expand Down
66 changes: 33 additions & 33 deletions sentry/src/main/java/io/sentry/SentryClient.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.sentry;

import io.sentry.hints.DiskFlushNotification;
import io.sentry.protocol.Contexts;
import io.sentry.protocol.SentryId;
import io.sentry.protocol.SentrySpan;
import io.sentry.protocol.SentryTransaction;
Expand Down Expand Up @@ -447,28 +448,16 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec
return transaction;
}

private @NotNull SentryTransaction applyScope(
@NotNull SentryTransaction transaction, final @Nullable Scope scope) {
if (scope != null) {
if (transaction.getRequest() == null) {
transaction.setRequest(scope.getRequest());
}
}
return transaction;
}

private @Nullable SentryEvent applyScope(
@NotNull SentryEvent event, final @Nullable Scope scope, final @Nullable Object hint) {
if (scope != null) {
applyScope(event, scope);
if (event.getTransaction() == null) {
event.setTransaction(scope.getTransactionName());
}
if (event.getUser() == null) {
event.setUser(scope.getUser());
}
if (event.getRequest() == null) {
event.setRequest(scope.getRequest());
}
if (event.getFingerprints() == null) {
event.setFingerprints(scope.getFingerprint());
}
Expand All @@ -477,15 +466,6 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec
} else {
sortBreadcrumbsByDate(event, scope.getBreadcrumbs());
}
if (event.getTags() == null) {
event.setTags(new HashMap<>(scope.getTags()));
} else {
for (Map.Entry<String, String> item : scope.getTags().entrySet()) {
if (!event.getTags().containsKey(item.getKey())) {
event.getTags().put(item.getKey(), item.getValue());
}
}
}
if (event.getExtras() == null) {
event.setExtras(new HashMap<>(scope.getExtras()));
} else {
Expand All @@ -495,17 +475,6 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec
}
}
}
try {
for (Map.Entry<String, Object> entry : scope.getContexts().clone().entrySet()) {
if (!event.getContexts().containsKey(entry.getKey())) {
event.getContexts().put(entry.getKey(), entry.getValue());
}
}
} catch (CloneNotSupportedException e) {
options
.getLogger()
.log(SentryLevel.ERROR, "An error has occurred when cloning Contexts", e);
}
// Level from scope exceptionally take precedence over the event
if (scope.getLevel() != null) {
event.setLevel(scope.getLevel());
Expand All @@ -521,6 +490,37 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec
return event;
}

private <T extends SentryBaseEvent> T applyScope(
final @NotNull T sentryBaseEvent, final @Nullable Scope scope) {
if (scope != null) {
if (sentryBaseEvent.getRequest() == null) {
sentryBaseEvent.setRequest(scope.getRequest());
}
if (sentryBaseEvent.getTags() == null) {
sentryBaseEvent.setTags(new HashMap<>(scope.getTags()));
} else {
for (Map.Entry<String, String> item : scope.getTags().entrySet()) {
if (!sentryBaseEvent.getTags().containsKey(item.getKey())) {
sentryBaseEvent.getTags().put(item.getKey(), item.getValue());
}
}
}
final Contexts contexts = sentryBaseEvent.getContexts();
try {
for (Map.Entry<String, Object> entry : scope.getContexts().clone().entrySet()) {
if (!contexts.containsKey(entry.getKey())) {
contexts.put(entry.getKey(), entry.getValue());
}
}
} catch (CloneNotSupportedException e) {
options
.getLogger()
.log(SentryLevel.ERROR, "An error has occurred when cloning Contexts", e);
}
}
return sentryBaseEvent;
}

private void sortBreadcrumbsByDate(
final @NotNull SentryEvent event, final @NotNull Collection<Breadcrumb> breadcrumbs) {
final List<Breadcrumb> sortedBreadcrumbs = event.getBreadcrumbs();
Expand Down
59 changes: 41 additions & 18 deletions sentry/src/test/java/io/sentry/SentryClientTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ class SentryClientTest {
@Test
fun `transactions are sent using connection`() {
val sut = fixture.getSut()
sut.captureTransaction(SentryTransaction(SentryTracer(TransactionContext("a-transaction", "op"), mock())), mock(), null)
sut.captureTransaction(SentryTransaction(SentryTracer(TransactionContext("a-transaction", "op"), mock())), Scope(fixture.sentryOptions), null)
verify(fixture.transport).send(check {
val transaction = it.items.first().getTransaction(fixture.sentryOptions.serializer)
assertNotNull(transaction)
Expand All @@ -771,14 +771,14 @@ class SentryClientTest {
span1.finish()
val span2 = transaction.startChild("span2")

sut.captureTransaction(SentryTransaction(transaction), mock(), null)
sut.captureTransaction(SentryTransaction(transaction), Scope(fixture.sentryOptions), null)
verify(fixture.transport).send(check {
val sentTransaction = it.items.first().getTransaction(fixture.sentryOptions.serializer)
assertNotNull(sentTransaction) { tx ->
val sentSpanIds = tx.spans.map { span -> span.spanId }
assertTrue(sentSpanIds.contains(span1.spanContext.spanId))
assertFalse(sentSpanIds.contains(span2.spanContext.spanId))
}
assertNotNull(sentTransaction) { tx ->
val sentSpanIds = tx.spans.map { span -> span.spanId }
assertTrue(sentSpanIds.contains(span1.spanContext.spanId))
assertFalse(sentSpanIds.contains(span2.spanContext.spanId))
}
}, eq(null))
}

Expand All @@ -800,6 +800,28 @@ class SentryClientTest {
verifyAttachmentsInEnvelope(transaction.eventId)
}

@Test
fun `when captureTransaction scope is applied to transaction`() {
val sut = fixture.getSut()
val scope = Scope(fixture.sentryOptions)
scope.setTag("tag1", "value1")
scope.setContexts("context-key", "context-value")
scope.request = Request().apply {
url = "/url"
}
sut.captureTransaction(SentryTransaction(SentryTracer(TransactionContext("a-transaction", "op"), mock())), scope, null)
verify(fixture.transport).send(check { envelope ->
val transaction = envelope.items.first().getTransaction(fixture.sentryOptions.serializer)
assertNotNull(transaction) {
assertEquals("value1", it.getTag("tag1"))
assertEquals(mapOf("value" to "context-value"), it.contexts["context-key"])
assertNotNull(it.request) { request ->
assertEquals("/url", request.url)
}
}
}, eq(null))
}

@Test
fun `when scope's active span is a transaction, transaction context is applied to an event`() {
val event = SentryEvent()
Expand Down Expand Up @@ -964,17 +986,18 @@ class SentryClientTest {
return Session("dis", User(), "env", release)
}

private val userFeedback: UserFeedback get() {
val eventId = SentryId("c2fb8fee2e2b49758bcb67cda0f713c7")
val userFeedback = UserFeedback(eventId)
userFeedback.apply {
name = "John"
email = "[email protected]"
comments = "comment"
}
private val userFeedback: UserFeedback
get() {
val eventId = SentryId("c2fb8fee2e2b49758bcb67cda0f713c7")
val userFeedback = UserFeedback(eventId)
userFeedback.apply {
name = "John"
email = "[email protected]"
comments = "comment"
}

return userFeedback
}
return userFeedback
}

internal class CustomTransportGate : ITransportGate {
override fun isConnected(): Boolean = false
Expand Down Expand Up @@ -1022,7 +1045,7 @@ class SentryClientTest {

val attachmentItemTooBig = attachmentItems.last()
assertFailsWith<SentryEnvelopeException>("Getting data from attachment should" +
"throw an exception, because the attachment is too big.") {
"throw an exception, because the attachment is too big.") {
attachmentItemTooBig.data
}
}, isNull())
Expand Down

0 comments on commit 838067f

Please sign in to comment.