Changing the User ID starts a new session in the server

[NoyGri]

As opposed to the Web & iOS SDKs, when updating the user ID during a visit and sending a new event, the idvisit of that event is greater than the idvisit of the last event that the same user has created.
The Web & iOS SDKs conform the fingerprint concept that Matomo uses to stitch 2 visit with different user_ids.

The Android SDK does not conform as it only sends the required event parameters at the *very first event of the visit (via the Tracker::injectInitialParams).
The solution would be very easy, simply move the following 3 lines from the Tracker::injectInitialParams to the Tracker::injectBaseParams.

trackMe.trySet(QueryParams.SCREEN_RESOLUTION, mDefaultTrackMe.get(QueryParams.SCREEN_RESOLUTION));
trackMe.trySet(QueryParams.USER_AGENT, mDefaultTrackMe.get(QueryParams.USER_AGENT));
trackMe.trySet(QueryParams.LANGUAGE, mDefaultTrackMe.get(QueryParams.LANGUAGE));

With that being said, was there a reason that those parameters where not included in the Tracker::injectBaseParams to begin with? If not, I would be happy to provide a PR with the aforementioned solution.

[d4rken]

The overlying reason to split up the injection was to reduce network traffic, but I wasn't aware of the fingerprinting mechanism when implementing that.

Can we come up with a better solution than to just inject these parameters into every event so we can still try to prevent unnecessary traffic? Maybe once after the id has changed?

[NoyGri]

We can add a simple AtomicBoolean flag to the Tracker object, and raise it when setUserId is called. Then in the following call to the track method we could check that flag after the call to injectBaseParams and if it's raised inject the "deviceParams" and lower the flag.
What do you think @d4rken ? (I ran a little test and seems to be working fine now)

[d4rken]

That could work, but I'm not sure if just an AtomicBoolean saves us from raceconditions.

What would happen if two threads call Tracker::track(TrackMe) at the same time? One thread might consume the flag, but the other thread could still put it's packet in the queue first. Would the visits "stitching" still work in that case? If so then we can do this with just an AtomicBoolean otherwise we might need a bit more thread control.

[NoyGri]

Yes you're right, that would be a problem.
OK this is still simple, what if we use the same code you used to lock a new session event at the top of the queue?
After the call to Tracker::tryNewSession in the Tracker::track(TrackMe) method, we will check if it was not a new session, then was the userId updated. We do that by comparing the received trackMe.get(QueryParams.USER_ID) to the member mLastEvent.get(QueryParams.USER_ID).

If they are different, set the mSessionStartLatch to 1, await, inject the Device Info params, and release.

Maybe I should change the name of the mSessionStartLatch to mCriticalInsertToEventQueueLatch or something like that.

[d4rken]

That would work but the code might be too complicated for my taste.

I'll have a look at the synchronization code again later, I think we can simplify the whole track method by synchronizing on the Tracker instance, if we then synchronize the methods to change the user id, we should be good. Currently trying to remember why i implemented the latch based solution... 😁

We do that by comparing the received trackMe.get(QueryParams.USER_ID) to the member mLastEvent.get(QueryParams.USER_ID).

That's a cool idea, saves us an extrar global var. 👍

[d4rken]

@NoyGri #210 should fix this, do you agree?

While simplifying the code I also noticed a bug that would cause the session-timeout to not work.

[NoyGri]

@d4rken yep, looks good to me. Also nicely done with the code cleanup :)

[d4rken]

v3.0.3 is available and contains the fixes.

Thanks for teaching me about the server-side stitching 😃