From 04c874d1ca5aaf343cff518ae1765242d68cea2e Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Fri, 19 Feb 2021 15:42:19 +0100 Subject: [PATCH 1/4] Fix: Discard unfinished Spans before sending them over to Sentry Fixes #1262 --- .../src/main/java/io/sentry/SentryClient.java | 7 +++++++ .../test/java/io/sentry/SentryClientTest.kt | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 46b071a45be..d93c84cbbcb 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -409,6 +409,13 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec } } } + final List unfinishedSpans = new ArrayList<>(); + for (Span span : transaction.getSpans()) { + if (!span.isFinished()) { + unfinishedSpans.add(span); + } + } + transaction.getSpans().removeAll(unfinishedSpans); return transaction; } diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 8a3f97a8371..e80b31b6d34 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -754,6 +754,25 @@ class SentryClientTest { }, eq(null)) } + @Test + fun `when captureTransactions unfinished spans are removed`() { + val sut = fixture.getSut() + val transaction = SentryTransaction("a-transaction", "op") + val span1 = transaction.startChild("span1") + span1.finish() + val span2 = transaction.startChild("span2") + + sut.captureTransaction(transaction, mock(), 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)) + } + }, eq(null)) + } + @Test fun `when captureTransaction with attachments`() { val transaction = SentryTransaction("a-transaction", "op") From 2f1eb53d8441c728f48448a0b0ab5fcf06f1fcbe Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Mon, 22 Feb 2021 06:10:43 +0100 Subject: [PATCH 2/4] Add warning message. --- sentry/src/main/java/io/sentry/SentryClient.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index d93c84cbbcb..b6c20c7af2d 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -415,6 +415,11 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec unfinishedSpans.add(span); } } + if (options.getLogger().isEnabled(SentryLevel.WARNING) && !unfinishedSpans.isEmpty()) { + options + .getLogger() + .log(SentryLevel.WARNING, "Dropping %d unfinished spans", unfinishedSpans.size()); + } transaction.getSpans().removeAll(unfinishedSpans); return transaction; } From 9fc3745c94444d2ca8b28676c6ba46681e321188 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Mon, 22 Feb 2021 06:15:04 +0100 Subject: [PATCH 3/4] changelog. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e2655c8f96..93ac5463d1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # vNext + * Fix: Remove experimental annotation for Attachment #1257 * Fix: Mark stacktrace as snapshot if captured at arbitrary moment #1231 * Enchancement: Improve EventProcessor nullability annotations (#1229). @@ -15,6 +16,7 @@ * Enchancement: Simplify Sentry configuration in Spring integration (#1259) * Fix: Fix SentryTransaction#getStatus NPE (#1273) * Enchancement: Optimize SentryTracingFilter when hub is disabled. +* Fix: Discard unfinished Spans before sending them over to Sentry (#1279) Breaking Changes: * Enchancement: SentryExceptionResolver should not send handled errors by default (#1248). From 26a18a9a205277b079f834ad933169c8cac5786b Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Mon, 22 Feb 2021 09:48:43 +0100 Subject: [PATCH 4/4] Remove check for log level. --- sentry/src/main/java/io/sentry/SentryClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index b6c20c7af2d..a779b4b5889 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -415,7 +415,7 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec unfinishedSpans.add(span); } } - if (options.getLogger().isEnabled(SentryLevel.WARNING) && !unfinishedSpans.isEmpty()) { + if (!unfinishedSpans.isEmpty()) { options .getLogger() .log(SentryLevel.WARNING, "Dropping %d unfinished spans", unfinishedSpans.size());