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 udp and tcp input #368

Merged
merged 6 commits into from
Jun 30, 2014
Merged

Add udp and tcp input #368

merged 6 commits into from
Jun 30, 2014

Conversation

repeatedly
Copy link
Member

General approach for resolving #363

@kiyoto Could you check this PR?

StreamInput based TcpInput is obsoleted since 2+ years ago.
So we can remove this class for newer plugin
@repeatedly repeatedly changed the title Add udp tcp input Add udp and tcp input Jun 26, 2014
@kiyoto
Copy link
Contributor

kiyoto commented Jun 26, 2014

I will take a closer look and test today. It might be a good idea to check with @frsyuki why the standard socket library wasn't used initially.

@kiyoto
Copy link
Contributor

kiyoto commented Jun 26, 2014

@repeatedly

I tested this locally on Ubuntu Precise VM.

  • Checked that it can handle both UDP and TCP payloads with csv/regex.
  • I noticed that for TCP + format none, the "\n" at the end is preserved. I suppose this is an expected behavior?
  • Most importantly, I checked that this works with syslog! Hooray! I will write a new solution page once this patch lands.

@repeatedly
Copy link
Member Author

@kiyoto Thank you for testing.

I noticed that for TCP + format none, the "\n" at the end is preserved

Hm. I can remove @delimiter from message easily.
Does in_tail removes \n? If so, keep same behaviour is better.

@repeatedly
Copy link
Member Author

I also add "source_host_key" option to add source host to records.
I'm not sure this is useful or not in general.
If users use in_udp or in_tcp for various sources, it might be useful for detecting log source.

@repeatedly
Copy link
Member Author

@frsyuki @kiyoto @tagomoris @sonots Do you have any concern to merge this PR?

@tagomoris
Copy link
Member

I think that using TcpInput directly is obsolete now.
If it is correct, we should add warn logs for direct use (maybe both of tcp/udp).

@repeatedly
Copy link
Member Author

No. Old TcpInput is obsoleted since 2 and half years ago so there is no users.
This PR introduces new TcpInput to handle general TCP access instead of old obsoleted TcpInput.

@tagomoris
Copy link
Member

I see.

@repeatedly
Copy link
Member Author

@kiyoto Follow in_tail way. Now in_udp and in_tcp removes a delimiter from message. Please check.

@repeatedly
Copy link
Member Author

If there is no problem, I will merge this PR tomorrow today.

repeatedly added a commit that referenced this pull request Jun 30, 2014
@repeatedly repeatedly merged commit 8da1871 into master Jun 30, 2014
@repeatedly repeatedly deleted the add-udp-tcp-input branch June 30, 2014 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request *Deprecated Label* Use enhancement label in general
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants