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

Add TCP support for GELF log driver #34758

Merged
merged 4 commits into from
Oct 10, 2017
Merged

Add TCP support for GELF log driver #34758

merged 4 commits into from
Oct 10, 2017

Conversation

ghislainbourgeois
Copy link
Contributor

@ghislainbourgeois 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 ghislainbourgeois changed the title [WIP] Add TCP support for GELF log driver Add TCP support for GELF log driver Sep 7, 2017
@thaJeztah
Copy link
Member

ping @vieux @cpuguy83 @mariussturm PTAL

@thaJeztah thaJeztah added the kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. label Sep 13, 2017
@vieux
Copy link
Contributor

vieux commented Sep 13, 2017

LGTM

Copy link
Member

@boaz0 boaz0 left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Member

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

@mariussturm
Copy link
Contributor

LGTM

Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@cpuguy83 cpuguy83 merged commit 3437f0f into moby:master Oct 10, 2017
@thaJeztah
Copy link
Member

Thanks for reviewing @mariussturm

@pdepaepe
Copy link

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

@ghislainbourgeois
Copy link
Contributor Author

@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
Copy link
Member

Choose a reason for hiding this comment

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

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

@Hermain
Copy link

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
Labels
area/logging impact/changelog kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. status/2-code-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GELF Driver does not support TCP Unable to use tcp in --gelf-address
10 participants