Page MenuHomePhabricator

Extend all EL schemas to include client-side timestamp
Closed, ResolvedPublic2 Story Points

Description

Right now events in MobileWikiAppSessions schema (and other schemas) are unreliable due to the absence of client-side timestamps.

For example, a session summary event summarizing the previous session is sent at the start of a new session. This can be days after the previous session. Furthermore, if the device goes offline those events are put into a queue and are pushed out when the device is online and the user opens the app. The only timestamps we have are server-side from when the events are received. This means we can't reliably track session length metrics over time and it also creates additional challenges for T184089 (hence "High" priority).

According to @Dbrant the app has a record of session start time that it uses for checking when to start a new session, so it shouldn't be too difficult to add that info to the sent events. He also agreed that it would best to standardize & fix all the funnels together rather than just the sessions one.

Details

Related Gerrit Patches:
apps/android/wikipedia : masterEvent logging schema updates, Part 1.

Event Timeline

mpopov triaged this task as High priority.Feb 8 2018, 12:04 AM
mpopov created this task.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 8 2018, 12:04 AM

Also a note that we should strive to make its format compatible with the general client-side timestamp planned as part of the extended app capsule in the upcoming general app analytics framework.

Charlotte set the point value for this task to 1.Feb 14 2018, 5:51 PM
Charlotte changed the point value for this task from 1 to 2.Feb 26 2018, 5:44 PM
mpopov added a comment.EditedMar 6 2018, 5:19 PM

Update: we decided to change this task to be about extending all schemas to include a client timestamp.

mpopov renamed this task from Add client timestamp to MobileWikiAppSessions to Extent all EL schemas to include client-side timestamp.Mar 6 2018, 5:30 PM
mpopov updated the task description. (Show Details)
mpopov added a comment.EditedMar 12 2018, 8:54 PM

I'm thinking for this one we should follow ISO 8601 with a minor deviation: in case of UTC, we wouldn't use 2018-03-10T14:03:56Z but 2018-03-10T14:03:56+0000

So we should use this format in general: YYYY-MM-DDThh:mm:ss±hhmm which is: date, time in UTC, timezone (including daylight savings) offset from UTC

For example:

  • If the user is in Pittsburgh, PA, the client-side timestamp would be 2018-03-10T14:03:56-0400
  • If the user is in India, the client-side timestamp would be 2018-03-10T14:03:56+0530

As you can see, we need the minute offset in there for certain timezones that have 30 and 45-minute offsets: https://www.timeanddate.com/time/time-zones-interesting.html

For funsies, we can then convert those timestamps to local times and look for interesting patterns:

SELECT CONVERT_TZ(LEFT('2018-03-10T14:03:56-04:00', 19), '+00:00', RIGHT('2018-03-10T14:03:56-04:00', 6)); -- returns: 2018-03-10 10:03:56

Edit: updated comment to reflect actual implemention (which has no separator between the hours and minutes in the offset)

Reedy renamed this task from Extent all EL schemas to include client-side timestamp to Extend all EL schemas to include client-side timestamp.Mar 12 2018, 8:58 PM

@mpopov Any preference for the name of the timestamp column? I was thinking something really terse, in the interest of minimizing the payload size, e.g. time, or even t?

@mpopov Any preference for the name of the timestamp column? I was thinking something really terse, in the interest of minimizing the payload size, e.g. time, or even t?

How about ts? It works and it's not used by any existing schema :) https://meta.wikimedia.org/w/index.php?search=ts+intitle%3AMobileWikiApp&title=Special:Search&profile=advanced&fulltext=1&ns470=1

Dbrant claimed this task.Mar 15 2018, 2:31 PM

Change 419739 had a related patch set uploaded (by Dbrant; owner: Dbrant):
[apps/android/wikipedia@master] Event logging schema updates, Part 1.

https://gerrit.wikimedia.org/r/419739

Change 419739 merged by jenkins-bot:
[apps/android/wikipedia@master] Event logging schema updates, Part 1.

https://gerrit.wikimedia.org/r/419739

BTW I updated the comment T186768#4044717 to reflect actual implemention (which has no separator between the hours and minutes in the offset)

Dbrant closed this task as Resolved.Apr 2 2018, 10:20 PM

@mpopov

date, time in UTC, timezone (including daylight savings) offset from UTC

ISO 8601 specifies that it’s the local time with the offset to UTC - is that acceptable?

As an alternative, given that mediawiki takes UTC with a trailing Z, we could we pass UTC to ts and a separate tz with an offset formatted as ZZZZZ?

@mpopov

date, time in UTC, timezone (including daylight savings) offset from UTC

ISO 8601 specifies that it’s the local time with the offset to UTC - is that acceptable?

Holy crap, you're right!!! Oh my god this is embarrassing as I've been going off a wrong assumption, so thank you! @Dbrant: I assume the DateUtil.getIso8601LocalDateFormat().format(new Date())) follows the actual ISO 8601 spec and not my weird fork of it, right?

As an alternative, given that mediawiki takes UTC with a trailing Z, we could we pass UTC to ts and a separate tz with an offset formatted as ZZZZZ?

I like this approach! Since Dmitry already updated all the funnels and schemas to use the ISO 8601 spec, I think Android app should stick with that approach. (Dmitry is welcome to disagree!) I don't think there's anything restricting you to do it our way, so if @chelsyx is okay with having the UTC timestamp and a timezone field, you're good to go!

:D yep, the Java implementation gives the local time followed by the timezone offset.