Make clearer to users that the optional timestamp argument should be provided in milliseconds

[pearsonhenri]

Ran into a bummer issue with the timestamps being reported from our PHP tracker. All of them are showing up in our atomic.events table as starting around roughly the Unix epoch (with some minor variation). They are being generated via a simple time() call and passed to the tracker. After digging into the code a bit, it looks like the timestamp should actually be multiplied by 1000:

https://github.com/snowplow/snowplow-php-tracker/blob/cff73c3ebfcbaf10c5323e1eb071beb8ae9450fb/src/Payload.php#L42

Not sure what the best method is here--probably just multiply the optional tstamp parameter by 1000 as well?

[BenFradet]

REQUEST_TIME_FLOAT seems to give microseconds precision: http://php.net/manual/en/reserved.variables.server.php maybe that could solve this issue? cc @mhadam

[pearsonhenri]

@BenFradet not sure that exactly addresses the issue here--it would make it so that the default time value doesn't need to be multiplied by 1000, but there's nothing to instruct users providing the optional timestamp arg to provide a timestamp in MS.

[BenFradet]

there's nothing to instruct users providing the optional timestamp arg to provide a timestamp in MS

Not sure I'm following, could you expand on your idea?

[pearsonhenri]

Ok maybe the specific case in which I ran into this issue will be more illustrative.

We are tracking events with the PHP tracker as follows:

$this->tracker->trackUnstructEvent($event, $schema, time());

I would expect this to give me an accurate representation for dvce_created_tstamp in our atomic.events table, but instead everything is off by a factor of 1000 (owing to the fact that the optional timestamp parameter we are passing is not being multiplied by 1000). My suggestion is that this optional parameter should also be multiplied by 1000 (as is done in the default case where the timestamp is not provided).

[BenFradet]

ah ok, this makes everything a lot clearer :+1: