diff --git a/CHANGELOG.md b/CHANGELOG.md index bfc76b7b..5d246db8 100644 --- 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 index f1cca0fa..9e6a6028 100644 --- 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 index 89f4b8f2..5d2037ea 100644 --- 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 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 index 0e364c7d..616e3a73 100644 --- 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 index b7e0057d..7f432a7e 100644 --- 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 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 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 index d66b60ee..550b2fc4 100644 --- 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/src/test/java/com/dynatrace/openkit/core/communication/BeaconSendingFlushSessionsStateTest.java b/src/test/java/com/dynatrace/openkit/core/communication/BeaconSendingFlushSessionsStateTest.java index fbd85388..00e56e65 100644 --- a/src/test/java/com/dynatrace/openkit/core/communication/BeaconSendingFlushSessionsStateTest.java +++ b/src/test/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 index 5a1e4fd8..e98f4951 100644 --- 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 index 21855cfc..b4a17746 100644 --- 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); }