quintilesims / layer0

Build, Manage, and Deploy Your Applications
Apache License 2.0
44 stars 20 forks source link

424: Modify L0 API ECS instance user data to send logs to CloudWatch #447

Closed jparsons04 closed 6 years ago

jparsons04 commented 6 years ago

What does this pull request do? The initial user data script that is executed at l0 api instance launch time will now stream 6 logs into their own log streams for the api instance's CloudWatch Log Group:

How should this be tested? Create a new layer0 api instance and observe the new log streams created for your l0 instance.

Note: You need to have the fixes from issue #455 and #456 applied to run this successfully.

~Checklist~ ~- [ ] Unit tests~ ~- [ ] Smoke tests (if applicable)~ ~- [ ] System tests (if applicable)~ ~- [ ] Documentation (if applicable)~

closes #424

zpatrick commented 6 years ago

@jparsons04: Have you verified this works in AWS? If so, :shipit:

diemonster commented 6 years ago

The only thing preventing this from being merged is whether or not it's worth having the debug logs as a default (e.g. if they're too chatty). Or: making this flag configurable, which honestly seems a bit heavy-handed.

@jparsons04 if you could post a gist of what the logs look like with this flag enabled, it would be appreciated.

jparsons04 commented 6 years ago

It's actually not enough to simply add these lines to the user data bash script. We also have to do a number of additional things to pipe the container's system logs to CloudWatch. This article has the basic idea, but as it stands, the changes I've made don't actually do anything and require additional work.

http://docs.aws.amazon.com/AmazonECS/latest/developerguide/using_cloudwatch_logs.html See the section Configuring CloudWatch Logs at Launch with User Data

sesh-kebab commented 6 years ago

As long as DEBUG is a good default among crit | error | warn | info | debug then this is good 👍

jparsons04 commented 6 years ago

After discussion, I think I'm going to turn down the logging levels to INFO for both the docker daemon and the ECS agent.

zpatrick commented 6 years ago

closing to reduce clutter