Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dates: UTC vs LocalDateTime #56

Closed
tobyweston opened this issue Feb 7, 2018 · 8 comments
Closed

Dates: UTC vs LocalDateTime #56

tobyweston opened this issue Feb 7, 2018 · 8 comments

Comments

@tobyweston
Copy link
Owner

tobyweston commented Feb 7, 2018

Some of the system uses UTC whilst others local data times.

Log is generated by Log4J and the pattern %d{yyyy-MM-dd HH:mm:ss:SSS} and so local time.

Logs

Instant is UTC and the LogParser and LogMessage use Instant, so it probably makes sense to convert the log code to use LocalDateTime.

Or, we could store UTC in the log file (add a Z to the log4j.properties) and present in the local time zone of the client.

Charts / RRD

RRD itself will use UTC as it uses System.currentTimeMillis under the covers (RrdUpdate.scala L16) with the below.

  public static long getTime() {
        return (System.currentTimeMillis() + 500L) / 1000L;
  }

The /temperatures endpoint returns seconds for the event time (used by the main current temperature display) whereas the /temperature.json endpoint returns millis (for the chart). Both as UTC.

@tobyweston
Copy link
Owner Author

#55 switched the log time format to UTC which also paves the way for switching timezones on the UI (as per @hobie22670 use case)

@hobie22670
Copy link

Additional info regarding time zones. I currently have multiple servers / clients with probes running in Northern Michigan (all in EST) UTC +5. I'm monitoring these from within the same time zone and charts showing local time is perfect.
Problem came up when another team in California (PST) UTC +8 was viewing the same systems - I was seeing a 10º C rise at 11:00 - they were seeing the jump at 08:00. Confusion. What I would like to see as default at all UI's (regardless of their TZ) would use the local TZ where the server is running, thus all viewers would be presented with the same chart. If a user required they could switch to use alternate local TZ.

@tobyweston
Copy link
Owner Author

Looks like chartjs doesn't support changing timezone (chartjs/Chart.js#4334 ) but we could map the UTC values coming from the server to an offset value set by the user.

So, if we know the browsers offset, we could reverse and overwrite the raw chart data. Taking a source UTC of +0000 and a target browser +0400, we could moment(x).minus("4", "hours").valueOf (or similar). Bit annoying to have to pre-process all data points though.

The other option would be to globally set the moment offset and as long as the charting library support it, it should just work.

moment.tz.setDefault("UTC");

This has the advantage of automatically affecting the logs 👍.

@tobyweston
Copy link
Owner Author

I've got something basic working, is this what you need? UTC is the default but can be switched via a dropdown. It's not pushed yet as it needs some styling attention...

screenflow

@tobyweston
Copy link
Owner Author

One thing I don't like when the server is not in a UTC zone. I'd prefer the default to be in the server's timezone.

tobyweston added a commit that referenced this issue Feb 12, 2018
…ng API change as temperatures PUT requests now send over the host's UTC offset as well as the host's name
tobyweston added a commit that referenced this issue Feb 12, 2018
tobyweston added a commit that referenced this issue Feb 17, 2018
…ne) is used to work out the index offset to write temperatures to the RRD and not just the host name
tobyweston added a commit that referenced this issue Feb 17, 2018
…ne) is used to work out the index offset to write temperatures to the RRD and not just the host name
tobyweston added a commit that referenced this issue Feb 17, 2018
@tobyweston
Copy link
Owner Author

tobyweston commented Feb 22, 2018

This is basically fixed in 18bdc61 now. A timezone picker is available that affects the log page and main chart.

Remaining issues:

  1. RRD charts don't update - I don't think I can do much about this
  2. I'd like to default the timezone (in the picker) to display the servers timezone

I'll leave the issue open for now until we resolve the above.

It looks like this:

screenflow

@tobyweston
Copy link
Owner Author

Default Timezone to Server's Timezone

This is a bit trickier than I thought. I'm in two minds how to do this. I don't really want to modify the Host class to include if it's the server of not but I don't think a JavaScript client can easily work out where it's been served from. If it could, it would know which of the /connections hosts it is and pick that timezone...

Options:

  • Modify the Host object to indicate if it's the server, look all /connections up and cross-ref the server... yuk
  • The UI could use datagrams to discover the server... yuk
  • Get the server IP from the address, cross-ref with connections... don't know if that can be done
  • What else?

@tobyweston
Copy link
Owner Author

Went for the server returning a X-Forwarded-Host header which if found in the /connections call, will be used for the default timezone. 👊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants