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

Doc: Expand info for URL options and other minor fixes #120

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

karenzone
Copy link
Contributor

Fix link formatting
Update outdated link to apache.org

Fix link formatting
Update outdated link to apache.org
@karenzone
Copy link
Contributor Author

karenzone commented Apr 30, 2021

@robbavey Much of this change is based on info you shared recently. Please let me know if you think this addition could be helpful. If so, anything else I should add?

Copy link
Contributor

@robbavey robbavey left a comment

Choose a reason for hiding this comment

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

Couple of suggestions, some of which predate your changes. I haven't taken a completely thorough look through the rest of the docs to see if there any more

@@ -369,7 +365,8 @@ Specify the truststore type here. One of `JKS` or `PKCS12`. Default is `JKS`
* Value type is <<string,string>>
* There is no default value for this setting.

URL to use
URL to use.
This option is {logstash-ref}/event-dependent-configuration.html#sprintf[sprintf]-compliant if your configuration does not use `json-batch` with the <<plugins-{type}s-{plugin}-content_type>> or <<plugins-{type}s-{plugin}-format>> options.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just the format option is relevant here -

Suggested change
This option is {logstash-ref}/event-dependent-configuration.html#sprintf[sprintf]-compliant if your configuration does not use `json-batch` with the <<plugins-{type}s-{plugin}-content_type>> or <<plugins-{type}s-{plugin}-format>> options.
This option is {logstash-ref}/event-dependent-configuration.html#sprintf[sprintf]-compliant if your configuration does not use the `json-batch` <<plugins-{type}s-{plugin}-format>> option.

See https://hc.apache.org/httpcomponents-client-ga/httpclient/apidocs/org/apache/http/impl/conn/PoolingHttpClientConnectionManager.html#setValidateAfterInactivity(int)[these docs for more info]
'Defines period of inactivity in milliseconds after which persistent connections must be re-validated prior to being leased to the consumer. Non-positive value passed to this method disables connection validation.
This check helps detect connections that have become stale (half-closed) while kept inactive in the pool.'
See the https://hc.apache.org/httpcomponents-client-5.0.x/[apache.org docs] for more info.
Copy link
Contributor

Choose a reason for hiding this comment

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

The link here is a tricky one to get right - this amended link points to the top level docs for the http client rather than the specific entry that was initially linked to. The version is also later than the version we actually use - the library that we are referring to here is pulled in by the manticore gem, which is pulled in by the logstash-mixin-http_client gem...

The current,correct active link is https://hc.apache.org/httpcomponents-client-4.5.x/current/httpclient/apidocs/org/apache/http/impl/conn/PoolingHttpClientConnectionManager.html#getValidateAfterInactivity()

based on [Manticore](https://github.com/cheald/manticore).
For an example of its usage see https://github.com/logstash-plugins/logstash-input-http_poller
Timeout (in seconds) for the entire request
This module helps you add a fully configured HTTP client to Logstash based on https://github.com/cheald/manticore[Manticore].
Copy link
Contributor

Choose a reason for hiding this comment

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

This section looks like it has been copy/pasted from here, and seems a bit out of place here in the middle of the configuration settings.

It can probably be replaced by

Timeout (in seconds) for the entire request

It would be nice to have configuration examples as in the http poller input docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants