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

Support Logentries Log Driver #870

Merged
merged 2 commits into from
Aug 1, 2017
Merged

Support Logentries Log Driver #870

merged 2 commits into from
Aug 1, 2017

Conversation

opsline-radek
Copy link
Contributor

@opsline-radek opsline-radek commented Jun 29, 2017

Summary

Adding support for Logentries log driver introduced in docker API 1.25 (docker version 1.13.0).

Implementation details

Adding logger names to appropriate places.

Testing

Tested on newest AWS Linux ECS by replacing the stock image with this one, updating ECS_AVAILABLE_LOGGING_DRIVERS variable, and scheduling a task with logentries log driver configured.

  • Builds on Linux (make release)
  • Builds on Windows (go build -out amazon-ecs-agent.exe ./agent)
  • Unit tests on Linux (make test-in-docker) pass
  • Unit tests on Windows (go test -timeout=25s ./agent/...) pass
  • Integration tests on Linux (make run-integ-tests) pass
  • Integration tests on Windows (.\scripts\run-integ-tests.ps1) pass
  • Functional tests on Linux (make run-functional-tests) pass
  • Functional tests on Windows (.\scripts\run-functional-tests.ps1) pass

New tests cover the changes: no

Description for the changelog

Support Logentries Log Driver

Licensing

This contribution is under the terms of the Apache 2.0 License: yes

@opsline-radek
Copy link
Contributor Author

Integration and functional tests fail to build, so can't execute.

Copy link
Contributor

@samuelkarp samuelkarp left a comment

Choose a reason for hiding this comment

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

Thanks for sending this! I believe we need a somewhat larger change to actually make this (I don't think the logentries driver will actually register because the 1.25 API version is not "supported"), but I've gone ahead and made the logic change in #875.

@@ -961,7 +961,8 @@
"journald",
"gelf",
"fluentd",
"awslogs"
"awslogs",
"logentries"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you undo the change here? This file is generated from a different one we have internally. Once we get this merged, I can make the change internally.

@@ -7479,6 +7479,9 @@ const (

// LogDriverAwslogs is a LogDriver enum value
LogDriverAwslogs = "awslogs"

// LogDriverLogentries is a LogDriver enum value
LogDriverLogentries = "logentries"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

@samuelkarp samuelkarp dismissed their stale review July 21, 2017 18:14

Changes have been made

@adnxn
Copy link
Contributor

adnxn commented Aug 1, 2017

changes successfully validated on a clean container instance using a Logentries trial account.

@adnxn adnxn mentioned this pull request Aug 1, 2017
8 tasks
@adnxn adnxn merged commit 95e3a98 into aws:dev Aug 1, 2017
@jhaynes jhaynes mentioned this pull request Aug 22, 2017
@samuelkarp samuelkarp added this to the 1.14.4 milestone Aug 22, 2017
@LyleScott
Copy link

Have people been experiencing an issue with this Driver? We've had problems for several months with containers seemingly stopping logging to LE. It seems consistent on only ECS containers using the driver (different apps, too). We've had to switch to awslog driver / app logging with some forwarding for out-of-app stuff :/ we are still trying to troubleshoot the issue

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.

5 participants