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

Add TCP support for GELF log driver #34758

Merged
merged 4 commits into from Oct 10, 2017

Conversation

@ghislainbourgeois
Contributor

ghislainbourgeois commented Sep 6, 2017

- What I did
I added TCP support to the GELF log driver. closes #33495 closes #29871

The documentation PR can be found here

- How I did it
The following changes were done:

  • Updated go-gelf to v2
  • Added unit tests to the gelf log driver
  • Refactored the gelf log driver to make room for the TCP support
  • Added TCP support

- How to verify it

  1. Run a Graylog server and configure a TCP GELF Input and a UDP GELF Input.

  2. Run a container with:

  • "--log-driver gelf --log-opt gelf-address=udp://<graylog_host>:<udp_port>"
  • "--log-driver gelf --log-opt gelf-address=tcp://<graylog_host>:<tcp_port>"
  1. Validate that the container logs are received in the Graylog server.

- Description for the changelog
Added TCP support to the GELF log driver.

- A picture of a cute animal (not mandatory but encouraged)
animal

ghislainbourgeois added some commits Sep 5, 2017

Update to latest go-gelf version and add tests
Signed-off-by: Ghislain Bourgeois <ghislain.bourgeois@gmail.com>
Remove empty gelf_unsupported.go
Signed-off-by: Ghislain Bourgeois <ghislain.bourgeois@gmail.com>
Add support for TCP parameters
Signed-off-by: Ghislain Bourgeois <ghislain.bourgeois@gmail.com>
Add TCP support for GELF log driver
Signed-off-by: Ghislain Bourgeois <ghislain.bourgeois@gmail.com>

@ghislainbourgeois ghislainbourgeois changed the title from [WIP] Add TCP support for GELF log driver to Add TCP support for GELF log driver Sep 7, 2017

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah
Member

thaJeztah commented Sep 13, 2017

@vieux

This comment has been minimized.

Show comment
Hide comment
@vieux

vieux Sep 13, 2017

Collaborator

LGTM

Collaborator

vieux commented Sep 13, 2017

LGTM

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Oct 9, 2017

Member

ping @mariussturm could you have a look at this one?

Member

thaJeztah commented Oct 9, 2017

ping @mariussturm could you have a look at this one?

@mariussturm

This comment has been minimized.

Show comment
Hide comment
@mariussturm

mariussturm Oct 10, 2017

Contributor

LGTM

Contributor

mariussturm commented Oct 10, 2017

LGTM

@cpuguy83

LGTM

@cpuguy83 cpuguy83 merged commit 3437f0f into moby:master Oct 10, 2017

7 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 36694 has succeeded
Details
janky Jenkins build Docker-PRs 45329 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 5727 has succeeded
Details
vendor Jenkins build Docker-PRs-vendor 3765 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 17041 has succeeded
Details
z Jenkins build Docker-PRs-s390x 5510 has succeeded
Details
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Oct 10, 2017

Member

Thanks for reviewing @mariussturm

Member

thaJeztah commented Oct 10, 2017

Thanks for reviewing @mariussturm

@pdepaepe

This comment has been minimized.

Show comment
Hide comment
@pdepaepe

pdepaepe Nov 14, 2017

Thanks a lot for this PR. Would you please also consider tcp+tls:// ?

pdepaepe commented Nov 14, 2017

Thanks a lot for this PR. Would you please also consider tcp+tls:// ?

@ghislainbourgeois

This comment has been minimized.

Show comment
Hide comment
@ghislainbourgeois

ghislainbourgeois Nov 17, 2017

Contributor

@pdepaepe I will look into what is involved, but do not expect it too quickly. This will need a PR in the go-gelf library before making the PR in moby. From my experience with this one, this could take a couple of months.

Contributor

ghislainbourgeois commented Nov 17, 2017

@pdepaepe I will look into what is involved, but do not expect it too quickly. This will need a PR in the go-gelf library before making the PR in moby. From my experience with this one, this could take a couple of months.

@@ -76,7 +76,7 @@ github.com/syndtr/gocapability 2c00daeb6c3b45114c80ac44119e7b8801fdd852
github.com/golang/protobuf 7a211bcf3bce0e3f1d74f9894916e6f116ae83b4
# gelf logging driver deps
github.com/Graylog2/go-gelf 7029da823dad4ef3a876df61065156acb703b2ea
github.com/Graylog2/go-gelf v2

This comment has been minimized.

@thaJeztah

thaJeztah Dec 11, 2017

Member

Whoops, turned out v2 here is a branch, not a tag

@thaJeztah

thaJeztah Dec 11, 2017

Member

Whoops, turned out v2 here is a branch, not a tag

@Hermain

This comment has been minimized.

Show comment
Hide comment
@Hermain

Hermain Jan 9, 2018

@ghislainbourgeois I'm trying to send logs to logstash. Unfortunately its not possible to set '\0' as delimiter in logstash. Should this maybe be configurable so that \n could be used (as this is the default for most log parsers).

Hermain commented Jan 9, 2018

@ghislainbourgeois I'm trying to send logs to logstash. Unfortunately its not possible to set '\0' as delimiter in logstash. Should this maybe be configurable so that \n could be used (as this is the default for most log parsers).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment