alexjustesen / speedtest-tracker

Speedtest Tracker is a self-hosted internet performance tracking application that runs speedtest checks against Ookla's Speedtest service.
https://speedtest-tracker.dev/
MIT License
2.87k stars 106 forks source link

Timezone issues #939

Closed hoorna closed 11 months ago

hoorna commented 11 months ago

In this post I try to think about the timezone issues of Speedtest-tracker. My suggestions are intended to help.

In the v0.14.0 version notes I read that the time zones are handled in a different way. If I understand correctly, from version 0.14.0 onwards, the UTC time of the measurement moment is stored in the database and not (anymore) the local time (as in previous versions). On the General web page the user sets the local time zone so that the correct local time of the measurement moment should be displayed on the Results web page (and not the UTC time from the database).

I wonder whether this is a wise choice, especially for regions where daylight saving time (DST) is applied. I'll try to explain why. Speedtest-tracker probably only records the UTC time of a measurement moment in the database and not the DST of that moment (let's call this the time of measurement on day 0). If the DST zone does not change, the correct local time of the measurement moment of day 0 will be displayed on day 1 on the Results web page. This display is determined by the region entered on the General web page. Nothing to worry about. If the DST zone does change the following night (at the start of day 2), Speedtest-tracker will from that moment on display all times on the Results web page in the new DST times. Speedtest-tracker follows the region time, which is entered on the General web page. The effect is that the Results web page now displays an incorrect time of the measurement on day 0 (namely in the 'new' DST time zone instead of the 'old').

If my reasoning is correct, then the only correct way is to record the local (DST) time of the region in the database for the measurement moments (and not the UTC time). This time should be determined by the TZ environment variable. The region setting on the General web page should be omitted in this approach. The Results page displays the correct time of the measurement as recorded in the database.

Finally, I would like to make a comment about DST and Speedtest-tracker. I don't know if this is correct, but I have the impression that some of the time zone issues reported so far are related to the DST zone users are in. My suspicion is that Speedtest-tracker has not taken enough account of this so far. This suspicion seems to be confirmed because in version 0.14.0 the time of the measurement moments on my Results web page is still displayed incorrectly (with a time difference of 1 hour in accordance with the DST zone in which I live: Europe/Amsterdam).

alexjustesen commented 11 months ago

Morning @hoorna thanks for the question. Let me go through this and see if I can break down each of your points...

In the v0.14.0 version notes I read that the time zones are handled in a different way.

The main goal of v0.14.0 is to simplify how time zones are being handled and to provide consistency across deployments. A good example of this is that not all host system OSs has the local time available to mount into the container (i.e. /etc/localtime:/etc/localtime:ro).

If I understand correctly, from version 0.14.0 onwards, the UTC time of the measurement moment is stored in the database and not (anymore) the local time (as in previous versions).

UTC time of the result has and will continue to be stored in the database this way so a consistent record can exist for the historic runs and then be calculated to the correct offset based on the user's location. This is generally considered best practice when recording and displaying of dates and times.

I wonder whether this is a wise choice, especially for regions where daylight saving time (DST) is applied.

Finally, I would like to make a comment about DST and Speedtest-tracker. I don't know if this is correct, but I have the impression that some of the time zone issues reported so far are related to the DST zone users are in. My suspicion is that Speedtest-tracker has not taken enough account of this so far.

PHP's DateTime interface is aware when a time zone is in or out of DST so the offset should be correct year round. I'm in America/New_York (ref) and this is currently showing I am out of DST which is correct.

Tested this assumption by dumping Carbon::create($tz = 'America/New_York')->format('I') which gave me false. Ps: Carbon is just a package to interface with the DateTime function.


Since you're in a different time zone than me would you be up for some testing once I tag the beta version? The changes I've made are working as intended for me and I haven't come across any incorrect running of the scheduler or displaying of datetime stamps yet.

hoorna commented 11 months ago

