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

Accept structured logs data over TCP by default. #7

Merged
merged 2 commits into from Jun 2, 2016
Merged

Accept structured logs data over TCP by default. #7

merged 2 commits into from Jun 2, 2016

Conversation

bokowski
Copy link
Contributor

To collect application-level logs, the fluentd documentation recommends
using the forward input plugin for all languages except PHP, see
http://docs.fluentd.org/v0.12/categories/logging-from-apps. By adding
this file, the story for sending application-level logs to Cloud Logging
becomes much easier.

To collect application-level logs, the fluentd documentation recommends
using the forward input plugin for all languages except PHP, see
http://docs.fluentd.org/v0.12/categories/logging-from-apps. By adding
this file, the story for sending application-level logs to Cloud Logging
becomes much easier.
@@ -0,0 +1,6 @@
<source>
@type forward
port 24224
Copy link
Member

Choose a reason for hiding this comment

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

This is the default, isn't it? Does it still make sense to set it explicitly?
@mr-salty for comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought it did make sense - we are asking users to edit the files in that
directory as necessary, and including the default in the file makes it
easier to see right there what the default is, and to decide whether it
needs to be changed. Otherwise, users will have to find the documentation
that's in a totally different spot.

On Thu, 2 Jun 2016, 18:57 igorpeshansky, notifications@github.com wrote:

In configs/config.d/forward.conf
#7 (comment)
:

@@ -0,0 +1,6 @@
+

  • @type forward
  • port 24224

This is the default, isn't it? Does it still make sense to set it
explicitly?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/GoogleCloudPlatform/fluentd-catch-all-config/pull/7/files/d87ae65a65f4452413bbbbacd3f0b242fba327c2#r65577922,
or mute the thread
https://github.com/notifications/unsubscribe/AALxtGnuVR7pHv2o0qXxrkJuK0dO1Yb0ks5qHwuUgaJpZM4Iez4l
.

Copy link
Member

Choose a reason for hiding this comment

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

I would at least add a "# Default." comment to this line. The problem is that other tools will attempt to connect to this port, so it makes sense to communicate to the user that changing this not advisable.

@igorpeshansky igorpeshansky self-assigned this Jun 2, 2016
@bokowski
Copy link
Contributor Author

bokowski commented Jun 2, 2016

Good point. I've added a comment saying that this is the default port.

@igorpeshansky
Copy link
Member

Ok, can you please squash the two commits? Otherwise LGTM.

@mr-salty
Copy link
Contributor

mr-salty commented Jun 2, 2016

FYI @igorpeshansky github now allows you to squash commits during merge, it's a fairly recent feature: https://github.com/blog/2141-squash-your-commits

@igorpeshansky
Copy link
Member

@mr-salty, doesn't look like this repo is configured to allow it. Plus, it's not at all clear from the documentation whether the authorship of the resulting commit will be retained.

@igorpeshansky igorpeshansky merged commit afc7766 into GoogleCloudPlatform:master Jun 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants