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

Determine session type on stream based on stream watcher events #50395

Merged

Conversation

gabrielcorado
Copy link
Contributor

@gabrielcorado gabrielcorado commented Dec 18, 2024

Related to #49164

Brief context: After the changes are made at #47309, when a request for session recording events (playback) is made, we query to find the last event to set the SessionType field on the audit log. Depending on the cluster's backend and the volume of audit events, this can cause increased latency.

This PR updates the logic determining the session type for the SessionRecordingAccess audit event to receive a callback from the streamer when the first event is available.

changelog: Improve session playback initial delay caused by an additional events query.

lib/auth/auth_with_roles.go Outdated Show resolved Hide resolved
Comment on lines 548 to 549
sessionStartCh := make(chan apievents.AuditEvent, 1)
if watcher, err := EventsWatcherFromContext(ctx); err == nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error cases above are missing the callback?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated it. Now the error cases closes the channel (causing the callback to called if any).

lib/auth/auth_with_roles.go Outdated Show resolved Hide resolved
@gabrielcorado gabrielcorado marked this pull request as ready for review December 23, 2024 15:06
@github-actions github-actions bot added audit-log Issues related to Teleports Audit Log size/md labels Dec 23, 2024
@zmb3
Copy link
Collaborator

zmb3 commented Dec 23, 2024

I haven't forgot about you @gabrielcorado - this is on my list to review/test later today!

lib/events/auditlog.go Outdated Show resolved Hide resolved
@gabrielcorado gabrielcorado added this pull request to the merge queue Dec 27, 2024
Merged via the queue into master with commit 2dbdfd2 Dec 27, 2024
43 checks passed
@gabrielcorado gabrielcorado deleted the gabrielcorado/stream-session-events-watcher-session-type branch December 27, 2024 17:13
@public-teleport-github-review-bot

@gabrielcorado See the table below for backport results.

Branch Result
branch/v17 Failed

gabrielcorado added a commit that referenced this pull request Dec 27, 2024
* refactor: determine session type on stream based on watcher

* refactor(auth): early return when is teleport server

* refactor: move to function and add tests

* chore(lib): fix lint

* refactor(events): make private function
github-merge-queue bot pushed a commit that referenced this pull request Dec 28, 2024
…) (#50592)

* refactor: determine session type on stream based on watcher

* refactor(auth): early return when is teleport server

* refactor: move to function and add tests

* chore(lib): fix lint

* refactor(events): make private function
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit-log Issues related to Teleports Audit Log backport/branch/v17 size/md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants