Skip to content

feat: add live logs #312

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

Open
wants to merge 5 commits into
base: 2.x.x
Choose a base branch
from
Open

feat: add live logs #312

wants to merge 5 commits into from

Conversation

surajgour-d11
Copy link
Collaborator

Odin version(x.y.z)

Features

Resources

@surajgour-d11 surajgour-d11 changed the title feat: add logs command feat: add live logs Apr 3, 2025
@surajgour-d11 surajgour-d11 force-pushed the feat/logs branch 3 times, most recently from fbb7d48 to 567e7f2 Compare April 3, 2025 13:38
if response != nil {
for _, logMessage := range response.Logs {
fmt.Println(logMessage.GetMessage())
lastLogTime = logMessage.GetTimestamp()
Copy link
Collaborator

Choose a reason for hiding this comment

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

what if this comes out of order? the lastLogTime will be updated to something before the actual lastLogTime

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ordering is guaranteed from deployer. But still, we can add > logic to prevent this

}
}
attempts++
time.Sleep(5 * time.Second)
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need to sleep even if there's no error?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sleep is to prevent burst retry if no logs are present for some time.

log.Info("Fetching live logs...")
}
select {
case <-streamCtx.Done(): // Listen for cancellation signal
Copy link
Collaborator

Choose a reason for hiding this comment

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

does cancel also send a Done signal?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this here sends the done signal

Signed-off-by: surajgour-d11 <[email protected]>
@surajgour-d11 surajgour-d11 force-pushed the feat/logs branch 2 times, most recently from 3afcb92 to 2e83084 Compare April 15, 2025 10:03
Signed-off-by: surajgour-d11 <[email protected]>
Signed-off-by: surajgour-d11 <[email protected]>
if logStreamErr != nil {
if errors.Is(logStreamErr, context.Canceled) || logStreamErr == io.EOF {
break
} else {

Choose a reason for hiding this comment

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

else can be removed here

"VALIDATE": {"FAILED": true},
}

var RetryableStatusCodes = []codes.Code{codes.DeadlineExceeded, codes.Canceled, codes.Unavailable}

Choose a reason for hiding this comment

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

there is one more. codes.Internal AND description contains "stream terminated by RST_STREAM"

var re retryable.Error
if errors.As(err, &re) {
if re.Retryable() {
log.Info("Connection with backend server lost. Retrying...")

Choose a reason for hiding this comment

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

i think it was agreed that "Connection with backend server lost." will only come first time. On subsequent retries ,Retrying... should be printed. lets check this with DC once

* chore: refractoring

Signed-off-by: surajgour-d11 <[email protected]>

* chore: refractoring

Signed-off-by: surajgour-d11 <[email protected]>

---------

Signed-off-by: surajgour-d11 <[email protected]>
// DeployService deploys service
func (e *Service) DeployService(ctx *context.Context, request *serviceProto.DeployServiceRequest) error {
log.Info("Deploying Service...")

Choose a reason for hiding this comment

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

Are we not doing this for operate service/component and deploy released service ?

Signed-off-by: surajgour-d11 <[email protected]>
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.

3 participants