Only send session update for dropped events if state changed (#2002)

* Change order of filtering mechanisms and add early return * Only send session update for dropped events if state changed * Extract variable for throwable * Add changelog * Add changelog * Rename method * Rename things * Make var final * Apply suggestions from code review Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> * Update sentry/src/main/java/io/sentry/SentryClient.java Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> * Add tests to verify order, session updates and sending * Add debug log for dropped events ... ... where the session update is not sent because it does not change the health of the sesssion * Fix merge mistake Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com>
getsentry · May 3, 2022 · e91f397 · e91f397
1 parent 4fd621e
commit e91f397
Show file tree

Hide file tree

Showing 3 changed files with 385 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 ### Fix
 
 * Change order of event filtering mechanisms (#2001)
+* Only send session update for dropped events if state changed (#2002)
 
 ## 6.0.0-beta.2
 

diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java
@@ -126,7 +126,10 @@ private boolean shouldApplyScopeData(
       return SentryId.EMPTY_ID;
     }
 
-    Session session = null;
+    @Nullable
+    Session sessionBeforeUpdate =
+        scope != null ? scope.withSession((@Nullable Session session) -> {}) : null;
+    @Nullable Session session = null;
 
     if (event != null) {
       session = updateSessionData(event, hint, scope);
@@ -146,6 +149,18 @@ private boolean shouldApplyScopeData(
       }
     }
 
+    final boolean shouldSendSessionUpdate =
+        shouldSendSessionUpdateForDroppedEvent(sessionBeforeUpdate, session);
+
+    if (event == null && !shouldSendSessionUpdate) {
+      options
+          .getLogger()
+          .log(
+              SentryLevel.DEBUG,
+              "Not sending session update for dropped event as it did not cause the session health to change.");
+      return SentryId.EMPTY_ID;
+    }
+
     SentryId sentryId = SentryId.EMPTY_ID;
     if (event != null && event.getEventId() != null) {
       sentryId = event.getEventId();
@@ -156,8 +171,9 @@ private boolean shouldApplyScopeData(
           scope != null && scope.getTransaction() != null
               ? scope.getTransaction().traceState()
               : null;
-      final SentryEnvelope envelope =
-          buildEnvelope(event, getAttachments(scope, hint), session, traceState, null);
+      final boolean shouldSendAttachments = event != null;
+      List<Attachment> attachments = shouldSendAttachments ? getAttachments(scope, hint) : null;
+      final SentryEnvelope envelope = buildEnvelope(event, attachments, session, traceState, null);
 
       if (envelope != null) {
         transport.send(envelope, hint);
@@ -172,6 +188,32 @@ private boolean shouldApplyScopeData(
     return sentryId;
   }
 
+  private boolean shouldSendSessionUpdateForDroppedEvent(
+      @Nullable Session sessionBeforeUpdate, @Nullable Session sessionAfterUpdate) {
+    if (sessionAfterUpdate == null) {
+      return false;
+    }
+
+    if (sessionBeforeUpdate == null) {
+      return true;
+    }
+
+    final boolean didSessionMoveToCrashedState =
+        sessionAfterUpdate.getStatus() == Session.State.Crashed
+            && sessionBeforeUpdate.getStatus() != Session.State.Crashed;
+    if (didSessionMoveToCrashedState) {
+      return true;
+    }
+
+    final boolean didSessionMoveToErroredState =
+        sessionAfterUpdate.errorCount() > 0 && sessionBeforeUpdate.errorCount() <= 0;
+    if (didSessionMoveToErroredState) {
+      return true;
+    }
+
+    return false;
+  }
+
   private @Nullable List<Attachment> getAttachments(
       final @Nullable Scope scope, final @NotNull Map<String, Object> hint) {
     List<Attachment> attachments = null;