The concept would be nice, but implementing it as another plugin isn't good idea.
Please imagine several tail plugins that are focused on each features.

• in_tail_feature1
• in_tail_feature2
• in_tail_feature3
• ...

If a user wants feature1 and feature3, which plugin the user should select?
We should integrate it into existing tail plugin.

On the other hand, current in_tail’s code is complicated, I think refactoring might be needed before adding new major features.
Until doing it, providing it as an external plugin would be nice.

The concept would be nice, but implementing it as another plugin isn't good idea. Please imagine several tail plugins that are focused on each features.

• in_tail_feature1
• in_tail_feature2
• in_tail_feature3
• ...

If a user wants feature1 and feature3, which plugin the user should select? We should integrate it into existing tail plugin.

Thank you for all your valuable feedback. In the in_tail plugin, one can rate limit using read_bytes_limit_per_second, but in this PR, in_tail_with_throttle provides a grouping mechanism for rate limiting files. These two features might seem mutually exclusive from an end-user point of view. That’s why we prepared a separate plugin that extends the functionality of the in_tail plugin. So, if one is interested in limiting each file by bytes per second, then one can use the in_tail plugin. Otherwise, you can add group rules in in_tail_with_throttle.

On the other hand, current in_tail’s code is complicated, I think refactoring might be needed before adding new major features. Until doing it, providing it as an external plugin would be nice.

Do you want to keep a separate plugin in this repository until in_tail is refactored and then merge it later?

@ashie can you further guide on this PR merging ? please note both in_tail and in_tail_with_throttle can co-exists and one can use both of them as set of . we didn't want to disturb the base line of in_tail hence we created a new plugin with this group based rate limiting functionality. let us know what step we need to take next.

My additional impressions:

• Equivalent feature might be able to realized by multiple <source> and adding regexp based path parameter to in_tail. Could you describe the advantage of your patch compared with other ideas?
  • I think one of the strength of your idea is fallback mechanism, it makes easy to configure unmatched paths.
• Again, implementing as an additional plugin isn't good idea, please consider integrating it into in_tail if you want to merge it into fluentd.
  • If you can integrate it cleanly, we might not need waiting refactoring.
• The config format seems too rely on k8s's concept. It would be nice if you can generalize it.
• We don't want maintaining Copy & Pasted codes. For example detach_watcher_after_rotate_wait and TailWatcher::IOHandler#rate_limit_handle_notify are almost same with parent's one. When parent one is modified, we have to modify both codes. Please consider integrating them.
• Although in_tail_with_throttle might change the behaviour of in_tail, in_tail's tests aren't executed against in_tail_with_throttle.

My additional impressions:

• Equivalent feature might be able to realized by multiple <source> and adding regexp based path parameter to in_tail. Could you describe the advantage of your patch compared with other ideas?

  • I think one of the strength of your idea is fallback mechanism, it makes easy to configure unmatched paths.

@ashie Our plugin has the following advantages :

1. Using multiple threads -> context switching and cpu usage

Adding regexp based path parameter to in_tail will add multiple source plugins which will create additional threads for reading from files. Hence cpu cycles will be wasted in context switching.

2. Paths can be overlapped when using multiple source plugins

in_tail_with_throttle allows you to have generic and context-specifc rules for log rate limit. It will be difficult to manage with in_tail's path parameter based grouping and can lead to files falling In more than one group.

For example, consider the following requirement:
1. Rate limit all files with a particular namespace (Generic rule).
2. Give priority to specific app name in that namespace with a different limit (Context Specific Rule)

Configuration for such requirement will be like: 
<group_rule>
	...
	<rule>
		## rule1
		namespace ABC
		limit 1000 ## Generic rules for all files in a namespace
	</rule>	
	<rule>
		## rule2
		namespace ABC
		appname DEF  ## Specific rules with adjustable limit
		limit 1000
	</rule>	
	...
</group_rule>

in_tail_with_throttle will add a file with DEF app name and ABC namespace to rule 2 (Higher precedence over rule1). Using in_tail's path parameter will add the file in two groups and hence repeated log lines will be collected.

3. Our plugin will take care of unexpected files.

