FluentD file buffer #322
FluentD file buffer #322
Conversation
…umologic-kubernetes-collection into vsinghal-file-buffer
…umologic-kubernetes-collection into vsinghal-file-buffer
…umologic-kubernetes-collection into vsinghal-file-buffer
…umologic-kubernetes-collection into vsinghal-file-buffer
| @@ -22,47 +22,120 @@ | |||
| @id sumologic.endpoint.metrics.apiserver | |||
| endpoint "#{ENV['SUMO_ENDPOINT_METRICS_APISERVER']}" | |||
| @include metrics.output.conf | |||
| <buffer> | |||
| {{- if eq .Values.sumologic.fluentdBuffer "file" }} | |||
| @type file | |||
There was a problem hiding this comment.
indent looks off on these, e.g. https://github.com/SumoLogic/sumologic-kubernetes-collection/pull/322/files#diff-77aa48b7684fe90d242ce9fdae9da300R58
I think they need to be something like
{{- if eq .Values.sumologic.fluentdBuffer "file" }}
@type file
path /fluentd/buffer/metrics.kubelet
{{- else }}
@type memory
{{- end }}
There was a problem hiding this comment.
good catch! Changed the indentation.
| @@ -17,6 +17,15 @@ | |||
| @type sumologic | |||
| @id sumologic.endpoint.logs.kubelet | |||
| @include logs.output.conf | |||
| <buffer> | |||
| {{- if eq .Values.sumologic.fluentdBuffer "file" }} | |||
| @type file | |||
deploy/helm/sumologic/values.yaml
Outdated
| @@ -159,6 +159,9 @@ sumologic: | |||
| # watchResourceEventsOverrides: | |||
| # pods: "v1" | |||
| # events: "events.k8s.io/v1beta1" | |||
|
|
|||
| ## Option to specify the fluentD buffer as file/memory. | |||
There was a problem hiding this comment.
nit: "fluentD" --> "Fluentd" to match our docs
There was a problem hiding this comment.
also can we make this
fluentd:
buffer: "memory"
There was a problem hiding this comment.
"FluentD" seems to be used in the values.yaml. Should update all those as well?
There was a problem hiding this comment.
LGTM, maybe in a future PR we can consider using Helm templating to make the metrics output conf more DRY, like the way they do in the fluentbit helm chart: https://github.com/helm/charts/blob/master/stable/fluent-bit/templates/config.yaml#L47
|
Is this PR sufficient to be released separately? Or do we need to waiting external volume? Also is there a doc/spec for the expected behavior (flush at shutdown, etc)? |
I will be creating a separate PR for the doc/spec. This PR will use the local file system to store the buffer files. In future, we can provide additional option to use external volumes to store the buffer. |
| @@ -57,55 +54,87 @@ data: | |||
| @id sumologic.endpoint.metrics.apiserver | |||
| endpoint "#{ENV['SUMO_ENDPOINT_METRICS_APISERVER']}" | |||
| @include metrics.output.conf | |||
| <buffer> | |||
| @type memory | |||
There was a problem hiding this comment.
@vsinghal13 is this change is only for helm based installation? For non-helm, we are still going to buffer to memory?
There was a problem hiding this comment.
I think the non-helm users can customize config by following these steps: https://github.com/SumoLogic/sumologic-kubernetes-collection/blob/master/deploy/docs/Non_Helm_Installation.md#customize-configuration
| @@ -30,12 +30,13 @@ | |||
| verify_ssl "#{ENV['VERIFY_SSL']}" | |||
| proxy_uri "#{ENV['PROXY_URI']}" | |||
| <buffer> | |||
| {{- if eq .Values.sumologic.fluentd.buffer "file" }} | |||
| @type file | |||
| path /fluentd/buffer/events | |||
There was a problem hiding this comment.
@vsinghal13 is this path local to node? If yes, all pods inside that node will store buffer in the same file?
There was a problem hiding this comment.
Yes, the path is local to node. Each buffer section needs to have a unique path. The pods will store will store in different paths based on the config path provided in the respective buffer section.
Description
Add option to use fluentD file buffer instead of in-memory buffer. Default will be
memory.Later on, we might add the functionality to write to an external volume, instead of local file system.
Testing performed