-
Notifications
You must be signed in to change notification settings - Fork 511
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
filter: kubernetes: add documentation for use_kubelet(#1948) #458
Conversation
pipeline/filters/kubernetes.md
Outdated
@@ -46,6 +46,8 @@ The plugin supports the following configuration parameters: | |||
| Dummy\_Meta | If set, use dummy-meta data \(for test/dev purposes\) | Off | | |||
| DNS\_Retries | DNS lookup retries N times until the network start working | 6 | | |||
| DNS\_Wait\_Time | DNS lookup interval between network status checks | 30 | | |||
| Use\_Kubelet | this is an optional feature flag to get metadata information from kubelet instead of calling Kube Server API to enhance the log. This could mitigate the Kube API heavy traffic issue for large cluster. This feature default to be disabled and user could enable it by set this flag to true | False | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the option supports On
or Off
as values as well right? (it should, boolean options in fluent bit config maps support that). I think it's preferable in the docs to use Off and On instead of true and false, even though both work. You can see this for other options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, lets change the wording here, this is my suggestion:
This is an optional feature flag to get metadata information from kubelet instead of calling Kube Server API to obtain kubernetes metadata. At large scale, issues have been reported with Fluent Bit making calls to the API Server, this option may mitigate those problems.
We can link to the issue instead of saying somethig vague, folks won't know what the "Kube API heavy traffic issue" means without more information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it supports On
and Off
. Changed the default value to Off
and updated the description.
pipeline/filters/kubernetes.md
Outdated
@@ -46,6 +46,8 @@ The plugin supports the following configuration parameters: | |||
| Dummy\_Meta | If set, use dummy-meta data \(for test/dev purposes\) | Off | | |||
| DNS\_Retries | DNS lookup retries N times until the network start working | 6 | | |||
| DNS\_Wait\_Time | DNS lookup interval between network status checks | 30 | | |||
| Use\_Kubelet | this is an optional feature flag to get metadata information from kubelet instead of calling Kube Server API to enhance the log. This could mitigate the Kube API heavy traffic issue for large cluster. This feature default to be disabled and user could enable it by set this flag to true | False | | |||
| Kubelet\_Port | kubelet port using for HTTP request, this only works when `Use_Kubelet` set to true. The default value is `10250` which is the same with offical website and could be setup based on kubeletconfig about the kubelet port.| 10250 | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default value is
10250
which is the same with offical website and could be setup based on kubeletconfig about the kubelet port.
I think all of this is unnecessary, since you note the default value in the next column. Also "which is the same with the official website" I am guessing you mean the official k8s docs? I am not sure what this sentence means. I think probably we can just remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed this sentence.
@@ -204,3 +206,149 @@ Under certain and not common conditions, a user would want to alter that hard-co | |||
|
|||
So at this point the filter is able to gather the values of _pod\_name_ and _namespace_, with that information it will check in the local cache \(internal hash table\) if some metadata for that key pair exists, if so, it will enrich the record with the metadata value, otherwise it will connect to the Kubernetes Master/API Server and retrieve that information. | |||
|
|||
## Optional Feature: Using Kubelet to Get Metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the explanation here is good. May be ignore my other comment on the config option and instead have a link in the description to this section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the link inside this section.
pipeline/filters/kubernetes.md
Outdated
## Optional Feature: Using Kubelet to Get Metadata | ||
|
||
There is an [issue](https://github.com/fluent/fluent-bit/issues/1948) reported about kube-apiserver fall over and become unresponsive when cluster is too large and too many requests are sent to it. | ||
We consider fluent bit would expected the same issue so provide this feature as an option to use. For this feature, fluent bit Kubernetes filter will send the request to kubelet /pods endpoint instead of kube-apiserver to retrieve the pods information and use it to enrich the log. Since Kubelet is running locally in nodes, the request would be responded faster and each node would only get one request one time. This could save kube-apiserver power to handle other requests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We consider fluent bit would expected the same issue so provide this feature as an option to use.
I know what you mean here, you are talking about Fluentd. But a reader will not unless they clicked on the issue. Always keep in mind what your reader might be thinking.
I think you can entirely remove this sentence, I don't think it adds value/it doesn't help the user
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
pipeline/filters/kubernetes.md
Outdated
|
||
There is an [issue](https://github.com/fluent/fluent-bit/issues/1948) reported about kube-apiserver fall over and become unresponsive when cluster is too large and too many requests are sent to it. | ||
We consider fluent bit would expected the same issue so provide this feature as an option to use. For this feature, fluent bit Kubernetes filter will send the request to kubelet /pods endpoint instead of kube-apiserver to retrieve the pods information and use it to enrich the log. Since Kubelet is running locally in nodes, the request would be responded faster and each node would only get one request one time. This could save kube-apiserver power to handle other requests. | ||
By enabling this feature, you should see no difference on enriching log part, but the Kube-apiserver bottleneck should be avoided when cluster is large. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to just say:
When this feature is enabled, you should see no difference in the kubernetes metadata added to logs.
the other bit is already explained by the previous sentences.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
pipeline/filters/kubernetes.md
Outdated
|
||
Now you are good to use this new feature! | ||
|
||
### Verify the New Feature Working |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Verify the New Feature Working" is a very vague header. You want to make sure someone skimming this doc would be able to understand what the header is about. The title should be clear.
May be: "Verify that the Use_Kubelet option is working"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
fcec1f6
to
bb6feef
Compare
pipeline/filters/kubernetes.md
Outdated
@@ -46,6 +46,8 @@ The plugin supports the following configuration parameters: | |||
| Dummy\_Meta | If set, use dummy-meta data \(for test/dev purposes\) | Off | | |||
| DNS\_Retries | DNS lookup retries N times until the network start working | 6 | | |||
| DNS\_Wait\_Time | DNS lookup interval between network status checks | 30 | | |||
| Use\_Kubelet | this is an optional feature flag to get metadata information from kubelet instead of calling Kube Server API to enhance the log. This could mitigate the Kube API heavy traffic issue for large cluster. | Off | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"the Kube API heavy traffic issue for large cluster" this should be a link to the standalone section you have down below on this feature ("## Optional Feature: Using Kubelet to Get Metadata"), otherwise to a new user this phrase won't make any sense.
You can do anchor links in github Markdown as shown here: https://gist.github.com/rachelhyman/b1f109155c9dafffe618
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated with anchor
7da7774
to
e8cbc04
Compare
e8cbc04
to
93d6f07
Compare
pipeline/filters/kubernetes.md
Outdated
### Verify that the Use_Kubelet option is working | ||
Basically you should see no difference about your experience for enriching your log files with Kubernetes metadata. | ||
|
||
By Checking if it's working in kubelet way, you can check fluent bit logs and there should be a log like this: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: "To check if Fluent Bit is using the kubelet, you can check..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
pipeline/filters/kubernetes.md
Outdated
Use_Kubelet true | ||
Kubelet_Port 10250 | ||
``` | ||
So for fluent bit configuration, we need to set the `Use_Kubelet` to true to enable this feature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: you not we
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed to you
pipeline/filters/kubernetes.md
Outdated
name: fluentbitds | ||
namespace: fluentbit-system | ||
``` | ||
The difference is that kubelet need a special permission for resource `nodes/proxy` to get HTTP request in. When creating the `role` or `clusterRole`, we need to add `nodes/proxy` into the rule for resource. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you is more typical in docs than we I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed to you
Signed-off-by: Drew Zhang <[email protected]>
93d6f07
to
ec11d51
Compare
This is for the documentation of issue: 3025
Signed-off-by: Drew Zhang [email protected]