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.

Sign up for GitHub

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

Jump to bottom

Handling large amounts of CloudWatch Logs in toHaveLog() #29

Closed
abby-huisman opened this issue May 30, 2019 · 6 comments · Fixed by #32
Closed

Handling large amounts of CloudWatch Logs in toHaveLog() #29

abby-huisman opened this issue May 30, 2019 · 6 comments · Fixed by #32

Comments

@abby-huisman
Copy link

Is your feature request related to a problem? Please describe.
We've been noticing our tests fail as the CloudWatch log streams grow and it is largely due to the AWS SDK returning multiple pages of CloudWatch Logs.

Describe the solution you'd like
At RobustWealth, we've added a wrapper around the SDK to page the logs along with giving a start and end date to retrieve a smaller subset of logs. Instead of rolling our own solution we would like to add this feature into this library and we'd be willing to contribute back our solution.

Describe alternatives you've considered
We've considered deleting our logs, this fixed our issues but then we lose the information we need to properly monitor our applications.
@erezrokah
Copy link
Owner

@abbyhuisman-rw thanks for opening this issue.
I'd be happy to port your solution to this library (I'm gladly accepting pull requests).
I'll go over the logs sdk as well :)

@erezrokah erezrokah mentioned this issue Jun 2, 2019
Closed
@erezrokah
Copy link
Owner

erezrokah commented Jun 2, 2019 •
edited
Loading

@abbyhuisman-rw I've looked at the library's code and I'm already limiting the results to 1 so I'm not sure I'll get into the pagination issue.
It does makes sense to me to filter by startTime in regards with testing (no point in looking too far back).
I've made a pull request for it here #32

Please let me know if it makes sense to you too :)

@abby-huisman
Copy link
Author

abby-huisman commented Jun 4, 2019 •
edited
Loading

@erezrokah Thanks for making that change. Our internal change looks almost exactly like your code so I think this PR will allow us to remove our home-grown solution. I didn't notice the limit one so hopefully that in tandem with the start time will fix our issue. Any idea when you will do a formal release of this code?

@erezrokah erezrokah mentioned this issue Jun 4, 2019
Merged
@erezrokah
Copy link
Owner

Thanks that’s great. I’m AFK, but I’ll do a formal release tomorrow and update (and close the issue) once I do.
Again, I appreciate the feedback :)

@erezrokah
Copy link
Owner

@abbyhuisman-rw Formal release v1.0.1 is out :
https://github.com/erezrokah/aws-testing-library/releases/tag/v1.0.2
https://www.npmjs.com/package/aws-testing-library/v/1.0.2

@erezrokah
Copy link
Owner

Note:
I've also updated the cicd process to allow me to release versions to npm faster (just by pushing a git tag, or creating a release on GitHub): #33

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@erezrokah @abby-huisman