Thanks @alexjustesen you for your detailed answers. I few minutes ago I installed version 0.14.0-beta1 as you asked for. Regarding the Speedtest-tracker container I didn't use the TZ environment variable nor the time volume definition and on the General web page I defined my Timezone Europe/Amsterdam. In the dockercompose file for the MariaDB container the TZ environment variable has the value Europe/Amsterdam. The latter is necessary because I also use this container for other applications. After that I queued a speedtest on 15:56 local time. Here are the results:

Let me further give some response on your reply:

PHP's DateTime interface is aware when a time zone is in or out of DST so the offset should be correct year round. I'm in America/New_York (ref) and this is currently showing I am out of DST which is correct.

Tested this assumption by dumping Carbon::create($tz = 'America/New_York')->format('I') which gave me false. Ps: Carbon is just a package to interface with the DateTime function.

I have not much knowledge about PHP nor Carbon but my question is if the routines you mentioned are accurate for times recorded in the past (i.e. times from measuments which are done with 'another' DST flag). The routines will of course return the correct DST time/flag of current moment but will they also do that for times recorded in a period with a different DST flag?

UTC time of the result has and will continue to be stored in the database this way so a consistent record can exist for the historic runs and then be calculated to the correct offset based on the user's location. This is generally considered best practice when recording and displaying of dates and times.

It is of course fine that you make your own choice and I respect that. After all, it is your app. :-) But I wonder if you're not complicating things unnecessarily. If you record the actual local time of measurements in the database, a problem will only arise if a user takes his database to another time zone. That chance seems zero and negligible to me.

Of course I would like to help you with further timezone tests. Just let us know what youre needs are.

alexjustesen commented 11 months ago

In the dockercompose file for the MariaDB container the TZ environment variable has the value Europe/Amsterdam. The latter is necessary because I also use this container for other applications.

That's an interesting nugget because I'm not passing any time zone to the database and I'm starting to think that could be causing the issue.

I'm in the office and probably won't circle back around to this until tomorrow but will absolutely be testing this as it differs from my test cases.

hoorna commented 11 months ago

That's an interesting nugget because I'm not passing any time zone to the database and I'm starting to think that could be causing the issue.

I'm in the office and probably won't circle back around to this until tomorrow but will absolutely be testing this as it differs from my test cases.

That is indeed a possible cause for the issues. I am of course very curious about your findings.

alexdelprete commented 11 months ago

Databases ALWAYS work with UTC. And even if you move them to other time-zones, data time continues to work properly, exactly because it stores times internally as UTC. It then transforms at the presentation layer timings in local format, depending on the OS configuration.

All apps should behave the same way. @alexjustesen: I think we already had a long discussion about this in a related issue, and we're here again at it, or maybe I'm wrong and it was another project/repo, I'm starting to get confused with all these services in the homelab. :)

hoorna commented 11 months ago

I think we already had a long discussion about this in a related issue, and we're here again at it, or maybe I'm wrong and it was another project/repo, I'm starting to get confused with all these services in the homelab. :)

I'm not sure exactly what you mean by this (Google Translate is failing me here). I hope that new contributions will continue to be welcomed :-) even though they may have already been discussed. After all, the time zone issues have been going on for a year now and despite all those discussions, they have still not been resolved.

alexdelprete commented 11 months ago
  1. My comment was for Alex more than you. I even tagged him. :)
  2. Google Translate perfectly translates that sentence, it's normal english.
  3. Correct: TZ issues and .env issues took 80% of all issues (just an estimate)
  4. You can open 1000 issues, but wouldn't call them "contribution". Some comments might be, the issues are not contributions. :)
alexdelprete commented 11 months ago

That's an interesting nugget because I'm not passing any time zone to the database and I'm starting to think that could be causing the issue.

From what I read regarding php/mysql:

<?php

date_default_timezone_set('Europe/Dublin'); 
$myDate = date("Y-m-d H:i:s"); // mysql timestamp: (YYYY-MM-DD HH:MM:SS)
mysql_query("insert into `mytable` set `mytime` = '$myDate'");

?>

image

What I don't understand is this: the records in the DB are correct. Why do you think you're storing it incorrectly? The problem is when you present them in the UI, not the raw record.

hoorna commented 11 months ago

