removing session tracking guard on hub and client (#338)

getsentry · Apr 2, 2020 · 3ca016c · 3ca016c
1 parent 314952b
commit 3ca016c
Show file tree

Hide file tree

Showing 9 changed files with 43 additions and 88 deletions.
diff --git a/sentry-android-core/proguard-rules.pro b/sentry-android-core/proguard-rules.pro
@@ -31,4 +31,7 @@
 # don't warn jetbrains annotations
 -dontwarn org.jetbrains.annotations.**
 
+# R8: Attribute Signature requires InnerClasses attribute. Check -keepattributes directive.
+-keepattributes InnerClasses
+
 ##---------------End: proguard configuration for Gson  ----------
diff --git a/sentry-core/src/main/java/io/sentry/core/Hub.java b/sentry-core/src/main/java/io/sentry/core/Hub.java
@@ -196,12 +196,6 @@ public void startSession() {
           .getLogger()
           .log(
               SentryLevel.WARNING, "Instance is disabled and this 'startSession' call is a no-op.");
-    } else if (!options.isEnableSessionTracking()) {
-      options
-          .getLogger()
-          .log(
-              SentryLevel.INFO,
-              "Session tracking is disabled and this 'startSession' call is a no-op.");
     } else {
       final StackItem item = this.stack.peek();
       if (item != null) {
@@ -227,12 +221,6 @@ public void endSession() {
       options
           .getLogger()
           .log(SentryLevel.WARNING, "Instance is disabled and this 'endSession' call is a no-op.");
-    } else if (!options.isEnableSessionTracking()) {
-      options
-          .getLogger()
-          .log(
-              SentryLevel.INFO,
-              "Session tracking is disabled and this 'endSession' call is a no-op.");
     } else {
       final StackItem item = this.stack.peek();
       if (item != null) {

diff --git a/sentry-core/src/main/java/io/sentry/core/SentryClient.java b/sentry-core/src/main/java/io/sentry/core/SentryClient.java
@@ -147,41 +147,38 @@ public SentryClient(final @NotNull SentryOptions options, @Nullable Connection c
   void updateSessionData(
       final @NotNull SentryEvent event, final @Nullable Object hint, final @Nullable Scope scope) {
     if (ApplyScopeUtils.shouldApplyScopeData(hint)) {
-      // safe guard
-      if (options.isEnableSessionTracking()) {
-        if (scope != null) {
-          scope.withSession(
-              session -> {
-                if (session != null) {
-                  Session.State status = null;
-                  if (event.isCrashed()) {
-                    status = Session.State.Crashed;
-                  }
+      if (scope != null) {
+        scope.withSession(
+            session -> {
+              if (session != null) {
+                Session.State status = null;
+                if (event.isCrashed()) {
+                  status = Session.State.Crashed;
+                }
 
-                  boolean crashedOrErrored = false;
-                  if (Session.State.Crashed == status || event.isErrored()) {
-                    crashedOrErrored = true;
-                  }
+                boolean crashedOrErrored = false;
+                if (Session.State.Crashed == status || event.isErrored()) {
+                  crashedOrErrored = true;
+                }
 
-                  String userAgent = null;
-                  if (event.getRequest() != null && event.getRequest().getHeaders() != null) {
-                    if (event.getRequest().getHeaders().containsKey("user-agent")) {
-                      userAgent = event.getRequest().getHeaders().get("user-agent");
-                    }
+                String userAgent = null;
+                if (event.getRequest() != null && event.getRequest().getHeaders() != null) {
+                  if (event.getRequest().getHeaders().containsKey("user-agent")) {
+                    userAgent = event.getRequest().getHeaders().get("user-agent");
                   }
+                }
 
-                  if (session.update(status, userAgent, crashedOrErrored)) {
-                    // a session update hint means its gonna only flush to the disk, but not to the
-                    // network
-                    captureSession(session, new SessionUpdateHint());
-                  }
-                } else {
-                  options.getLogger().log(SentryLevel.INFO, "Session is null on scope.withSession");
+                if (session.update(status, userAgent, crashedOrErrored)) {
+                  // a session update hint means its gonna only flush to the disk, but not to the
+                  // network
+                  captureSession(session, new SessionUpdateHint());
                 }
-              });
-        } else {
-          options.getLogger().log(SentryLevel.INFO, "Scope is null on client.captureEvent");
-        }
+              } else {
+                options.getLogger().log(SentryLevel.INFO, "Session is null on scope.withSession");
+              }
+            });
+      } else {
+        options.getLogger().log(SentryLevel.INFO, "Scope is null on client.captureEvent");
       }
     }
   }

diff --git a/sentry-core/src/main/java/io/sentry/core/SentryOptions.java b/sentry-core/src/main/java/io/sentry/core/SentryOptions.java
@@ -671,16 +671,16 @@ public void setAttachThreads(boolean attachThreads) {
   }
 
   /**
-   * Returns if the session tracking is enabled or not
+   * Returns if the automatic session tracking is enabled or not
    *
-   * @return trye if enabled or false otherwise
+   * @return true if enabled or false otherwise
    */
   public boolean isEnableSessionTracking() {
     return enableSessionTracking;
   }
 
   /**
-   * Enable or disable the session tracking
+   * Enable or disable the automatic session tracking
    *
    * @param enableSessionTracking true if enabled or false otherwise
    */

diff --git a/sentry-core/src/test/java/io/sentry/core/HubTest.kt b/sentry-core/src/test/java/io/sentry/core/HubTest.kt
@@ -321,9 +321,8 @@ class HubTest {
     }
 
     @Test
-    fun `when captureEvent is called session tracking is enabled but no session started, it should not capture a session`() {
+    fun `when captureEvent is called but no session started, it should not capture a session`() {
         val options = SentryOptions()
-        options.isEnableSessionTracking = true
         options.cacheDirPath = file.absolutePath
         options.dsn = "https://key@sentry.io/proj"
         options.setSerializer(mock())
@@ -864,24 +863,9 @@ class HubTest {
         verify(mockClient, never()).captureSession(any(), any())
     }
 
-    @Test
-    fun `when startSession is called and session tracking is disabled, do nothing`() {
-        val options = SentryOptions()
-        options.cacheDirPath = file.absolutePath
-        options.dsn = "https://key@sentry.io/proj"
-        options.setSerializer(mock())
-        val sut = Hub(options)
-        val mockClient = mock<ISentryClient>()
-        sut.bindClient(mockClient)
-
-        sut.startSession()
-        verify(mockClient, never()).captureSession(any(), any())
-    }
-
     @Test
     fun `when startSession is called, starts a session`() {
         val options = SentryOptions()
-        options.isEnableSessionTracking = true
         options.cacheDirPath = file.absolutePath
         options.dsn = "https://key@sentry.io/proj"
         options.setSerializer(mock())
@@ -894,9 +878,8 @@ class HubTest {
     }
 
     @Test
-    fun `when startSession is called and theres a session, stops it and starts a new one`() {
+    fun `when startSession is called and there's a session, stops it and starts a new one`() {
         val options = SentryOptions()
-        options.isEnableSessionTracking = true
         options.cacheDirPath = file.absolutePath
         options.dsn = "https://key@sentry.io/proj"
         options.setSerializer(mock())
@@ -944,7 +927,6 @@ class HubTest {
     @Test
     fun `when endSession is called, end a session`() {
         val options = SentryOptions()
-        options.isEnableSessionTracking = true
         options.cacheDirPath = file.absolutePath
         options.dsn = "https://key@sentry.io/proj"
         options.setSerializer(mock())
@@ -959,9 +941,8 @@ class HubTest {
     }
 
     @Test
-    fun `when endSession is called and theres no session, do nothing`() {
+    fun `when endSession is called and there's no session, do nothing`() {
         val options = SentryOptions()
-        options.isEnableSessionTracking = true
         options.cacheDirPath = file.absolutePath
         options.dsn = "https://key@sentry.io/proj"
         options.setSerializer(mock())

diff --git a/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt b/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt
@@ -454,7 +454,6 @@ class SentryClientTest {
 
     @Test
     fun `When event is Fatal or not handled, mark session as Crashed`() {
-        fixture.sentryOptions.isEnableSessionTracking = true
         val scope = Scope(fixture.sentryOptions)
         val session = scope.startSession().current
         val event = SentryEvent().apply {
@@ -466,7 +465,6 @@ class SentryClientTest {
 
     @Test
     fun `When event is non fatal, keep level as it is`() {
-        fixture.sentryOptions.isEnableSessionTracking = true
         val scope = Scope(fixture.sentryOptions)
         val session = scope.startSession().current
         val level = session.status
@@ -477,7 +475,6 @@ class SentryClientTest {
 
     @Test
     fun `When event is Fatal, increase errorCount`() {
-        fixture.sentryOptions.isEnableSessionTracking = true
         val scope = Scope(fixture.sentryOptions)
         val session = scope.startSession().current
         val event = SentryEvent().apply {
@@ -489,7 +486,6 @@ class SentryClientTest {
 
     @Test
     fun `When event is Errored, increase errorCount`() {
-        fixture.sentryOptions.isEnableSessionTracking = true
         val scope = Scope(fixture.sentryOptions)
         val session = scope.startSession().current
         val exceptions = mutableListOf<SentryException>()
@@ -502,8 +498,7 @@ class SentryClientTest {
     }
 
     @Test
-    fun `When event is non fatal or error, do not increase errorsCount`() {
-        fixture.sentryOptions.isEnableSessionTracking = true
+    fun `When event is non fatal nor errored, do not increase errorsCount`() {
         val scope = Scope(fixture.sentryOptions)
         val session = scope.startSession().current
         val errorCount = session.errorCount()
@@ -514,7 +509,6 @@ class SentryClientTest {
 
     @Test
     fun `When event has userAgent, set it into session`() {
-        fixture.sentryOptions.isEnableSessionTracking = true
         val scope = Scope(fixture.sentryOptions)
         val session = scope.startSession().current
         val event = SentryEvent().apply {
@@ -528,7 +522,6 @@ class SentryClientTest {
 
     @Test
     fun `When event has no userAgent, keep as it is`() {
-        fixture.sentryOptions.isEnableSessionTracking = true
         val scope = Scope(fixture.sentryOptions)
         val session = scope.startSession().current
         val userAgent = session.userAgent
@@ -541,8 +534,7 @@ class SentryClientTest {
     }
 
     @Test
-    fun `When capture an event and theres no session, do nothing`() {
-        fixture.sentryOptions.isEnableSessionTracking = true
+    fun `When capture an event and there's no session, do nothing`() {
         val scope = Scope(fixture.sentryOptions)
         val event = SentryEvent()
         fixture.getSut().updateSessionData(event, null, scope)
@@ -551,18 +543,9 @@ class SentryClientTest {
         }
     }
 
-    @Test
-    fun `When capture an event and session tracking is disabled, do nothing`() {
-        val scope = mock<Scope>()
-        val event = SentryEvent()
-        fixture.getSut().updateSessionData(event, null, scope)
-        verify(scope, never()).withSession(any())
-    }
-
     @Test
     fun `when captureEvent with sampling, session is still updated`() {
         fixture.sentryOptions.sampleRate = 1.0
-        fixture.sentryOptions.isEnableSessionTracking = true
         val sut = fixture.getSut()
 
         val event = SentryEvent().apply {

diff --git a/sentry-native-sample/build.gradle.kts b/sentry-native-sample/build.gradle.kts
@@ -6,7 +6,7 @@ plugins {
     id("com.ydq.android.gradle.native-aar.import")
 
     // apply sentry android gradle plugin
-//    id("io.sentry.android.gradle")
+    // id("io.sentry.android.gradle")
 }
 
 android {

diff --git a/sentry-native-sample/proguard-rules.pro b/sentry-native-sample/proguard-rules.pro
@@ -19,3 +19,6 @@
 # If you keep the line number information, uncomment this to
 # hide the original source file name.
 #-renamesourcefileattribute SourceFile
+
+# R8: Attribute Signature requires InnerClasses attribute. Check -keepattributes directive.
+-keepattributes InnerClasses
diff --git a/sentry-sample/src/main/AndroidManifest.xml b/sentry-sample/src/main/AndroidManifest.xml
@@ -55,7 +55,7 @@
 <!--    how to set a custom release-->
 <!--    <meta-data android:name="io.sentry.release" android:value="io.sentry.sample@1.0.0+10001" />-->
 
-<!--    how to enable session tracking-->
+<!--    how to enable automatic session tracking-->
     <meta-data android:name="io.sentry.session-tracking.enable" android:value="true" />
 
 <!--    how to set an environment-->