Merge pull request #128 in OP/openkit-java from bugfix/ONE-42563-fix-…

…end-session-issue-after-splitting to release/2.0 * commit '97327bc04e280c35c21f5dab7f85e3ba4699d751': Do not send end session event for split sessions GitOrigin-RevId: 68bbca0251f8abf32f0ef6405ecc6eba50384fd8
Dynatrace · Jul 7, 2020 · eaa9ac3 · eaa9ac3
1 parent c58df70
commit eaa9ac3
Show file tree

Hide file tree

Showing 9 changed files with 100 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,12 @@
 
 ## [Unreleased](https://github.com/Dynatrace/openkit-java/compare/v2.0.0...HEAD)
 
+### Changed
+- Fix issue with sessions being closed after splitting.
+  This happened because OpenKit was sending an end session event right after splitting.
+  New behavior is to only send the end session event if explicitly requested via
+  the `Session.end()` method and only for the active session.
+
 ## 2.0.0 [Release date: 2020-06-24]
 [GitHub Releases](https://github.com/Dynatrace/openkit-java/releases/tag/v2.0.0)
 

diff --git a/src/main/java/com/dynatrace/openkit/core/SessionWatchdogContext.java b/src/main/java/com/dynatrace/openkit/core/SessionWatchdogContext.java
@@ -105,7 +105,7 @@ private long closeExpiredSessions() {
         }
 
         for (SessionImpl session : sessionsToEnd) {
-            session.end();
+            session.end(false);
         }
 
         return sleepTimeInMillis;

diff --git a/src/main/java/com/dynatrace/openkit/core/communication/BeaconSendingFlushSessionsState.java b/src/main/java/com/dynatrace/openkit/core/communication/BeaconSendingFlushSessionsState.java
@@ -49,7 +49,7 @@ void doExecute(BeaconSendingContext context) {
         // end open sessions -> will be flushed afterwards
         List<SessionImpl> openSessions = context.getAllOpenAndConfiguredSessions();
         for (SessionImpl openSession : openSessions) {
-            openSession.end();
+            openSession.end(false);
         }
 
         // flush already finished (and previously ended) sessions

diff --git a/src/main/java/com/dynatrace/openkit/core/objects/SessionImpl.java b/src/main/java/com/dynatrace/openkit/core/objects/SessionImpl.java
@@ -170,6 +170,10 @@ public WebRequestTracer traceWebRequest(String url) {
 
     @Override
     public void end() {
+        end(true);
+    }
+
+    public void end(boolean sendSessionEndEvent) {
         if (logger.isDebugEnabled()) {
             logger.debug(this + "end()");
         }
@@ -191,8 +195,10 @@ public void end() {
             }
         }
 
-        // create end session data on beacon
-        beacon.endSession();
+        // send the end event, only if a session is explicitly ended
+        if (sendSessionEndEvent) {
+            beacon.endSession();
+        }
 
         state.markAsFinished();
 
@@ -215,7 +221,7 @@ public boolean tryEnd() {
             }
 
             if (getChildCount() == 0) {
-                end();
+                end(false);
                 return true;
             }
 
@@ -304,7 +310,7 @@ void onChildClosed(OpenKitObject childObject) {
             removeChildFromList(childObject);
 
             if (state.wasTriedForEnding() && getChildCount() == 0) {
-                end();
+                end(false);
             }
         }
     }

diff --git a/src/main/java/com/dynatrace/openkit/core/objects/SessionProxyImpl.java b/src/main/java/com/dynatrace/openkit/core/objects/SessionProxyImpl.java
@@ -28,6 +28,7 @@
 
 import java.io.IOException;
 import java.net.URLConnection;
+import java.util.Iterator;
 import java.util.List;
 
 /**
@@ -208,18 +209,46 @@ public void end() {
             isFinished = true;
         }
 
+        closeChildObjects();
+
+        parent.onChildClosed(this);
+        sessionWatchdog.removeFromSplitByTimeout(this);
+    }
+
+    /**
+     * Close all child objects of this {@link SessionProxyImpl} which are still open.
+     */
+    void closeChildObjects() {
         List<OpenKitObject> childObjects = getCopyOfChildObjects();
-        for (OpenKitObject childObject : childObjects) {
-            try {
-                childObject.close();
-            } catch (IOException e) {
-                // should not happen, nevertheless let's log an error
-                logger.error(this + "Caught IOException while closing OpenKitObject (" + childObject + ")", e);
+
+        SessionImpl lastSession = null;
+        Iterator<OpenKitObject> childObjectsIterator = childObjects.iterator();
+        while (childObjectsIterator.hasNext()) {
+            OpenKitObject childObject = childObjectsIterator.next();
+            if (childObject instanceof SessionImpl) {
+                // child object is a session - special treatment is needed for sessions
+                SessionImpl childSession = (SessionImpl)childObject;
+                // end the child session and send the end session event
+                // if the child session is the current session
+                childSession.end(childSession == currentSession);
+            } else {
+                closeChildObject(childObject);
             }
         }
+    }
 
-        parent.onChildClosed(this);
-        sessionWatchdog.removeFromSplitByTimeout(this);
+    /**
+     * Close single child object.
+     *
+     * @param childObject Child object to close.
+     */
+    private void closeChildObject(OpenKitObject childObject) {
+        try {
+            childObject.close();
+        } catch (IOException e) {
+            // should not happen, nevertheless let's log an error
+            logger.error(this + "Caught IOException while closing OpenKitObject (" + childObject + ")", e);
+        }
     }
 
     /**

diff --git a/src/test/java/com/dynatrace/openkit/core/SessionWatchdogContextTest.java b/src/test/java/com/dynatrace/openkit/core/SessionWatchdogContextTest.java
@@ -190,7 +190,7 @@ public void executeEndsSessionsWithExpiredGracePeriod() {
         target.execute();
 
         // then
-        verify(mockSession, times(1)).end();
+        verify(mockSession, times(1)).end(false);
         assertThat(target.getSessionsToClose().size(), is(0));
     }
 
@@ -208,7 +208,7 @@ public void executeEndsSessionsWithGraceEndTimeSameAsCurrentTime() {
         target.execute();
 
         // then
-        verify(mockSession, times(1)).end();
+        verify(mockSession, times(1)).end(false);
         assertThat(target.getSessionsToClose().size(), is(0));
     }
 
@@ -245,7 +245,7 @@ public void executeSleepsDefaultTimeIfSessionIsExpiredAndNoFurtherNonExpiredSess
 
         // then
         verify(mockTimingProvider, times(1)).sleep(SessionWatchdogContext.DEFAULT_SLEEP_TIME_IN_MILLIS);
-        verify(mockSession, times(1)).end();
+        verify(mockSession, times(1)).end(false);
     }
 
     @Test

diff --git a/...st/java/com/dynatrace/openkit/core/communication/BeaconSendingFlushSessionsStateTest.java b/...st/java/com/dynatrace/openkit/core/communication/BeaconSendingFlushSessionsStateTest.java
@@ -127,8 +127,8 @@ public void aBeaconSendingFlushSessionsClosesOpenSessions() {
         target.doExecute(mockContext);
 
         // verify that open sessions are closed
-        verify(mockSession1Open, times(1)).end();
-        verify(mockSession2Open, times(1)).end();
+        verify(mockSession1Open, times(1)).end(false);
+        verify(mockSession2Open, times(1)).end(false);
     }
 
     @Test

diff --git a/src/test/java/com/dynatrace/openkit/core/objects/SessionImplTest.java b/src/test/java/com/dynatrace/openkit/core/objects/SessionImplTest.java
@@ -347,13 +347,27 @@ public void reportCrashLogsInvocation() {
     public void endSessionFinishesSessionOnBeacon() {
         // given
         when(mockBeacon.getCurrentTimestamp()).thenReturn(1234L);
-        SessionImpl target = createSession().build();
+        SessionImpl target = spy(createSession().build());
 
         // when
         target.end();
 
         // then
         verify(mockBeacon, times(1)).endSession();
+        verify(target, times(1)).end(true);
+    }
+
+    @Test
+    public void endSessionDoesNotFinishSessionOnBeacon() {
+        // given
+        when(mockBeacon.getCurrentTimestamp()).thenReturn(1234L);
+        SessionImpl target = spy(createSession().build());
+
+        // when
+        target.end(false);
+
+        // then
+        verify(mockBeacon, times(0)).endSession();
     }
 
     @Test
@@ -460,7 +474,9 @@ public void tryEndEndsSessionIfNoMoreChildObjects() {
 
         // then
         assertThat(obtained, is(true));
-        verify(mockBeacon, times(1)).endSession();
+        verify(mockBeacon, times(0)).endSession(); // no end session event is sent
+        verify(mockParent, times(1)).onChildClosed(target);
+
     }
 
     @Test

diff --git a/src/test/java/com/dynatrace/openkit/core/objects/SessionProxyImplTest.java b/src/test/java/com/dynatrace/openkit/core/objects/SessionProxyImplTest.java
@@ -17,6 +17,7 @@
 
 import com.dynatrace.openkit.api.Logger;
 import com.dynatrace.openkit.api.RootAction;
+import com.dynatrace.openkit.api.Session;
 import com.dynatrace.openkit.api.WebRequestTracer;
 import com.dynatrace.openkit.core.BeaconSender;
 import com.dynatrace.openkit.core.SessionWatchdog;
@@ -1073,7 +1074,7 @@ public void endingAnAlreadyEndedSessionDoesNothing() {
     @Test
     public void endingASessionImplicitlyClosesAllOpenChildObjects() throws IOException {
         // given
-        final OpenKitObject childObjectOne = mock(OpenKitObject.class);
+        OpenKitObject childObjectOne = mock(OpenKitObject.class);
         OpenKitObject childObjectTwo = mock(OpenKitObject.class);
         SessionProxyImpl target = createSessionProxy();
 
@@ -1148,6 +1149,26 @@ public void closeSessionEndsTheSession() {
         verify(target, times(1)).end();
     }
 
+    @Test
+    public void endCallsDifferentMethodsForSessions() {
+        // given
+        SessionImpl childObjectOne = mock(SessionImpl.class);
+        SessionImpl childObjectTwo = mock(SessionImpl.class);
+        SessionProxyImpl target = createSessionProxy();
+
+        target.storeChildInList(childObjectOne);
+        target.storeChildInList(childObjectTwo);
+
+        // when
+        target.end();
+
+        // then
+        verify(mockSession, times(1)).end(true);
+        verify(childObjectOne, times(1)).end(false);
+        verify(childObjectTwo, times(1)).end(false);
+        verifyNoMoreInteractions(childObjectOne, childObjectTwo);
+    }
+
     ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
     /// split session by time tests
     ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
@@ -1667,7 +1688,6 @@ public void toStringReturnsAppropriateResult() {
         assertThat(obtained, is(equalTo("SessionProxyImpl [sn=37, seq=73]")));
     }
 
-
     private SessionProxyImpl createSessionProxy() {
         return new SessionProxyImpl(mockLogger, mockParent, mockSessionCreator, mockTimingProvider, mockBeaconSender, mockSessionWatchdog);
     }