@alexjustesen, did you found some time to test passing an TZ environment vairable to your database container? I am curious.

alexjustesen commented 11 months ago

@alexjustesen, did you found some time to test passing an TZ environment vairable to your database container? I am curious.

Unfortunately not yet, the day job and clients have been keeping me busy the last couple of days. Hopefully tonight I will be able to.

alexjustesen commented 11 months ago

What I don't understand is this: the records in the DB are correct. Why do you think you're storing it incorrectly? The problem is when you present them in the UI, not the raw record.

Not concerned about how it's stored as I'm considering that solid but I am trying to figure out why only a handful of time zones are an issue at this point. We're in this it works fine in my tz circle right now.

alexdelprete commented 11 months ago

If you follow all reports you'll get crazy. :)

TZ on the db container doesn't change anything, since records are already created correctly.

It's something related to the code or Laravel setting, on the presentation layer, since the data layer seems solid.

hoorna commented 11 months ago

Hopefully tonight I will be able to.

Good luck, but ..... there is no huge rush. Think about your night's sleep.

hoorna commented 11 months ago

I am trying to figure out why only a handful of time zones are an issue at this point

Thanks for that! It is pretty annoying that the displayed times are not right and also that the scheduled times does not fit. I am happy with your intention to fix this despite others who are stating that you do not have to follow all reports. That people have probably no problems I suppose: that's easy.

Remember that I am gladly willing to help where I can.

alexdelprete commented 11 months ago

despite others who are stating that you do not have to follow all reports

I was referring to finding the bug, not following the issues, try to understand the context instead of misinterpreting.

TZ on the database container has nothing to do with this issue.

alexjustesen commented 11 months ago

v0.14.0-beta2 is currently building with a time zone debug page which is available at /debug/timezone. For an explanation of the fields and what values to expect check out #946.

hoorna commented 11 months ago

Thanks!! I will look after it as soon as I can and I will report back.

hoorna commented 11 months ago

Here is the output from /debug/timezone with version 0.14.0-beta3. It seems to me that all values are correct for my timezone.

Schermafbeelding 2023-11-30 004913

alexdelprete commented 11 months ago

The debug page looks ok: image

But the issue is there: image

alexjustesen commented 11 months ago

The debug page looks ok

But the issue is there

I'm going to pull the latest test result into the debug page so we can also assess against the DB records. -beta4 probably tomorrow morning.

hoorna commented 11 months ago

I'm going to pull the latest test result into the debug page so we can also assess against the DB records.

That seems like a good addition to me. If I may make a suggestion: also add the Speedtest schedule from the General web page. You then also have information about when speed tests should have started according to the schedule.

alexjustesen commented 11 months ago

v0.14.0-beta4 is building now which brings the latest speed test result into the debug page.

hoorna commented 11 months ago

This is the result of the debug page with v0.14.0-beta4. I think all times seems correct, exept that the result time differs from the time I scheduled the speedtest. This scheduled time is 01:15. There is a uncertainty though: I cannot check/see anywhere on which time the speedtest really ran (01:15 or 02:15).

Schermafbeelding 2023-11-30 131937

alexdelprete commented 11 months ago

Here is beta4. Last result is the same I show you via screenshots, as expected. +1 ahead.

image

alexdelprete commented 11 months ago

BTW: I checked inside the container, and date command shows it's UTC based.

> dcx -it speedtest-tracker bash
root@851f45b24c39:/var/www/html# date
Thu Nov 30 12:42:57 UTC 2023

In order to correct my issue, I reintroduce /etc/localtime and here are the results of debug page and date shell command:

image

root@9120a23c4ddc:/var/www/html# date
Thu Nov 30 13:46:35 CET 2023
root@9120a23c4ddc:/var/www/html#

As you can see above, when it's working correctly for me, Local Time and UTC Time are the same, and that's wrong.

hoorna commented 11 months ago

The date command inside the container (without TZ nor timezone vollume variable) return "UTC" (so 1 hour back).

