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

Generate multiline parsers for fluent bit & support multiple parsers per filter #225

Merged
merged 21 commits into from
Oct 8, 2021
Merged

Conversation

dehaansa
Copy link
Collaborator

@dehaansa dehaansa commented Oct 2, 2021

This config validated in test for multiline is copied from cassandra development, it works properly for cassandra system logs parsing.

The multiple parsers per filter has not yet been as thoroughly validated, but intended to be useable in situations like MySQL having a different error log format for version 8+.

@dehaansa dehaansa added the kokoro:force-run Forces kokoro to run integration tests on a CL label Oct 2, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Forces kokoro to run integration tests on a CL label Oct 2, 2021
@dehaansa dehaansa changed the title Generate multiline parsers for fluent bit Generate multiline parsers for fluent bit & support multiple parsers per filter Oct 2, 2021
@dehaansa dehaansa added the kokoro:force-run Forces kokoro to run integration tests on a CL label Oct 2, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Forces kokoro to run integration tests on a CL label Oct 2, 2021
@dehaansa dehaansa mentioned this pull request Oct 4, 2021
Copy link
Contributor

@qingling128 qingling128 left a comment

Choose a reason for hiding this comment

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

My main concern is that we are introducing a new Ops Agent user visible receiver parse_regex_complex. We do want to support multi format parsers. But it likely would also be applicable for json parsers and needs more time to design and integration test it.

Can we leave the multi parsers code at fluent bit subagent level for now in this PR since the goal is to facilitate specific use cases of some 3rd party applications?

@dehaansa
Copy link
Collaborator Author

dehaansa commented Oct 4, 2021

My main concern is that we are introducing a new Ops Agent user visible receiver parse_regex_complex. We do want to support multi format parsers. But it likely would also be applicable for json parsers and needs more time to design and integration test it.

Can we leave the multi parsers code at fluent bit subagent level for now in this PR since the goal is to facilitate specific use cases of some 3rd party applications?

Does this also apply to the parse_regex_multiline receiver? It is currently user visible, but I can not expose it as well and keep it internally useable by fluent bit.

@qingling128
Copy link
Contributor

Yes, it applies to parse_regex_multiline as well.

@dehaansa
Copy link
Collaborator Author

dehaansa commented Oct 4, 2021

Code coverage reduction is due to the removal of user-facing parse_regex_complex & parse_regex_multiline, which prevents me from writing a confgenerator/testdata unit test for either of them. When we have log receivers using them, that code will get code coverage again. Alternatively, I could write an out of band unit test to cover the code.

@dehaansa dehaansa mentioned this pull request Oct 5, 2021
confgenerator/logging_processors.go Outdated Show resolved Hide resolved
confgenerator/logging_processors.go Outdated Show resolved Hide resolved
confgenerator/logging_processors.go Outdated Show resolved Hide resolved
confgenerator/fluentbit/modular.go Outdated Show resolved Hide resolved
confgenerator/fluentbit/modular.go Outdated Show resolved Hide resolved
confgenerator/logging_processors.go Outdated Show resolved Hide resolved
@qingling128 qingling128 added the kokoro:force-run Forces kokoro to run integration tests on a CL label Oct 7, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Forces kokoro to run integration tests on a CL label Oct 7, 2021
Copy link
Contributor

@qingling128 qingling128 left a comment

Choose a reason for hiding this comment

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

LGTM, modulo the outstanding comments get resolved.

Copy link
Member

@quentinmit quentinmit left a comment

Choose a reason for hiding this comment

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

LGTM modulo a resolution for the comment thread below

@qingling128 qingling128 merged commit 8bd47ae into GoogleCloudPlatform:master Oct 8, 2021
@dehaansa dehaansa deleted the multiline-parsers branch October 8, 2021 11:03
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.

5 participants