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

set Suppress_Type_Name to On for ElasticSearch output #464

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SlavikCA
Copy link

For ElasticSearch output, the default value of Suppress_Type_Name is Off:
https://docs.fluentbit.io/manual/pipeline/outputs/elasticsearch

That means, that out of the box FluentBit can only work with ElasticSearch version 6.x.
If output sent to ElasticSearch 8.x, the output will fail.

Support for Elastic 6.x was ended ~2 years ago.

Changing the value of Suppress_Type_Name to On will enable it to work with version 8.x out of the box.

@stevehipwell
Copy link
Collaborator

@SlavikCA do you use the output value without modification? IMHO they're just there as an example and shouldn't be used, a better pattern would be to switch the value to a stdout output so the chart can be deployed in a working state.

@SlavikCA
Copy link
Author

@stevehipwell
I took that values.yaml file, modified it and used it.

better pattern would be to switch the value to a stdout output

Can you clarify? Are you saying that instead of ElasticSearch output, the better idea is to use stdout?

@stevehipwell
Copy link
Collaborator

@SlavikCA when you modified values.yaml did you change config.outputs? If so the values you're submitting here would just be overridden anyway.

AFAIK the current config.outputs value would only work for someone who had an ES cluster running in the same namespace with the hostname (service name) elasticsearch-master which makes it unsuitable for just bringing up an instance to test.

@SlavikCA
Copy link
Author

I'm using ElasticSearch which is not hosted in the Kubernetes. It runs standalone.
So, yes, I updated config.outputs to point to my ES, username, password. But I based it on this example of values.yaml. And it didn't work with ES 8 because of Suppress_Type_Name.

So, this PR changes this example to make the example more modern - it will work for ES 8 now.

@stevehipwell
Copy link
Collaborator

@SlavikCA my issue here is that to make the change, which will almost certainly be overridden by all end users, requires a new chart release which will cause a significant amount of churn. So as to my second point, if we were making changes here I'd prefer to remove the ES config so it can't be incorrect and make a default installation work without needing to override this part.

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

Successfully merging this pull request may close these issues.

2 participants