The date command with the TZ environment variable set (Europe/Amsterdam) gives "CET", which is correct). The output from the /debug/timezone web page is in for me in this case exactly the same as with no TZ nor timezone volume environment variable: UTC time and Local time differs one hour. This is another outcome than @alexdelprete reports above (UTC time and local time the same), but he was using the timezone volume environment variable.

Furthermore: a question: Isn't it strange that with the TZ environment set (to Europe/Amsterdam) the /debug/timezone output for "PHP time zone" and "App time zone" stays at UTC?

alexjustesen commented 11 months ago

The date command inside the container (without TZ nor timezone vollume variable) return "UTC" (so 1 hour back).

The date command with the TZ environment variable set (Europe/Amsterdam) gives "CET", which is correct). The output from the /debug/timezone web page is in for me in this case exactly the same as with no TZ nor timezone volume environment variable: UTC time and Local time differs one hour. This is another outcome than @alexdelprete reports above (UTC time and local time the same), but he was using the timezone volume environment variable.

Furthermore: a question: Isn't it strange that with the TZ environment set (to Europe/Amsterdam) the /debug/timezone output for "PHP time zone" and "App time zone" stays at UTC?

So...

When passing TZ environment variable into the container that only sets the time zone of the operating system, PHP's and in turn the app's is separate.

The time zone of Ubuntu (in this case) is out of scope of this issue and doesn't (or at least shouldn't) effect the date time of PHP.

PHP should continue to operate under the UTC time but what I'm trying to accomplish here is the schedulers time and the display time being correct by setting a time zone in the UI (Settings -> General).

alexjustesen commented 11 months ago

⚠️ The problem

Alright... so after some pretty extensive testing the only way I could reproduce result records having the wrong offset were if TZ environment variable was being passed to the database container or the database instance has had it's time zone changed.

In a nutshell Speedtest Tracker assumes the timestamp from the created_at field it's receiving from the database is a time from the UTC time zone with no offset. From there it automatically will calculate the offset to your local time based on the time zone setting set in the UI.

Example

If your database is set to the Europe/Rome timezone which is UTC +1 and you then set Speedtest Tracker's time zone to Europe/Rome it's going to actually calculate the offset twice which accounts for the display time being an hour off.

🤔 How can I find out my database's time?

To test you database's local time use date command inside the database's VM or container. If it's other than UTC (example below) then the database configuration is incorrect.

image

@hoorna and @alexdelprete would like some validation on the assumption above but at this point I'm out of options for testing and will likely close this issue down.

alexdelprete commented 11 months ago

In a nutshell Speedtest Tracker assumes the timestamp from the created_at field it's receiving from the database is a time from the UTC time zone with no offset. From there it automatically will calculate the offset to your local time based on the time zone setting set in the UI.

That is a totally wrong assumption, and since created_at is a TIMESTAMP field, MySQL/MariaDB stores the value as UTC after converting it based on the connection timezone, and gives back the value depending on the connection timezone. If you don't set the connection timezone, the default is the server's timezone.

source image

f it's other than UTC (example below) then the database configuration is incorrect.

Totally wrong. An app should never rely on the configuration of the database, since the database could be used for several apps (in a cloud scenario for example). So setting the correct TZ for the database container is not a wrong configuration, but the correct one. I use MariaDB as my central instance for all homelab apps/services, and only Speedtest-Tracker has issues with timestamps.

The problem is that you're not setting the correct timezone for the connection, or you're forcing UTC for the connection, and that's wrong. You should set the connection based on user's selected timezone.

would like some validation on the assumption above but at this point I'm out of options for testing and will likely close this issue down.

Hope this clears the problem once and for all. :)

alexdelprete commented 11 months ago

Checked the ST code, and in config/database.php there's no time zone set for the connection. So when ST writes created_at records, mysql is using its time zone for the conversion to UTC.

This is what I found to set the time zone at connection level:

'options' => array(
               \PDO::MYSQL_ATTR_INIT_COMMAND => 'SET time_zone = $offset'
           )