In cases where we forget to add rules for some files, in_tail_with_throttle will add them to the default group and still collect logs. in_tail plugin with regex based path parameter grouping will fail in these situations.

4. Config will get pretty cluttered.

Adding multiple source configs for in_tail's path based grouping will make your configuration file big in size. It will be difficult to manage and update configurations in case of very large number of groups.

My additional impressions:

• The config format seems too rely on k8s's concept. It would be nice if you can generalize it.

@ashie can you please elaborate on what this generalisation means? Should we replace our namespace and app-name fields with something else?

For example,

<rule> 
    ## level1 instead of namespace
    level1 ABC

    ## level2 instead of appname
    level2 DEF

    limit 1000
</rule>

@ashie Could you please comment?

@cosmo0920 would you also be able to take a look?

The big picture seems good, I'll check further more details, please wait for a few days...

A test always fails:

2022-02-07T07:27:29.7369178Z Error: test_shorter_than_rotate_wait(TailInputTest::singleline::log throttling per file::EOF with reads_bytes_per_second): Fluent::Test::Driver::TestTimedOut: Test case timed out with hard limit.
2022-02-07T07:27:29.7398448Z /home/runner/work/fluentd/fluentd/lib/fluent/test/driver/base.rb:201:in `rescue in run_actual'
2022-02-07T07:27:29.7398936Z /home/runner/work/fluentd/fluentd/lib/fluent/test/driver/base.rb:196:in `run_actual'
2022-02-07T07:27:29.7399358Z /home/runner/work/fluentd/fluentd/lib/fluent/test/driver/base.rb:95:in `run'
2022-02-07T07:27:29.7399753Z /home/runner/work/fluentd/fluentd/lib/fluent/test/driver/base_owner.rb:130:in `run'
2022-02-07T07:27:29.7400214Z /home/runner/work/fluentd/fluentd/test/plugin/test_in_tail.rb:714:in `test_shorter_than_rotate_wait'
2022-02-07T07:27:29.7400553Z      711:             tw
2022-02-07T07:27:29.7400872Z      712:           end.twice
2022-02-07T07:27:29.7401240Z      713: 
2022-02-07T07:27:29.7401498Z   => 714:           d.run(timeout: 10) do
2022-02-07T07:27:29.7403168Z      715:             until detached do
2022-02-07T07:27:29.7403680Z      716:               if d.events.size > 0 && !rotated
2022-02-07T07:27:29.7405460Z      717:                 cleanup_file("#{TMP_DIR}/tail.txt")
2022-02-07T07:27:29.7406123Z ===============================================================================
2022-02-07T07:29:08.6385152Z ...............................................................................
2022-02-07T07:29:08.6500370Z ...............................................................................
2022-02-07T07:29:08.6725292Z ...............................................................................

Hmm, it takes too long time to run tests, more than 1 minutes longer than before:

master branch:

$ time bundle exec rake test TEST=test/plugin/test_in_tail.rb
...
real	2m33.910s
user	0m3.410s
sys	0m0.322s

this branch:

$ time bundle exec rake test TEST=test/plugin/test_in_tail.rb
...
real	3m36.044s
user	0m3.807s
sys	0m0.335s

We should shorten it if it's possible since it's affect to whole debugging efficiency.

Hmm, it takes too long time to run tests, more than 1 minutes longer than before:

Trying to improve it by efa7d2d
It seems fine on my local environment but not sure on CI yet.

Hmm, it takes too long time to run tests, more than 1 minutes longer than before:

Trying to improve it by efa7d2d It seems fine on my local environment but not sure on CI yet.

It's also fine on CI.

I'll merge this after checking group_watch.rb once more.

Hmm, it takes too long time to run tests, more than 1 minutes longer than before:

Trying to improve it by efa7d2d It seems fine on my local environment but not sure on CI yet.

$ time bundle exec rake test TEST=test/plugin/test_in_tail.rb
...
real	2m45.993s
user	0m3.642s
sys	0m0.266s

Merged.
I'm sorry for taking long time to merge this.
And thank you for contributing a nice feature 😃

Thanks @ashie @cosmo0920 @agup006 for your help and support! I am glad that it got merged! 🎉