Dedot fluentbit filter for labels and annotations

[nutellinoit]

Hi team, i added a little lua script on fluentbit to dedot labels and annotations before sending data to fluentd.
Source here: fluent/fluent-bit#1134 (comment)
This way, there will be no conflict on elasticsearch side because everything will be a string instead of an object.

I tested it and it's working, but before merging or do more test (eg on a real cluster) tell me what you think about it

[angelbarrera92]

How it affects current running setups?
I mean... after applying the patch, do we have to perform any ES operations?

The patch LGTM!

[nutellinoit]

No ES operation needed, because the logs are always sent to the same indexes.
The only thing that changes are the annotation/labels keys, no more an object but a single string with underscores.

IDEA:
What if we do some e2e tests on logging, for example, with some sample applications outputting a finite number of logs, and checking that the expected number of entries on ES is the one we are expecting to see?

[angelbarrera92]

No ES operation needed, because the logs are always sent to the same indexes.
The only thing that changes are the annotation/labels keys, no more an object but a single string with underscores.

IDEA:
What if we do some e2e tests on logging, for example, with some sample applications outputting a finite number of logs, and checking that the expected number of entries on ES is the one we are expecting to see?

Not sure if it is "easy" to do. But worth a test

[angelbarrera92]

Can you check the linter? it is complaining about Lua :)
If it is a false positive, or you want to deactivate the Lua linter, take a look to: https://github.com/sighupio/fury-kubernetes-policeman

[nutellinoit]

Hi @angelbarrera92 , can you help me with the linter?

[nutellinoit]

And side note, this change is now running on two clusters

[angelbarrera92]

Hi @angelbarrera92 , can you help me with the linter?

If you copy-pasted the code, maybe could be better to "exclude" it somehow. Is this the case?

[nutellinoit]

Hi @angelbarrera92 , can you help me with the linter?

If you copy-pasted the code, maybe could be better to "exclude" it somehow. Is this the case?

Yes, that's the case

[ralgozino]

Hi @nutellinoit !

The patch itself lgtm, but I'm missing a piece. What is this patch trying to solve? Is this an issue with our current implementation? if that's the case, is it fixed upstream? should we instead wait for the upstream patch and update the hole module? or is this a backport of a hotfix?

[nutellinoit]

Hi @ralgozino , the patch is solving the problem that causes issue if in the same cluster you have a deployment with a labels for example app and another deployment having the label app.something ( for example, if you deploy stuff with kustomize and with helm). This cause a mapping conflict on ES side.

I did this workaround because the dedot filter on fluentbit is present only on elasticsearch output, but we are not directly sending logs to ES, we are sending logs to fluentd.

Also it's easier to manage fluentbit configuration instead of fluentd.

If you want we can pair and i will show you the patch up&running

[angelbarrera92]

Hi @angelbarrera92 , can you help me with the linter?

Done here: 7cb863e

[nutellinoit]

Hi @angelbarrera92 , can you help me with the linter?

Done here: 7cb863e

Thank you @angelbarrera92 !

[ralgozino]

Hi @ralgozino , the patch is solving the problem that causes issue if in the same cluster you have a deployment with a labels for example app and another deployment having the label app.something ( for example, if you deploy stuff with kustomize and with helm). This cause a mapping conflict on ES side.

I did this workaround because the dedot filter on fluentbit is present only on elasticsearch output, but we are not directly sending logs to ES, we are sending logs to fluentd.

Also it's easier to manage fluentbit configuration instead of fluentd.

If you want we can pair and i will show you the patch up&running

Thank you for the explanation Samu, there's no need to pair but thanks for offering it!

[ralgozino]

lgtm

[angelbarrera92]

LGTM!