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 tail_path option to in_tail plugin #951

Merged
merged 1 commit into from
May 16, 2016
Merged

Add tail_path option to in_tail plugin #951

merged 1 commit into from
May 16, 2016

Conversation

sonots
Copy link
Member

@sonots sonots commented May 16, 2016

Fix #945

@@ -252,6 +254,7 @@ def flush_buffer(tw)
else
@tag
end
record[@path_key] ||= tw.path unless @path_key.nil?
Copy link
Member

Choose a reason for hiding this comment

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

Using ||= is for safety?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

Copy link
Member Author

Choose a reason for hiding this comment

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

I mean that path_key is not overwritten if record already has it.
This was the specification of the original implementation (#281), but we can change the spec now. Do you prefer to overwrite?

Copy link
Member

@repeatedly repeatedly May 16, 2016

Choose a reason for hiding this comment

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

Either is fine for me.
BTW, user can check path_key value is not contained or not easily so
additional check is not needed for me.

Copy link
Member Author

@sonots sonots May 16, 2016

Choose a reason for hiding this comment

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

Okay, let us keep as is (not to overwrite).

Copy link
Member

Choose a reason for hiding this comment

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

I follow your opinion

@repeatedly repeatedly merged commit 808a6e8 into master May 16, 2016
@repeatedly
Copy link
Member

Thx!

@sonots sonots deleted the tail_path branch May 16, 2016 06:43
@repeatedly repeatedly added v0.12 enhancement Feature request or improve operations labels May 16, 2016
repeatedly added a commit that referenced this pull request May 16, 2016
Add tail_path option to in_tail plugin
@tagomoris
Copy link
Member

Tests added by this change in test_in_tail fails very frequently.
For example: https://travis-ci.org/fluent/fluentd/jobs/130508869
@sonots Could you fix it?

@sonots sonots mentioned this pull request May 16, 2016
@sonots
Copy link
Member Author

sonots commented May 16, 2016

@tagomoris sorry, fixed the unstable test.

cosmo0920 added a commit to cosmo0920/fluent-plugin-tail_path that referenced this pull request Dec 14, 2016
Because Fluentd v0.14 includes this feature. see:
fluent/fluentd#951
cosmo0920 added a commit to cosmo0920/fluentd-website that referenced this pull request Feb 7, 2017
Because in_tail_path had been merged in in_tail in v0.14.

ref: fluent/fluentd#945
ref: fluent/fluentd#951
cosmo0920 added a commit to cosmo0920/fluentd-website that referenced this pull request Feb 7, 2017
Because in_tail_path had been merged in in_tail in v0.12.

ref: fluent/fluentd#945
ref: fluent/fluentd#951
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request or improve operations v0.12 v0.14
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants