Add Harvester Limit to the Apache Tomcat Logs

[ishleenk17]

Proposed commit message

• WHAT: Addition of harvester limit to the Apache Tomcat logs to control the number of files to be opened in parallel
• WHY: There are users who have more files to harvest than the system limit.

Checklist

• I have reviewed tips for building integrations and this pull request is aligned with them.
• I have verified that all data streams collect metrics or logs.
• I have added an entry to my package's changelog.yml file.
• I have verified that Kibana version constraints are current according to guidelines.

Related issues

• Closes https://github.com/elastic/enhancements/issues/21308

Screenshots

[ishleenk17]

@belimawr : Since this is the very first Integration that is using Harvester Limit config option.
Could you please review as well.

Also, I see mentioned in the harvester_limit documentation that it is advisable to use the close_* config along with the harvester limit. Thoughts on that?

[elasticmachine]

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

[harnish-elastic]

Link that you have attached is broken. Can you please fix it?

[harnish-elastic]

LGTM! 🚀

[belimawr]

[Suggestion]

I'm not completely sure how big the description can be, however it would be nice to mention a couple of things:

• zero means no limit
• This is per input, not a global setting.

The second point can definitely be left for the documentation linked, however the first one I feel it is important to have stated.

[belimawr]

@belimawr : Since this is the very first Integration that is using Harvester Limit config option. Could you please review as well.

Also, I see mentioned in the harvester_limit documentation that it is advisable to use the close_* config along with the harvester limit. Thoughts on that?

Because the harvesters are limited, this means not all files will be opened/harvested at the same time. A new file will only be opened after:

• A previously open file is closed
• and there is another scan for new files/changed files.

Also because the next file to be opened is picked randomly, there is no guarantee of how long a file will wait to be harvested.

Because of this considerations our documentation suggests to set some of the close_* fields to close files more often, hence increasing the rotation between the files being harvested.

The default to close a file is after 5 minutes of inactivity. In some environments that might be too long.

Thinking about it, I believe this PR should also include some of the close_* settings. At least close_inactive and close_eof also exposed.

[belimawr]

Sorry for changing my review, but something just crossed my mind: Is this integration using Filestream or Log input?

The file changed is called filestream.yml.hbs, however there is no type: filestream set and the documentation link is for the Log input. I'm a bit confused here.

[ishleenk17]

Sorry for changing my review, but something just crossed my mind: Is this integration using Filestream or Log input?

The file changed is called filestream.yml.hbs, however there is no type: filestream set and the documentation link is for the Log input. I'm a bit confused here.

This Integration uses Filestream input type here. Documentation is for log input since there exists no documentation for harvester_limit for the filestream input.

[belimawr]

Sorry for changing my review, but something just crossed my mind: Is this integration using Filestream or Log input?
The file changed is called filestream.yml.hbs, however there is no type: filestream set and the documentation link is for the Log input. I'm a bit confused here.

This Integration uses Filestream input type here. Documentation is for log input since there exists no documentation for harvester_limit for the filestream input.

That can be confusing for users. Even though the behaviour of the log input and filestream are the same, some of the configuration keys are different. I'm a bit concerned that this might cause some SDHs. However I don't see it as a blocker, as we can update the documentation link once we document it in Filestream.

One of the key difference are the close_* settings that on Filestream are under a different key, e.g: close.on_state_change.inactive and close.on_state_change.removed.

I understand the lack of documentation is a shortcoming from Filestream :/;

If it is not super urgent to merge this PR, maybe we can try to get the filestream documentation released first.

[ishleenk17]

If it is not super urgent to merge this PR, maybe we can try to get the filestream documentation released first.

The PR can wait. What would be the timeline for documenting this ib Filestream ?

[ishleenk17]

If it is not super urgent to merge this PR, maybe we can try to get the filestream documentation released first.

The PR can wait. What would be the timeline for documenting this ib Filestream ?

@belimawr : Can you share when this documentation would be ready ?

[belimawr]

@pierrehilbert do you think we can squeeze elastic/beats#38784 in our current sprint?

It is a small doc update and it will help to keep the documentation for this integration consistent.

[pierrehilbert]

If you are able to finalize your P0 first, I have no objections with having it in the current sprint.

[belimawr]

I added some small suggestions, they apply to all entries.

[ishleenk17]

I am planning to remove this link. Once the beats changes are out for this documentation. Will link it then.
@belimawr

[ritalwar]

same, no need for ""

[ritalwar]

In descriptions, we're sometimes using quotation marks and sometimes not. I think we should be consistent, and ideally, they're not needed.

[ritalwar]

same, no need for ""

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 76902f0

History

• 💚 Build #11376 succeeded 5b6fd02
• 💚 Build #11213 succeeded 2c674b3
• 💚 Build #10372 succeeded 6b777f6
• 💚 Build #10320 succeeded 1538cb6

cc @ishleenk17

[elastic-sonarqube]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube

[elasticmachine]

Package apache_tomcat - 1.5.0 containing this change is available at https://epr.elastic.co/search?package=apache_tomcat

[htioekb]

Hello @ishleenk17,

{{#if close.on_state_change.inactive}}
close.on_state_change.inactive: 
{{/if}}

are we missing a value here?

After editing and saving the integration in Kibana I get the following error message:
can not read a block mapping entry; a multiline key may not be an implicit key at line 10, column 31: close.on_state_change.inactive: ^

[ishleenk17]

Hello @ishleenk17,

{{#if close.on_state_change.inactive}}
close.on_state_change.inactive: 
{{/if}}

are we missing a value here?

After editing and saving the integration in Kibana I get the following error message: can not read a block mapping entry; a multiline key may not be an implicit key at line 10, column 31: close.on_state_change.inactive: ^

The issue is being fixed.