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

🚀 feat: implement logs streaming for pods #568

Open
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

naineel1209
Copy link
Contributor

closes #555

📑 Description

[BE]

  • Implemented streaming logs of pod continuously to the client using SSE.
  • Utilized previously coded SSE implementation and made channels to stream the logs to the FE.

[FE]

  • Changed logs state to store array of strings to easily manage logs by using logs.join("\n").
  • Implemented logStream function which streams the logs and sets it in the state.
  • Handled scenario when no logs are available, container tabs are changed and when the log modal is cancelled or OKed.
  • Introduced a new logsSignalController state to be passed to the logStream function to handle the SSE connection scenarios i.e. close the connection via signal when modal is closed, close the current connection via signal and make a new signal for the new connection when the container tab is changed.

Streamed Logs Demo + No more events fired after the modal is closed:
https://github.com/user-attachments/assets/1f3f5fdf-102d-4697-9188-002092279098

✅ Checks

  • [✅] I have tested my code (provide screenshots or screen recordings of a working solution)
  • [✅] I have performed a self-review of my code

ℹ Additional context

@naineel1209 naineel1209 requested a review from a team as a code owner September 12, 2024 18:26
Copy link
Collaborator

@petar-cvit petar-cvit left a comment

Choose a reason for hiding this comment

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

@naineel1209 thanks for the PR! Looks awesome 🧡

Left some comments on code structure

cyclops-ui/src/utils/api/sse/resources.tsx Outdated Show resolved Hide resolved
cyclops-ctrl/pkg/cluster/k8sclient/client.go Outdated Show resolved Hide resolved
cyclops-ui/yarn.lock Outdated Show resolved Hide resolved
Copy link
Collaborator

@petar-cvit petar-cvit left a comment

Choose a reason for hiding this comment

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

@naineel1209, I tried testing it and noticed that when I fetch the logs, those are streamed as expected, but when the window with my Cyclops is not in focus or I change the tab in my browser, it stops the connection and refetches all the logs again. This results in duplicated logs.

Could Abort controller be at fault? To be honest if its the root cause we can remove it, but could you look into it?

@naineel1209
Copy link
Contributor Author

naineel1209 commented Sep 19, 2024

Hi @petar-cvit, I examined the issue, and it was happening due to the default behaviour of the @microsoft/fetch-event-source library, which closes the SSE events when the document is not visible. This caused our request to close as soon as the tab's visibility changed and restart when the tab was in focus again, resulting in fetching the latest 100 logs and streaming the new logs while our old logs were already present.

I have resolved this issue by setting the openWhenHidden flag to true, which keeps the SSE connection open when the tab's visibility changes.

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

Successfully merging this pull request may close these issues.

Stream pod logs
2 participants