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

Make it more palatable for ECS environments #29

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vagmi
Copy link

@vagmi vagmi commented Feb 20, 2019

ECS creates log stream per container and the current watch method takes about 2 minutes in my environment to get log streams. Since ECS creates log group per task, I added a --recent flag to find streams based on EventTime.

@vagmi vagmi changed the title Make it more palatable for docker-compose environments Make it more palatable for ECS environments Feb 20, 2019
@TylerBrock
Copy link
Owner

This is great! Apologizes, I've been busy and haven't gotten a chance to look at this yet. I'll try and review the code soon. Thanks for contributing.

@@ -150,6 +169,7 @@ func (b *Blade) StreamEvents() {
}
return !lastPage
}
fmt.Printf("starting to tail logs from %d streams ...\n", len(input.LogStreamNames))
Copy link
Owner

Choose a reason for hiding this comment

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

Would you mind removing this bit? As part of the design I like there to be no other output than data or errors (this helps with things downstream if you pipe the output of saw into other programs)

Comment on lines +103 to +106
if lastPage || len(streams) >= 100 {
return false
}
return true

Choose a reason for hiding this comment

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

We might return the bool directly here.
Maybe:

return !lastPage && len(streams) < 100

@ipmb ipmb mentioned this pull request Apr 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants