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

Partial render of sessions due to timezones #2798

Closed
klizhentas opened this issue Jun 26, 2019 · 13 comments
Closed

Partial render of sessions due to timezones #2798

klizhentas opened this issue Jun 26, 2019 · 13 comments

Comments

@klizhentas
Copy link
Contributor

Have a question? Please use Our Forum

What happened:

In case if the session end event does not end up being present in the requested range
for the sessions in the user interface, the sessions are rendered incomplete, because
not all session events are fetched.

What you expected to happen:

Regardless of the timezone, the end event should be included for the sessions.

How to reproduce it (as minimally and precisely as possible):

Create a session, make sure the session is longer lasting. Tweak the timezone to include the session start, but exclude session end.

The session will render with incomplete results - no duration or playback will be available

Environment:

  • Teleport version (use teleport version):
    4.0
  • Tsh version (use tsh version):
    4.0
  • OS (e.g. from /etc/os-release):

Browser environment

  • Browser Version (for UI-related issues):
  • Install tools:
  • Others:

Relevant Debug Logs If Applicable

  • tsh --debug
  • teleport --debug
@klizhentas klizhentas added this to the 4.1 "Seattle" milestone Jun 26, 2019
@klizhentas
Copy link
Contributor Author

@kontsevoy @benarent my proposal is always use UTC on the client for selection purposes to avoid confusion like this, instead of relying on the browser client timezone. The session dates should always be displayed in UTC as well.

@klizhentas
Copy link
Contributor Author

I'm currently thinking on the appropriate fix for the backend as well.

@alex-kovoy
Copy link
Contributor

We use UTC format in web ui when making session queries, so a user needs to make sure that browser/OS timezone is correctly configured. For example, if you are on the east cost but your OS/Browser uses Western time zone, then your search query wont yield the expected results.

@klizhentas
Copy link
Contributor Author

My point is that we should ignore the browser timezone and always display and query in UTC. We should also make it clear that all times are in UTC. This is quite common for the server/infrastructure software.

@benarent
Copy link
Contributor

+1 for the solution. We already have UTC defined for the session list, and the date pickers doesn't define time so working on the backend fix seems appropriate. No need to update the UI for now.

@klizhentas
Copy link
Contributor Author

well, UI has to be updated in the sense that it has to stop using local timezone, and always use UTC, what's not the case it seems like.

@alex-kovoy
Copy link
Contributor

In case if the session end event does not end up being present in the requested range
for the sessions in the user interface, the sessions are rendered incomplete, because
not all session events are fetched.

Based on these repro steps it looks like the issue is with the way we handle stored sessions in general. If session.end event is outside of requested range, the session wont appear in the stored session list. Currently this is expected behavior because fetch request wont return this event.

@alex-kovoy
Copy link
Contributor

In other words, there must be session.end event to mark a session as completed even if its session.start event falls into requested range.

@klizhentas
Copy link
Contributor Author

@kontsevoy to confirm with you, @alex-kovoy mentioned that the new UI won't actually have sessions tab for plabyack - as long as session.end event shows up in the audit log, users will be able to play it back, otherwise they could try to join. Is that correct? In this case the fix on the backend will not be necessary as there will be no ambiguity.

However, there is still a question on how to reliably determine on whether the session is active or not, do we use some other method of determining it @alex-kovoy ?

@alex-kovoy
Copy link
Contributor

@klizhentas there is a separate endpoint just for active sessions and this is not related to events. So yes, we can reliably display the list of active sessions.

@kontsevoy
Copy link
Contributor

@klizhentas yes this makes sense to me. But 4.0 does not currently offer "join" button at all, just FYI. I created #2839 for this.

I don't have anything to offer regarding the missing session_end events. This is a software engineering/design limitation. Either have an internal API which returns the list of active sessions, or assume that any session_start events older than X hours indicate a completed session?

@klizhentas
Copy link
Contributor Author

Moving this to 4.2 because new UI changes should address this issue - the playback will be available only on the "session.end" event.

@benarent benarent modified the milestones: 4.2 "Alameda", 4.3 "Concord" Nov 1, 2019
@benarent
Copy link
Contributor

benarent commented Nov 4, 2019

Closing this ticket and going to reference it into #2800 so we've one ticket for this.

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

5 participants