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

[breaking change] remove log_format #53

Closed
wants to merge 4 commits into from

Conversation

samjsong
Copy link
Contributor

As part of our new kubernetes solution, we are pursuing more declarative config all around. Some configuration that we currently support like log_format or add_timestamp could instead be done in a filter plugin like the record_modifier plugin.

However, in order to support both text type formats and json type formats, while also preserving _sumo_metadata, we need to change the expected format of the input. Now the log message to be sent to sumo is wrapped in the "message" key, as shown in the unit tests

This is a breaking change and will require a major release 2.0.0

Copy link
Contributor

@frankreno frankreno left a comment

Choose a reason for hiding this comment

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

LGTM - with one thing we need to do. When we cut this release. We should provide examples in the Readme of how customers could use other fluentD plugins with this to render the logs in the exact same format. They may have a ton of searches/dashbaords and alerts that depend on this and fixing those can be a non-trivial task so they need a migration path that clearly documents this. @samjsong lets make sure we add that before releasing a new major release.

@samjsong
Copy link
Contributor Author

Makes sense, thanks for the feedback @frankreno . I'll make sure to add some documentation as part of this PR, to clearly communicate the differences in the expected input.

@samjsong samjsong closed this Apr 22, 2020
@andrzej-stencel andrzej-stencel deleted the ssong-remove-log-format branch February 9, 2024 12:55
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.

4 participants