Others report this also to be working:

        'mysql' => [
            'driver'    => 'mysql',
            'host'      => env('DB_HOST', 'localhost'),
            'database'  => env('DB_DATABASE', 'forge'),
            'username'  => env('DB_USERNAME', 'forge'),
            'password'  => env('DB_PASSWORD', ''),
            'charset'   => 'utf8',
            'collation' => 'utf8_unicode_ci',
            'prefix'    => '',
            'strict'    => false,
            'timezone'  => '$offset'
        ],

And I also read this:

try {

    $conn = new PDO( "mysql:host=$hostname;dbname=$dbname", $username, $password );

    // Set DB Timezone using the Offset
    $conn->exec( "SET time_zone='$offset';" );

    echo "Connected successfully";
}
catch( PDOException $e ) {

    echo "Failed to connect: " . $e->getMessage();
}

So 3 different ways, it needs to be tested I guess.

To calculate the offset:

// Define Timezone globally
define( 'TIMEZONE', 'America/Los_Angeles' );

// Set PHP Timezone
date_default_timezone_set( TIMEZONE );

// Obtain the Offset
$time   = new DateTime();
$offset = $time->format( 'P' );

Unfortunately $offset has to be used because time zone names (like Europe/Rome) are not by default present in system database mysql/time_zone_name, so in order to populate this a shell mysql command needs to be run (I tested it now and it works on my MariaDB instance):

mysql_tzinfo_to_sql /usr/share/zoneinfo | mysql -u root -p mysql

Some articles about this topic:

hoorna commented 11 months ago

In a nutshell Speedtest Tracker assumes the timestamp from the created_at field it's receiving from the database is a time from the UTC time zone with no offset.

@alexjustesen, I agree with @alexdelprete that the assumption from Speedtest Test tracker is incorrect. The data from my server and running containers prove this. See the data at the bottom of this message. In my MariaDB database, the "created_at" column displays the local recorded time and not the UTC time. I think @alex clearly explains why that is the case. Furthermore, as you can see in my data the only time errors which seems to occur are the times on the Results tab from Speedtest. So is likely that @alexjustesen is right with his analyses that the offset times are calculated 2 times on the Results tab.

I also agree with @alexdelprete that for both containers the TZ environment variable has to be set. Just like him I need the TZ variable definition for the database container because of other running applications/containers. Furthermore, it is a normal procedure for containers to define the TZ environment variable.

Finally, I notice that the "data" column in the database contains the recorded UTC time. If you really need the recorded UTC time for displaying the time on the Results tab, why not using the time from the "data" column?

Possible solutions

To keep it simple I think there are two possible solutions for the timezone issue on the Speedtest results tab.

@alexjustesen, I really hope this helps for solving the timezone issue! I fully understand your frustration with this issue and therefore remain available for assistance.

My data (Release v0.14.0-beta5)

Localtime

With TZ=Europe/Amsterdam set on both Speedtest and MariaDB container en no timezone volume defintion set

alexjustesen commented 11 months ago

Alright I spent a couple of days thinking about this and the path forward, so here's the final decision so I can move on...

Final Decision

  1. This application will assume the database stores and responds with timestamps in UTC.
  2. TZ environment variable SHOULD NOT be passed to the database container or the application container.
  3. To adjust the application's display time zone and the time zone that effects the schedule that can be done in the admin panel under General -> Settings -> Time zone.

This vs. That

Just like him I need the TZ variable definition for the database container because of other running applications/containers. Furthermore, it is a normal procedure for containers to define the TZ environment variable.

I agree this is a viable pattern and a requirement of other applications and containers. However for this application the standard will be the database should run in UTC and not your local time zone. Broadly speaking we don't change the time zone of the database in any of the other applications, websites and tools I've developed so I'm not deviating from that standard here either.

One Last Comment...

I'll be closing this issue after this comment and sticking a nail in it's coffin so I can move on to other features. I'll be tagging v0.14.0 tomorrow morning and updating the docs to reflect this decision. @hoorna and @alexdelprete I really appreciate all the input and time you both have spent, the insight into how others have their home labs setup has been invaluable.

alexjustesen commented 11 months ago

Linking to #972 and #984 should anyone come across this looking for resolution.